Keep Location-based plugin code working with Locatables

docs/plugins.md

@@ -613,10 +613,11 @@ A node's bounds are computed once, when it's constructed, from the children and
 A position describes where the code came from in the original source, so it's used as-is for source maps and diagnostics. Don't change the position of a parsed token or node. Only give one to a synthetic token or node you're creating:
 - **Borrow it from a real token or node in the same file** (preferred): `util.setBounds(node, someToken)`. This shares the file's real `sourceInfo`, so the node combines correctly with the rest of that file's AST. The `create*` functions do the same when given a token or node: `createToken(TokenKind.Identifier, 'name', someToken)`.
 - **From a `Location` in a file**: `util.setLocation(node, location, file.sourceInfo)`. This maps the location into that file, so the node combines with the rest of its AST. If the location is in a different file (or its lines aren't in the file), the node is left without a position. To pass one to a `create*` function, convert it first: `createToken(TokenKind.Identifier, 'name', util.setLocation({} as Locatable, location, file.sourceInfo))`.
+- **From a `Location` without the file's source**: the `create*` functions also accept a `Location` directly, and assigning `node.location` or `token.location` moves an item to that location (see `util.assignLocation()`). When the file's source isn't known, the position goes in a placeholder source for the location's uri. `util.getLocation()` gives back the same location, and items placed this way combine with each other, but not with the file's parsed AST, so prefer the options above when you can.
 
 Code parsed from a snippet (i.e. `Parser.parse('...')` without a `srcPath`) has positions, but no uri. Those positions aren't in the file the code is added to, so lookups by position in that file (hover, definition, etc.) skip it, it isn't included in the bounds of nodes that also contain code from a file, and it doesn't get source map entries.
 
-`Location` is only used for language server output (diagnostics, hovers, etc.), so use `util.getLocation()` when you need one. Tokens and nodes still have read-only `location` and `range` properties to help existing plugins migrate, but they're deprecated and will be removed in a future release. They build a new `Location` on every read (so don't compare them with `===`). Tokens created by hand (i.e. `{ kind: ..., text: ... }` or `{ ...token }`) don't have them.
+`Location` is only used for language server output (diagnostics, hovers, etc.), so use `util.getLocation()` when you need one. Tokens and nodes still have `location` and `range` properties to help existing plugins migrate, but they're deprecated and will be removed in a future release. They build a new `Location` on every read (so don't compare them with `===`). Tokens created by hand (i.e. `{ kind: ..., text: ... }` or `{ ...token }`) don't have them, so create tokens with `createToken()` or `util.cloneToken()` instead.
 
 ## Modifying code
 Sometimes plugins will want to modify code before the project is transpiled. While you can technically edit the AST directly at any point in the file's lifecycle, this is not recommended as those changes will remain changed as long as that file exists in memory and could cause issues with file validation if the plugin is used in a language-server context (i.e. inside vscode).

src/astUtils/creators.spec.ts

@@ -1,9 +1,10 @@
 import { expect } from '../chai-config.spec';
-import { createIdentifier, createStringLiteral, createToken, createVariableExpression } from './creators';
+import { createCall, createIdentifier, createStringLiteral, createToken, createVariableExpression } from './creators';
 import { TokenKind } from '../lexer/TokenKind';
 import { Lexer } from '../lexer/Lexer';
 import { util } from '../util';
 import type { Locatable } from '../lexer/Token';
+import type { Location } from 'vscode-languageserver';
 
 describe('creators', () => {
 
@@ -44,5 +45,26 @@ describe('creators', () => {
         it('leaves the token synthetic when no position is given', () => {
             expect(util.getLocation(createToken(TokenKind.Identifier, 'xyz'))).to.be.undefined;
         });
+
+        it('accepts a line/character location', () => {
+            const location = util.createLocation(12, 4, 12, 7, 'file:///source/main.brs');
+            expect(util.getLocation(createToken(TokenKind.Identifier, 'xyz', location))).to.eql(location);
+            expect(createToken(TokenKind.Identifier, 'xyz', location).location).to.eql(location);
+            expect(util.getLocation(createIdentifier('xyz', location))).to.eql(location);
+            expect(util.getLocation(createVariableExpression('xyz', location))).to.eql(location);
+            expect(util.getLocation(createStringLiteral('xyz', location))).to.eql(location);
+        });
+
+        it('combines items created from line/character locations in the same file', () => {
+            const expression = createCall(
+                createVariableExpression('doSomething', util.createLocation(3, 4, 3, 15, 'file:///source/main.brs')),
+                [createStringLiteral('a', util.createLocation(3, 16, 3, 19, 'file:///source/main.brs'))]
+            );
+            expect(expression.location).to.eql(util.createLocation(3, 4, 3, 19, 'file:///source/main.brs'));
+        });
+
+        it('leaves the token synthetic for a location without a range', () => {
+            expect(util.getLocation(createToken(TokenKind.Identifier, 'xyz', { uri: 'file:///source/main.brs' } as Location))).to.be.undefined;
+        });
     });
 });

