Refactor Location tracking to be an offset-based Locatable model

docs/plugins.md

@@ -582,8 +582,7 @@ export default function () {
                             event.program.diagnostics.register([{
                                 code: 9000,
                                 message: 'Do not use underscores in function names',
-                                range: funcStmt.tokens.name.range,
-                                file
+                                location: util.getLocation(funcStmt.tokens.name)
                             }]);
                         }
                     }
@@ -596,6 +595,30 @@ export default function () {
 };
 ```
 
+## Locations
+To keep memory down, tokens and AST nodes don't store a `Location`. Instead, each one stores where it starts and ends, counted from the start of the file (`startPosition` and `endPosition`), plus a reference to info about the file it was parsed from (`sourceInfo`), which is shared by everything from the same parse. `sourceInfo.linePositions` holds where each line of the file starts, which is how positions get converted to lines and characters.
+
+Positions are the gaps between characters, like an editor cursor (and like an LSP `Position`). For a file that starts with `print`, the `print` token's `startPosition` is `0` (before the `p`) and its `endPosition` is `5` (after the `t`), so `endPosition - startPosition` is its length.
+
+Build the line/character `Location` on demand with `util.getLocation()`:
+
+```typescript
+const location = util.getLocation(funcStmt.tokens.name); // { uri, range }
+```
+
+A node's bounds are computed once, when it's constructed, from the children and tokens it's given. They describe where the node came from in the original source, so they don't change if a plugin edits the AST afterwards (i.e. adding statements to a block doesn't move the block).
+
+`util.getLocation()` returns `undefined` for synthetic tokens and nodes (ones that were created in code rather than parsed from a file), since they have no position. A synthetic node won't get a source map entry, and diagnostics can't point at it.
+
+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 `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/CommentFlagProcessor.spec.ts

