From c33fcbc83d90588086b3f0d3c97fc253c66242d7 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 27 Jan 2023 15:43:52 -0500 Subject: [PATCH 1/2] Use an object type to represent `Position` --- actions-languageservice/src/utils/find-token.ts | 11 +++++++---- actions-languageservice/src/utils/range.ts | 4 ++-- actions-languageservice/src/validate.ts | 4 ++-- .../src/templates/template-reader.ts | 10 +++++----- .../src/templates/tokens/template-token.ts | 4 ++-- .../src/templates/tokens/token-range.ts | 8 +++++++- .../src/workflows/yaml-object-reader.ts | 8 ++++---- 7 files changed, 29 insertions(+), 20 deletions(-) diff --git a/actions-languageservice/src/utils/find-token.ts b/actions-languageservice/src/utils/find-token.ts index b133001..bf2b744 100644 --- a/actions-languageservice/src/utils/find-token.ts +++ b/actions-languageservice/src/utils/find-token.ts @@ -141,13 +141,16 @@ function posInToken(pos: Position, token: TemplateToken): boolean { const tokenChar = pos.character + 1; // Check lines - if (r.start[0] > tokenLine || tokenLine > r.end[0]) { + if (r.start.line > tokenLine || tokenLine > r.end.line) { return false; } // Position is within the token lines. Check character/column if pos line matches // start or end - if ((r.start[0] === tokenLine && tokenChar < r.start[1]) || (r.end[0] === tokenLine && tokenChar > r.end[1])) { + if ( + (r.start.line === tokenLine && tokenChar < r.start.column) || + (r.end.line === tokenLine && tokenChar > r.end.column) + ) { return false; } @@ -163,14 +166,14 @@ function onSameLine(pos: Position, key: TemplateToken, value: TemplateToken): bo return false; } - if (value.range.start[0] !== value.range.end[0]) { + if (value.range.start.line !== value.range.end.line) { // Token occupies multiple lines, can't be an empty node return false; } // TokenRange is one-based, Position is zero-based const posLine = pos.line + 1; - if (posLine != value.range.start[0]) { + if (posLine != value.range.start.line) { return false; } diff --git a/actions-languageservice/src/utils/range.ts b/actions-languageservice/src/utils/range.ts index 13e6c5a..bba22cc 100644 --- a/actions-languageservice/src/utils/range.ts +++ b/actions-languageservice/src/utils/range.ts @@ -23,7 +23,7 @@ export function mapRange(range: TokenRange | undefined): Range { export function mapPosition(position: TokenPosition): Position { return { - line: position[0] - 1, - character: position[1] - 1 + line: position.line - 1, + character: position.column - 1 }; } diff --git a/actions-languageservice/src/validate.ts b/actions-languageservice/src/validate.ts index 0f82985..fdf592a 100644 --- a/actions-languageservice/src/validate.ts +++ b/actions-languageservice/src/validate.ts @@ -168,8 +168,8 @@ function getProviderContext( ): WorkflowContext { const {parent, path} = findToken( { - line: token.range!.start[0] - 1, - character: token.range!.start[1] - 1 + line: token.range!.start.line - 1, + character: token.range!.start.column - 1 }, root ); diff --git a/actions-workflow-parser/src/templates/template-reader.ts b/actions-workflow-parser/src/templates/template-reader.ts index 8ee3286..d00ea75 100644 --- a/actions-workflow-parser/src/templates/template-reader.ts +++ b/actions-workflow-parser/src/templates/template-reader.ts @@ -500,11 +500,11 @@ class TemplateReader { ); let tr = token.range!; - if (tr.start[0] === tr.end[0]) { + if (tr.start.line === tr.end.line) { // If it's a single line expression, adjust the range to only cover the sub-expression tr = { - start: [tr.start[0], tr.start[1] + startExpression], - end: [tr.end[0], tr.start[1] + endExpression + 1] + start: {line: tr.start.line, column: tr.start.column + startExpression}, + end: {line: tr.end.line, column: tr.start.column + endExpression + 1} }; } else { // Adjust the range to only cover the expression for multi-line strings @@ -515,8 +515,8 @@ class TemplateReader { const adjustedEnd = endExpression - beginningOfLine + 1; tr = { - start: [tr.start[0] + adjustedStartLine, adjustedStart], - end: [tr.start[0] + adjustedStartLine, adjustedEnd] + start: {line: tr.start.line + adjustedStartLine, column: adjustedStart}, + end: {line: tr.start.line + adjustedStartLine, column: adjustedEnd} }; } diff --git a/actions-workflow-parser/src/templates/tokens/template-token.ts b/actions-workflow-parser/src/templates/tokens/template-token.ts index 5257b3c..9aab233 100644 --- a/actions-workflow-parser/src/templates/tokens/template-token.ts +++ b/actions-workflow-parser/src/templates/tokens/template-token.ts @@ -42,11 +42,11 @@ export abstract class TemplateToken { } public get line(): number | undefined { - return this.range?.start[0]; + return this.range?.start.line; } public get col(): number | undefined { - return this.range?.start[1]; + return this.range?.start.column; } public get definition(): Definition | undefined { diff --git a/actions-workflow-parser/src/templates/tokens/token-range.ts b/actions-workflow-parser/src/templates/tokens/token-range.ts index 34b1f90..03d5f8e 100644 --- a/actions-workflow-parser/src/templates/tokens/token-range.ts +++ b/actions-workflow-parser/src/templates/tokens/token-range.ts @@ -1,4 +1,10 @@ -export type Position = [line: number, character: number]; +/** Represents the position within a template object */ +export type Position = { + /** The one-based line value */ + line: number; + /** The one-based column value */ + column: number; +}; export type TokenRange = { start: Position; diff --git a/actions-workflow-parser/src/workflows/yaml-object-reader.ts b/actions-workflow-parser/src/workflows/yaml-object-reader.ts index e4bc3d9..d64c4c5 100644 --- a/actions-workflow-parser/src/workflows/yaml-object-reader.ts +++ b/actions-workflow-parser/src/workflows/yaml-object-reader.ts @@ -89,8 +89,8 @@ export class YamlObjectReader implements ObjectReader { const elp = this.lineCounter.linePos(endPos); return { - start: [slp.line, slp.col], - end: [elp.line, elp.col] + start: {line: slp.line, column: slp.col}, + end: {line: elp.line, column: elp.col} }; } @@ -216,7 +216,7 @@ function rangeFromLinePos(linePos: [LinePos] | [LinePos, LinePos] | undefined): return; } // TokenRange and linePos are both 1-based - const start: Position = [linePos[0].line, linePos[0].col]; - const end: Position = linePos.length == 2 ? [linePos[1].line, linePos[1].col] : start; + const start: Position = {line: linePos[0].line, column: linePos[0].col}; + const end: Position = linePos.length == 2 ? {line: linePos[1].line, column: linePos[1].col} : start; return {start, end}; } From 16ad1addf5522d09acfb12438f11cb4c7e3057ec Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 27 Jan 2023 15:54:51 -0500 Subject: [PATCH 2/2] Update position tuples in tests --- actions-workflow-parser/src/expressions.test.ts | 16 ++++++++-------- actions-workflow-parser/src/index.test.ts | 8 ++++---- .../src/workflows/yaml-object-reader.test.ts | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/actions-workflow-parser/src/expressions.test.ts b/actions-workflow-parser/src/expressions.test.ts index f45eaea..8bdc916 100644 --- a/actions-workflow-parser/src/expressions.test.ts +++ b/actions-workflow-parser/src/expressions.test.ts @@ -85,15 +85,15 @@ jobs: [ "${{ github.event_name }}", { - start: [7, 16], - end: [7, 40] + start: {line: 7, column: 16}, + end: {line: 7, column: 40} } ], [ "${{ github.ref }}", { - start: [8, 24], - end: [8, 41] + start: {line: 8, column: 24}, + end: {line: 8, column: 41} } ] ]); @@ -160,16 +160,16 @@ jobs: { prefix: "test.yaml (Line: 6, Col: 14)", range: { - start: [7, 16], - end: [7, 40] + start: {line: 7, column: 16}, + end: {line: 7, column: 40} }, rawMessage: "Unrecognized function: 'fromJSON2'" }, { prefix: "test.yaml (Line: 6, Col: 14)", range: { - start: [8, 24], - end: [8, 51] + start: {line: 8, column: 24}, + end: {line: 8, column: 51} }, rawMessage: "Unrecognized function: 'toJSON2'" } diff --git a/actions-workflow-parser/src/index.test.ts b/actions-workflow-parser/src/index.test.ts index de071cb..b316089 100644 --- a/actions-workflow-parser/src/index.test.ts +++ b/actions-workflow-parser/src/index.test.ts @@ -32,8 +32,8 @@ describe("parseWorkflow", () => { expect(result.context.errors.getErrors()).toEqual([ new TemplateValidationError("Required property is missing: runs-on", "test.yaml (Line: 4, Col: 5)", undefined, { - start: [4, 5], - end: [5, 24] + start: {line: 4, column: 5}, + end: {line: 5, column: 24} }) ]); }); @@ -62,8 +62,8 @@ jobs: "test.yaml (Line: 6, Col: 13)", undefined, { - start: [6, 13], - end: [6, 37] + start: {line: 6, column: 13}, + end: {line: 6, column: 37} } ) ]); diff --git a/actions-workflow-parser/src/workflows/yaml-object-reader.test.ts b/actions-workflow-parser/src/workflows/yaml-object-reader.test.ts index 9813dff..4ff3b13 100644 --- a/actions-workflow-parser/src/workflows/yaml-object-reader.test.ts +++ b/actions-workflow-parser/src/workflows/yaml-object-reader.test.ts @@ -74,8 +74,8 @@ it("YAML errors include range information", () => { const error = context.errors.getErrors()[0]; expect(error.range).toEqual({ - start: [7, 38], - end: [7, 63] + start: {line: 7, column: 38}, + end: {line: 7, column: 63} }); });