src/astUtils/creators.ts

@@ -7,6 +7,7 @@ import type { Expression, Statement } from '../parser/AstNode';
 import { LiteralExpression, CallExpression, DottedGetExpression, VariableExpression, FunctionExpression } from '../parser/Expression';
 import { AssignmentStatement, Block, DottedSetStatement, IfStatement, IndexedSetStatement, MethodStatement } from '../parser/Statement';
 import { TokenObject } from '../lexer/TokenObject';
+import util from '../util';
 
 const tokenDefaults = {
     [TokenKind.BackTick]: '`',
@@ -79,7 +80,13 @@ const tokenDefaults = {
     [TokenKind.Whitespace]: ' '
 };
 
-export function createToken<T extends TokenKind>(kind: T, text?: string, locatable?: Locatable): Token & { kind: T } {
+/**
+ * Where a created item goes: the position of a `Locatable` (such as an existing token or node), or a line/character `Location`
+ */
+export type CreatorPosition = Locatable | Location;
+
+export function createToken<T extends TokenKind>(kind: T, text?: string, position?: CreatorPosition): Token & { kind: T } {
+    const locatable = util.toLocatable(position);
     return new TokenObject(
         kind,
         text ?? (tokenDefaults[kind as string] as string) ?? kind.toString().toLowerCase(),
@@ -91,7 +98,8 @@ export function createToken<T extends TokenKind>(kind: T, text?: string, locatab
     ) as Token as Token & { kind: T };
 }
 
-export function createIdentifier(name: string, locatable?: Locatable): Identifier {
+export function createIdentifier(name: string, position?: CreatorPosition): Identifier {
+    const locatable = util.toLocatable(position);
     return new TokenObject(
         TokenKind.Identifier,
         name,
@@ -103,11 +111,11 @@ export function createIdentifier(name: string, locatable?: Locatable): Identifie
     ) as Identifier;
 }
 
-export function createVariableExpression(ident: string, locatable?: Locatable): VariableExpression {
+export function createVariableExpression(ident: string, locatable?: CreatorPosition): VariableExpression {
     return new VariableExpression({ name: createToken(TokenKind.Identifier, ident, locatable) });
 }
 
-export function createDottedIdentifier(path: string[], locatable?: Locatable): DottedGetExpression {
+export function createDottedIdentifier(path: string[], locatable?: CreatorPosition): DottedGetExpression {
     const ident = path.pop();
     const obj = path.length > 1 ? createDottedIdentifier(path, locatable) : createVariableExpression(path[0], locatable);
     return new DottedGetExpression({
@@ -122,29 +130,29 @@ export function createDottedIdentifier(path: string[], locatable?: Locatable): D
  * Since brightscript doesn't support strings with quotes in them, we can safely auto-detect and wrap the value in quotes in this function.
  * @param value - the string value. (value will be wrapped in quotes if they are missing)
  */
-export function createStringLiteral(value: string, locatable?: Locatable) {
+export function createStringLiteral(value: string, locatable?: CreatorPosition) {
     //wrap the value in double quotes
     if (!value.startsWith('"') && !value.endsWith('"')) {
         value = '"' + value + '"';
     }
     return new LiteralExpression({ value: createToken(TokenKind.StringLiteral, value, locatable) });
 }
-export function createIntegerLiteral(value: string, locatable?: Locatable) {
+export function createIntegerLiteral(value: string, locatable?: CreatorPosition) {
     return new LiteralExpression({ value: createToken(TokenKind.IntegerLiteral, value, locatable) });
 }
-export function createFloatLiteral(value: string, locatable?: Locatable) {
+export function createFloatLiteral(value: string, locatable?: CreatorPosition) {
     return new LiteralExpression({ value: createToken(TokenKind.FloatLiteral, value, locatable) });
 }
-export function createDoubleLiteral(value: string, locatable?: Locatable) {
+export function createDoubleLiteral(value: string, locatable?: CreatorPosition) {
     return new LiteralExpression({ value: createToken(TokenKind.DoubleLiteral, value, locatable) });
 }
-export function createLongIntegerLiteral(value: string, locatable?: Locatable) {
+export function createLongIntegerLiteral(value: string, locatable?: CreatorPosition) {
     return new LiteralExpression({ value: createToken(TokenKind.LongIntegerLiteral, value, locatable) });
 }
-export function createInvalidLiteral(value?: string, locatable?: Locatable) {
+export function createInvalidLiteral(value?: string, locatable?: CreatorPosition) {
     return new LiteralExpression({ value: createToken(TokenKind.Invalid, value, locatable) });
 }
-export function createBooleanLiteral(value: string, locatable?: Locatable) {
+export function createBooleanLiteral(value: string, locatable?: CreatorPosition) {
     return new LiteralExpression({ value: createToken(value === 'true' ? TokenKind.True : TokenKind.False, value, locatable) });
 }
 export function createFunctionExpression(kind: TokenKind.Sub | TokenKind.Function) {

src/astUtils/reflection.spec.ts

@@ -193,6 +193,7 @@ describe('reflection', () => {
             startPosition: undefined,
             endPosition: undefined,
             sourceInfo: undefined,
+            location: undefined,
             charCode: 0,
             leadingTrivia: []
         };

src/lexer/Token.ts

@@ -1,4 +1,5 @@
 import { TokenKind } from './TokenKind';
+import type { Location } from 'vscode-languageserver';
 
 /**
  * Represents a chunk of BrightScript scanned by the lexer.
@@ -21,6 +22,11 @@ export interface Token extends Locatable {
      * Any tokens starting on the next line of the previous token, up to the start of this token
      */
     leadingTrivia?: Token[];
+    /**
+     * Where the token was found. Assigning it moves the token to that location.
+     * @deprecated use `util.getLocation(token)`. This is computed on every read
+     */
+    location: Location;
 }
 
 /**

src/lexer/TokenObject.ts

@@ -19,12 +19,17 @@ export class TokenObject implements Token {
     ) { }
 
     /**
+     * Where the token was found. Assigning it moves the token to that location (see `util.assignLocation()`).
      * @deprecated use `util.getLocation(token)`. This is computed on every read
      */
     public get location(): Location | undefined {
         return util.getLocation(this);
     }
 
+    public set location(value: Location | undefined) {
+        util.assignLocation(this, value, this.sourceInfo);
+    }
+
     /**
      * @deprecated use `util.getLocation(token)?.range`. This is computed on every read
      */

src/parser/AstNode.spec.ts

@@ -1,4 +1,5 @@
 import { util } from '../util';
+import type { Location } from 'vscode-languageserver';
 import * as fsExtra from 'fs-extra';
 import { Program } from '../Program';
 import type { BrsFile } from '../files/BrsFile';
@@ -578,17 +579,47 @@ describe('AstNode', () => {
             expect((createToken(TokenKind.Identifier, 'b') as any).location).to.be.undefined;
         });
 
-        it('does not allow setting the location, since it describes where the code came from', () => {
+        it('moves synthetic nodes and tokens to an assigned location', () => {
             const location = util.createLocation(2, 3, 2, 7, 'file:///a.brs');
             const node = createVariableExpression('x');
-            expect(() => {
-                (node as any).location = location;
-            }).to.throw();
+            node.location = location;
+            expect(node.location).to.eql(location);
 
             const token = createToken(TokenKind.Identifier, 'x');
-            expect(() => {
-                (token as any).location = location;
-            }).to.throw();
+            token.location = location;
+            expect(token.location).to.eql(location);
+            expect(util.getLocation(token)).to.eql(location);
+        });
+
+        it('keeps an assigned location in the original source when it is in that source', () => {
+            const { ast } = Parser.parse('sub main()\n    a = 1\nend sub');
+            const name = ast.findChild<AssignmentStatement>(isAssignmentStatement).tokens.name;
+            const sourceInfo = name.sourceInfo;
+            name.location = util.createLocation(2, 0, 2, 3, sourceInfo.uri);
+            expect(name.sourceInfo).to.equal(sourceInfo);
+            expect(name.location.range).to.eql(util.createRange(2, 0, 2, 3));
+        });
+
+        it('keeps an assigned location that is outside of the original source', () => {
+            const { ast } = Parser.parse('sub main()\n    a = 1\nend sub');
+            const name = ast.findChild<AssignmentStatement>(isAssignmentStatement).tokens.name;
+            name.location = util.createLocation(20, 0, 20, 300, 'file:///other.brs');
+            expect(name.location).to.eql(util.createLocation(20, 0, 20, 300, 'file:///other.brs'));
+        });
+
+        it('clears the location when assigned undefined', () => {
+            const { ast } = Parser.parse('sub main()\n    a = 1\nend sub');
+            const assignment = ast.findChild<AssignmentStatement>(isAssignmentStatement);
+            assignment.location = undefined;
+            expect(assignment.location).to.be.undefined;
+            assignment.tokens.name.location = undefined;
+            expect(assignment.tokens.name.location).to.be.undefined;
+        });
+
+        it('lets tokens be used where a `{ location: Location }` is expected', () => {
+            const getLine = (item: { text: string; location: Location }) => item.location.range.start.line;
+            const { ast } = Parser.parse('sub main()\n    a = 1\nend sub');
+            expect(getLine(ast.findChild<AssignmentStatement>(isAssignmentStatement).tokens.name)).to.equal(1);
         });
     });
 

src/parser/AstNode.ts

@@ -33,12 +33,17 @@ export abstract class AstNode implements Locatable {
     public abstract sourceInfo: SourceInfo | undefined;
 
     /**
+     * The starting and ending location of the node. Assigning it moves the node to that location (see `util.assignLocation()`).
      * @deprecated use `util.getLocation(node)`. This is computed on every read
      */
     public get location(): Location | undefined {
         return util.getLocation(this);
     }
 
+    public set location(value: Location | undefined) {
+        util.assignLocation(this, value, this.sourceInfo);
+    }
+
     /**
      * @deprecated use `util.getLocation(node)?.range`. This is computed on every read
      */

src/parser/Statement.ts

@@ -3516,34 +3516,10 @@ export class MethodStatement extends FunctionStatement {
         const superCall = new ExpressionStatement({
             expression: new CallExpression({
                 callee: new VariableExpression({
-                    name: {
-                        kind: TokenKind.Identifier,
-                        text: 'super',
-                        isReserved: false,
-                        startPosition: state.classStatement.tokens.name.startPosition,
-                        endPosition: state.classStatement.tokens.name.endPosition,
-                        sourceInfo: state.classStatement.tokens.name.sourceInfo,
-                        leadingTrivia: []
-                    }
+                    name: createIdentifier('super', state.classStatement.tokens.name)
                 }),
-                openingParen: {
-                    kind: TokenKind.LeftParen,
-                    text: '(',
-                    isReserved: false,
-                    startPosition: state.classStatement.tokens.name.startPosition,
-                    endPosition: state.classStatement.tokens.name.endPosition,
-                    sourceInfo: state.classStatement.tokens.name.sourceInfo,
-                    leadingTrivia: []
-                },
-                closingParen: {
-                    kind: TokenKind.RightParen,
-                    text: ')',
-                    isReserved: false,
-                    startPosition: state.classStatement.tokens.name.startPosition,
-                    endPosition: state.classStatement.tokens.name.endPosition,
-                    sourceInfo: state.classStatement.tokens.name.sourceInfo,
-                    leadingTrivia: []
-                },
+                openingParen: createToken(TokenKind.LeftParen, '(', state.classStatement.tokens.name),
+                closingParen: createToken(TokenKind.RightParen, ')', state.classStatement.tokens.name),
                 args: []
             })
         });

src/parser/tests/Parser.spec.ts

@@ -14,6 +14,7 @@ export function token(kind: TokenKind, text?: string): Token {
         startPosition: undefined,
         endPosition: undefined,
         sourceInfo: undefined,
+        location: undefined,
         leadingTrivia: []
     };
 }

src/testHelpers.spec.ts

@@ -42,8 +42,10 @@ const testSource: SourceInfo = { uri: undefined, linePositions: Array.from({ len
 /**
  * Create a `Locatable` for a hand-built test token from line/character positions
  */
-export function testLocatable(startLine: number, startCharacter: number, endLine: number, endCharacter: number): Locatable {
-    return { startPosition: (startLine * 1000) + startCharacter, endPosition: (endLine * 1000) + endCharacter, sourceInfo: testSource };
+export function testLocatable(startLine: number, startCharacter: number, endLine: number, endCharacter: number): Locatable & { location: Location } {
+    const locatable: Locatable = { startPosition: (startLine * 1000) + startCharacter, endPosition: (endLine * 1000) + endCharacter, sourceInfo: testSource };
+    //hand-built tokens are plain objects without the deprecated `location` getter, so give them the matching value
+    return { ...locatable, location: util.getLocation(locatable) };
 }
 const sinon = createSandbox();
 

src/util.spec.ts

@@ -1235,8 +1235,8 @@ describe('util', () => {
 
         it('clears the position when given no location or source', () => {
             const sourceInfo = Lexer.scan('a = 1', { srcPath: srcPath }).tokens[0].sourceInfo;
-            expect(util.setLocation(testLocatable(0, 0, 0, 1), undefined, sourceInfo)).to.eql({ startPosition: undefined, endPosition: undefined, sourceInfo: undefined });
-            expect(util.setLocation(testLocatable(0, 0, 0, 1), util.createLocation(0, 0, 0, 1), undefined)).to.eql({ startPosition: undefined, endPosition: undefined, sourceInfo: undefined });
+            expect(util.setLocation(testLocatable(0, 0, 0, 1), undefined, sourceInfo)).to.include({ startPosition: undefined, endPosition: undefined, sourceInfo: undefined });
+            expect(util.setLocation(testLocatable(0, 0, 0, 1), util.createLocation(0, 0, 0, 1), undefined)).to.include({ startPosition: undefined, endPosition: undefined, sourceInfo: undefined });
         });
 
         it('clears the position for a location in a different file', () => {