@@ -122,7 +122,7 @@ describe('CommentFlagProcessor', () => {
         it('tokenizes bs:disable comment with codes', () => {
             const token = Lexer.scan(`'bs:disable:1 2 3`).tokens[0].leadingTrivia[0];
             expect(
-                processor['tokenize'](token.text, token.location.range)
+                processor['tokenize'](token.text, util.getLocation(token).range)
             ).to.eql({
                 commentTokenText: `'`,
                 directive: 'disable',
@@ -143,7 +143,7 @@ describe('CommentFlagProcessor', () => {
             const tokens = Lexer.scan(`print "hi" 'bs:disable-line: 123456 999999   aaaab`).tokens;
             const token = tokens[2].leadingTrivia[1];
             expect(
-                processor['tokenize'](token.text, token.location.range)
+                processor['tokenize'](token.text, util.getLocation(token).range)
             ).to.eql({
                 commentTokenText: `'`,
                 directive: 'line',
@@ -163,7 +163,7 @@ describe('CommentFlagProcessor', () => {
         it('tokenizes bs:disable-line comment with codes', () => {
             const token = Lexer.scan(`'bs:disable-line:1 2 3`).tokens[0].leadingTrivia[0];
             expect(
-                processor['tokenize'](token.text, token.location?.range)
+                processor['tokenize'](token.text, util.getLocation(token)?.range)
             ).to.eql({
                 commentTokenText: `'`,
                 directive: 'line',
@@ -184,7 +184,7 @@ describe('CommentFlagProcessor', () => {
             const tokens = Lexer.scan(`' bs:disable-line:1`).tokens;
             const token = tokens[0].leadingTrivia[0];
             expect(
-                processor['tokenize'](token.text, token.location?.range)
+                processor['tokenize'](token.text, util.getLocation(token)?.range)
             ).to.eql({
                 commentTokenText: `'`,
                 directive: 'line',
@@ -199,7 +199,7 @@ describe('CommentFlagProcessor', () => {
             const tokens = Lexer.scan(`'\tbs:disable-line:1`).tokens;
             const token = tokens[0].leadingTrivia[0];
             expect(
-                processor['tokenize'](token.text, token.location?.range)
+                processor['tokenize'](token.text, util.getLocation(token)?.range)
             ).to.eql({
                 commentTokenText: `'`,
                 directive: 'line',

src/CommentFlagProcessor.ts

@@ -1,7 +1,8 @@
 import type { Range } from 'vscode-languageserver';
 import { DiagnosticMessages } from './DiagnosticMessages';
 import type { BscFile } from './files/BscFile';
-import type { BsDiagnostic, CommentFlag, DiagnosticCode } from './interfaces';
+import type { BsDiagnosticInput, CommentFlag, DiagnosticCode } from './interfaces';
+import type { Locatable } from './lexer/Token';
 import { util } from './util';
 
 export class CommentFlagProcessor {
@@ -29,16 +30,21 @@ export class CommentFlagProcessor {
     /**
      * List of diagnostics generated during processing
      */
-    public diagnostics = [] as BsDiagnostic[];
+    public diagnostics = [] as BsDiagnosticInput[];
 
     /**
      * Block-level `bs:disable` / `bs:enable` directives, recorded in source order
      * by `tryAdd` and resolved into `CommentFlag`s by `finalize()`.
      */
     private blockDirectives = [] as BlockDirective[];
 
-    public tryAdd(text: string, range: Range) {
-        const tokenized = this.tokenize(text, range);
+    /**
+     * @param text the text of the comment
+     * @param range the range of the comment
+     * @param locatable the comment token, if available. Used for the location of any diagnostics instead of `range`
+     */
+    public tryAdd(text: string, range: Range, locatable?: Locatable) {
+        const tokenized = this.tokenize(text, range, locatable);
         if (!tokenized) {
             return;
         }
@@ -157,7 +163,7 @@ export class CommentFlagProcessor {
      * Pushes diagnostics for any unknown numeric codes. Returns `null` when no codes were specified
      * (i.e. a bare `bs:disable` / `bs:enable`), and an array otherwise.
      */
-    private collectCodes(rawCodes: Array<{ code: string; range: Range }>): DiagnosticCode[] | null {
+    private collectCodes(rawCodes: RawCode[]): DiagnosticCode[] | null {
         if (rawCodes.length === 0) {
             return null;
         }
@@ -172,7 +178,7 @@ export class CommentFlagProcessor {
             } else {
                 this.diagnostics.push({
                     ...DiagnosticMessages.unknownDiagnosticCode(codeInt),
-                    location: util.createLocationFromFileRange(this.file, codeToken.range)
+                    location: codeToken.locatable ?? util.createLocationFromFileRange(this.file, codeToken.range)
                 });
             }
         }
@@ -182,7 +188,7 @@ export class CommentFlagProcessor {
     /**
      * Small tokenizer for `bs:` directive comments.
      */
-    private tokenize(text: string, range: Range): DisableToken | null {
+    private tokenize(text: string, range: Range, locatable?: Locatable): DisableToken | null {
         let lowerText = text.toLowerCase();
         let offset = 0;
         let commentTokenText: string | null = null;
@@ -230,17 +236,26 @@ export class CommentFlagProcessor {
         }
 
         const items = this.tokenizeByWhitespace(lowerText);
-        const codes = [] as Array<{ code: string; range: Range }>;
+        const codes = [] as RawCode[];
         for (const item of items) {
-            codes.push({
+            const codeOffset = offset + item.startIndex;
+            const code: RawCode = {
                 code: item.text,
                 range: util.createRange(
                     range.start.line,
-                    range.start.character + offset + item.startIndex,
+                    range.start.character + codeOffset,
                     range.start.line,
-                    range.start.character + offset + item.startIndex + item.text.length
+                    range.start.character + codeOffset + item.text.length
                 )
-            });
+            };
+            if (locatable?.sourceInfo) {
+                code.locatable = {
+                    startPosition: locatable.startPosition + codeOffset,
+                    endPosition: locatable.startPosition + codeOffset + item.text.length,
+                    sourceInfo: locatable.sourceInfo
+                };
+            }
+            codes.push(code);
         }
 
         return {
@@ -293,10 +308,19 @@ interface Token {
 interface DisableToken {
     commentTokenText: string | null;
     directive: 'line' | 'next-line' | 'disable' | 'enable';
-    codes: {
-        code: string;
-        range: Range;
-    }[];
+    codes: RawCode[];
+}
+
+/**
+ * A code from a directive comment
+ */
+interface RawCode {
+    code: string;
+    range: Range;
+    /**
+     * The position of the code, when the comment was given as a locatable
+     */
+    locatable?: Locatable;
 }
 
 interface BlockDirective {
@@ -305,6 +329,6 @@ interface BlockDirective {
      * The raw code tokens parsed from the directive comment. `finalize()` runs `collectCodes` over
      * these to validate them and emit any unknown-code diagnostics. An empty array means a bare directive.
      */
-    rawCodes: Array<{ code: string; range: Range }>;
+    rawCodes: RawCode[];
     range: Range;
 }

src/CrossScopeValidator.ts

@@ -19,6 +19,7 @@ import { ParseMode } from './parser/Parser';
 import { globalFile } from './globalCallables';
 import type { DottedGetExpression, VariableExpression } from './parser/Expression';
 import type { InheritableType } from './types';
+import type { Locatable } from './lexer/Token';
 
 
 interface FileSymbolPair {
@@ -861,16 +862,17 @@ export class CrossScopeValidator {
                                 type AstNodeWithName = VariableExpression | DottedGetExpression | EnumStatement | ClassStatement | ConstStatement | EnumMemberStatement | InterfaceStatement;
 
                                 const thatNodeKindName = otherIsGlobal ? 'Global Function' : util.getAstNodeFriendlyName(otherDupeNode) ?? 'Item';
-                                let thisNameRange = (dupeNode as AstNodeWithName)?.tokens?.name?.location?.range ?? dupeNode.location?.range;
-                                let thatNameRange = (otherDupeNode as AstNodeWithName)?.tokens?.name?.location?.range ?? otherDupeNode?.location?.range;
+                                let thisNameLocatable = util.firstLocatable<Locatable>((dupeNode as AstNodeWithName)?.tokens?.name, dupeNode);
+                                let thatNameLocatable = util.firstLocatable<Locatable>((otherDupeNode as AstNodeWithName)?.tokens?.name, otherDupeNode);
 
-                                const relatedInformation = thatNameRange ? [{
+                                const relatedInformation = thatNameLocatable ? [{
                                     message: `${thatNodeKindName} declared here`,
-                                    location: util.createLocationFromFileRange(otherDupe.file, thatNameRange)
+                                    location: thatNameLocatable
                                 }] : undefined;
                                 this.program.diagnostics.register({
                                     ...DiagnosticMessages.nameCollision(thisNodeKindName, thatNodeKindName, thatName),
-                                    location: util.createLocationFromFileRange(dupe.file, thisNameRange),
+                                    //keep the file's uri even if the node has no location
+                                    location: thisNameLocatable ?? util.createLocationFromFileRange(dupe.file, undefined),
                                     relatedInformation: relatedInformation
                                 }, {
                                     scope: scope,
@@ -906,7 +908,7 @@ export class CrossScopeValidator {
             for (const scope of scopeList) {
                 this.program.diagnostics.register({
                     ...this.getCannotFindDiagnostic(scope, symbol, typeChainResult),
-                    location: typeChainResult.location
+                    location: typeChainResult.locatable
                 }, {
                     scope: scope,
                     tags: [CrossScopeValidatorDiagnosticTag]
@@ -925,7 +927,7 @@ export class CrossScopeValidator {
                 const scopeList = [...incompatibleScopes.values()].map(s => s.name);
                 this.program.diagnostics.register({
                     ...DiagnosticMessages.incompatibleSymbolDefinition(typeChainResult.fullChainName, { scopes: scopeList }),
-                    location: typeChainResult.location
+                    location: typeChainResult.locatable
                 }, {
                     tags: [CrossScopeValidatorDiagnosticTag]
                 });