diff --git a/docs/plugins.md b/docs/plugins.md index e1c5fa9b7..c54a4d7e4 100644 --- a/docs/plugins.md +++ b/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). diff --git a/src/CommentFlagProcessor.spec.ts b/src/CommentFlagProcessor.spec.ts index b68e28987..92b4d3698 100644 --- a/src/CommentFlagProcessor.spec.ts +++ b/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', diff --git a/src/CommentFlagProcessor.ts b/src/CommentFlagProcessor.ts index 1a95ce891..a05f7c02a 100644 --- a/src/CommentFlagProcessor.ts +++ b/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,7 +30,7 @@ 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 @@ -37,8 +38,13 @@ export class CommentFlagProcessor { */ 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; } diff --git a/src/CrossScopeValidator.ts b/src/CrossScopeValidator.ts index cb80fb5b2..8d1069e24 100644 --- a/src/CrossScopeValidator.ts +++ b/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((dupeNode as AstNodeWithName)?.tokens?.name, dupeNode); + let thatNameLocatable = util.firstLocatable((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] }); diff --git a/src/DiagnosticManager.spec.ts b/src/DiagnosticManager.spec.ts index 61e32cbd3..e80da8db6 100644 --- a/src/DiagnosticManager.spec.ts +++ b/src/DiagnosticManager.spec.ts @@ -3,8 +3,11 @@ import type { BrsFile } from './files/BrsFile'; import type { BsDiagnostic } from './interfaces'; import { Program } from './Program'; import { expectDiagnostics, expectZeroDiagnostics } from './testHelpers.spec'; -import util from './util'; -import { expect } from 'chai'; +import util, { standardizePath as s } from './util'; +import { expect } from './chai-config.spec'; +import { createSandbox } from 'sinon'; +import { TokenKind } from './lexer/TokenKind'; +import { createToken } from './astUtils/creators'; import { DiagnosticMessages } from './DiagnosticMessages'; @@ -383,6 +386,242 @@ describe('DiagnosticManager', () => { }); }); + describe('register', () => { + it('merges relatedInformation when the same diagnostic is registered more than once', () => { + const location = { uri: 'source/main.brs', range: util.createRange(1, 2, 3, 4) }; + const related1 = { message: 'related1', location: { uri: 'source/a.brs', range: util.createRange(1, 1, 1, 1) } }; + const related2 = { message: 'related2', location: { uri: 'source/b.brs', range: util.createRange(2, 2, 2, 2) } }; + program.diagnostics.register({ message: 'test', location: location, relatedInformation: [related1] }); + program.diagnostics.register({ message: 'test', location: location, relatedInformation: [related1, related2] }); + + const diagnostics = program.getDiagnostics(); + expect(diagnostics).to.be.lengthOf(1); + expect(diagnostics[0].relatedInformation).to.eql([related1, related2]); + }); + }); + + describe('locatables', () => { + const sinon = createSandbox(); + afterEach(() => { + sinon.restore(); + }); + + function getTokens(code: string) { + const file = program.setFile('source/main.brs', code) as BrsFile; + return { file: file, tokens: file.parser.tokens }; + } + + it('resolves a locatable location in getDiagnostics', () => { + const { tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + program.diagnostics.register({ message: 'test', location: printToken }); + expectDiagnostics(program.getDiagnostics(), [ + { message: 'test', location: util.getLocation(printToken) } + ]); + }); + + it('resolves locatable related information in getDiagnostics', () => { + const { tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + const subToken = tokens.find(x => x.kind === TokenKind.Sub); + program.diagnostics.register({ + message: 'test', + location: printToken, + relatedInformation: [{ message: 'related', location: subToken }] + }); + const diagnostics = program.getDiagnostics(); + expect(diagnostics[0].relatedInformation).to.eql([ + { message: 'related', location: util.getLocation(subToken) } + ]); + }); + + it('resolves a token ending in a newline to the same location as util.getLocation', () => { + const { tokens } = getTokens('sub main()\r\n print "hello"\nend sub\n'); + const newlineTokens = tokens.filter(x => x.kind === TokenKind.Newline); + program.diagnostics.register(newlineTokens.map((token, i) => ({ message: `test${i}`, location: token }))); + expectDiagnostics(program.getDiagnostics(), newlineTokens.map((token, i) => ({ + message: `test${i}`, + location: util.getLocation(token) + }))); + }); + + it('does not retain the locatable itself', () => { + const { tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + const subToken = tokens.find(x => x.kind === TokenKind.Sub); + program.diagnostics.register({ + message: 'test', + location: printToken, + relatedInformation: [{ message: 'related', location: subToken }] + }); + const cached = [...program.diagnostics['diagnosticsCache'].values()][0].diagnostic; + expect(cached.location).not.to.equal(printToken); + expect(cached.location).to.eql({ startPosition: printToken.startPosition, endPosition: printToken.endPosition, sourceInfo: printToken.sourceInfo }); + expect(cached.relatedInformation[0].location).not.to.equal(subToken); + expect(cached.relatedInformation[0].location).to.eql({ startPosition: subToken.startPosition, endPosition: subToken.endPosition, sourceInfo: subToken.sourceInfo }); + }); + + it('only computes Locations during getDiagnostics', () => { + const { file, tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + const subToken = tokens.find(x => x.kind === TokenKind.Sub); + const scope = new Scope('scope1', program); + const getLocationSpy = sinon.spy(util, 'getLocation'); + + program.diagnostics.register({ + message: 'test', + location: printToken, + relatedInformation: [{ message: 'related', location: subToken }] + }, { scope: scope, tags: ['tag1'] }); + program.diagnostics.register({ message: 'test2', location: subToken }, { tags: ['tag1'] }); + program.diagnostics.clearByFilter({ tag: 'tag2' }); + program.diagnostics.clearForFile('source/other.brs'); + expect(getLocationSpy.called).to.be.false; + + expectDiagnostics(program.getDiagnostics(), [{ message: 'test' }, { message: 'test2' }]); + expect(getLocationSpy.called).to.be.true; + + program.diagnostics.clearForFile(file.srcPath); + expectZeroDiagnostics(program.getDiagnostics()); + }); + + it('clears by file for both locatable and Location diagnostics', () => { + const { file, tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + const otherUri = util.pathToUri(s`${file.srcPath}/../other.brs`); + program.diagnostics.register([ + { message: 'locatable', location: printToken }, + { message: 'location', location: util.getLocation(printToken) }, + { message: 'other', location: { uri: otherUri, range: util.createRange(1, 2, 3, 4) } } + ]); + + program.diagnostics.clearForFile(file.srcPath); + expectDiagnostics(program.getDiagnostics(), [{ message: 'other' }]); + }); + + it('clears by fileUri filter for locatable diagnostics', () => { + const { file, tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + program.diagnostics.register({ message: 'test', location: printToken }, { tags: ['tag1'] }); + + program.diagnostics.clearByFilter({ fileUri: util.pathToUri(file.srcPath), tag: 'tag1' }); + expectZeroDiagnostics(program.getDiagnostics()); + }); + + it('dedupes the same locatable diagnostic from multiple scopes', () => { + const { tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + const subToken = tokens.find(x => x.kind === TokenKind.Sub); + const scope1 = new Scope('scope1', program); + const scope2 = new Scope('scope2', program); + const getDiagnostic = () => ({ + message: 'test', + location: printToken, + relatedInformation: [{ message: 'related', location: subToken }] + }); + program.diagnostics.register([ + { diagnostic: getDiagnostic(), context: { scope: scope1 } }, + { diagnostic: getDiagnostic(), context: { scope: scope2 } } + ]); + + const diagnostics = program.getDiagnostics(); + expect(diagnostics).to.be.lengthOf(1); + expect(diagnostics[0].relatedInformation).to.eql([ + { message: 'related', location: util.getLocation(subToken) }, + { message: `In scope 'scope1'`, location: util.getLocation(printToken) }, + { message: `In scope 'scope2'`, location: util.getLocation(printToken) } + ]); + }); + + it('suppresses locatable diagnostics with comment flags', () => { + const { tokens } = getTokens('sub main()\n \'bs:disable-next-line\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + program.diagnostics.register({ message: 'test', code: 1234, location: printToken }); + expectZeroDiagnostics(program.getDiagnostics()); + }); + + it('treats tokens and nodes with deprecated range/location getters as locatables', () => { + const { tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + //mimic the deprecated `range`/`location` getters that tokens and nodes can have + const tokenWithGetters = Object.create({ + get range() { + return util.getLocation(printToken).range; + }, + get location() { + return util.getLocation(printToken); + } + }, Object.getOwnPropertyDescriptors(printToken)); + program.diagnostics.register({ message: 'test', location: tokenWithGetters }); + + const cached = [...program.diagnostics['diagnosticsCache'].values()][0].diagnostic; + expect(cached.location).to.eql({ startPosition: printToken.startPosition, endPosition: printToken.endPosition, sourceInfo: printToken.sourceInfo }); + expectDiagnostics(program.getDiagnostics(), [ + { message: 'test', location: util.getLocation(printToken) } + ]); + }); + + it('does not return or retain a synthetic token that has no position', () => { + const { file } = getTokens('sub main()\nend sub\n'); + program.diagnostics.locationResolver = () => util.createLocation(0, 0, 0, 100, file.srcPath); + const token = createToken(TokenKind.Identifier, 'synthetic'); + program.diagnostics.register({ message: 'test', location: token }); + + const cached = [...program.diagnostics['diagnosticsCache'].values()][0].diagnostic; + expect(cached.location).not.to.equal(token); + expectDiagnostics(program.getDiagnostics(), [{ + message: 'test (location unknown, added here for visibility)', + location: util.createLocation(0, 0, 0, 100, file.srcPath) + }]); + }); + + it('uses the locationResolver for synthetic locatables', () => { + const { file } = getTokens('sub main()\nend sub\n'); + program.diagnostics.locationResolver = () => util.createLocation(0, 0, 0, 100, file.srcPath); + program.diagnostics.register({ message: 'test', location: { startPosition: 0, endPosition: 0, sourceInfo: undefined } }); + expectDiagnostics(program.getDiagnostics(), [{ + message: 'test (location unknown, added here for visibility)', + location: util.createLocation(0, 0, 0, 100, file.srcPath) + }]); + }); + + it('keeps the diagnostic source string and does not mistake it for a locatable', () => { + const { file, tokens } = getTokens('sub main()\n print "hello"\nend sub\n'); + const printToken = tokens.find(x => x.kind === TokenKind.Print); + const subToken = tokens.find(x => x.kind === TokenKind.Sub); + const location = util.createLocation(0, 0, 0, 3, printToken.sourceInfo.uri); + program.diagnostics.register([ + { + message: 'location', + source: 'bslint', + location: location, + relatedInformation: [{ message: 'related', location: location }] + }, + { + message: 'locatable', + source: 'bslint', + location: printToken, + relatedInformation: [{ message: 'related', location: subToken }] + } + ]); + + const diagnostics = program.getDiagnostics(); + const locationDiagnostic = diagnostics.find(x => x.message === 'location'); + expect(locationDiagnostic.source).to.eql('bslint'); + expect(locationDiagnostic.location).to.eql(location); + expect(locationDiagnostic.relatedInformation).to.eql([{ message: 'related', location: location }]); + + const locatableDiagnostic = diagnostics.find(x => x.message === 'locatable'); + expect(locatableDiagnostic.source).to.eql('bslint'); + expect(locatableDiagnostic.location).to.eql(util.getLocation(printToken)); + expect(locatableDiagnostic.relatedInformation).to.eql([{ message: 'related', location: util.getLocation(subToken) }]); + + //both should be cleared by their file + program.diagnostics.clearForFile(file.srcPath); + expectZeroDiagnostics(program.getDiagnostics()); + }); + }); + describe('canSkipScopeValidationForFile', () => { it('returns true if the DiagnosticFilterer says to filter the file', () => { const file = program.setFile('source/main.brs', '') as BrsFile; diff --git a/src/DiagnosticManager.ts b/src/DiagnosticManager.ts index c727d5d88..2aac417bc 100644 --- a/src/DiagnosticManager.ts +++ b/src/DiagnosticManager.ts @@ -1,10 +1,11 @@ -import type { DiagnosticContext, BsDiagnostic, DiagnosticContextPair } from './interfaces'; +import type { DiagnosticContext, BsDiagnostic, DiagnosticContextPair, BsDiagnosticInput, DiagnosticLocationInput } from './interfaces'; +import type { Locatable } from './lexer/Token'; import type { AstNode } from './parser/AstNode'; import type { Scope } from './Scope'; import { util } from './util'; import { Cache } from './Cache'; import { isBlock, isBrsFile, isBsDiagnostic, isXmlScope } from './astUtils/reflection'; -import type { DiagnosticRelatedInformation, Location, Range } from 'vscode-languageserver-protocol'; +import type { Location, Position, Range } from 'vscode-languageserver-protocol'; import { DiagnosticFilterer } from './DiagnosticFilterer'; import { DiagnosticSeverityAdjuster } from './DiagnosticSeverityAdjuster'; import type { FinalizedBsConfig } from './BsConfig'; @@ -17,20 +18,40 @@ import { DiagnosticCodeMap, DiagnosticMessages } from './DiagnosticMessages'; import * as path from 'path'; interface DiagnosticWithContexts { - diagnostic: BsDiagnosticWithKey; + diagnostic: StoredDiagnostic; contexts: Set; } -interface BsDiagnosticWithKey extends BsDiagnostic { +/** + * The position details of a `Locatable`, detached from the locatable itself so the node/token (and its AST) is not retained. + * `text` is only set (to a newline constant) when the locatable ended in a newline, so `util.getLocation()` resolves it the same way + */ +interface DetachedLocatable extends Locatable { + text?: string; +} + +/** + * Either an already-resolved `Location`, or a detached locatable that is only resolved in `getDiagnostics()` + */ +export type StoredLocation = Location | DetachedLocatable; + +export interface StoredRelatedInformation { + message: string; + location: StoredLocation; +} + +export interface StoredDiagnostic extends Omit { key: string; + location: StoredLocation; + relatedInformation: StoredRelatedInformation[]; } export interface LocationResolverArgs { - diagnostic: BsDiagnosticWithKey; + diagnostic: StoredDiagnostic; contexts: Set; } -type LocationResolver = (options: LocationResolverArgs) => Location | undefined; +type LocationResolver = (options: LocationResolverArgs) => DiagnosticLocationInput | undefined; /** * Manages all diagnostics for a program. @@ -72,12 +93,14 @@ export class DiagnosticManager { /** * Registers a diagnostic (or multiple diagnostics) for a program. - * Diagnostics can optionally be associated with a context + * Diagnostics can optionally be associated with a context. + * Locations (including in `relatedInformation`) may be `Locatable`s. Only their position details are kept, + * and they are not resolved into a `Location` until `getDiagnostics()` */ - public register(diagnostic: BsDiagnostic, context?: DiagnosticContext); - public register(diagnostics: Array, context?: DiagnosticContext); + public register(diagnostic: BsDiagnosticInput, context?: DiagnosticContext); + public register(diagnostics: Array, context?: DiagnosticContext); public register(diagnostics: Array); - public register(diagnosticArg: BsDiagnostic | Array, context?: DiagnosticContext) { + public register(diagnosticArg: BsDiagnosticInput | Array, context?: DiagnosticContext) { this.registerDiagnostics(diagnosticArg, context, false); } @@ -86,24 +109,28 @@ export class DiagnosticManager { * Unlike other diagnostics, these are dropped while the code they describe is in an inactive `#if` / `#else if` / `#else` branch, * because the device never compiles that code. The branches follow the file's most recent validation. */ - public registerParseDiagnostics(diagnostics: BsDiagnostic[]) { + public registerParseDiagnostics(diagnostics: BsDiagnosticInput[]) { this.registerDiagnostics(diagnostics, undefined, true); } - private registerDiagnostics(diagnosticArg: BsDiagnostic | Array, context: DiagnosticContext | undefined, isParseDiagnostic: boolean) { + private registerDiagnostics(diagnosticArg: BsDiagnosticInput | Array, context: DiagnosticContext | undefined, isParseDiagnostic: boolean) { const diagnostics = Array.isArray(diagnosticArg) ? diagnosticArg : [{ diagnostic: diagnosticArg, context: context }]; for (const diagnosticData of diagnostics) { - const diagnostic = isBsDiagnostic(diagnosticData) ? diagnosticData : diagnosticData.diagnostic; + const diagnostic = isBsDiagnostic(diagnosticData) ? diagnosticData as BsDiagnosticInput : (diagnosticData as DiagnosticContextPair).diagnostic; const diagContext = (diagnosticData as DiagnosticContextPair)?.context ?? context; - const key = this.getDiagnosticKey(diagnostic); + const location = this.storeLocation(diagnostic.location); + const relatedInformation = (diagnostic.relatedInformation ?? []).map(x => ({ + message: x.message, + location: this.storeLocation(x.location) + })); + const key = this.getDiagnosticKey(diagnostic, location); let fromCache = true; const cacheData = this.diagnosticsCache.getOrAdd(key, () => { - - if (!diagnostic.relatedInformation) { - diagnostic.relatedInformation = []; - } fromCache = false; - return { diagnostic: { key: key, ...diagnostic }, contexts: new Set() }; + return { + diagnostic: { ...diagnostic, key: key, location: location, relatedInformation: relatedInformation }, + contexts: new Set() + }; }); const cachedDiagnostic = cacheData.diagnostic; @@ -112,8 +139,8 @@ export class DiagnosticManager { } else { this.parseDiagnosticKeys.delete(key); } - if (!fromCache && diagnostic.relatedInformation) { - this.mergeRelatedInformation(cachedDiagnostic.relatedInformation, diagnostic.relatedInformation); + if (fromCache) { + this.mergeRelatedInformation(cachedDiagnostic.relatedInformation, relatedInformation); } const contexts = cacheData.contexts; if (diagContext) { @@ -123,8 +150,8 @@ export class DiagnosticManager { } } - private addToMaps(diagnostic: BsDiagnosticWithKey, context?: DiagnosticContext) { - const uriLower = util.pathToUri(diagnostic.location?.uri?.toLowerCase()); + private addToMaps(diagnostic: StoredDiagnostic, context?: DiagnosticContext) { + const uriLower = util.pathToUri(this.getUri(diagnostic.location)?.toLowerCase()); if (uriLower) { if (!this.fileUriMap.has(uriLower)) { this.fileUriMap.set(uriLower, new Set()); @@ -184,11 +211,29 @@ export class DiagnosticManager { const results = [] as Array; const isInInactiveBranch = this.createInactiveBranchDetector(); for (const cachedDiagnostic of this.diagnosticsCache.values()) { - if (isInInactiveBranch(cachedDiagnostic.diagnostic)) { + const storedDiagnostic = cachedDiagnostic.diagnostic; + if (isInInactiveBranch(storedDiagnostic)) { + continue; + } + let location = storedDiagnostic.location; + let message = storedDiagnostic.message; + if (!this.getUri(location)) { + location = this.storeLocation(this.locationResolver?.(cachedDiagnostic)); + if (location) { + //if we found a location, tweak the message a bit to let devs know this was not the original location + message = `${message} (location unknown, added here for visibility)`; + } + } + //check suppression before resolving any locations, so suppressed diagnostics never pay for it + if (this.isSuppressed(storedDiagnostic.code, storedDiagnostic.legacyCode, location)) { continue; } - const diagnostic = { ...cachedDiagnostic.diagnostic }; - const relatedInformation = [...cachedDiagnostic.diagnostic.relatedInformation]; + + const originalLocation = this.resolveLocation(storedDiagnostic.location); + const relatedInformation = storedDiagnostic.relatedInformation.map(x => ({ + message: x.message, + location: this.resolveLocation(x.location) + })); const affectedScopes = new Set(); for (const context of cachedDiagnostic.contexts.values()) { if (context.scope) { @@ -207,25 +252,19 @@ export class DiagnosticManager { } else { relatedInformation.push({ message: `In scope '${scope.name}'`, - location: diagnostic.location + location: originalLocation }); } } - diagnostic.relatedInformation = relatedInformation; - if (!diagnostic.location?.uri) { - diagnostic.location = this.locationResolver?.(cachedDiagnostic); - if (diagnostic.location) { - //if we found a location, tweak the message a bit to let devs know this was not the original location - diagnostic.message = `${diagnostic.message} (location unknown, added here for visibility)`; - } - } - results.push(diagnostic); + results.push({ + ...storedDiagnostic, + message: message, + location: location === storedDiagnostic.location ? originalLocation : this.resolveLocation(location), + relatedInformation: relatedInformation + }); } - const filteredResults = results.filter((x) => { - return !this.isDiagnosticSuppressed(x); - }); - return filteredResults; + return results; } /** @@ -234,10 +273,13 @@ export class DiagnosticManager { */ private createInactiveBranchDetector() { const inactiveRangesByUri = new Map(); - return (diagnostic: BsDiagnosticWithKey) => { - const uri = diagnostic.location?.uri; - const start = diagnostic.location?.range?.start; - if (!uri || !start || !this.parseDiagnosticKeys.has(diagnostic.key)) { + return (diagnostic: StoredDiagnostic) => { + if (!this.parseDiagnosticKeys.has(diagnostic.key)) { + return false; + } + const uri = this.getUri(diagnostic.location); + const start = this.getStart(diagnostic.location); + if (!uri || !start) { return false; } const uriLower = uri.toLowerCase(); @@ -249,8 +291,8 @@ export class DiagnosticManager { // eslint-disable-next-line @typescript-eslint/dot-notation for (const statement of file['_cachedLookups'].conditionalCompileStatements) { for (const branch of [statement.thenBranch, statement.elseBranch]) { - if (isBlock(branch) && branch.isActive === false && branch.location) { - inactiveRanges.push(branch.location.range); + if (isBlock(branch) && branch.isActive === false && branch.sourceInfo) { + inactiveRanges.push(util.getLocation(branch).range); } } } @@ -266,17 +308,27 @@ export class DiagnosticManager { * Determine whether this diagnostic should be supressed or not, based on brs comment-flags */ public isDiagnosticSuppressed(diagnostic: BsDiagnostic) { - const diagnosticCode = typeof diagnostic.code === 'string' ? diagnostic.code.toLowerCase() : diagnostic.code?.toString() ?? undefined; - const diagnosticLegacyCode = typeof diagnostic.legacyCode === 'string' ? diagnostic.legacyCode.toLowerCase() : diagnostic.legacyCode; - const file = this.program?.getFile(diagnostic.location?.uri); + return this.isSuppressed(diagnostic.code, diagnostic.legacyCode, diagnostic.location); + } + + private isSuppressed(code: number | string | undefined, legacyCode: number | string | undefined, location: StoredLocation) { + const diagnosticCode = typeof code === 'string' ? code.toLowerCase() : code?.toString() ?? undefined; + const diagnosticLegacyCode = typeof legacyCode === 'string' ? legacyCode.toLowerCase() : legacyCode; + const file = this.program?.getFile(this.getUri(location)); if (diagnosticCode === DiagnosticCodeMap.unknownDiagnosticCode) { return false; } + //only computed if the file actually has comment flags + let start: Position | undefined; + let isStartComputed = false; for (let flag of file?.commentFlags ?? []) { - - if (!diagnostic.location?.range || !util.rangeContains(flag.affectedRange, diagnostic.location.range.start)) { + if (!isStartComputed) { + start = this.getStart(location); + isStartComputed = true; + } + if (!start || !util.rangeContains(flag.affectedRange, start)) { continue; } //if this flag explicitly re-enables the code, it's not suppressed here, keep looking @@ -466,7 +518,7 @@ export class DiagnosticManager { isMatch = context.scope?.name === filter.scope.name; } if (isMatch && needToMatch.fileUri) { - isMatch = diagnostic.location?.uri === filter.fileUri; + isMatch = this.getUri(diagnostic.location) === filter.fileUri; } if (isMatch && needToMatch.segment) { isMatch = context.segment === filter.segment; @@ -484,7 +536,7 @@ export class DiagnosticManager { } } - private deleteContextsFromDiagnostic(diagnostic: BsDiagnosticWithKey, contexts: DiagnosticContext[]) { + private deleteContextsFromDiagnostic(diagnostic: StoredDiagnostic, contexts: DiagnosticContext[]) { const key = diagnostic.key; const cachedData = this.diagnosticsCache.get(key); for (const context of contexts) { @@ -530,25 +582,25 @@ export class DiagnosticManager { } } - private removeDiagnosticIfNoContexts(diagnostic: BsDiagnosticWithKey) { + private removeDiagnosticIfNoContexts(diagnostic: StoredDiagnostic) { const key = diagnostic.key; const cachedData = this.diagnosticsCache.get(key); if (cachedData.contexts.size === 0) { this.diagnosticsCache.delete(key); this.parseDiagnosticKeys.delete(key); - this.fileUriMap.get(diagnostic.location?.uri?.toLowerCase())?.delete(key); + this.fileUriMap.get(this.getUri(diagnostic.location)?.toLowerCase())?.delete(key); } } - private getDiagnosticKey(diagnostic: BsDiagnostic) { - return `${diagnostic.location?.uri ?? 'No uri'} ${util.rangeToString(diagnostic.location?.range)} - ${diagnostic.code} - ${diagnostic.message}`; + private getDiagnosticKey(diagnostic: BsDiagnosticInput, location: StoredLocation) { + return `${this.getLocationKey(location)} - ${diagnostic.code} - ${diagnostic.message}`; } - private mergeRelatedInformation(target: DiagnosticRelatedInformation[], source: DiagnosticRelatedInformation[]) { - function getRiKey(relatedInfo: DiagnosticRelatedInformation) { - return `${relatedInfo.message} - ${relatedInfo.location?.uri} - ${util.rangeToString(relatedInfo.location?.range)}`.toLowerCase(); - } + private mergeRelatedInformation(target: StoredRelatedInformation[], source: StoredRelatedInformation[]) { + const getRiKey = (relatedInfo: StoredRelatedInformation) => { + return `${relatedInfo.message} - ${this.getLocationKey(relatedInfo.location)}`.toLowerCase(); + }; const existingKeys = target.map(ri => getRiKey(ri)); @@ -560,6 +612,49 @@ export class DiagnosticManager { } } + private isDetachedLocatable(location: StoredLocation | DiagnosticLocationInput): location is DetachedLocatable { + //a `Location` always has a `uri` and never a `startPosition`. Tokens and nodes always have a `startPosition` key (even synthetic ones, where it's undefined), + //and may also have deprecated `range`/`location` getters, so those can't be used to tell them apart. + //never check for `source` here: that's the diagnostic's own `source` string (i.e. 'brs' or 'bslint'), not a locatable field + return !!location && ('startPosition' in location || !('uri' in location)); + } + + /** + * Convert an incoming location into the form we keep in the cache. Locatables are copied down to just their position details + * so the node/token they came from (and its whole AST) is not retained by the diagnostic + */ + private storeLocation(location: DiagnosticLocationInput | undefined): StoredLocation | undefined { + if (!this.isDetachedLocatable(location)) { + return location; + } + const detached: DetachedLocatable = { startPosition: location.startPosition, endPosition: location.endPosition, sourceInfo: location.sourceInfo }; + //`util.getLocation()` keeps a newline-terminated token on its own line by looking at its text, so keep just enough text for that + const newlineLength = location.sourceInfo ? location.endPosition - util.getContentEnd(location) : 0; + if (newlineLength > 0) { + detached.text = newlineLength === 2 ? '\r\n' : '\n'; + } + return detached; + } + + private getUri(location: StoredLocation | undefined): string | undefined { + return this.isDetachedLocatable(location) ? location.sourceInfo?.uri : location?.uri; + } + + private getStart(location: StoredLocation | undefined): Position | undefined { + return this.isDetachedLocatable(location) ? util.getStartPosition(location) : location?.range?.start; + } + + private getLocationKey(location: StoredLocation | undefined) { + if (this.isDetachedLocatable(location)) { + return `${location.sourceInfo?.uri ?? 'No uri'} @${location.startPosition}-${location.endPosition}`; + } + return `${location?.uri ?? 'No uri'} ${util.rangeToString(location?.range)}`; + } + + private resolveLocation(location: StoredLocation | undefined): Location | undefined { + return this.isDetachedLocatable(location) ? util.getLocation(location) : location; + } + /** * Are the diagnostics for this file completely filtered? * If so, we can skip any Scope-based validation on this file at all, which can save a lot of time for large files diff --git a/src/FunctionScope.ts b/src/FunctionScope.ts index e1af48c26..550cea7e0 100644 --- a/src/FunctionScope.ts +++ b/src/FunctionScope.ts @@ -1,5 +1,6 @@ import type { LabelDeclaration, VariableDeclaration } from './interfaces'; import type { FunctionExpression } from './parser/Expression'; +import { util } from './util'; //TODO I think this class can be eliminated in favor of moving some of these onto the FunctionExpression AST node export class FunctionScope { @@ -13,7 +14,7 @@ export class FunctionScope { * and ends after the final `n` in `end function` or `b` in end sub. */ public get range() { - return this.func?.location?.range; + return util.getLocation(this.func)?.range; } /** * The scopes that are children of this scope diff --git a/src/PluginInterface.spec.ts b/src/PluginInterface.spec.ts index 93f2f96d6..b7b735957 100644 --- a/src/PluginInterface.spec.ts +++ b/src/PluginInterface.spec.ts @@ -61,4 +61,14 @@ describe('PluginInterface', () => { pluginInterface.emit('beforeSerializeProgram', {} as any); expect(beforeSerializeProgram.callCount).to.equal(1); }); + + it('converts the old afterProgramValidate event to afterValidateProgram', () => { + const afterProgramValidate = sinon.spy(); + pluginInterface.add({ + name: 'converts the old afterProgramValidate event', + afterProgramValidate: afterProgramValidate + } as any); + pluginInterface.emit('afterValidateProgram', {} as any); + expect(afterProgramValidate.callCount).to.equal(1); + }); }); diff --git a/src/PluginInterface.ts b/src/PluginInterface.ts index 05f1dba26..6b97be51e 100644 --- a/src/PluginInterface.ts +++ b/src/PluginInterface.ts @@ -148,6 +148,7 @@ export default class PluginInterface { afterProgramCreate: 'afterProvideProgram', beforeProgramValidate: 'beforeValidateProgram', onProgramValidate: 'validateProgram', + afterProgramValidate: 'afterValidateProgram', beforeProgramDispose: 'beforeRemoveProgram', afterScopeCreate: 'afterProvideScope', beforeScopeDispose: 'beforeRemoveScope', diff --git a/src/Program.spec.ts b/src/Program.spec.ts index b55c436be..6d4c7395c 100644 --- a/src/Program.spec.ts +++ b/src/Program.spec.ts @@ -3527,6 +3527,266 @@ describe('Program', () => { // We don't necessarily expect specific results, just that it doesn't crash expect(signatureHelp).to.be.an('array'); }); + + describe('location boundaries', () => { + //every test in this block builds its source from an explicit list of lines (no indentation stripping) + //so that the line/character values below can be counted by hand + const greetLabel = 'sub greet(name as string, age as integer)'; + + function expectSignature(srcPath: string, line: number, character: number, label: string, index: number) { + const signatureHelp = program.getSignatureHelp(srcPath, util.createPosition(line, character)); + expect( + signatureHelp.map(x => ({ label: x.signature?.label, index: x.index })), + `wrong signature help at ${line},${character}` + ).to.eql([{ label: label, index: index }]); + } + + function expectNoSignature(srcPath: string, line: number, character: number) { + const signatureHelp = program.getSignatureHelp(srcPath, util.createPosition(line, character)); + expect(signatureHelp, `expected no signature help at ${line},${character}`).to.eql([]); + } + + function testSingleLineCall(srcPath: string, eol: string) { + program.setFile(srcPath, [ + 'sub main()', //0 + ' greet("bob", 42)', //1 + 'end sub', //2 + 'sub greet(name as string, age as integer)', //3 + 'end sub' //4 + ].join(eol)); + program.validate(); + expectZeroDiagnostics(program); + // greet|("bob", 42) + expectNoSignature(srcPath, 1, 9); + // greet(|"bob", 42) + expectSignature(srcPath, 1, 10, greetLabel, 0); + // greet("bob"|, 42) + expectSignature(srcPath, 1, 15, greetLabel, 0); + // greet("bob",| 42) + expectSignature(srcPath, 1, 16, greetLabel, 1); + // greet("bob", |42) + expectSignature(srcPath, 1, 17, greetLabel, 1); + // greet("bob", 42|) + expectSignature(srcPath, 1, 19, greetLabel, 1); + // greet("bob", 42)| + expectSignature(srcPath, 1, 20, greetLabel, 2); + //the lines before and after the call + expectNoSignature(srcPath, 0, 10); + expectNoSignature(srcPath, 2, 0); + } + + it('handles every boundary of a single-line call in a .bs file', () => { + testSingleLineCall('source/main.bs', '\n'); + }); + + it('handles every boundary of a single-line call in a .brs file', () => { + testSingleLineCall('source/main.brs', '\n'); + }); + + it('handles every boundary of a single-line call in a file with CRLF line endings', () => { + testSingleLineCall('source/main.bs', '\r\n'); + }); + + it('handles every boundary of a single-line call in a .brs file with CRLF line endings', () => { + testSingleLineCall('source/main.brs', '\r\n'); + }); + + it('handles nested calls', () => { + program.setFile('source/main.bs', [ + 'sub main()', //0 + ' outer(inner(1, 2), 3)', //1 + 'end sub', //2 + 'sub outer(a as dynamic, b as integer)', //3 + 'end sub', //4 + 'function inner(x as integer, y as integer)', //5 + ' return x + y', //6 + 'end function' //7 + ].join('\n')); + program.validate(); + const outerLabel = 'sub outer(a as dynamic, b as integer)'; + const innerLabel = 'function inner(x as integer, y as integer)'; + // outer(|inner(1, 2), 3) + expectSignature('source/main.bs', 1, 10, outerLabel, 0); + // outer(inner(|1, 2), 3) + expectSignature('source/main.bs', 1, 16, innerLabel, 0); + // outer(inner(1,| 2), 3) (the space after a nested comma isn't inside any argument, so this resolves to the outer call, same as v1) + expectSignature('source/main.bs', 1, 18, outerLabel, 0); + // outer(inner(1, |2), 3) + expectSignature('source/main.bs', 1, 19, innerLabel, 1); + // outer(inner(1, 2|), 3) + expectSignature('source/main.bs', 1, 20, innerLabel, 1); + // outer(inner(1, 2)|, 3) + expectSignature('source/main.bs', 1, 21, outerLabel, 0); + // outer(inner(1, 2),| 3) + expectSignature('source/main.bs', 1, 22, outerLabel, 1); + // outer(inner(1, 2), 3|) + expectSignature('source/main.bs', 1, 24, outerLabel, 1); + }); + + it('handles a call that spans multiple lines', () => { + program.setFile('source/main.bs', [ + 'sub main()', //0 + ' greet(', //1 + ' "bob",', //2 + ' 42', //3 + ' )', //4 + 'end sub', //5 + 'sub greet(name as string, age as integer)', //6 + 'end sub' //7 + ].join('\n')); + program.validate(); + expectZeroDiagnostics(program); + // greet|( + expectNoSignature('source/main.bs', 1, 9); + // greet(| + expectSignature('source/main.bs', 1, 10, greetLabel, 0); + //| "bob", + expectSignature('source/main.bs', 2, 0, greetLabel, 0); + // |"bob", + expectSignature('source/main.bs', 2, 8, greetLabel, 0); + // "bob",| + expectSignature('source/main.bs', 2, 14, greetLabel, 1); + //| 42 + expectSignature('source/main.bs', 3, 0, greetLabel, 1); + // 42| + expectSignature('source/main.bs', 3, 10, greetLabel, 1); + //the cursor is past the end of the last argument, so the index is one past it (same as for a single-line call) + // |) + expectSignature('source/main.bs', 4, 4, greetLabel, 2); + expectNoSignature('source/main.bs', 5, 0); + }); + + it('handles a call that spans multiple lines in a file with CRLF line endings', () => { + program.setFile('source/main.bs', [ + 'sub main()', //0 + ' greet(', //1 + ' "bob",', //2 + ' 42', //3 + ' )', //4 + 'end sub', //5 + 'sub greet(name as string, age as integer)', //6 + 'end sub' //7 + ].join('\r\n')); + program.validate(); + expectZeroDiagnostics(program); + // greet(| + expectSignature('source/main.bs', 1, 10, greetLabel, 0); + //| "bob", + expectSignature('source/main.bs', 2, 0, greetLabel, 0); + // "bob",| + expectSignature('source/main.bs', 2, 14, greetLabel, 1); + //| 42 + expectSignature('source/main.bs', 3, 0, greetLabel, 1); + // 42| + expectSignature('source/main.bs', 3, 10, greetLabel, 1); + expectNoSignature('source/main.bs', 5, 0); + }); + + it('handles a surrogate-pair emoji before the target on the same line', () => { + program.setFile('source/main.bs', [ + 'sub main()', //0 + //each emoji is 2 UTF-16 code units + ' greet("😀😀", 42)', //1 + ' x = "😀" : greet("bob", 42)', //2 + 'end sub', //3 + 'sub greet(name as string, age as integer)', //4 + 'end sub' //5 + ].join('\n')); + program.validate(); + // greet(|"😀😀", 42) + expectSignature('source/main.bs', 1, 10, greetLabel, 0); + // greet("😀😀"|, 42) + expectSignature('source/main.bs', 1, 16, greetLabel, 0); + // greet("😀😀",| 42) + expectSignature('source/main.bs', 1, 17, greetLabel, 1); + // greet("😀😀", 42|) + expectSignature('source/main.bs', 1, 20, greetLabel, 1); + + // x = "😀" : greet|("bob", 42) + expectNoSignature('source/main.bs', 2, 20); + // x = "😀" : greet(|"bob", 42) + expectSignature('source/main.bs', 2, 21, greetLabel, 0); + // x = "😀" : greet("bob",| 42) + expectSignature('source/main.bs', 2, 27, greetLabel, 1); + // x = "😀" : greet("bob", 42|) + expectSignature('source/main.bs', 2, 30, greetLabel, 1); + }); + + it('handles calls inside and after a multi-line template string', () => { + program.setFile('source/main.bs', [ + 'sub main()', //0 + // eslint-disable-next-line no-template-curly-in-string + ' msg = `first ${greet("a", 1)}', //1 + // eslint-disable-next-line no-template-curly-in-string + 'second ${greet("b", 2)} third`', //2 + ' greet("c", 3)', //3 + 'end sub', //4 + 'function greet(name as string, age as integer)', //5 + ' return name', //6 + 'end function' //7 + ].join('\n')); + program.validate(); + const label = 'function greet(name as string, age as integer)'; + // msg = `first ${greet("a",| 1)} + expectSignature('source/main.bs', 1, 29, label, 1); + //second ${greet(|"b", 2)} third` + expectSignature('source/main.bs', 2, 15, label, 0); + //second ${greet("b",| 2)} third` + expectSignature('source/main.bs', 2, 19, label, 1); + //second ${greet("b", 2|)} third` + expectSignature('source/main.bs', 2, 21, label, 1); + //second ${greet("b", 2)} th|ird` + expectNoSignature('source/main.bs', 2, 26); + // greet("c",| 3) + expectSignature('source/main.bs', 3, 14, label, 1); + }); + + it('handles a call on the last line of a file with no trailing newline', () => { + program.setFile('source/main.bs', [ + //signature help crashes when the declaration is the very first token of the file (it has no leading trivia), so start with an empty line + '', //0 + 'sub greet(name as string, age as integer)', //1 + 'end sub', //2 + 'sub main() : greet("bob", 42) : end sub' //3 + ].join('\n')); + program.validate(); + expectZeroDiagnostics(program); + //sub main() : greet(|"bob", 42) : end sub + expectSignature('source/main.bs', 3, 19, greetLabel, 0); + //sub main() : greet("bob",| 42) : end sub + expectSignature('source/main.bs', 3, 25, greetLabel, 1); + //sub main() : greet("bob", 42|) : end sub + expectSignature('source/main.bs', 3, 28, greetLabel, 1); + //sub main() : greet("bob", 42) : end sub| + expectNoSignature('source/main.bs', 3, 39); + }); + + it('computes the exact signature label when the declaration contains an emoji and uses CRLF line endings', () => { + program.setFile('source/main.bs', [ + 'sub main()', //0 + ' greet("bob", 42)', //1 + 'end sub', //2 + 'sub greet(name = "😀" as string, age = 1 as integer)', //3 + ' print name', //4 + 'end sub' //5 + ].join('\r\n')); + program.validate(); + // greet("bob",| 42) + expectSignature('source/main.bs', 1, 16, 'sub greet(name = "😀" as string, age = 1 as integer)', 1); + }); + + it('computes the exact signature label for a declaration with an empty body on the last line of a file', () => { + program.setFile('source/main.bs', [ + 'sub main()', //0 + ' greet("bob", 42)', //1 + 'end sub', //2 + 'sub greet(name as string, age as integer) : end sub' //3 + ].join('\n')); + program.validate(); + // greet("bob",| 42) + expectSignature('source/main.bs', 1, 16, greetLabel, 1); + }); + }); }); describe('plugins', () => { diff --git a/src/astUtils/CachedLookups.ts b/src/astUtils/CachedLookups.ts index 55eb45e6e..913a7597f 100644 --- a/src/astUtils/CachedLookups.ts +++ b/src/astUtils/CachedLookups.ts @@ -269,7 +269,7 @@ export class CachedLookups { node = node.obj; } else { //some expression we don't understand. log it and quit the loop - this.file.program.logger.debug('Encountered unknown expression while calculating function expression chain', node.kind, node.location); + this.file.program.logger.debug('Encountered unknown expression while calculating function expression chain', node.kind, util.getLocation(node)); break; } } diff --git a/src/astUtils/creators.spec.ts b/src/astUtils/creators.spec.ts index 644c8674b..d7223e239 100644 --- a/src/astUtils/creators.spec.ts +++ b/src/astUtils/creators.spec.ts @@ -1,5 +1,10 @@ import { expect } from '../chai-config.spec'; -import { createStringLiteral } 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', () => { @@ -18,4 +23,48 @@ describe('creators', () => { expect(createStringLiteral('hello world"').tokens.value.text).to.equal('hello world"'); }); }); + + describe('position argument', () => { + it('copies the bounds of a locatable', () => { + const original = Lexer.scan('\n abc').tokens[1]; + const token = createToken(TokenKind.Identifier, 'xyz', original); + expect(token.sourceInfo).to.equal(original.sourceInfo); + expect(util.getLocation(token)).to.eql(util.getLocation(original)); + }); + + it('accepts a location from util.setLocation', () => { + const sourceInfo = Lexer.scan('\n\n abc', { srcPath: 'source/main.brs' }).tokens[0].sourceInfo; + const location = util.createLocation(2, 4, 2, 7, sourceInfo.uri); + const locatable = util.setLocation({} as Locatable, location, sourceInfo); + expect(util.getLocation(createToken(TokenKind.Identifier, 'xyz', locatable))).to.eql(location); + expect(util.getLocation(createIdentifier('xyz', locatable))).to.eql(location); + expect(util.getLocation(createVariableExpression('xyz', locatable))).to.eql(location); + expect(util.getLocation(createStringLiteral('xyz', locatable))).to.eql(location); + }); + + 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; + }); + }); }); diff --git a/src/astUtils/creators.ts b/src/astUtils/creators.ts index a55e697e5..c12ebdf04 100644 --- a/src/astUtils/creators.ts +++ b/src/astUtils/creators.ts @@ -1,11 +1,13 @@ import type { Location } from 'vscode-languageserver'; -import type { Identifier, Token } from '../lexer/Token'; +import type { Identifier, Locatable, Token } from '../lexer/Token'; import type { SGToken } from '../parser/SGTypes'; import { SGAttribute, SGComponent, SGInterface, SGInterfaceField, SGInterfaceFunction, SGScript } from '../parser/SGTypes'; import { TokenKind } from '../lexer/TokenKind'; 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]: '`', @@ -78,37 +80,48 @@ const tokenDefaults = { [TokenKind.Whitespace]: ' ' }; -export function createToken(kind: T, text?: string, location?: Location): Token & { kind: T } { - return { - kind: kind, - text: text ?? tokenDefaults[kind as string] ?? kind.toString().toLowerCase(), - isReserved: !text || text === kind.toString(), - location: location, - leadingTrivia: [] - }; +/** + * 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(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(), + !text || text === kind.toString(), + locatable?.startPosition, + locatable?.endPosition, + locatable?.sourceInfo, + [] + ) as Token as Token & { kind: T }; } -export function createIdentifier(name: string, location?: Location): Identifier { - return { - kind: TokenKind.Identifier, - text: name, - isReserved: false, - location: location, - leadingTrivia: [] - }; +export function createIdentifier(name: string, position?: CreatorPosition): Identifier { + const locatable = util.toLocatable(position); + return new TokenObject( + TokenKind.Identifier, + name, + false, + locatable?.startPosition, + locatable?.endPosition, + locatable?.sourceInfo, + [] + ) as Identifier; } -export function createVariableExpression(ident: string, location?: Location): VariableExpression { - return new VariableExpression({ name: createToken(TokenKind.Identifier, ident, location) }); +export function createVariableExpression(ident: string, locatable?: CreatorPosition): VariableExpression { + return new VariableExpression({ name: createToken(TokenKind.Identifier, ident, locatable) }); } -export function createDottedIdentifier(path: string[], location?: Location): DottedGetExpression { +export function createDottedIdentifier(path: string[], locatable?: CreatorPosition): DottedGetExpression { const ident = path.pop(); - const obj = path.length > 1 ? createDottedIdentifier(path, location) : createVariableExpression(path[0], location); + const obj = path.length > 1 ? createDottedIdentifier(path, locatable) : createVariableExpression(path[0], locatable); return new DottedGetExpression({ obj: obj, - name: createToken(TokenKind.Identifier, ident, location), - dot: createToken(TokenKind.Dot, '.', location) + name: createToken(TokenKind.Identifier, ident, locatable), + dot: createToken(TokenKind.Dot, '.', locatable) }); } @@ -117,30 +130,30 @@ export function createDottedIdentifier(path: string[], location?: Location): Dot * 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, location?: Location) { +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, location) }); + return new LiteralExpression({ value: createToken(TokenKind.StringLiteral, value, locatable) }); } -export function createIntegerLiteral(value: string, location?: Location) { - return new LiteralExpression({ value: createToken(TokenKind.IntegerLiteral, value, location) }); +export function createIntegerLiteral(value: string, locatable?: CreatorPosition) { + return new LiteralExpression({ value: createToken(TokenKind.IntegerLiteral, value, locatable) }); } -export function createFloatLiteral(value: string, location?: Location) { - return new LiteralExpression({ value: createToken(TokenKind.FloatLiteral, value, location) }); +export function createFloatLiteral(value: string, locatable?: CreatorPosition) { + return new LiteralExpression({ value: createToken(TokenKind.FloatLiteral, value, locatable) }); } -export function createDoubleLiteral(value: string, location?: Location) { - return new LiteralExpression({ value: createToken(TokenKind.DoubleLiteral, value, location) }); +export function createDoubleLiteral(value: string, locatable?: CreatorPosition) { + return new LiteralExpression({ value: createToken(TokenKind.DoubleLiteral, value, locatable) }); } -export function createLongIntegerLiteral(value: string, location?: Location) { - return new LiteralExpression({ value: createToken(TokenKind.LongIntegerLiteral, value, location) }); +export function createLongIntegerLiteral(value: string, locatable?: CreatorPosition) { + return new LiteralExpression({ value: createToken(TokenKind.LongIntegerLiteral, value, locatable) }); } -export function createInvalidLiteral(value?: string, location?: Location) { - return new LiteralExpression({ value: createToken(TokenKind.Invalid, value, location) }); +export function createInvalidLiteral(value?: string, locatable?: CreatorPosition) { + return new LiteralExpression({ value: createToken(TokenKind.Invalid, value, locatable) }); } -export function createBooleanLiteral(value: string, location?: Location) { - return new LiteralExpression({ value: createToken(value === 'true' ? TokenKind.True : TokenKind.False, value, location) }); +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) { return new FunctionExpression({ diff --git a/src/astUtils/reflection.spec.ts b/src/astUtils/reflection.spec.ts index 21a0d3548..3d47d1653 100644 --- a/src/astUtils/reflection.spec.ts +++ b/src/astUtils/reflection.spec.ts @@ -189,8 +189,11 @@ describe('reflection', () => { const charCode: Token & { charCode: number } = { kind: TokenKind.EscapedCharCodeLiteral, text: '0', - location: undefined, isReserved: false, + startPosition: undefined, + endPosition: undefined, + sourceInfo: undefined, + location: undefined, charCode: 0, leadingTrivia: [] }; diff --git a/src/bscPlugin/CallExpressionInfo.ts b/src/bscPlugin/CallExpressionInfo.ts index 6a563cb78..4d8170228 100644 --- a/src/bscPlugin/CallExpressionInfo.ts +++ b/src/bscPlugin/CallExpressionInfo.ts @@ -96,7 +96,7 @@ export class CallExpressionInfo { if (!this.callExpression) { return false; } - let boundingRange = util.createBoundingRange(this.callExpression.tokens.openingParen?.location, this.callExpression.tokens.closingParen?.location); + let boundingRange = util.createBoundingRange(util.getLocation(this.callExpression.tokens.openingParen), util.getLocation(this.callExpression.tokens.closingParen)); return util.rangeContains(boundingRange, this.position); } @@ -159,7 +159,7 @@ export class CallExpressionInfo { } for (let i = this.callExpression.args.length - 1; i > -1; i--) { let argExpression = this.callExpression.args[i]; - let comparison = util.comparePositionToRange(this.position, argExpression.location?.range); + let comparison = util.comparePositionToRange(this.position, util.getLocation(argExpression)?.range); if (comparison >= 0) { return i + comparison; } diff --git a/src/bscPlugin/SignatureHelpUtil.ts b/src/bscPlugin/SignatureHelpUtil.ts index d2075472f..697310cf9 100644 --- a/src/bscPlugin/SignatureHelpUtil.ts +++ b/src/bscPlugin/SignatureHelpUtil.ts @@ -75,7 +75,7 @@ export class SignatureHelpUtil { return undefined; } const func = statement.func; - const funcStartPosition = func.location?.range.start; + const funcStartPosition = util.getLocation(func)?.range.start; // Get function comments in reverse order const trivia = util.concatAnnotationLeadingTrivia(func).reverse(); @@ -85,7 +85,7 @@ export class SignatureHelpUtil { if (!currentToken) { break; } - if (currentToken.location?.range.start.line + 1 < funcStartPosition.line) { + if (util.getLocation(currentToken)?.range.start.line + 1 < funcStartPosition.line) { if (functionComments.length === 0) { break; } @@ -121,7 +121,7 @@ export class SignatureHelpUtil { key += param.tokens.name.text; } - let label = util.getTextForRange(lines, util.createRangeFromPositions(func.tokens.functionType?.location?.range.start, func.body.location?.range.start)).trim(); + let label = util.getTextForRange(lines, util.createRangeFromPositions(util.getLocation(func.tokens.functionType)?.range.start, util.getLocation(func.body)?.range.start)).trim(); if (namespaceName) { label = label.replace(/^(sub | function )/gim, `$1${namespaceName}.`); } diff --git a/src/bscPlugin/codeActions/CodeActionsProcessor.ts b/src/bscPlugin/codeActions/CodeActionsProcessor.ts index 6fd3c861d..9bf1eb285 100644 --- a/src/bscPlugin/codeActions/CodeActionsProcessor.ts +++ b/src/bscPlugin/codeActions/CodeActionsProcessor.ts @@ -264,7 +264,7 @@ export class CodeActionsProcessor { for (const commentToken of commentTokens) { //`tokens` is an `any[]` union of brs/xml token shapes (see declaration above) //eslint-disable-next-line @typescript-eslint/no-unsafe-argument - const tokenRange: Range = isXml ? rangeFromTokenValue(commentToken) : commentToken.location.range; + const tokenRange: Range = isXml ? rangeFromTokenValue(commentToken) : util.getLocation(commentToken).range; const tokenText: string = isXml ? commentToken.image : commentToken.text; const parsed = parseDisableComment(tokenText); if (!parsed) { @@ -353,7 +353,7 @@ export class CodeActionsProcessor { // eslint-disable-next-line @typescript-eslint/dot-notation const importStatements = (this.event.file as BrsFile)['_cachedLookups'].importStatements; //find the position of the first import statement, or the top of the file if there is none - const insertPosition = importStatements[importStatements.length - 1]?.tokens.import.location.range?.start ?? util.createPosition(0, 0); + const insertPosition = util.getLocation(importStatements[importStatements.length - 1]?.tokens.import)?.range?.start ?? util.createPosition(0, 0); //find all files that reference this function for (const file of files) { @@ -410,7 +410,7 @@ export class CodeActionsProcessor { const file = this.event.file; // eslint-disable-next-line @typescript-eslint/dot-notation const importStatements = file['_cachedLookups'].importStatements; - const insertPosition = importStatements[importStatements.length - 1]?.tokens.import.location.range?.start ?? util.createPosition(0, 0); + const insertPosition = util.getLocation(importStatements[importStatements.length - 1]?.tokens.import)?.range?.start ?? util.createPosition(0, 0); const changes: InsertChange[] = []; const addedPaths = new Set(); @@ -542,9 +542,9 @@ export class CodeActionsProcessor { kind: CodeActionKind.QuickFix, changes: [ //function - { type: 'replace', filePath: this.event.file.srcPath, range: func.tokens.functionType.location.range, newText: functionTypeText }, + { type: 'replace', filePath: this.event.file.srcPath, range: util.getLocation(func.tokens.functionType).range, newText: functionTypeText }, //end function - { type: 'replace', filePath: this.event.file.srcPath, range: func.tokens.endFunctionType.location.range, newText: endFunctionTypeText } + { type: 'replace', filePath: this.event.file.srcPath, range: util.getLocation(func.tokens.endFunctionType).range, newText: endFunctionTypeText } ] }) ); @@ -606,7 +606,7 @@ export class CodeActionsProcessor { if (!fn) { continue; } - const fnKey = `${fn.location.range.start.line}:${fn.location.range.start.character}`; + const fnKey = `${util.getLocation(fn).range.start.line}:${util.getLocation(fn).range.start.character}`; if (seenFunctions.has(fnKey)) { continue; } @@ -618,7 +618,7 @@ export class CodeActionsProcessor { addVoidChanges.push({ type: 'insert', filePath: this.event.file.srcPath, - position: fn.tokens.rightParen.location.range.end, + position: util.getLocation(fn.tokens.rightParen).range.end, newText: ` ${asText ?? 'as'} ${voidText ?? 'void'}` }); } @@ -647,8 +647,8 @@ export class CodeActionsProcessor { diagnostics: [diagnostics[0]], kind: CodeActionKind.QuickFix, changes: [ - { type: 'replace', filePath: file.srcPath, range: func.tokens.functionType.location.range, newText: subText ?? 'sub' }, - { type: 'replace', filePath: file.srcPath, range: func.tokens.endFunctionType.location.range, newText: endSubText ?? 'end sub' } + { type: 'replace', filePath: file.srcPath, range: util.getLocation(func.tokens.functionType).range, newText: subText ?? 'sub' }, + { type: 'replace', filePath: file.srcPath, range: util.getLocation(func.tokens.endFunctionType).range, newText: endSubText ?? 'end sub' } ] }) ); @@ -723,10 +723,10 @@ export class CodeActionsProcessor { file.ast.walk((node) => { if ( isMethodStatement(node) && - node.location?.range?.start?.line === diagnostic.location.range.start.line && - node.location?.range?.start?.character === diagnostic.location.range.start.character + util.getLocation(node)?.range?.start?.line === diagnostic.location.range.start.line && + util.getLocation(node)?.range?.start?.character === diagnostic.location.range.start.character ) { - insertPosition = (node as MethodStatement).func.tokens.functionType?.location?.range?.start; + insertPosition = util.getLocation((node as MethodStatement).func.tokens.functionType)?.range?.start; } }, { walkMode: WalkMode.visitStatementsRecursive }); @@ -824,10 +824,10 @@ export class CodeActionsProcessor { filePath: this.event.file.srcPath, // )| as | range: util.createRange( - func.tokens.rightParen.location.range.start.line, - func.tokens.rightParen.location.range.start.character + 1, - func.returnTypeExpression.location.range.end.line, - func.returnTypeExpression.location.range.end.character + util.getLocation(func.tokens.rightParen).range.start.line, + util.getLocation(func.tokens.rightParen).range.start.character + 1, + util.getLocation(func.returnTypeExpression).range.end.line, + util.getLocation(func.returnTypeExpression).range.end.character ) }; } diff --git a/src/bscPlugin/completions/CompletionsProcessor.spec.ts b/src/bscPlugin/completions/CompletionsProcessor.spec.ts index 0fd3f745a..67b10f926 100644 --- a/src/bscPlugin/completions/CompletionsProcessor.spec.ts +++ b/src/bscPlugin/completions/CompletionsProcessor.spec.ts @@ -2772,4 +2772,292 @@ describe('CompletionsProcessor', () => { }]); }); }); + + describe('location boundaries', () => { + //every test in this block builds its source from an explicit list of lines (no indentation stripping) + //so that the line/character values below can be counted by hand + const personClassLines = [ + 'class Person', //0 + ' name as string', //1 + ' sub speak()', //2 + ' end sub', //3 + 'end class' //4 + ]; + + function getLabels(srcPath: string, line: number, character: number) { + return program.getCompletions(srcPath, util.createPosition(line, character)).map(x => x.label); + } + + it('finds members directly after a dot', () => { + program.setFile('source/main.bs', [ + ...personClassLines, + 'sub main()', //5 + ' p = new Person()', //6 + ' p.', //7 + 'end sub' //8 + ].join('\n')); + program.validate(); + // p.| + const labels = getLabels('source/main.bs', 7, 6); + expect(labels).to.include.members(['name', 'speak']); + expect(labels).not.to.include('main'); + }); + + it('finds members when the cursor is in the middle of an identifier after a dot', () => { + program.setFile('source/main.bs', [ + ...personClassLines, + 'sub main()', //5 + ' p = new Person()', //6 + ' p.spe', //7 + 'end sub' //8 + ].join('\n')); + program.validate(); + // p.s|pe + const labels = getLabels('source/main.bs', 7, 7); + expect(labels).to.include.members(['name', 'speak']); + expect(labels).not.to.include('main'); + }); + + it('finds locals when the cursor is in the middle of an identifier', () => { + program.setFile('source/main.brs', [ + 'sub main()', //0 + ' person = 1', //1 + ' pers', //2 + 'end sub' //3 + ].join('\n')); + program.validate(); + // pe|rs + expect(getLabels('source/main.brs', 2, 6)).to.include.members(['person', 'main']); + }); + + it('finds locals for an identifier at column 0 of a line', () => { + program.setFile('source/main.brs', [ + 'sub main()', //0 + ' person = 1', //1 + 'pe', //2 + 'end sub' //3 + ].join('\n')); + program.validate(); + //|pe + expect(getLabels('source/main.brs', 2, 0)).to.include.members(['person', 'main']); + //pe| + expect(getLabels('source/main.brs', 2, 2)).to.include.members(['person', 'main']); + }); + + it('finds locals at column 0 of an empty line', () => { + program.setFile('source/main.brs', [ + 'sub main()', //0 + ' person = 1', //1 + '', //2 + ' print person', //3 + 'end sub' //4 + ].join('\n')); + program.validate(); + expect(getLabels('source/main.brs', 2, 0)).to.include.members(['person', 'main']); + }); + + it('finds parameters inside an empty function body', () => { + program.setFile('source/main.bs', [ + 'sub main(person as string)', //0 + '', //1 + 'end sub' //2 + ].join('\n')); + program.validate(); + expect(getLabels('source/main.bs', 1, 0)).to.include.members(['person', 'main']); + }); + + it('finds parameters inside an empty, whitespace-only function body', () => { + program.setFile('source/main.bs', [ + 'sub main(person as string)', //0 + ' ', //1 + 'end sub' //2 + ].join('\n')); + program.validate(); + // | + expect(getLabels('source/main.bs', 1, 4)).to.include.members(['person', 'main']); + // | + expect(getLabels('source/main.bs', 1, 2)).to.include.members(['person', 'main']); + }); + + it('finds parameters inside a function body with no lines between the signature and `end sub`', () => { + program.setFile('source/main.bs', [ + 'sub main(person as string)', //0 + 'end sub' //1 + ].join('\n')); + program.validate(); + //sub main(person as string)| + expect(getLabels('source/main.bs', 0, 26)).to.include.members(['person', 'main']); + }); + + it('finds members after a dot when a surrogate-pair emoji precedes the target on the same line', () => { + program.setFile('source/main.bs', [ + ...personClassLines, + 'sub main()', //5 + ' p = new Person()', //6 + //each emoji is 2 UTF-16 code units, so the dot is at character 17 + ' x = "😀😀": p.', //7 + 'end sub' //8 + ].join('\n')); + program.validate(); + // x = "😀😀": p.| + const labels = getLabels('source/main.bs', 7, 18); + expect(labels).to.include.members(['name', 'speak']); + expect(labels).not.to.include('main'); + }); + + it('finds members after a dot in a file with CRLF line endings', () => { + program.setFile('source/main.bs', [ + ...personClassLines, + 'sub main()', //5 + ' p = new Person()', //6 + ' p.', //7 + 'end sub' //8 + ].join('\r\n')); + program.validate(); + // p.| + const labels = getLabels('source/main.bs', 7, 6); + expect(labels).to.include.members(['name', 'speak']); + expect(labels).not.to.include('main'); + }); + + it('finds locals mid-identifier in a file with CRLF line endings', () => { + program.setFile('source/main.brs', [ + 'sub main()', //0 + ' person = 1', //1 + ' pers', //2 + 'end sub' //3 + ].join('\r\n')); + program.validate(); + // pe|rs + expect(getLabels('source/main.brs', 2, 6)).to.include.members(['person', 'main']); + }); + + it('finds completions inside `${}` on a later line of a multi-line template string, and after the template string', () => { + program.setFile('source/main.bs', [ + ...personClassLines, + 'sub main()', //5 + ' name = "bob"', //6 + ' p = new Person()', //7 + // eslint-disable-next-line no-template-curly-in-string + ' msg = `hello ${name}', //8 + // eslint-disable-next-line no-template-curly-in-string + 'and ${na} again`', //9 + ' p.', //10 + 'end sub' //11 + ].join('\n')); + program.validate(); + //and ${n|a} again` + expect(getLabels('source/main.bs', 9, 7)).to.include.members(['name', 'p', 'main']); + // p.| + const labels = getLabels('source/main.bs', 10, 6); + expect(labels).to.include.members(['name', 'speak']); + expect(labels).not.to.include('main'); + }); + + it('finds members on the last line of a file with no trailing newline', () => { + program.setFile('source/main.bs', [ + ...personClassLines, + 'sub main(p as Person) : p.speak() : end sub' //5 + ].join('\n')); + program.validate(); + //sub main(p as Person) : p.|speak() : end sub + const labels = getLabels('source/main.bs', 5, 26); + expect(labels).to.include.members(['name', 'speak']); + expect(labels).not.to.include('main'); + }); + + it('returns exact pkg path textEdit ranges for a string literal', () => { + program.setFile('source/main.brs', [ + 'sub main()', //0 + ' print "pkg:"', //1 + 'end sub' //2 + ].join('\n')); + program.validate(); + // print "pk|g:" + const completions = program.getCompletions('source/main.brs', util.createPosition(1, 13)); + expect(completions.map(x => x.label)).to.eql(['pkg:/source/main.brs']); + //replaces the text between the quotes + expect(completions[0].textEdit).to.eql({ + newText: 'pkg:/source/main.brs', + range: util.createRange(1, 11, 1, 15) + }); + }); + + it('returns exact pkg path textEdit ranges when a surrogate-pair emoji precedes the string on the same line', () => { + program.setFile('source/main.brs', [ + 'sub main()', //0 + //each emoji is 2 UTF-16 code units + ' x = "😀" + "pkg:"', //1 + 'end sub' //2 + ].join('\n')); + program.validate(); + // x = "😀" + "pk|g:" + const completions = program.getCompletions('source/main.brs', util.createPosition(1, 18)); + expect(completions.map(x => x.label)).to.eql(['pkg:/source/main.brs']); + expect(completions[0].textEdit).to.eql({ + newText: 'pkg:/source/main.brs', + range: util.createRange(1, 16, 1, 20) + }); + }); + + it('returns exact pkg path textEdit ranges in a file with CRLF line endings', () => { + program.setFile('source/main.brs', [ + 'sub main()', //0 + ` ' 😀 comment`, //1 + ' print "pkg:"', //2 + 'end sub' //3 + ].join('\r\n')); + program.validate(); + // print "pk|g:" + const completions = program.getCompletions('source/main.brs', util.createPosition(2, 13)); + expect(completions.map(x => x.label)).to.eql(['pkg:/source/main.brs']); + expect(completions[0].textEdit).to.eql({ + newText: 'pkg:/source/main.brs', + range: util.createRange(2, 11, 2, 15) + }); + }); + + it('returns exact pkg path textEdit ranges on a later line of a multi-line template string', () => { + program.setFile('source/main.bs', [ + 'sub main()', //0 + // eslint-disable-next-line no-template-curly-in-string + ' x = `line one ${1}', //1 + 'pkg:`', //2 + 'end sub' //3 + ].join('\n')); + program.validate(); + //pk|g:` + const completions = program.getCompletions('source/main.bs', util.createPosition(2, 2)); + //the pkg path is the transpiled destination, so `.bs` becomes `.brs` + expect(completions.map(x => x.label)).to.eql(['pkg:/source/main.brs']); + expect(completions[0].textEdit).to.eql({ + newText: 'pkg:/source/main.brs', + range: util.createRange(2, 0, 2, 4) + }); + }); + + it('returns exact import path textEdit ranges in a file with CRLF line endings', () => { + program.setFile('source/lib.bs', [ + 'sub libFunc()', + 'end sub' + ].join('\n')); + program.setFile('source/main.bs', [ + `' 😀 header`, //0 + 'import "lib.bs"', //1 + 'sub main()', //2 + 'end sub' //3 + ].join('\r\n')); + program.validate(); + //import "li|b.bs" + const completions = program.getCompletions('source/main.bs', util.createPosition(1, 10)); + expect(completions.map(x => x.label).sort()).to.eql(['lib.bs', 'pkg:/source/lib.bs']); + //the range covers the path without its quotes (the import statement trims them from the token) + for (const completion of completions) { + expect(completion.textEdit).to.eql({ + newText: completion.label, + range: util.createRange(1, 8, 1, 14) + }); + } + }); + }); }); diff --git a/src/bscPlugin/completions/CompletionsProcessor.ts b/src/bscPlugin/completions/CompletionsProcessor.ts index 4598543b5..19c8bebd3 100644 --- a/src/bscPlugin/completions/CompletionsProcessor.ts +++ b/src/bscPlugin/completions/CompletionsProcessor.ts @@ -178,7 +178,7 @@ export class CompletionsProcessor { public getBrsFileCompletions(position: Position, file: BrsFile, scopesToProcess: Scope[]): { global: CompletionItem[]; scoped: CompletionItem[] } { let result = [] as CompletionItem[]; const currentTokenByFilePosition = file.getTokenAt(position); - const currentToken = currentTokenByFilePosition ?? file.getTokenAt(file.getClosestExpression(position)?.location?.range.start); + const currentToken = currentTokenByFilePosition ?? file.getTokenAt(util.getLocation(file.getClosestExpression(position))?.range.start); const emptyResult = { global: [], scoped: [] }; @@ -206,12 +206,12 @@ export class CompletionsProcessor { if (this.isTokenAdjacentTo(file, currentToken, TokenKind.Dot)) { const dotToken = this.getAdjacentToken(file, currentToken, TokenKind.Dot); beforeDotToken = file.getTokenBefore(dotToken); - expression = file.getClosestExpression(beforeDotToken?.location?.range.end); + expression = file.getClosestExpression(util.getLocation(beforeDotToken)?.range.end); shouldLookForMembers = true; } else if (this.isTokenAdjacentTo(file, currentToken, TokenKind.Callfunc)) { const dotToken = this.getAdjacentToken(file, currentToken, TokenKind.Callfunc); beforeDotToken = file.getTokenBefore(dotToken); - expression = file.getClosestExpression(beforeDotToken?.location?.range.end); + expression = file.getClosestExpression(util.getLocation(beforeDotToken)?.range.end); shouldLookForCallFuncMembers = true; } else if (this.isTokenAdjacentTo(file, currentToken, TokenKind.As)) { if (file.parseMode === ParseMode.BrightScript) { @@ -239,7 +239,7 @@ export class CompletionsProcessor { return emptyResult; } - const tokenBefore = file.getTokenBefore(file.getClosestToken(expression.location?.range?.start)); + const tokenBefore = file.getTokenBefore(file.getClosestToken(util.getLocation(expression)?.range?.start)); // helper to check get correct symbol tables for look ups function getSymbolTableForLookups() { @@ -543,12 +543,12 @@ export class CompletionsProcessor { label: pkgPath, textEdit: TextEdit.replace( util.createRange( - currentToken.location?.range.start.line, + util.getLocation(currentToken)?.range.start.line, //+1 to step past the opening quote - currentToken.location?.range.start.character + (openingQuote ? 1 : 0), - currentToken.location?.range.end.line, + util.getLocation(currentToken)?.range.start.character + (openingQuote ? 1 : 0), + util.getLocation(currentToken)?.range.end.line, //-1 to exclude the closing quotemark (or the end character if there is no closing quotemark) - currentToken.location?.range.end.character + (currentToken.text.endsWith('"') ? -1 : 0) + util.getLocation(currentToken)?.range.end.character + (currentToken.text.endsWith('"') ? -1 : 0) ), pkgPath ), @@ -658,7 +658,7 @@ export class CompletionsProcessor { const nextNonComment = file.getNextTokenByPredicate(currentToken, (t: Token) => !AllowedTriviaTokens.includes(t.kind), 1); const firstComment = nextNonComment?.leadingTrivia?.find(t => t.kind === TokenKind.Comment); - if (firstComment && util.comparePosition(position, firstComment?.location?.range.start) >= 0) { + if (firstComment && util.comparePosition(position, util.getLocation(firstComment)?.range.start) >= 0) { return true; } return false; diff --git a/src/bscPlugin/definition/DefinitionProvider.spec.ts b/src/bscPlugin/definition/DefinitionProvider.spec.ts index b0e7de516..2aa0ec7b0 100644 --- a/src/bscPlugin/definition/DefinitionProvider.spec.ts +++ b/src/bscPlugin/definition/DefinitionProvider.spec.ts @@ -751,4 +751,217 @@ describe('DefinitionProvider', () => { ); }); }); + + describe('location boundaries', () => { + //NOTE: these tests intentionally only use public `Program` APIs (Position in, Location out) so they + //can be run against any implementation of token/node locations. All expected values are hand-counted. + + /** + * Join lines with the given line ending. No trailing newline is added + */ + function lines(eol: string, ...items: string[]) { + return items.join(eol); + } + + function definitionAt(srcPath: string, line: number, character: number) { + return program.getDefinition(srcPath, util.createPosition(line, character)); + } + + function loc(srcPath: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) { + return { + uri: util.pathToUri(srcPath), + range: util.createRange(startLine, startCharacter, endLine, endCharacter) + }; + } + + it('finds the identifier at its first character, middle, and immediately after its last character', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + ' alpha = 1', + ' print alpha', + 'end sub' + )); + program.validate(); + const expected = [loc(file.srcPath, 1, 4, 1, 9)]; + //print |alpha + expect(definitionAt(file.srcPath, 2, 10)).to.eql(expected); + //print al|pha + expect(definitionAt(file.srcPath, 2, 12)).to.eql(expected); + //print alpha| (the end of a range is inclusive) + expect(definitionAt(file.srcPath, 2, 15)).to.eql(expected); + + //print| alpha (immediately after `print`, which has no definition) + expect(definitionAt(file.srcPath, 2, 9)).to.eql([]); + //leading whitespace + expect(definitionAt(file.srcPath, 2, 2)).to.eql([]); + }); + + it('finds identifiers at column 0 of a line and at the end of a line', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + 'beta = 1', + 'print beta', + 'end sub' + )); + program.validate(); + const expected = [loc(file.srcPath, 1, 0, 1, 4)]; + //|beta = 1 + expect(definitionAt(file.srcPath, 1, 0)).to.eql(expected); + //print beta| + expect(definitionAt(file.srcPath, 2, 10)).to.eql(expected); + //print |beta + expect(definitionAt(file.srcPath, 2, 6)).to.eql(expected); + }); + + it('handles CRLF line endings and returns the uri of the other file for cross-file definitions', () => { + const libFile = program.setFile('source/lib.brs', lines('\r\n', + `' helpers`, + 'function helper()', + ' return 1', + 'end function' + )); + const mainFile = program.setFile('source/main.brs', lines('\r\n', + 'sub main()', + ' helper()', + 'helper()', + ' x = helper()', + 'end sub' + )); + program.validate(); + const expected = [loc(libFile.srcPath, 1, 0, 3, 12)]; + //|helper() + expect(definitionAt(mainFile.srcPath, 1, 4)).to.eql(expected); + //hel|per() + expect(definitionAt(mainFile.srcPath, 1, 7)).to.eql(expected); + //helper|() + expect(definitionAt(mainFile.srcPath, 1, 10)).to.eql(expected); + //column 0 after a `\r\n` + expect(definitionAt(mainFile.srcPath, 2, 0)).to.eql(expected); + //x = helper|() + expect(definitionAt(mainFile.srcPath, 3, 14)).to.eql(expected); + }); + + it('uses utf-16 code units for characters after a surrogate pair emoji on the same line', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + ' name = "x"', + ' print "😀😀" + name', + ' x = "😀": y = 1', + ' print y', + 'end sub' + )); + program.validate(); + //each emoji is 2 utf-16 code units, so `name` starts at 19 (not 17) + const expected = [loc(file.srcPath, 1, 4, 1, 8)]; + expect(definitionAt(file.srcPath, 2, 19)).to.eql(expected); + expect(definitionAt(file.srcPath, 2, 21)).to.eql(expected); + expect(definitionAt(file.srcPath, 2, 23)).to.eql(expected); + + //the declaration of `y` comes after an emoji on its line + expect(definitionAt(file.srcPath, 4, 10)).to.eql([loc(file.srcPath, 3, 14, 3, 15)]); + //y| = 1 + expect(definitionAt(file.srcPath, 3, 15)).to.eql([loc(file.srcPath, 3, 14, 3, 15)]); + }); + + it('counts lines through multi-line template strings', () => { + const file = program.setFile('source/main.bs', lines('\n', + 'sub main()', + ' name = "bob"', + /* eslint-disable no-template-curly-in-string */ + ' msg = `hello ${name}', + 'line two ${name}', + 'and ${name} three`', + /* eslint-enable no-template-curly-in-string */ + ' after = msg', + ' print after + name', + 'end sub' + )); + program.validate(); + const nameDeclaration = [loc(file.srcPath, 1, 4, 1, 8)]; + //inside `${}` on later lines of the template string + expect(definitionAt(file.srcPath, 3, 13)).to.eql(nameDeclaration); + expect(definitionAt(file.srcPath, 3, 15)).to.eql(nameDeclaration); + expect(definitionAt(file.srcPath, 4, 8)).to.eql(nameDeclaration); + expect(definitionAt(file.srcPath, 4, 10)).to.eql(nameDeclaration); + + //after the template string (both the declaration and the usage come after it) + expect(definitionAt(file.srcPath, 6, 10)).to.eql([loc(file.srcPath, 5, 4, 5, 9)]); + expect(definitionAt(file.srcPath, 6, 15)).to.eql([loc(file.srcPath, 5, 4, 5, 9)]); + //after = msg| + expect(definitionAt(file.srcPath, 5, 15)).to.eql([loc(file.srcPath, 2, 4, 2, 7)]); + //print after + name| + expect(definitionAt(file.srcPath, 6, 22)).to.eql(nameDeclaration); + }); + + it('finds items on the last line of a file with no trailing newline', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + ' last()', + 'end sub', + 'sub last(): value = 1: print value: end sub' + )); + program.validate(); + //print value|: + expect(definitionAt(file.srcPath, 3, 34)).to.eql([loc(file.srcPath, 3, 12, 3, 17)]); + //print |value + expect(definitionAt(file.srcPath, 3, 29)).to.eql([loc(file.srcPath, 3, 12, 3, 17)]); + //the function statement ends at the very end of the file + expect(definitionAt(file.srcPath, 1, 4)).to.eql([loc(file.srcPath, 3, 0, 3, 43)]); + }); + + it('finds symbols in namespaces, classes, and empty function bodies across files', () => { + const libFile = program.setFile('source/lib.bs', lines('\n', + 'namespace alpha.beta', + ' function noop()', + ' end function', + ' class Widget', + ' sub render()', + ' end sub', + ' end class', + 'end namespace', + 'sub empty()', + 'end sub' + )); + const mainFile = program.setFile('source/main.bs', lines('\n', + 'sub main()', + ' alpha.beta.noop()', + ' w = new alpha.beta.Widget()', + ' empty()', + 'end sub' + )); + program.validate(); + //new alpha.beta.|Widget() + expect(definitionAt(mainFile.srcPath, 2, 24)).to.eql([loc(libFile.srcPath, 3, 10, 3, 16)]); + //new alpha.beta.Widget|() + expect(definitionAt(mainFile.srcPath, 2, 29)).to.eql([loc(libFile.srcPath, 3, 10, 3, 16)]); + //alpha.beta.noop|() (an empty function inside a namespace) + expect(definitionAt(mainFile.srcPath, 1, 19)).to.eql([loc(libFile.srcPath, 1, 4, 2, 16)]); + //empty|() (an empty function) + expect(definitionAt(mainFile.srcPath, 3, 9)).to.eql([loc(libFile.srcPath, 8, 0, 9, 7)]); + }); + + it('finds namespaced consts and enum members in another file', () => { + const libFile = program.setFile('source/lib.bs', lines('\n', + 'namespace alpha', + ' const PI = 3.14', + ' enum Direction', + ' up = "up"', + ' end enum', + 'end namespace' + )); + const mainFile = program.setFile('source/main.bs', lines('\n', + 'sub main()', + ' print alpha.PI', + ' print alpha.Direction.up', + 'end sub' + )); + program.validate(); + //print alpha.PI| + expect(definitionAt(mainFile.srcPath, 1, 18)).to.eql([loc(libFile.srcPath, 1, 10, 1, 12)]); + //print alpha.Direction.up| + expect(definitionAt(mainFile.srcPath, 2, 28)).to.eql([loc(libFile.srcPath, 3, 8, 3, 10)]); + //print alpha.Direc|tion.up + expect(definitionAt(mainFile.srcPath, 2, 21)).to.eql([loc(libFile.srcPath, 2, 9, 2, 18)]); + }); + }); }); diff --git a/src/bscPlugin/definition/DefinitionProvider.ts b/src/bscPlugin/definition/DefinitionProvider.ts index 2b0fc0643..05ae5dcea 100644 --- a/src/bscPlugin/definition/DefinitionProvider.ts +++ b/src/bscPlugin/definition/DefinitionProvider.ts @@ -227,7 +227,7 @@ export class DefinitionProvider { const constant = scope?.getConstFileLink(fullName, containingNamespace); if (constant) { this.event.definitions.push( - constant.item.tokens.name?.location + util.getLocation(constant.item.tokens.name) ); return; } @@ -236,14 +236,14 @@ export class DefinitionProvider { const enumLink = scope.getEnumFileLink(fullName, containingNamespace); if (enumLink) { this.event.definitions.push( - enumLink.item.tokens.name.location + util.getLocation(enumLink.item.tokens.name) ); return; } const enumMemberLink = scope.getEnumMemberFileLink(fullName, containingNamespace); if (enumMemberLink) { this.event.definitions.push( - enumMemberLink.item.tokens.name.location + util.getLocation(enumMemberLink.item.tokens.name) ); return; } @@ -251,7 +251,7 @@ export class DefinitionProvider { const interfaceFileLink = scope.getInterfaceFileLink(fullName, containingNamespace); if (interfaceFileLink) { this.event.definitions.push( - interfaceFileLink.item.tokens.name.location + util.getLocation(interfaceFileLink.item.tokens.name) ); return; } @@ -259,7 +259,7 @@ export class DefinitionProvider { const classFileLink = scope.getClassFileLink(fullName, containingNamespace); if (classFileLink) { this.event.definitions.push( - classFileLink.item.tokens.name.location + util.getLocation(classFileLink.item.tokens.name) ); return; } @@ -268,7 +268,7 @@ export class DefinitionProvider { let textToSearchFor = token.text.toLowerCase(); - const previousToken = file.getTokenAt({ line: token.location?.range.start.line, character: token.location?.range.start.character }); + const previousToken = file.getTokenAt({ line: util.getLocation(token)?.range.start.line, character: util.getLocation(token)?.range.start.character }); if (previousToken?.kind === TokenKind.Callfunc) { for (const scope of this.event.program.getScopes()) { @@ -282,7 +282,7 @@ export class DefinitionProvider { const callable = scope.getAllCallables().find((c) => c.callable.name.toLowerCase() === textToSearchFor); // eslint-disable-line @typescript-eslint/no-loop-func if (callable) { this.event.definitions.push( - util.createLocationFromRange(util.pathToUri((callable.callable.file as BrsFile).srcPath), callable.callable.functionStatement.tokens.name.location?.range) + util.createLocationFromRange(util.pathToUri((callable.callable.file as BrsFile).srcPath), util.getLocation(callable.callable.functionStatement.tokens.name)?.range) ); } } @@ -294,12 +294,12 @@ export class DefinitionProvider { // eslint-disable-next-line @typescript-eslint/dot-notation let classToken = file['getTokenBefore'](token, TokenKind.Class); if (classToken) { - let cs = file.parser.ast.findChild((klass) => isClassStatement(klass) && klass.tokens.class.location?.range === classToken.location?.range); + let cs = file.parser.ast.findChild((klass) => isClassStatement(klass) && klass.tokens.class === classToken); if (cs?.parentClassName) { const nameParts = cs.parentClassName.getNameParts(); let extendedClass = file.getClassFileLink(nameParts[nameParts.length - 1], nameParts.slice(0, -1).join('.')); if (extendedClass) { - this.event.definitions.push(util.createLocationFromRange(util.pathToUri(extendedClass.file.srcPath), extendedClass.item.location?.range)); + this.event.definitions.push(util.createLocationFromRange(util.pathToUri(extendedClass.file.srcPath), util.getLocation(extendedClass.item)?.range)); } } return; @@ -327,10 +327,10 @@ export class DefinitionProvider { pathValue, file.pkgPath, util.createRange( - token.location.range.start.line, - token.location.range.start.character + 1, - token.location.range.end.line, - token.location.range.end.character - 1 + util.getLocation(token).range.start.line, + util.getLocation(token).range.start.character + 1, + util.getLocation(token).range.end.line, + util.getLocation(token).range.end.character - 1 ) ); if (link) { @@ -391,7 +391,7 @@ export class DefinitionProvider { FunctionStatement: (statement: FunctionStatement) => { if (statement.getName(file.parseMode).toLowerCase() === textToSearchFor) { const uri = util.pathToUri(file.srcPath); - this.event.definitions.push(util.createLocationFromRange(uri, statement.location?.range)); + this.event.definitions.push(util.createLocationFromRange(uri, util.getLocation(statement)?.range)); } } }), { @@ -418,7 +418,7 @@ export class DefinitionProvider { const namespaceItemStatementHandler = (statement: ClassStatement | FunctionStatement) => { if (!location && statement.tokens.name.text.toLowerCase() === endName) { const uri = util.pathToUri(file.srcPath); - location = util.createLocationFromRange(uri, statement.location?.range); + location = util.createLocationFromRange(uri, util.getLocation(statement)?.range); } }; diff --git a/src/bscPlugin/hover/HoverProcessor.spec.ts b/src/bscPlugin/hover/HoverProcessor.spec.ts index ba4965d07..b1d3ae68b 100644 --- a/src/bscPlugin/hover/HoverProcessor.spec.ts +++ b/src/bscPlugin/hover/HoverProcessor.spec.ts @@ -1336,4 +1336,288 @@ describe('HoverProcessor', () => { expect(hover?.contents).eql([fence('function (bytes as roByteArray) as integer')]); }); }); + + describe('location boundaries', () => { + //NOTE: these tests intentionally only use public `Program` APIs (Position in, Range out) so they + //can be run against any implementation of token/node locations. All expected values are hand-counted. + + /** + * Join lines with the given line ending. No trailing newline is added + */ + function lines(eol: string, ...items: string[]) { + return items.join(eol); + } + + function hoverAt(srcPath: string, line: number, character: number) { + return program.getHover(srcPath, util.createPosition(line, character)); + } + + it('finds the identifier at its first character, middle, and immediately after its last character', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + ' alpha = 1', + ' print alpha', + 'end sub' + )); + program.validate(); + const expected = [{ + range: util.createRange(2, 10, 2, 15), + contents: [fence('alpha as integer')] + }]; + //print |alpha + expect(hoverAt(file.srcPath, 2, 10)).to.eql(expected); + //print al|pha + expect(hoverAt(file.srcPath, 2, 12)).to.eql(expected); + //print alpha| (the end of a range is inclusive) + expect(hoverAt(file.srcPath, 2, 15)).to.eql(expected); + + //alpha| = 1 (immediately after the declaration, which is followed by whitespace) + expect(hoverAt(file.srcPath, 1, 9)).to.eql([{ + range: util.createRange(1, 4, 1, 9), + contents: [fence('alpha as integer')] + }]); + + //print| alpha (immediately after `print`, which is not hoverable) + expect(hoverAt(file.srcPath, 2, 9)).to.eql([]); + //pri|nt + expect(hoverAt(file.srcPath, 2, 6)).to.eql([]); + //leading whitespace + expect(hoverAt(file.srcPath, 2, 2)).to.eql([]); + }); + + it('finds identifiers at column 0 of a line and at the end of a line', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + 'beta = 1', + 'print beta', + 'end sub' + )); + program.validate(); + //|beta = 1 + expect(hoverAt(file.srcPath, 1, 0)).to.eql([{ + range: util.createRange(1, 0, 1, 4), + contents: [fence('beta as integer')] + }]); + //print beta| + expect(hoverAt(file.srcPath, 2, 10)).to.eql([{ + range: util.createRange(2, 6, 2, 10), + contents: [fence('beta as integer')] + }]); + //|end sub + expect(hoverAt(file.srcPath, 3, 0)).to.eql([{ + range: util.createRange(3, 0, 3, 7), + contents: [fence('sub main() as void')] + }]); + }); + + it('handles CRLF line endings', () => { + const file = program.setFile('source/main.brs', lines('\r\n', + 'sub main()', + ' gamma = 1', + ' print gamma', + ' print gamma + 1', + 'delta = gamma', + 'end sub' + )); + program.validate(); + const gammaLine3 = [{ + range: util.createRange(3, 10, 3, 15), + contents: [fence('gamma as integer')] + }]; + expect(hoverAt(file.srcPath, 3, 10)).to.eql(gammaLine3); + expect(hoverAt(file.srcPath, 3, 13)).to.eql(gammaLine3); + expect(hoverAt(file.srcPath, 3, 15)).to.eql(gammaLine3); + + //end of the line, right before the `\r\n` + expect(hoverAt(file.srcPath, 2, 15)).to.eql([{ + range: util.createRange(2, 10, 2, 15), + contents: [fence('gamma as integer')] + }]); + + //column 0 after a `\r\n` + expect(hoverAt(file.srcPath, 4, 0)).to.eql([{ + range: util.createRange(4, 0, 4, 5), + contents: [fence('delta as integer')] + }]); + //last identifier on the line before the `\r\n` + expect(hoverAt(file.srcPath, 4, 13)).to.eql([{ + range: util.createRange(4, 8, 4, 13), + contents: [fence('gamma as integer')] + }]); + expect(hoverAt(file.srcPath, 5, 0)).to.eql([{ + range: util.createRange(5, 0, 5, 7), + contents: [fence('sub main() as void')] + }]); + }); + + it('uses utf-16 code units for characters after a surrogate pair emoji on the same line', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + ' name = "x"', + ' print "😀😀" + name', + ` ' 😀 comment`, + ' print name', + 'end sub' + )); + program.validate(); + //each emoji is 2 utf-16 code units, so `name` starts at 19 (not 17) + const expected = [{ + range: util.createRange(2, 19, 2, 23), + contents: [fence('name as string')] + }]; + expect(hoverAt(file.srcPath, 2, 19)).to.eql(expected); + expect(hoverAt(file.srcPath, 2, 21)).to.eql(expected); + expect(hoverAt(file.srcPath, 2, 23)).to.eql(expected); + + //the line after the emoji comment is unaffected + expect(hoverAt(file.srcPath, 4, 10)).to.eql([{ + range: util.createRange(4, 10, 4, 14), + contents: [fence('name as string')] + }]); + }); + + it('counts lines through multi-line template strings', () => { + const file = program.setFile('source/main.bs', lines('\n', + 'sub main()', + ' name = "bob"', + /* eslint-disable no-template-curly-in-string */ + ' msg = `hello ${name}', + 'line two ${name}', + 'and ${name} three`', + /* eslint-enable no-template-curly-in-string */ + ' print msg + name', + 'end sub' + )); + program.validate(); + //after the template string + const nameAfter = [{ + range: util.createRange(5, 16, 5, 20), + contents: [fence('name as string')] + }]; + expect(hoverAt(file.srcPath, 5, 16)).to.eql(nameAfter); + expect(hoverAt(file.srcPath, 5, 18)).to.eql(nameAfter); + expect(hoverAt(file.srcPath, 5, 20)).to.eql(nameAfter); + + expect(hoverAt(file.srcPath, 5, 10)).to.eql([{ + range: util.createRange(5, 10, 5, 13), + contents: [fence('msg as string')] + }]); + + //the first character of an identifier directly after `${` is also the (inclusive) end of the `${` + //token, which is found first and is not hoverable + expect(hoverAt(file.srcPath, 2, 19)).to.eql([]); + expect(hoverAt(file.srcPath, 3, 11)).to.eql([]); + expect(hoverAt(file.srcPath, 4, 6)).to.eql([]); + + //inside `${}` on the first line of the template string + expect(hoverAt(file.srcPath, 2, 20)).to.eql([{ + range: util.createRange(2, 19, 2, 23), + contents: [fence('name as string')] + }]); + //inside `${}` on the second line of the template string + expect(hoverAt(file.srcPath, 3, 12)).to.eql([{ + range: util.createRange(3, 11, 3, 15), + contents: [fence('name as string')] + }]); + expect(hoverAt(file.srcPath, 3, 15)).to.eql([{ + range: util.createRange(3, 11, 3, 15), + contents: [fence('name as string')] + }]); + //inside `${}` on the third line of the template string + expect(hoverAt(file.srcPath, 4, 8)).to.eql([{ + range: util.createRange(4, 6, 4, 10), + contents: [fence('name as string')] + }]); + expect(hoverAt(file.srcPath, 4, 10)).to.eql([{ + range: util.createRange(4, 6, 4, 10), + contents: [fence('name as string')] + }]); + }); + + it('finds items on the last line of a file with no trailing newline', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + 'end sub', + 'sub last(): value = 1: print value: end sub' + )); + program.validate(); + //sub |last() + expect(hoverAt(file.srcPath, 2, 4)).to.eql([{ + range: util.createRange(2, 4, 2, 8), + contents: [fence('sub last() as void')] + }]); + //value| = 1 + expect(hoverAt(file.srcPath, 2, 17)).to.eql([{ + range: util.createRange(2, 12, 2, 17), + contents: [fence('value as integer')] + }]); + //print value|: + expect(hoverAt(file.srcPath, 2, 34)).to.eql([{ + range: util.createRange(2, 29, 2, 34), + contents: [fence('value as integer')] + }]); + //end sub| + expect(hoverAt(file.srcPath, 2, 43)).to.eql([{ + range: util.createRange(2, 36, 2, 43), + contents: [fence('sub last() as void')] + }]); + }); + + it('finds symbols in namespaces, classes, and empty function bodies', () => { + const file = program.setFile('source/main.bs', lines('\n', + 'namespace alpha.beta', + ' function noop()', + ' end function', + ' class Widget', + ' sub render()', + ' end sub', + ' end class', + 'end namespace', + 'sub main()', + ' alpha.beta.noop()', + ' w = new alpha.beta.Widget()', + ' w.render()', + 'end sub' + )); + program.validate(); + //function |noop() (a function with an empty body) + const noopDeclaration = [{ + range: util.createRange(1, 13, 1, 17), + contents: [fence('function alpha.beta.noop() as dynamic')] + }]; + expect(hoverAt(file.srcPath, 1, 13)).to.eql(noopDeclaration); + expect(hoverAt(file.srcPath, 1, 15)).to.eql(noopDeclaration); + expect(hoverAt(file.srcPath, 1, 17)).to.eql(noopDeclaration); + + //alpha.beta.|noop() (the inclusive end of the `.` token wins, and `.` is not hoverable) + expect(hoverAt(file.srcPath, 9, 15)).to.eql([]); + //alpha.beta.n|oop() + expect(hoverAt(file.srcPath, 9, 16)).to.eql([{ + range: util.createRange(9, 15, 9, 19), + contents: [fence('function alpha.beta.noop() as dynamic')] + }]); + //alpha.beta.noop|() + expect(hoverAt(file.srcPath, 9, 19)).to.eql([{ + range: util.createRange(9, 15, 9, 19), + contents: [fence('function alpha.beta.noop() as dynamic')] + }]); + + //sub |render() (a method with an empty body) + expect(hoverAt(file.srcPath, 4, 12)).to.eql([{ + range: util.createRange(4, 12, 4, 18), + contents: [fence('sub alpha.beta.Widget.render() as void')] + }]); + + //w.render|() + expect(hoverAt(file.srcPath, 11, 12)).to.eql([{ + range: util.createRange(11, 6, 11, 12), + contents: [fence('sub alpha.beta.Widget.render() as void')] + }]); + + //new alpha.beta.Widget|() + expect(hoverAt(file.srcPath, 10, 29)[0]?.range).to.eql(util.createRange(10, 23, 10, 29)); + //class |Widget + expect(hoverAt(file.srcPath, 3, 10)[0]?.range).to.eql(util.createRange(3, 10, 3, 16)); + }); + }); }); diff --git a/src/bscPlugin/hover/HoverProcessor.ts b/src/bscPlugin/hover/HoverProcessor.ts index 963c651c5..bcd2f864b 100644 --- a/src/bscPlugin/hover/HoverProcessor.ts +++ b/src/bscPlugin/hover/HoverProcessor.ts @@ -70,7 +70,7 @@ export class HoverProcessor { } private getConstHover(token: Token, file: BrsFile, scope: Scope, expression: Expression) { - let containingNamespace = file.getNamespaceStatementForPosition(expression?.location?.range?.start)?.getName(ParseMode.BrighterScript); + let containingNamespace = file.getNamespaceStatementForPosition(util.getLocation(expression)?.range?.start)?.getName(ParseMode.BrighterScript); const fullName = util.getAllDottedGetParts(expression)?.map(x => x.text).join('.'); //find a constant with this name @@ -242,7 +242,7 @@ export class HoverProcessor { } } return { - range: token?.location.range, + range: util.getLocation(token).range, contents: hoverContents }; } diff --git a/src/bscPlugin/inlayHints/InlayHintProcessor.ts b/src/bscPlugin/inlayHints/InlayHintProcessor.ts index f6bc2af8c..2b4ac7e93 100644 --- a/src/bscPlugin/inlayHints/InlayHintProcessor.ts +++ b/src/bscPlugin/inlayHints/InlayHintProcessor.ts @@ -37,13 +37,15 @@ export class InlayHintProcessor { file.ast.walk(createVisitor({ CallExpression: (call) => { - if (!call.location?.range || !util.rangesIntersectOrTouch(call.location.range, range)) { + const callRange = util.getLocation(call)?.range; + if (!callRange || !util.rangesIntersectOrTouch(callRange, range)) { return; } this.emitParameterNameHints(file, call); }, CallfuncExpression: (call) => { - if (!call.location?.range || !util.rangesIntersectOrTouch(call.location.range, range)) { + const callRange = util.getLocation(call)?.range; + if (!callRange || !util.rangesIntersectOrTouch(callRange, range)) { return; } this.emitParameterNameHintsForCallfunc(file, call); @@ -191,7 +193,7 @@ export class InlayHintProcessor { const arg = args[i]; const param = params[i]; const paramName = param?.tokens?.name?.text; - const argRange = arg?.location?.range; + const argRange = util.getLocation(arg)?.range; if (!paramName || !argRange) { continue; } diff --git a/src/bscPlugin/references/ReferencesProvider.spec.ts b/src/bscPlugin/references/ReferencesProvider.spec.ts index b6df2f2ea..cfb4080bf 100644 --- a/src/bscPlugin/references/ReferencesProvider.spec.ts +++ b/src/bscPlugin/references/ReferencesProvider.spec.ts @@ -285,6 +285,214 @@ describe('ReferencesProvider', () => { ).to.eql([]); }); + describe('location boundaries', () => { + //NOTE: these tests intentionally only use public `Program` APIs (Position in, Location out) so they + //can be run against any implementation of token/node locations. All expected values are hand-counted. + + /** + * Join lines with the given line ending. No trailing newline is added + */ + function lines(eol: string, ...items: string[]) { + return items.join(eol); + } + + /** + * Get the references at the position, sorted by uri, then by position + */ + function referencesAt(srcPath: string, line: number, character: number) { + return program.getReferences(srcPath, util.createPosition(line, character)).sort((a, b) => { + if (a.uri !== b.uri) { + return a.uri < b.uri ? -1 : 1; + } + return (a.range.start.line - b.range.start.line) || (a.range.start.character - b.range.start.character); + }); + } + + function loc(srcPath: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) { + return { + uri: util.pathToUri(srcPath), + range: util.createRange(startLine, startCharacter, endLine, endCharacter) + }; + } + + it('finds the identifier at its first character, middle, and immediately after its last character', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + ' alpha = 1', + ' print alpha', + ' alpha = alpha + 1', + 'end sub' + )); + program.validate(); + const expected = [ + loc(file.srcPath, 1, 4, 1, 9), + loc(file.srcPath, 2, 10, 2, 15), + loc(file.srcPath, 3, 4, 3, 9), + loc(file.srcPath, 3, 12, 3, 17) + ]; + //print |alpha + expect(referencesAt(file.srcPath, 2, 10)).to.eql(expected); + //print al|pha + expect(referencesAt(file.srcPath, 2, 12)).to.eql(expected); + //print alpha| (the end of a range is inclusive) + expect(referencesAt(file.srcPath, 2, 15)).to.eql(expected); + //alpha| = 1 + expect(referencesAt(file.srcPath, 1, 9)).to.eql(expected); + + //print| alpha (immediately after `print`, so we search for `print` instead) + expect(referencesAt(file.srcPath, 2, 9)).to.eql([]); + }); + + it('finds identifiers at column 0 of a line and at the end of a line', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + 'beta = 1', + 'print beta', + 'end sub' + )); + program.validate(); + const expected = [ + loc(file.srcPath, 1, 0, 1, 4), + loc(file.srcPath, 2, 6, 2, 10) + ]; + //|beta = 1 + expect(referencesAt(file.srcPath, 1, 0)).to.eql(expected); + //print beta| + expect(referencesAt(file.srcPath, 2, 10)).to.eql(expected); + }); + + it('handles CRLF line endings and returns the uri of each file for cross-file references', () => { + const mainFile = program.setFile('source/main.brs', lines('\r\n', + 'sub main()', + ' gamma = 1', + ' print gamma', + 'gamma = 2', + 'end sub' + )); + const otherFile = program.setFile('source/other.brs', lines('\r\n', + 'sub other()', + ' print gamma', + 'end sub' + )); + program.validate(); + const expected = [ + loc(mainFile.srcPath, 1, 4, 1, 9), + loc(mainFile.srcPath, 2, 10, 2, 15), + loc(mainFile.srcPath, 3, 0, 3, 5), + loc(otherFile.srcPath, 1, 10, 1, 15) + ]; + //column 0 after a `\r\n` + expect(referencesAt(mainFile.srcPath, 3, 0)).to.eql(expected); + //end of the line, right before the `\r\n` + expect(referencesAt(mainFile.srcPath, 2, 15)).to.eql(expected); + //from the other file + expect(referencesAt(otherFile.srcPath, 1, 12)).to.eql(expected); + }); + + it('uses utf-16 code units for characters after a surrogate pair emoji on the same line', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + ' name = "😀"', + ' print "😀😀" + name', + ' x = "😀": name = name + "😀"', + 'end sub' + )); + program.validate(); + //each emoji is 2 utf-16 code units + const expected = [ + loc(file.srcPath, 1, 4, 1, 8), + loc(file.srcPath, 2, 19, 2, 23), + loc(file.srcPath, 3, 14, 3, 18), + loc(file.srcPath, 3, 21, 3, 25) + ]; + expect(referencesAt(file.srcPath, 2, 19)).to.eql(expected); + expect(referencesAt(file.srcPath, 2, 21)).to.eql(expected); + expect(referencesAt(file.srcPath, 2, 23)).to.eql(expected); + expect(referencesAt(file.srcPath, 3, 25)).to.eql(expected); + }); + + it('counts lines through multi-line template strings', () => { + const file = program.setFile('source/main.bs', lines('\n', + 'sub main()', + ' name = "bob"', + /* eslint-disable no-template-curly-in-string */ + ' msg = `hello ${name}', + 'line two ${name}', + 'and ${name} three`', + /* eslint-enable no-template-curly-in-string */ + ' print msg + name', + 'end sub' + )); + program.validate(); + const expected = [ + loc(file.srcPath, 1, 4, 1, 8), + loc(file.srcPath, 2, 19, 2, 23), + loc(file.srcPath, 3, 11, 3, 15), + loc(file.srcPath, 4, 6, 4, 10), + loc(file.srcPath, 5, 16, 5, 20) + ]; + //inside `${}` on later lines of the template string + expect(referencesAt(file.srcPath, 3, 13)).to.eql(expected); + expect(referencesAt(file.srcPath, 4, 10)).to.eql(expected); + //after the template string + expect(referencesAt(file.srcPath, 5, 20)).to.eql(expected); + + //print msg| + expect(referencesAt(file.srcPath, 5, 13)).to.eql([ + loc(file.srcPath, 2, 4, 2, 7), + loc(file.srcPath, 5, 10, 5, 13) + ]); + }); + + it('finds items on the last line of a file with no trailing newline', () => { + const file = program.setFile('source/main.brs', lines('\n', + 'sub main()', + 'end sub', + 'sub last(): value = 1: print value: end sub' + )); + program.validate(); + const expected = [ + loc(file.srcPath, 2, 12, 2, 17), + loc(file.srcPath, 2, 29, 2, 34) + ]; + //print value|: + expect(referencesAt(file.srcPath, 2, 34)).to.eql(expected); + //|value = 1 + expect(referencesAt(file.srcPath, 2, 12)).to.eql(expected); + }); + + it('finds symbols in namespaces, classes, and empty function bodies', () => { + const file = program.setFile('source/main.bs', lines('\n', + 'namespace alpha.beta', + ' function noop()', + ' end function', + ' class Widget', + ' sub render()', + ' count = 1', + ' print count', + ' end sub', + ' end class', + ' sub caller()', + ' noop()', + ' end sub', + 'end namespace' + )); + program.validate(); + //variables in a class method inside a namespace + const expected = [ + loc(file.srcPath, 5, 12, 5, 17), + loc(file.srcPath, 6, 18, 6, 23) + ]; + expect(referencesAt(file.srcPath, 6, 23)).to.eql(expected); + expect(referencesAt(file.srcPath, 5, 12)).to.eql(expected); + + //call to an empty function from inside a namespace + expect(referencesAt(file.srcPath, 10, 12)).to.eql([ + loc(file.srcPath, 10, 8, 10, 12) + ]); + }); + }); + function locationToString(loc: Location) { return `${URI.parse(loc.uri).fsPath}:${loc.range.start.line}:${loc.range.start.character}-${loc.range.end.line}:${loc.range.end.character}`; } diff --git a/src/bscPlugin/references/ReferencesProvider.ts b/src/bscPlugin/references/ReferencesProvider.ts index 0cea7f564..829f72d5a 100644 --- a/src/bscPlugin/references/ReferencesProvider.ts +++ b/src/bscPlugin/references/ReferencesProvider.ts @@ -49,12 +49,12 @@ export class ReferencesProvider { file.ast.walk(createVisitor({ AssignmentStatement: (s) => { if (s.tokens.name?.text?.toLowerCase() === searchFor) { - this.event.references.push(util.createLocationFromRange(util.pathToUri(file.srcPath), s.tokens.name.location?.range)); + this.event.references.push(util.createLocationFromRange(util.pathToUri(file.srcPath), util.getLocation(s.tokens.name)?.range)); } }, VariableExpression: (e) => { if (e.tokens.name.text.toLowerCase() === searchFor) { - this.event.references.push(util.createLocationFromRange(util.pathToUri(file.srcPath), e.location?.range)); + this.event.references.push(util.createLocationFromRange(util.pathToUri(file.srcPath), util.getLocation(e)?.range)); } } }), { diff --git a/src/bscPlugin/selectionRanges/SelectionRangesProcessor.spec.ts b/src/bscPlugin/selectionRanges/SelectionRangesProcessor.spec.ts index fe88f32b0..5e1f8b94d 100644 --- a/src/bscPlugin/selectionRanges/SelectionRangesProcessor.spec.ts +++ b/src/bscPlugin/selectionRanges/SelectionRangesProcessor.spec.ts @@ -352,4 +352,381 @@ describe('SelectionRangesProcessor', () => { const hasCustom = ranges.some(r => r.range.start.line === 0 && r.range.end.character === 10); expect(hasCustom).to.be.true; }); + + describe('location boundaries', () => { + type RangeTuple = [number, number, number, number]; + + /** + * Get the full selection range chain (innermost to outermost) for each position, requesting each position individually + */ + function getChains(pkgPath: string, code: string, positions: Array<[number, number]>) { + const file = program.setFile(pkgPath, code); + program.validate(); + return positions.map(([line, character]) => { + const ranges = program.getSelectionRanges(file.srcPath, [util.createPosition(line, character)]); + expect(ranges).to.have.length(1, `expected exactly one selection range for position ${line}:${character}`); + return flatten(ranges[0]); + }); + } + + const loopsCode = [ + 'sub main()', + ' value = 1', + ' while value < 10', + ' value = value + 1', + ' if value = 5', + ' exitwhile', + ' end if', + ' end while', + ' for i = 0 to 3', + ' print i', + ' end for', + 'end sub', + 'sub empty()', + 'end sub' + ].join('\n'); + + it('computes exact chains at identifier boundaries, column 0, and inside loops with exitwhile', () => { + const [atStart, afterEnd, column0, onExit, afterExit, inFor, onIf] = getChains('source/main.brs', loopsCode, [ + [1, 4], // |value = 1 + [1, 9], // value| = 1 + [2, 0], // | while value < 10 + [5, 12], // |exitwhile + [5, 16], // exit|while + [9, 14], // print |i + [4, 8] // |if value = 5 + ]); + const outer: RangeTuple[] = [ + [1, 4, 10, 11], // function body block + [0, 0, 11, 7], // sub main()...end sub + [0, 0, 13, 7] // whole file + ]; + const whileChain: RangeTuple[] = [ + [4, 8, 6, 14], // if value = 5 ... end if + [3, 8, 6, 14], // while body block + [2, 4, 7, 13], // while ... end while + ...outer + ]; + expect(atStart).to.eql([ + [1, 4, 1, 9], // value + [1, 4, 1, 13], // value = 1 + ...outer + ]); + expect(afterEnd).to.eql(atStart); + expect(column0).to.eql(outer); + expect(onExit).to.eql([ + [5, 12, 5, 16], // exit (split from `exitwhile`) + [5, 12, 5, 21], // exitwhile + ...whileChain + ]); + expect(afterExit).to.eql(onExit); + expect(inFor).to.eql([ + [9, 14, 9, 15], // i + [9, 8, 9, 15], // print i + [8, 4, 10, 11], // for ... end for + ...outer + ]); + expect(onIf).to.eql([ + [4, 8, 4, 10], // if + ...whileChain + ]); + }); + + it('computes exact chains inside an empty function body followed by a column-0 `end sub`', () => { + const [afterParen, atEnd] = getChains('source/main.brs', loopsCode, [ + [12, 11], // sub empty()| + [13, 0] // |end sub + ]); + expect(afterParen).to.eql([ + [12, 10, 12, 11], // ) + [12, 11, 13, 0], // empty body block (from the end of `)` to the start of `end sub`) + [12, 0, 13, 7], // sub empty()...end sub + [0, 0, 13, 7] // whole file + ]); + expect(atEnd).to.eql([ + [13, 0, 13, 7], // end sub + [12, 11, 13, 0], // empty body block + [12, 0, 13, 7], // sub empty()...end sub + [0, 0, 13, 7] // whole file + ]); + }); + + it('computes exact chains for empty-bodied constructs and empty if/else/while/for blocks', () => { + const code = [ + 'namespace alpha', + 'end namespace', + 'class Beta', + 'end class', + 'interface Charlie', + 'end interface', + 'enum Delta', + 'end enum', + 'sub main()', + ' if true then', + ' else if false then', + ' else', + ' end if', + ' while true', + ' end while', + ' for i = 0 to 1', + ' end for', + 'end sub' + ].join('\n'); + const chains = getChains('source/main.bs', code, [ + [1, 0], // |end namespace + [3, 0], // |end class + [5, 0], // |end interface + [7, 0], // |end enum + [9, 16], // if true then| + [10, 0], // | else if false then + [11, 0], // | else + [11, 8], // else| + [12, 0], // | end if + [13, 14], // while true| + [14, 0], // | end while + [16, 0] // | end for + ]); + const file: RangeTuple = [0, 0, 17, 7]; + const main: RangeTuple[] = [ + [9, 4, 16, 11], // function body block + [8, 0, 17, 7], // sub main()...end sub + file + ]; + const ifChain: RangeTuple[] = [ + [9, 4, 12, 10], // if ... end if + ...main + ]; + const elseIfChain: RangeTuple[] = [ + [10, 9, 12, 10], // the nested `if false then ... end if` statement + ...ifChain + ]; + expect(chains).to.eql([ + [[1, 0, 1, 13], [0, 0, 1, 13], file], + [[3, 0, 3, 9], [2, 0, 3, 9], file], + [[5, 0, 5, 13], [4, 0, 5, 13], file], + [[7, 0, 7, 8], [6, 0, 7, 8], file], + // `then` token, then the empty `then` block (end of `then` to the start of `else`) + [[9, 12, 9, 16], [9, 16, 10, 4], ...ifChain], + [[9, 16, 10, 4], ...ifChain], + // the empty `else if` block (end of `then` to the start of `else`) + [[10, 22, 11, 4], ...elseIfChain], + // `else` token, then the empty `else` block (end of `else` to the start of `end if`) + [[11, 4, 11, 8], [11, 8, 12, 4], ...elseIfChain], + [[11, 8, 12, 4], ...elseIfChain], + // `true` token, then while statement + [[13, 10, 13, 14], [13, 4, 14, 13], ...main], + // empty while block + [[13, 14, 14, 4], [13, 4, 14, 13], ...main], + // empty for block + [[15, 18, 16, 4], [15, 4, 16, 11], ...main] + ]); + }); + + it('computes exact chains in a .brs file with CRLF line endings, emoji, and elseif', () => { + const code = [ + '\' 😀 header comment', + 'function alpha(a, b)', + ' if a then', + ' print "😀"', + ' elseif b then', + ' print 2', + ' else', + ' print 3', + ' end if', + 'end function', + 'sub beta() : print "😀😀" : end sub : sub charlie() : end sub', + 'sub empty()', + 'end sub', + '' + ].join('\r\n'); + const [onPrint, onElse, onElseIf, inElseIf, onEndSub, onCharlie, column0, afterParen, atEnd] = getChains('source/main.brs', code, [ + [3, 8], // |print "😀" + [4, 4], // |elseif b then + [4, 8], // else|if b then + [5, 8], // |print 2 + [10, 30], // en|d sub (after the emoji string) + [10, 42], // sub |charlie() (after the emoji string) + [2, 0], // | if a then + [11, 11], // sub empty()| + [12, 0] // |end sub + ]); + const body: RangeTuple = [1, 0, 12, 7]; + const alpha: RangeTuple[] = [ + [2, 4, 8, 10], // if ... end if + [1, 0, 9, 12], // function alpha...end function + body + ]; + expect(onPrint).to.eql([ + [3, 8, 3, 13], // print + [3, 8, 3, 18], // print "😀" + ...alpha + ]); + expect(onElse).to.eql([ + [4, 4, 4, 8], // else (split from `elseif`) + ...alpha + ]); + expect(onElseIf).to.eql([ + [4, 4, 4, 8], // else (split from `elseif`) + [4, 8, 8, 10], // if b then ... end if + ...alpha + ]); + expect(inElseIf).to.eql([ + [5, 8, 5, 13], // print + [5, 8, 5, 15], // print 2 + [4, 8, 8, 10], // if b then ... end if + ...alpha + ]); + expect(onEndSub).to.eql([ + [10, 28, 10, 35], // end sub + [10, 0, 10, 35], // sub beta() : print "😀😀" : end sub + body + ]); + expect(onCharlie).to.eql([ + [10, 42, 10, 49], // charlie + [10, 38, 10, 61], // sub charlie() : end sub + body + ]); + expect(column0).to.eql([ + [1, 0, 9, 12], // function alpha...end function + body + ]); + expect(afterParen).to.eql([ + [11, 10, 11, 11], // ) + [11, 11, 12, 0], // empty body block (from the end of `)` to the start of `end sub`) + [11, 0, 12, 7], // sub empty()...end sub + body + ]); + expect(atEnd).to.eql([ + [12, 0, 12, 7], // end sub + [11, 11, 12, 0], // empty body block + [11, 0, 12, 7], // sub empty()...end sub + body + ]); + }); + + it('uses utf-16 code units for positions that follow an emoji on the same line', () => { + const code = [ + 'const A = "😀" : const B = 2', + 'enum E', + ' x = "😀😀" : y = "b"', + 'end enum' + ].join('\n'); + const chains = getChains('source/main.bs', code, [ + [0, 22], // const| B = 2 + [0, 23], // const |B = 2 + [2, 17], // |y = "b" + [2, 24] // y = "b"| + ]); + expect(chains).to.eql([ + [[0, 17, 0, 22], [0, 17, 0, 28], [0, 0, 3, 8]], + [[0, 23, 0, 24], [0, 17, 0, 28], [0, 0, 3, 8]], + [[2, 17, 2, 18], [2, 17, 2, 24], [1, 0, 3, 8], [0, 0, 3, 8]], + [[2, 21, 2, 24], [2, 17, 2, 24], [1, 0, 3, 8], [0, 0, 3, 8]] + ]); + }); + + it('computes exact chains inside and after a multi-line template string', () => { + const code = [ + 'sub main(who as string)', + ' count = 1', + ' text = `line one 😀', + // eslint-disable-next-line no-template-curly-in-string + 'two ${who} and ${count}', + // eslint-disable-next-line no-template-curly-in-string + 'three ${ lcase(who) }`', + ' print text', + 'end sub', + 'function after()', + 'end function', + '' + ].join('\n'); + const chains = getChains('source/main.bs', code, [ + [3, 6], // two ${|who} + [4, 9], // three ${ |lcase(who) } + [4, 15], // three ${ lcase(|who) } + [7, 9] // function |after() + ]); + const template: RangeTuple[] = [ + [2, 11, 4, 22], // the whole template string + [2, 4, 4, 22], // text = `...` + [1, 4, 5, 14], // function body block + [0, 0, 6, 7], // sub main...end sub + [0, 0, 8, 12] // whole file + ]; + expect(chains).to.eql([ + [[3, 4, 3, 6], [3, 6, 3, 9], ...template], + [[4, 9, 4, 14], [4, 9, 4, 19], ...template], + [[4, 14, 4, 15], [4, 15, 4, 18], [4, 9, 4, 19], ...template], + [[7, 9, 7, 14], [7, 0, 8, 12], [0, 0, 8, 12]] + ]); + }); + + it('computes exact chains for nested namespaces and a symbol on the last line with no trailing newline', () => { + const code = [ + 'namespace alpha', + ' namespace beta.charlie', + ' class Person', + ' name as string', + ' sub speak()', + ' end sub', + ' end class', + ' end namespace', + ' enum Direction', + ' up = "up"', + ' down = "down"', + ' end enum', + ' interface Shape', + ' width as integer', + ' function area() as float', + ' end interface', + ' const PI = 3.14', + 'end namespace', + 'function empty()', + 'end function', + 'const LAST_ONE = true' + ].join('\n'); + const chains = getChains('source/main.bs', code, [ + [20, 6], // const |LAST_ONE = true + [20, 21], // const LAST_ONE = true| + [16, 10], // const |PI = 3.14 + [1, 19], // namespace beta.|charlie + [9, 13], // up = |"up" + [18, 16], // function empty()| + [19, 0] // |end function + ]); + const alphaBody: RangeTuple[] = [ + [1, 4, 16, 19], // namespace alpha body + [0, 0, 17, 13], // namespace alpha...end namespace + [0, 0, 20, 21] // whole file + ]; + expect(chains).to.eql([ + [[20, 6, 20, 14], [20, 0, 20, 21], [0, 0, 20, 21]], + [[20, 17, 20, 21], [20, 0, 20, 21], [0, 0, 20, 21]], + [[16, 10, 16, 12], [16, 4, 16, 19], ...alphaBody], + [[1, 18, 1, 19], [1, 14, 1, 26], [1, 4, 7, 17], ...alphaBody], + [[9, 13, 9, 17], [9, 8, 9, 17], [8, 4, 11, 12], ...alphaBody], + // `)`, then the empty body block (end of `)` to the start of `end function`) + [[18, 15, 18, 16], [18, 16, 19, 0], [18, 0, 19, 12], [0, 0, 20, 21]], + [[19, 0, 19, 12], [18, 16, 19, 0], [18, 0, 19, 12], [0, 0, 20, 21]] + ]); + }); + + it('returns the same chains when all positions are requested in a single call', () => { + const file = program.setFile('source/main.brs', loopsCode); + program.validate(); + const positions = [ + util.createPosition(1, 4), + util.createPosition(5, 12), + util.createPosition(9, 14) + ]; + const combined = program.getSelectionRanges(file.srcPath, positions).map(x => flatten(x)); + const individual = positions.map(x => flatten(program.getSelectionRanges(file.srcPath, [x])[0])); + expect(combined).to.eql(individual); + expect(combined.map(x => x[0])).to.eql([ + [1, 4, 1, 9], + [5, 12, 5, 16], + [9, 14, 9, 15] + ]); + }); + }); }); diff --git a/src/bscPlugin/selectionRanges/SelectionRangesProcessor.ts b/src/bscPlugin/selectionRanges/SelectionRangesProcessor.ts index a7d6b8755..119d32f7b 100644 --- a/src/bscPlugin/selectionRanges/SelectionRangesProcessor.ts +++ b/src/bscPlugin/selectionRanges/SelectionRangesProcessor.ts @@ -3,6 +3,7 @@ import type { Range, Position } from 'vscode-languageserver-protocol'; import { isBrsFile } from '../../astUtils/reflection'; import type { ProvideSelectionRangesEvent } from '../../interfaces'; import type { AstNode } from '../../parser/AstNode'; +import { util } from '../../util'; export class SelectionRangesProcessor { public constructor( @@ -30,7 +31,7 @@ export class SelectionRangesProcessor { // Find the deepest AST node containing this position const innerNode = file.ast.findChildAtPosition(position); - const innerNodeRange = innerNode?.location?.range; + const innerNodeRange = util.getLocation(innerNode)?.range; if (!innerNodeRange) { return undefined; } @@ -43,7 +44,7 @@ export class SelectionRangesProcessor { // the JS/TS smart-select behaviour — we look up the exact lexer token at the // cursor position and use its range as the initial step. const token = file.getTokenAt(position); - const tokenRange = token?.location?.range; + const tokenRange = util.getLocation(token)?.range; if (tokenRange && !rangesEqual(tokenRange, innerNodeRange)) { ranges.push(tokenRange); } @@ -53,7 +54,7 @@ export class SelectionRangesProcessor { // only child statement, or an ExpressionStatement === its expression). let node: AstNode | undefined = innerNode; while (node) { - const nodeRange = node.location?.range; + const nodeRange = util.getLocation(node)?.range; if (nodeRange && !rangesEqual(nodeRange, ranges[ranges.length - 1])) { ranges.push(nodeRange); } diff --git a/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.spec.ts b/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.spec.ts index 78be51bbb..092be7783 100644 --- a/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.spec.ts +++ b/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.spec.ts @@ -635,4 +635,176 @@ describe('BrsFileSemanticTokensProcessor', () => { [SemanticTokenTypes.function, 2, 16, 2, 20] ]); }); + + describe('location boundaries', () => { + it('computes exact token ranges for nested multi-line constructs in a .bs file with no trailing newline', () => { + const file = program.setFile('source/main.bs', [ + 'namespace alpha', + ' namespace beta.charlie', + ' class Person', + ' name as string', + ' sub speak()', + ' end sub', + ' end class', + ' end namespace', + ' enum Direction', + ' up = "up"', + ' down = "down"', + ' end enum', + ' interface Shape', + ' width as integer', + ' function area() as float', + ' end interface', + ' const PI = 3.14', + 'end namespace', + 'function empty()', + 'end function', + 'const LAST_ONE = true' + ].join('\n')); + expectSemanticTokens(file, [ + // namespace |alpha| + [SemanticTokenTypes.namespace, 0, 10, 0, 15], + // class |Person| + [SemanticTokenTypes.class, 2, 14, 2, 20], + // enum |Direction| + [SemanticTokenTypes.enum, 8, 9, 8, 18], + // interface |Shape| + [SemanticTokenTypes.interface, 12, 14, 12, 19], + // const |PI| = 3.14 + [SemanticTokenTypes.variable, 16, 10, 16, 12, [SemanticTokenModifiers.readonly, SemanticTokenModifiers.static]], + // function |empty|() + [SemanticTokenTypes.function, 18, 9, 18, 14], + // const |LAST_ONE| = true + [SemanticTokenTypes.variable, 20, 6, 20, 14, [SemanticTokenModifiers.readonly, SemanticTokenModifiers.static]] + ]); + }); + + it('computes exact token ranges in a .brs file with CRLF line endings, emoji, and elseif', () => { + const file = program.setFile('source/main.brs', [ + '\' 😀 header comment', + 'function alpha(a, b)', + ' if a then', + ' print "😀"', + ' elseif b then', + ' print 2', + ' else', + ' print 3', + ' end if', + 'end function', + 'sub beta() : print "😀😀" : end sub : sub charlie() : end sub', + 'sub empty()', + 'end sub', + '' + ].join('\r\n')); + expectSemanticTokens(file, [ + // function |alpha|(a, b) + [SemanticTokenTypes.function, 1, 9, 1, 14], + // function alpha(|a|, b) + [SemanticTokenTypes.parameter, 1, 15, 1, 16], + // function alpha(a, |b|) + [SemanticTokenTypes.parameter, 1, 18, 1, 19], + // sub |beta|() : print "😀😀" : end sub : sub charlie() : end sub + [SemanticTokenTypes.function, 10, 4, 10, 8], + // sub beta() : print "😀😀" : end sub : sub |charlie|() : end sub + [SemanticTokenTypes.function, 10, 42, 10, 49], + // sub |empty|() + [SemanticTokenTypes.function, 11, 4, 11, 9] + ]); + }); + + it('uses utf-16 code units for tokens that follow an emoji on the same line', () => { + const file = program.setFile('source/main.bs', [ + 'const A = "😀" : const B = 2', + 'enum E', + ' x = "😀😀" : y = "b"', + 'end enum' + ].join('\n')); + expectSemanticTokens(file, [ + // const |A| = "😀" : const B = 2 + [SemanticTokenTypes.variable, 0, 6, 0, 7, [SemanticTokenModifiers.readonly, SemanticTokenModifiers.static]], + // const A = "😀" : const |B| = 2 + [SemanticTokenTypes.variable, 0, 23, 0, 24, [SemanticTokenModifiers.readonly, SemanticTokenModifiers.static]], + // enum |E| + [SemanticTokenTypes.enum, 1, 5, 1, 6] + ]); + }); + + it('computes exact token ranges inside and after a multi-line template string', () => { + const file = program.setFile('source/main.bs', [ + 'sub main(who as string)', + ' count = 1', + ' text = `line one 😀', + // eslint-disable-next-line no-template-curly-in-string + 'two ${who} and ${count}', + // eslint-disable-next-line no-template-curly-in-string + 'three ${ lcase(who) }`', + ' print text', + 'end sub', + 'function after()', + 'end function', + '' + ].join('\n')); + expectSemanticTokens(file, [ + // sub |main|(who as string) + [SemanticTokenTypes.function, 0, 4, 0, 8], + // sub main(|who| as string) + [SemanticTokenTypes.parameter, 0, 9, 0, 12], + // |count| = 1 + [SemanticTokenTypes.variable, 1, 4, 1, 9], + // |text| = `line one 😀 + [SemanticTokenTypes.variable, 2, 4, 2, 8], + // two ${|who|} and ${count} + [SemanticTokenTypes.variable, 3, 6, 3, 9], + // two ${who} and ${|count|} + [SemanticTokenTypes.variable, 3, 17, 3, 22], + // three ${ |lcase|(who) }` + [SemanticTokenTypes.function, 4, 9, 4, 14], + // three ${ lcase(|who|) }` + [SemanticTokenTypes.variable, 4, 15, 4, 18], + // print |text| + [SemanticTokenTypes.variable, 5, 10, 5, 14], + // function |after|() + [SemanticTokenTypes.function, 7, 9, 7, 14] + ]); + }); + + it('computes exact token ranges in loops, exitwhile, and an empty function at the end of a .brs file', () => { + const file = program.setFile('source/main.brs', [ + 'sub main()', + ' value = 1', + ' while value < 10', + ' value = value + 1', + ' if value = 5', + ' exitwhile', + ' end if', + ' end while', + ' for i = 0 to 3', + ' print i', + ' end for', + 'end sub', + 'sub empty()', + 'end sub' + ].join('\n')); + expectSemanticTokens(file, [ + // sub |main|() + [SemanticTokenTypes.function, 0, 4, 0, 8], + // |value| = 1 + [SemanticTokenTypes.variable, 1, 4, 1, 9], + // while |value| < 10 + [SemanticTokenTypes.variable, 2, 10, 2, 15], + // |value| = value + 1 + [SemanticTokenTypes.variable, 3, 8, 3, 13], + // value = |value| + 1 + [SemanticTokenTypes.variable, 3, 16, 3, 21], + // if |value| = 5 + [SemanticTokenTypes.variable, 4, 11, 4, 16], + // for |i| = 0 to 3 + [SemanticTokenTypes.variable, 8, 8, 8, 9], + // print |i| + [SemanticTokenTypes.variable, 9, 14, 9, 15], + // sub |empty|() + [SemanticTokenTypes.function, 12, 4, 12, 9] + ]); + }); + }); }); diff --git a/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.ts b/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.ts index 0f6bd42fa..78f025916 100644 --- a/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.ts +++ b/src/bscPlugin/semanticTokens/BrsFileSemanticTokensProcessor.ts @@ -100,8 +100,9 @@ export class BrsFileSemanticTokensProcessor { private addToken(locatable: Locatable, type: SemanticTokenTypes, modifiers: SemanticTokenModifiers[] = []) { //only keep a single token per range. Last-in wins - this.result.set(util.rangeToString(locatable.location.range), { - range: locatable.location.range, + const range = util.getLocation(locatable).range; + this.result.set(util.rangeToString(range), { + range: range, tokenType: type, tokenModifiers: modifiers }); diff --git a/src/bscPlugin/symbols/DocumentSymbolProcessor.spec.ts b/src/bscPlugin/symbols/DocumentSymbolProcessor.spec.ts index 07b8cdb8e..994dabacf 100644 --- a/src/bscPlugin/symbols/DocumentSymbolProcessor.spec.ts +++ b/src/bscPlugin/symbols/DocumentSymbolProcessor.spec.ts @@ -5,6 +5,7 @@ import { rootDir } from '../../testHelpers.spec'; import type { DocumentSymbol } from 'vscode-languageserver-types'; import { SymbolKind } from 'vscode-languageserver-types'; import type { BrsFile } from '../../files/BrsFile'; +import { util } from '../../util'; let sinon = createSandbox(); describe('DocumentSymbolProcessor', () => { @@ -288,6 +289,157 @@ describe('DocumentSymbolProcessor', () => { }); }); + describe('location boundaries', () => { + /** + * Flatten the document symbols into `name|kind|range|selectionRange` strings, indenting children by 2 spaces per level + */ + function getSymbolStrings(srcPath: string) { + const result: string[] = []; + function walk(symbols: DocumentSymbol[], indent: string) { + for (const symbol of symbols ?? []) { + result.push(`${indent}${symbol.name}|${SymbolKindMap.get(symbol.kind)}|${util.rangeToString(symbol.range)}|${util.rangeToString(symbol.selectionRange)}`); + walk(symbol.children, indent + ' '); + } + } + walk(program.getDocumentSymbols(srcPath), ''); + return result; + } + + it('computes exact ranges for nested multi-line constructs in a .bs file with no trailing newline', () => { + program.setFile('source/main.bs', [ + 'namespace alpha', + ' namespace beta.charlie', + ' class Person', + ' name as string', + ' sub speak()', + ' end sub', + ' end class', + ' end namespace', + ' enum Direction', + ' up = "up"', + ' down = "down"', + ' end enum', + ' interface Shape', + ' width as integer', + ' function area() as float', + ' end interface', + ' const PI = 3.14', + 'end namespace', + 'function empty()', + 'end function', + 'const LAST_ONE = true' + ].join('\n')); + expect(getSymbolStrings('source/main.bs')).to.eql([ + 'alpha|Namespace|0:0-17:13|0:10-0:15', + ' charlie|Namespace|1:4-7:17|1:14-1:26', + ' Person|Class|2:8-6:17|2:14-2:20', + ' name|Field|3:12-3:26|3:12-3:16', + ' speak|Method|4:12-5:19|4:16-4:21', + ' Direction|Enum|8:4-11:12|8:9-8:18', + ' up|EnumMember|9:8-9:17|9:8-9:10', + ' down|EnumMember|10:8-10:21|10:8-10:12', + ' Shape|Interface|12:4-15:17|12:14-12:19', + ' width|Field|13:8-13:24|13:8-13:13', + ' area|Method|14:8-14:32|14:17-14:21', + ' PI|Constant|16:4-16:19|16:10-16:12', + 'empty|Function|18:0-19:12|18:9-18:14', + 'LAST_ONE|Constant|20:0-20:21|20:6-20:14' + ]); + }); + + it('computes exact ranges for empty-bodied constructs', () => { + program.setFile('source/main.bs', [ + 'namespace alpha', + 'end namespace', + 'class Beta', + 'end class', + 'interface Charlie', + 'end interface', + 'enum Delta', + 'end enum', + 'sub main()', + ' if true then', + ' else if false then', + ' else', + ' end if', + ' while true', + ' end while', + ' for i = 0 to 1', + ' end for', + 'end sub' + ].join('\n')); + expect(getSymbolStrings('source/main.bs')).to.eql([ + 'alpha|Namespace|0:0-1:13|0:10-0:15', + 'Beta|Class|2:0-3:9|2:6-2:10', + 'Charlie|Interface|4:0-5:13|4:10-4:17', + 'Delta|Enum|6:0-7:8|6:5-6:10', + 'main|Function|8:0-17:7|8:4-8:8' + ]); + }); + + it('computes exact ranges in a .brs file with CRLF line endings, emoji, and elseif', () => { + program.setFile('source/main.brs', [ + '\' 😀 header comment', + 'function alpha(a, b)', + ' if a then', + ' print "😀"', + ' elseif b then', + ' print 2', + ' else', + ' print 3', + ' end if', + 'end function', + 'sub beta() : print "😀😀" : end sub : sub charlie() : end sub', + 'sub empty()', + 'end sub', + '' + ].join('\r\n')); + expect(getSymbolStrings('source/main.brs')).to.eql([ + 'alpha|Function|1:0-9:12|1:9-1:14', + 'beta|Function|10:0-10:35|10:4-10:8', + 'charlie|Function|10:38-10:61|10:42-10:49', + 'empty|Function|11:0-12:7|11:4-11:9' + ]); + }); + + it('uses utf-16 code units for symbols that follow an emoji on the same line', () => { + program.setFile('source/main.bs', [ + 'const A = "😀" : const B = 2', + 'enum E', + ' x = "😀😀" : y = "b"', + 'end enum' + ].join('\n')); + expect(getSymbolStrings('source/main.bs')).to.eql([ + 'A|Constant|0:0-0:14|0:6-0:7', + 'B|Constant|0:17-0:28|0:23-0:24', + 'E|Enum|1:0-3:8|1:5-1:6', + ' x|EnumMember|2:4-2:14|2:4-2:5', + ' y|EnumMember|2:17-2:24|2:17-2:18' + ]); + }); + + it('computes exact ranges for symbols after a multi-line template string', () => { + program.setFile('source/main.bs', [ + 'sub main(who as string)', + ' count = 1', + ' text = `line one 😀', + // eslint-disable-next-line no-template-curly-in-string + 'two ${who} and ${count}', + // eslint-disable-next-line no-template-curly-in-string + 'three ${ lcase(who) }`', + ' print text', + 'end sub', + 'function after()', + 'end function', + '' + ].join('\n')); + expect(getSymbolStrings('source/main.bs')).to.eql([ + 'main|Function|0:0-6:7|0:4-0:8', + 'after|Function|7:0-8:12|7:9-7:14' + ]); + }); + }); + function expectSymbols(documentSymbols: DocumentSymbol[], expected: SymbolTree) { expect( symbolKindToString(createSymbolTree(documentSymbols)) diff --git a/src/bscPlugin/symbols/WorkspaceSymbolProcessor.spec.ts b/src/bscPlugin/symbols/WorkspaceSymbolProcessor.spec.ts index 2142cc102..7f4480453 100644 --- a/src/bscPlugin/symbols/WorkspaceSymbolProcessor.spec.ts +++ b/src/bscPlugin/symbols/WorkspaceSymbolProcessor.spec.ts @@ -279,4 +279,120 @@ describe('WorkspaceSymbolProcessor', () => { ['g', SymbolKind.EnumMember] ]); }); + + describe('location boundaries', () => { + /** + * Get every workspace symbol as a `name|kind|containerName|uri|range` string, sorted for stable comparison + */ + function getSymbolStrings() { + return program.getWorkspaceSymbols().map((x: any) => { + return `${x.name}|${SymbolKindMap.get(x.kind)}|${x.containerName}|${x.location.uri}|${util.rangeToString(x.location.range)}`; + }).sort(); + } + + function uri(pkgPath: string) { + return util.pathToUri(s`${rootDir}/${pkgPath}`); + } + + it('computes exact locations across .bs and .brs files, including CRLF, emoji, and nested namespaces', () => { + program.setFile('source/main.bs', [ + 'namespace alpha', + ' namespace beta.charlie', + ' class Person', + ' name as string', + ' sub speak()', + ' end sub', + ' end class', + ' end namespace', + ' enum Direction', + ' up = "up"', + ' down = "down"', + ' end enum', + ' interface Shape', + ' width as integer', + ' function area() as float', + ' end interface', + ' const PI = 3.14', + 'end namespace', + 'function empty()', + 'end function', + 'const LAST_ONE = true' + ].join('\n')); + program.setFile('source/lib.brs', [ + '\' 😀 header comment', + 'function alpha(a, b)', + ' if a then', + ' print "😀"', + ' elseif b then', + ' print 2', + ' end if', + 'end function', + 'sub beta() : print "😀😀" : end sub : sub charlie() : end sub', + 'sub emptyBrs()', + 'end sub', + '' + ].join('\r\n')); + const main = uri('source/main.bs'); + const lib = uri('source/lib.brs'); + expect(getSymbolStrings()).to.eql([ + `alpha|Function|undefined|${lib}|1:9-1:14`, + `alpha|Namespace|undefined|${main}|0:10-0:15`, + `area|Method|Shape|${main}|14:17-14:21`, + `beta|Function|undefined|${lib}|8:4-8:8`, + `charlie|Function|undefined|${lib}|8:42-8:49`, + `charlie|Namespace|alpha|${main}|1:14-1:26`, + `Direction|Enum|alpha|${main}|8:9-8:18`, + `down|EnumMember|Direction|${main}|10:8-10:12`, + `empty|Function|undefined|${main}|18:9-18:14`, + `emptyBrs|Function|undefined|${lib}|9:4-9:12`, + `LAST_ONE|Constant|undefined|${main}|20:6-20:14`, + `name|Field|Person|${main}|3:12-3:16`, + `Person|Class|charlie|${main}|2:14-2:20`, + `PI|Constant|alpha|${main}|16:10-16:12`, + `Shape|Interface|alpha|${main}|12:14-12:19`, + `speak|Method|Person|${main}|4:16-4:21`, + `up|EnumMember|Direction|${main}|9:8-9:10`, + `width|Field|Shape|${main}|13:8-13:13` + ].sort()); + }); + + it('uses utf-16 code units for symbols that follow an emoji on the same line', () => { + program.setFile('source/main.bs', [ + 'const A = "😀" : const B = 2', + 'enum E', + ' x = "😀😀" : y = "b"', + 'end enum' + ].join('\n')); + const main = uri('source/main.bs'); + expect(getSymbolStrings()).to.eql([ + `A|Constant|undefined|${main}|0:6-0:7`, + `B|Constant|undefined|${main}|0:23-0:24`, + `E|Enum|undefined|${main}|1:5-1:6`, + `x|EnumMember|E|${main}|2:4-2:5`, + `y|EnumMember|E|${main}|2:17-2:18` + ].sort()); + }); + + it('computes exact locations for symbols after a multi-line template string', () => { + program.setFile('source/main.bs', [ + 'sub main(who as string)', + ' count = 1', + ' text = `line one 😀', + // eslint-disable-next-line no-template-curly-in-string + 'two ${who} and ${count}', + // eslint-disable-next-line no-template-curly-in-string + 'three ${ lcase(who) }`', + ' print text', + 'end sub', + 'function after()', + 'end function', + '' + ].join('\n')); + const main = uri('source/main.bs'); + expect(getSymbolStrings()).to.eql([ + `after|Function|undefined|${main}|7:9-7:14`, + `main|Function|undefined|${main}|0:4-0:8` + ].sort()); + }); + }); }); diff --git a/src/bscPlugin/symbols/symbolUtils.ts b/src/bscPlugin/symbols/symbolUtils.ts index d1c1b740f..0bc4696c3 100644 --- a/src/bscPlugin/symbols/symbolUtils.ts +++ b/src/bscPlugin/symbols/symbolUtils.ts @@ -98,57 +98,57 @@ function getSymbolsFromAstNode(node: AstNode): SymbolInfo[] { node.walk(createVisitor({ FunctionStatement: (statement) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Function, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Function, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, ClassStatement: (statement, parent) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Class, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Class, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, FieldStatement: (statement, parent) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Field, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Field, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, MethodStatement: (statement, parent) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Method, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Method, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, InterfaceStatement: (statement, parent) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Interface, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Interface, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, InterfaceFieldStatement: (statement, parent) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Field, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Field, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, InterfaceMethodStatement: (statement, parent) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Method, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Method, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, ConstStatement: (statement) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Constant, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Constant, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, NamespaceStatement: (statement) => { if (statement.nameExpression) { - addSymbol(statement, statement.getNameParts().pop().text, SymbolKind.Namespace, statement.location?.range, statement.nameExpression.location?.range); + addSymbol(statement, statement.getNameParts().pop().text, SymbolKind.Namespace, util.getLocation(statement)?.range, util.getLocation(statement.nameExpression)?.range); } }, EnumStatement: (statement) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.Enum, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.Enum, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } }, EnumMemberStatement: (statement) => { if (statement.tokens.name?.text) { - addSymbol(statement, statement.tokens.name.text, SymbolKind.EnumMember, statement.location?.range, statement.tokens.name.location?.range); + addSymbol(statement, statement.tokens.name.text, SymbolKind.EnumMember, util.getLocation(statement)?.range, util.getLocation(statement.tokens.name)?.range); } } }), { diff --git a/src/bscPlugin/transpile/BrsFileTranspileProcessor.ts b/src/bscPlugin/transpile/BrsFileTranspileProcessor.ts index e3f284383..34362af46 100644 --- a/src/bscPlugin/transpile/BrsFileTranspileProcessor.ts +++ b/src/bscPlugin/transpile/BrsFileTranspileProcessor.ts @@ -4,7 +4,7 @@ import { isDottedGetExpression, isLiteralExpression, isVariableExpression, isUna import { createVisitor, InternalWalkMode, WalkMode } from '../../astUtils/visitors'; import type { BrsFile } from '../../files/BrsFile'; import type { ExtraSymbolData, OnPrepareFileEvent } from '../../interfaces'; -import type { Identifier, Token } from '../../lexer/Token'; +import type { Identifier, Locatable, Token } from '../../lexer/Token'; import { TokenKind } from '../../lexer/TokenKind'; import type { AstNode, Expression, Statement } from '../../parser/AstNode'; import type { AALiteralExpression, NullCoalescingExpression, TernaryExpression } from '../../parser/Expression'; @@ -12,7 +12,6 @@ import { ArrayLiteralExpression, BinaryExpression, DottedGetExpression, LiteralE import { ParseMode } from '../../parser/Parser'; import type { Block, Body, ConstStatement, NamespaceStatement } from '../../parser/Statement'; import { AugmentedAssignmentStatement, ExpressionStatement, type AliasStatement, type IfStatement } from '../../parser/Statement'; -import type { Location } from 'vscode-languageserver'; import type { Scope } from '../../Scope'; import { SymbolTypeFlag } from '../../SymbolTypeFlag'; import util from '../../util'; @@ -127,18 +126,18 @@ export class BrsFilePreTranspileProcessor { return; } const editor = this.event.editor; - const location = expression.tokens.questionQuestion?.location; + const locatable = expression.tokens.questionQuestion; //`if = invalid then = end if` const createFallback = (name: string) => createIfStatement({ condition: new BinaryExpression({ - left: createVariableExpression(name, location), - operator: createToken(TokenKind.Equal, '=', location), - right: createInvalidLiteral('invalid', location) + left: createVariableExpression(name, locatable), + operator: createToken(TokenKind.Equal, '=', locatable), + right: createInvalidLiteral('invalid', locatable) }), thenBranch: createBlock({ statements: [ - createAssignmentStatement({ name: createIdentifier(name, location), value: expression.alternate }) + createAssignmentStatement({ name: createIdentifier(name, locatable), value: expression.alternate }) ] }) }); @@ -152,10 +151,10 @@ export class BrsFilePreTranspileProcessor { } else { const tmpName = '__bsc_tmp_coalesce'; newStatements = [ - createAssignmentStatement({ name: createIdentifier(tmpName, location), value: expression.consequent }), + createAssignmentStatement({ name: createIdentifier(tmpName, locatable), value: expression.consequent }), createFallback(tmpName) ]; - editor.setProperty(statement, 'value', createVariableExpression(tmpName, location)); + editor.setProperty(statement, 'value', createVariableExpression(tmpName, locatable)); editor.arraySplice(block.statements, index, 0, ...newStatements); } @@ -209,16 +208,16 @@ export class BrsFilePreTranspileProcessor { // - `m.list = [...]`: every follow-up statement would re-evaluate `m.list`; a local is 10-40% faster (bsbench) // - `list = [...list, 4]`: assigning the trimmed literal first would clobber `list` before we read it const tmpName = '__bsc_tmp_spread'; - const createTarget = (leadingTrivia?: Token[]) => this.createTargetExpression(tmpName, literal.location, leadingTrivia); + const createTarget = (leadingTrivia?: Token[]) => this.createTargetExpression(tmpName, literal, leadingTrivia); statements = [ - createAssignmentStatement({ name: createIdentifier(tmpName, literal.location), value: literal }), + createAssignmentStatement({ name: createIdentifier(tmpName, literal), value: literal }), ...this.createSpreadStatements(createTarget, trailing, isArray) ]; editor.setProperty(statement, 'value', createTarget()); editor.arraySplice(block.statements, index, 0, ...statements); } else { //local variable target: assign the trimmed literal, then append/push/set the rest directly on it - const createTarget = (leadingTrivia?: Token[]) => this.createTargetExpression(statement.tokens.name.text, statement.tokens.name.location, leadingTrivia); + const createTarget = (leadingTrivia?: Token[]) => this.createTargetExpression(statement.tokens.name.text, statement.tokens.name, leadingTrivia); statements = this.createSpreadStatements(createTarget, trailing, isArray); editor.arraySplice(block.statements, index + 1, 0, ...statements); } @@ -244,7 +243,7 @@ export class BrsFilePreTranspileProcessor { if (isArray && run.length >= ARRAY_SPREAD_APPEND_THRESHOLD) { //elements keep their own comments here: inside an array literal they transpile fine on their own lines const literal = new ArrayLiteralExpression({ elements: run }); - statements.push(this.createMethodCallStatement(createTarget(), 'append', literal, run[0].location)); + statements.push(this.createMethodCallStatement(createTarget(), 'append', literal, run[0])); } else { for (const element of run) { const target = createTarget(this.takeLeadingTrivia(element)); @@ -257,7 +256,7 @@ export class BrsFilePreTranspileProcessor { if (isSpreadExpression(element)) { flushRun(); const target = createTarget(this.takeLeadingTrivia(element)); - statements.push(this.createMethodCallStatement(target, 'append', element.expression, element.location)); + statements.push(this.createMethodCallStatement(target, 'append', element.expression, element)); } else { run.push(element); } @@ -270,8 +269,8 @@ export class BrsFilePreTranspileProcessor { * A fresh reference to the variable the follow-up statements operate on, optionally carrying the comments * that should be emitted above the statement */ - private createTargetExpression(name: string, location: Location, leadingTrivia?: Token[]) { - const target = createVariableExpression(name, location); + private createTargetExpression(name: string, locatable: Locatable, leadingTrivia?: Token[]) { + const target = createVariableExpression(name, locatable); if (leadingTrivia?.length > 0) { target.tokens.name.leadingTrivia = leadingTrivia; } @@ -300,7 +299,7 @@ export class BrsFilePreTranspileProcessor { private createElementStatement(target: Expression, element: Expression, isArray: boolean): Statement { if (isArray) { //`target.push(value)` - return this.createMethodCallStatement(target, 'push', element, element.location); + return this.createMethodCallStatement(target, 'push', element, element); } if (isAAIndexedMemberExpression(element)) { //`[key]: value` -> `target[key] = value` @@ -323,13 +322,13 @@ export class BrsFilePreTranspileProcessor { /** * `obj.methodName(arg)` as a standalone statement */ - private createMethodCallStatement(obj: Expression, methodName: string, arg: Expression, location: Location) { + private createMethodCallStatement(obj: Expression, methodName: string, arg: Expression, locatable: Locatable) { return new ExpressionStatement({ expression: createCall( new DottedGetExpression({ obj: obj, - name: createIdentifier(methodName, location), - dot: createToken(TokenKind.Dot, '.', location) + name: createIdentifier(methodName, locatable), + dot: createToken(TokenKind.Dot, '.', locatable) }), [arg] ) @@ -361,9 +360,9 @@ export class BrsFilePreTranspileProcessor { if (isAssignmentStatement(parent)) { ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', ternaryExpression.tokens.questionMark.location), + if: createToken(TokenKind.If, 'if', ternaryExpression.tokens.questionMark), condition: ternaryExpression.test, - then: createToken(TokenKind.Then, 'then', ternaryExpression.tokens.questionMark.location), + then: createToken(TokenKind.Then, 'then', ternaryExpression.tokens.questionMark), thenBranch: createBlock({ statements: [ createAssignmentStatement({ @@ -373,7 +372,7 @@ export class BrsFilePreTranspileProcessor { }) ] }), - else: createToken(TokenKind.Else, 'else', ternaryExpression.tokens.questionMark.location), + else: createToken(TokenKind.Else, 'else', ternaryExpression.tokens.questionMark), elseBranch: createBlock({ statements: [ createAssignmentStatement({ @@ -383,13 +382,13 @@ export class BrsFilePreTranspileProcessor { }) ] }), - endIf: createToken(TokenKind.EndIf, 'end if', ternaryExpression.tokens.questionMark.location) + endIf: createToken(TokenKind.EndIf, 'end if', ternaryExpression.tokens.questionMark) }); } else if (isDottedSetStatement(parent)) { ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', ternaryExpression.tokens.questionMark.location), + if: createToken(TokenKind.If, 'if', ternaryExpression.tokens.questionMark), condition: ternaryExpression.test, - then: createToken(TokenKind.Then, 'then', ternaryExpression.tokens.questionMark.location), + then: createToken(TokenKind.Then, 'then', ternaryExpression.tokens.questionMark), thenBranch: createBlock({ statements: [ createDottedSetStatement({ @@ -400,7 +399,7 @@ export class BrsFilePreTranspileProcessor { }) ] }), - else: createToken(TokenKind.Else, 'else', ternaryExpression.tokens.questionMark.location), + else: createToken(TokenKind.Else, 'else', ternaryExpression.tokens.questionMark), elseBranch: createBlock({ statements: [ createDottedSetStatement({ @@ -411,15 +410,15 @@ export class BrsFilePreTranspileProcessor { }) ] }), - endIf: createToken(TokenKind.EndIf, 'end if', ternaryExpression.tokens.questionMark.location) + endIf: createToken(TokenKind.EndIf, 'end if', ternaryExpression.tokens.questionMark) }); //if this is an indexedSetStatement, and the ternary expression is NOT an index } else if (isIndexedSetStatement(parent) && !parent.indexes?.includes(ternaryExpression)) { ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', ternaryExpression.tokens.questionMark.location), + if: createToken(TokenKind.If, 'if', ternaryExpression.tokens.questionMark), condition: ternaryExpression.test, - then: createToken(TokenKind.Then, 'then', ternaryExpression.tokens.questionMark.location), + then: createToken(TokenKind.Then, 'then', ternaryExpression.tokens.questionMark), thenBranch: createBlock({ statements: [ createIndexedSetStatement({ @@ -432,7 +431,7 @@ export class BrsFilePreTranspileProcessor { }) ] }), - else: createToken(TokenKind.Else, 'else', ternaryExpression.tokens.questionMark.location), + else: createToken(TokenKind.Else, 'else', ternaryExpression.tokens.questionMark), elseBranch: createBlock({ statements: [ createIndexedSetStatement({ @@ -445,13 +444,13 @@ export class BrsFilePreTranspileProcessor { }) ] }), - endIf: createToken(TokenKind.EndIf, 'end if', ternaryExpression.tokens.questionMark.location) + endIf: createToken(TokenKind.EndIf, 'end if', ternaryExpression.tokens.questionMark) }); } else if (isAugmentedAssignmentStatement(parent)) { ifStatement = createIfStatement({ - if: createToken(TokenKind.If, 'if', ternaryExpression.tokens.questionMark.location), + if: createToken(TokenKind.If, 'if', ternaryExpression.tokens.questionMark), condition: ternaryExpression.test, - then: createToken(TokenKind.Then, 'then', ternaryExpression.tokens.questionMark.location), + then: createToken(TokenKind.Then, 'then', ternaryExpression.tokens.questionMark), thenBranch: createBlock({ statements: [ new AugmentedAssignmentStatement({ @@ -461,7 +460,7 @@ export class BrsFilePreTranspileProcessor { }) ] }), - else: createToken(TokenKind.Else, 'else', ternaryExpression.tokens.questionMark.location), + else: createToken(TokenKind.Else, 'else', ternaryExpression.tokens.questionMark), elseBranch: createBlock({ statements: [ new AugmentedAssignmentStatement({ @@ -471,7 +470,7 @@ export class BrsFilePreTranspileProcessor { }) ] }), - endIf: createToken(TokenKind.EndIf, 'end if', ternaryExpression.tokens.questionMark.location) + endIf: createToken(TokenKind.EndIf, 'end if', ternaryExpression.tokens.questionMark) }); } @@ -628,7 +627,7 @@ export class BrsFilePreTranspileProcessor { // skip any changes in an Alias Statement return; } - let containingNamespaceStmt = this.event.file.getNamespaceStatementForPosition(expression.location?.range.start); + let containingNamespaceStmt = this.event.file.getNamespaceStatementForPosition(util.getLocation(expression)?.range.start); let containingNamespace = containingNamespaceStmt?.getName(ParseMode.BrighterScript); const parts = util.splitExpression(expression); diff --git a/src/bscPlugin/validation/BrsFileValidator.spec.ts b/src/bscPlugin/validation/BrsFileValidator.spec.ts index 77795ce49..1e962c09c 100644 --- a/src/bscPlugin/validation/BrsFileValidator.spec.ts +++ b/src/bscPlugin/validation/BrsFileValidator.spec.ts @@ -1455,7 +1455,7 @@ describe('BrsFileValidator', () => { it('reports the diagnostics of the branch that a plugin activates by editing the condition', () => { const diagnostics = validateAfterEdit((file) => { const statement = file.ast.findChild(isConditionalCompileStatement); - (statement.tokens as any).condition = createToken(TokenKind.True, 'true', statement.tokens.condition.location); + (statement.tokens as any).condition = createToken(TokenKind.True, 'true', statement.tokens.condition); }); expectDiagnostics(diagnostics, [ { code: 'unexpected-token', location: { range: util.createRange(3, 28, 3, 29) } }, diff --git a/src/bscPlugin/validation/BrsFileValidator.ts b/src/bscPlugin/validation/BrsFileValidator.ts index 854025816..6e26b11d8 100644 --- a/src/bscPlugin/validation/BrsFileValidator.ts +++ b/src/bscPlugin/validation/BrsFileValidator.ts @@ -13,8 +13,8 @@ import { SymbolTypeFlag } from '../../SymbolTypeFlag'; import { AssociativeArrayType } from '../../types/AssociativeArrayType'; import { DynamicType } from '../../types/DynamicType'; import util from '../../util'; -import type { Location, Position, Range } from 'vscode-languageserver'; -import type { Token } from '../../lexer/Token'; +import type { Location } from 'vscode-languageserver'; +import type { Locatable, SourceInfo, Token } from '../../lexer/Token'; import type { BrightScriptDoc } from '../../parser/BrightScriptDocParser'; import brsDocParser from '../../parser/BrightScriptDocParser'; import { TypeStatementType } from '../../types/TypeStatementType'; @@ -69,7 +69,7 @@ export class BrsFileValidator { /** * Parser diagnostics sorted by start position. Each entry also holds the furthest end among the entries up to and including it. */ - private parseDiagnosticIndex: Array<{ start: Position; furthestEnd: Position }> | undefined; + private parseDiagnosticIndex: Array<{ startPosition: number; furthestEndPosition: number }> | undefined; public process() { const unlinkGlobalSymbolTable = this.event.file.parser.symbolTable.pushParentProvider(() => this.event.program.globalScope.symbolTable); @@ -125,7 +125,7 @@ export class BrsFileValidator { if (firstLabelByName.has(lowerName)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.duplicateLabel(labelName), - location: label.tokens.name.location + location: label.tokens.name }); } else { firstLabelByName.set(lowerName, label); @@ -133,7 +133,7 @@ export class BrsFileValidator { if (this.isInsideTryBranch(label, func)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.labelInTryBlock(labelName), - location: label.tokens.name.location + location: label.tokens.name }); } } @@ -147,12 +147,12 @@ export class BrsFileValidator { if (!targetLabel) { this.event.program.diagnostics.register({ ...DiagnosticMessages.labelNotFound(labelToken.text), - location: labelToken.location + location: labelToken }); } else if (this.jumpsIntoForLoop(gotoStatement, targetLabel, func)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.gotoIntoForLoop(labelToken.text), - location: labelToken.location + location: labelToken }); } } @@ -211,24 +211,23 @@ export class BrsFileValidator { if (node.args.length > 5) { this.event.program.diagnostics.register({ ...DiagnosticMessages.callfuncHasToManyArgs(node.args.length), - location: node.tokens.methodName.location + location: node.tokens.methodName }); } }, DottedGetExpression: (node) => { if (node.tokens.dot?.kind === TokenKind.QuestionDot) { - this.validateMinFirmwareVersionForOptionalChaining(node.tokens.dot?.location?.range); + this.validateMinFirmwareVersionForOptionalChaining(node.tokens.dot); } }, IndexedGetExpression: (node) => { if (node.tokens.questionDot || node.tokens.openingSquare?.kind === TokenKind.QuestionLeftSquare) { - const range = node.tokens.questionDot?.location?.range ?? node.tokens.openingSquare?.location?.range; - this.validateMinFirmwareVersionForOptionalChaining(range); + this.validateMinFirmwareVersionForOptionalChaining(util.firstLocatable(node.tokens.questionDot, node.tokens.openingSquare)); } }, CallExpression: (node) => { if (node.tokens.openingParen?.kind === TokenKind.QuestionLeftParen) { - this.validateMinFirmwareVersionForOptionalChaining(node.tokens.openingParen?.location?.range); + this.validateMinFirmwareVersionForOptionalChaining(node.tokens.openingParen); } this.validateGlobalCallableAvailability(node); }, @@ -236,7 +235,7 @@ export class BrsFileValidator { this.validateRegexLiteral(node); }, EnumStatement: (node) => { - this.validateDeclarationLocations(node, 'enum', () => util.createBoundingRange(node.tokens.enum, node.tokens.name)); + this.validateDeclarationLocations(node, 'enum', () => util.setBounds({} as Locatable, node.tokens.enum, node.tokens.name)); this.validateEnumDeclaration(node); @@ -252,12 +251,12 @@ export class BrsFileValidator { if (!node?.tokens?.name) { return; } - this.validateDeclarationLocations(node, 'class', () => util.createBoundingRange(node.tokens.class, node.tokens.name)); + this.validateDeclarationLocations(node, 'class', () => util.setBounds({} as Locatable, node.tokens.class, node.tokens.name)); for (const constructor of node.getConditionalCompileConstructors()) { this.event.program.diagnostics.register({ ...DiagnosticMessages.classConstructorNotAllowedInConditionalCompile(), - location: constructor.tokens.name.location + location: constructor.tokens.name }); } @@ -315,11 +314,11 @@ export class BrsFileValidator { if (!node?.nameExpression) { return; } - this.validateDeclarationLocations(node, 'namespace', () => util.createBoundingRange(node.tokens.namespace, node.nameExpression)); + this.validateDeclarationLocations(node, 'namespace', () => util.setBounds({} as Locatable, node.tokens.namespace, node.nameExpression)); //Namespace Types are added at the Scope level - This is handled when the SymbolTables get linked }, FunctionStatement: (node) => { - this.validateDeclarationLocations(node, 'function', () => util.createBoundingRange(node.func.tokens.functionType, node.tokens.name)); + this.validateDeclarationLocations(node, 'function', () => util.setBounds({} as Locatable, node.func.tokens.functionType, node.tokens.name)); const funcType = node.getType({ flags: SymbolTypeFlag.typetime }); if (node.tokens.name?.text) { @@ -407,7 +406,7 @@ export class BrsFileValidator { if (!node.tokens.name) { return; } - this.validateDeclarationLocations(node, 'interface', () => util.createBoundingRange(node.tokens.interface, node.tokens.name)); + this.validateDeclarationLocations(node, 'interface', () => util.setBounds({} as Locatable, node.tokens.interface, node.tokens.name)); const nodeType = node.getType({ flags: SymbolTypeFlag.typetime }); // eslint-disable-next-line no-bitwise @@ -417,7 +416,7 @@ export class BrsFileValidator { if (!node.tokens.name) { return; } - this.validateDeclarationLocations(node, 'const', () => util.createBoundingRange(node.tokens.const, node.tokens.name)); + this.validateDeclarationLocations(node, 'const', () => util.setBounds({} as Locatable, node.tokens.const, node.tokens.name)); const nodeType = node.getType({ flags: SymbolTypeFlag.runtime }); node.parent.getSymbolTable().addSymbol(node.tokens.name.text, { definingNode: node, isInstance: true }, nodeType, SymbolTypeFlag.runtime); }, @@ -448,7 +447,7 @@ export class BrsFileValidator { } else { this.event.program.diagnostics.register({ ...DiagnosticMessages.expectedExceptionVarToFollowCatch(), - location: node.exceptionVariableExpression?.location ?? node.tokens.catch?.location + location: util.firstLocatable(node.exceptionVariableExpression, node.tokens.catch) }); } }, @@ -470,7 +469,7 @@ export class BrsFileValidator { if (node.value) { this.event.program.diagnostics.register({ ...DiagnosticMessages.voidFunctionMayNotReturnValue(func.tokens.functionType?.text), - location: node.location + location: node }); } @@ -479,7 +478,7 @@ export class BrsFileValidator { if (!node.value) { this.event.program.diagnostics.register({ ...DiagnosticMessages.nonVoidFunctionMustReturnValue(func?.tokens.functionType?.text), - location: node.location + location: node }); } } @@ -512,7 +511,7 @@ export class BrsFileValidator { ConditionalCompileErrorStatement: (node) => { this.event.program.diagnostics.register({ ...DiagnosticMessages.hashError(node.tokens.message.text), - location: node.location + location: node }); }, AliasStatement: (node) => { @@ -524,7 +523,7 @@ export class BrsFileValidator { }, TypeStatement: (node) => { - this.validateDeclarationLocations(node, 'type', () => util.createBoundingRange(node.tokens.type, node.tokens.name)); + this.validateDeclarationLocations(node, 'type', () => util.setBounds({} as Locatable, node.tokens.type, node.tokens.name)); const wrappedNodeType = node.getType({ flags: SymbolTypeFlag.runtime }); const typeStmtType = new TypeStatementType(node.tokens.name.text, wrappedNodeType); node.parent.getSymbolTable().addSymbol(node.tokens.name.text, { definingNode: node, isFromTypeStatement: true }, typeStmtType, SymbolTypeFlag.typetime); @@ -539,7 +538,7 @@ export class BrsFileValidator { if (!target || target.isInLoop) { this.event.program.diagnostics.register({ ...(target ? DiagnosticMessages.exitSelectInLoop() : DiagnosticMessages.exitSelectOutsideSelectCase()), - location: node.location + location: node }); } } @@ -564,7 +563,7 @@ export class BrsFileValidator { blockSymbolTable.isOrdered = true; } if (!isFunctionExpression(node.parent) && node.parent) { - node.symbolTable.name = `Block-${node.parent.kind}@${node.location?.range?.start?.line}`; + node.symbolTable.name = `Block-${node.parent.kind}@${util.getLocation(node)?.range?.start?.line}`; // we're a block inside another block (or body). This block is a pocket in the bigger block node.parent.getSymbolTable().addPocketTable({ index: node.parent.statementIndex, @@ -598,7 +597,7 @@ export class BrsFileValidator { ) { this.event.program.diagnostics.register({ ...DiagnosticMessages.reservedBuiltinUsedAsValue(name), - location: node.tokens.name.location + location: node.tokens.name }); } }, @@ -663,7 +662,7 @@ export class BrsFileValidator { * - inside a namespace * This is applicable to things like FunctionStatement, ClassStatement, NamespaceStatement, EnumStatement, InterfaceStatement */ - private validateDeclarationLocations(statement: Statement, keyword: string, rangeFactory?: () => (Range | undefined)) { + private validateDeclarationLocations(statement: Statement, keyword: string, locatableFactory?: () => Locatable) { //if nested inside a namespace, or defined at the root of the AST (i.e. in a body that has no parent) const isOkDeclarationLocation = (parentNode: AstNode | undefined) => { return isNamespaceStatement(parentNode?.parent) || (isBody(parentNode) && !parentNode?.parent); @@ -693,7 +692,7 @@ export class BrsFileValidator { //the statement was defined in the wrong place. Flag it. this.event.program.diagnostics.register({ ...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel(keyword), - location: rangeFactory ? util.createLocationFromFileRange(this.event.file, rangeFactory()) : statement.location + location: locatableFactory ? locatableFactory() : statement }); } @@ -726,7 +725,7 @@ export class BrsFileValidator { if (name && name.length > BrsFileValidator.MaxFunctionNameLength) { this.event.program.diagnostics.register({ ...DiagnosticMessages.functionNameTooLong(name, name.length, BrsFileValidator.MaxFunctionNameLength), - location: node.tokens.name?.location ?? node.location + location: util.firstLocatable(node.tokens.name, node) }); } } @@ -759,7 +758,7 @@ export class BrsFileValidator { for (let i = CallExpression.MaximumArguments; i < func.parameters.length; i++) { this.event.program.diagnostics.register({ ...DiagnosticMessages.tooManyCallableParameters(func.parameters.length, CallExpression.MaximumArguments), - location: func.parameters[i]?.tokens.name?.location ?? func.parameters[i]?.location ?? func.location + location: util.firstLocatable(func.parameters[i]?.tokens.name, func.parameters[i], func) }); } } @@ -771,7 +770,7 @@ export class BrsFileValidator { if (count > PrintStatement.MaximumPrintCount) { this.event.program.diagnostics.register({ ...DiagnosticMessages.tooManyPrintItems(count, PrintStatement.MaximumPrintCount), - location: statement.location + location: statement }); } } @@ -781,52 +780,53 @@ export class BrsFileValidator { * Nested functions have their own limit and are not included. Names that are only read are not counted, so the count is a lower bound. */ private validateFunctionVariableCount(func: FunctionExpression) { - const variableLocations = new Map(); - const addVariable = (nameToken: Token | undefined, fallbackLocation: Location) => { + //keep the locatable for each variable, and only build a `Location` for the one we report + const variableLocatables = new Map(); + const addVariable = (nameToken: Token | undefined, fallback: Locatable) => { const name = nameToken?.text?.toLowerCase(); - if (name && !variableLocations.has(name)) { - variableLocations.set(name, nameToken.location ?? fallbackLocation); + if (name && !variableLocatables.has(name)) { + variableLocatables.set(name, nameToken.sourceInfo ? nameToken : fallback); } }; // count all the parameters as they count as variables for (const parameter of func.parameters) { - addVariable(parameter.tokens.name, parameter.location); + addVariable(parameter.tokens.name, parameter); } // count all the variables in the function body func.body?.walk(createVisitor({ AssignmentStatement: (statement) => { - addVariable(statement.tokens.name, statement.location); + addVariable(statement.tokens.name, statement); }, AugmentedAssignmentStatement: (statement) => { if (isVariableExpression(statement.item)) { - addVariable(statement.item.tokens.name, statement.location); + addVariable(statement.item.tokens.name, statement); } }, DimStatement: (statement) => { - addVariable(statement.tokens.name, statement.location); + addVariable(statement.tokens.name, statement); }, IncrementStatement: (statement) => { if (isVariableExpression(statement.value)) { - addVariable(statement.value.tokens.name, statement.location); + addVariable(statement.value.tokens.name, statement); } }, ForEachStatement: (statement) => { - addVariable(statement.tokens.item, statement.location); + addVariable(statement.tokens.item, statement); }, CatchStatement: (statement) => { const exceptionVariable = isTypecastExpression(statement.exceptionVariableExpression) ? statement.exceptionVariableExpression.obj : statement.exceptionVariableExpression; if (isVariableExpression(exceptionVariable)) { - addVariable(exceptionVariable.tokens.name, statement.location); + addVariable(exceptionVariable.tokens.name, statement); } } }), { walkMode: WalkMode.visitAll }); - if (variableLocations.size > FunctionExpression.MaximumVariables) { - const firstOverLimitLocation = [...variableLocations.values()][FunctionExpression.MaximumVariables]; + if (variableLocatables.size > FunctionExpression.MaximumVariables) { + const firstOverLimit = [...variableLocatables.values()][FunctionExpression.MaximumVariables]; this.event.program.diagnostics.register({ - ...DiagnosticMessages.tooManyFunctionVariables(variableLocations.size, FunctionExpression.MaximumVariables), - location: firstOverLimitLocation ?? func.tokens.functionType?.location ?? func.location + ...DiagnosticMessages.tooManyFunctionVariables(variableLocatables.size, FunctionExpression.MaximumVariables), + location: util.firstLocatable(firstOverLimit, func.tokens.functionType, func) }); } } @@ -844,7 +844,7 @@ export class BrsFileValidator { */ private collectFunctionLiterals(func: FunctionExpression) { const pools = this.literalPools; - const addToPool = (poolName: LiteralPoolName, key: string | number, location: Location) => { + const addToPool = (poolName: LiteralPoolName, key: string | number, location: Locatable) => { let pool = pools[poolName]; if (!pool) { pool = { values: new Set(), firstOverLimitLocation: undefined }; @@ -865,24 +865,24 @@ export class BrsFileValidator { const text = token.text; switch (token.kind) { case TokenKind.IntegerLiteral: - addToPool('integer', /^&h/i.test(text) ? parseInt(text.slice(2), 16) : Number(text.replace('%', '')), literal.location); + addToPool('integer', /^&h/i.test(text) ? parseInt(text.slice(2), 16) : Number(text.replace('%', '')), literal); break; case TokenKind.LongIntegerLiteral: - addToPool('longinteger', getLongIntegerKey(text), literal.location); + addToPool('longinteger', getLongIntegerKey(text), literal); break; case TokenKind.FloatLiteral: - addToPool('float', Math.fround(getFloatingPointValue(text)), literal.location); + addToPool('float', Math.fround(getFloatingPointValue(text)), literal); break; case TokenKind.DoubleLiteral: - addToPool('double', getFloatingPointValue(text), literal.location); + addToPool('double', getFloatingPointValue(text), literal); break; case TokenKind.StringLiteral: - addToPool('string', getStringLiteralKey(text), literal.location); + addToPool('string', getStringLiteralKey(text), literal); break; case TokenKind.TemplateStringQuasi: //empty quasi strings are dropped from template strings, but tagged templates keep them if (text !== '' || isTaggedTemplateStringExpression(literal.parent?.parent)) { - addToPool('string', text, literal.location); + addToPool('string', text, literal); } break; default: @@ -894,12 +894,12 @@ export class BrsFileValidator { return; } for (const part of this.getSourceLiteralReplacements(sourceLiteral, this.event.file)) { - addToPool(part.type, part.value, sourceLiteral.location); + addToPool(part.type, part.value, sourceLiteral); } const literalKind = sourceLiteral.tokens.value.kind; const lineRange = sourceLiteral.location?.range; if (lineRange && (literalKind === TokenKind.LineNumLiteral || literalKind === TokenKind.PkgLocationLiteral)) { - addToPool('integer', (lineRange.start.line + 1) % 65536, sourceLiteral.location); + addToPool('integer', (lineRange.start.line + 1) % 65536, sourceLiteral); } }, AAMemberExpression: (member) => { @@ -907,14 +907,14 @@ export class BrsFileValidator { if (keyToken?.kind !== TokenKind.StringLiteral || member.findAncestor(isFunctionExpression) !== func) { return; } - addToPool('string', getStringLiteralKey(keyToken.text), keyToken.location ?? member.location); + addToPool('string', getStringLiteralKey(keyToken.text), util.firstLocatable(keyToken, member)); }, //template string escapes (like `\n` or `"`) transpile to `chr(charCode)` EscapedCharCodeLiteralExpression: (escape) => { if (escape.findAncestor(isFunctionExpression) !== func) { return; } - addToPool('integer', escape.tokens.value.charCode, escape.location); + addToPool('integer', escape.tokens.value.charCode, escape); }, //`/pattern/flags` transpiles to `CreateObject("roRegex", "pattern", "flags")`, with each `"` in the pattern split out as `chr(34)` RegexLiteralExpression: (regex) => { @@ -929,14 +929,14 @@ export class BrsFileValidator { flags = flagMatch[1]; } const patternParts = text.substring(1, text.length - 1).split('"'); - addToPool('string', 'roRegex', regex.location); + addToPool('string', 'roRegex', regex); for (const part of patternParts) { - addToPool('string', part, regex.location); + addToPool('string', part, regex); } if (patternParts.length > 1) { - addToPool('integer', 34, regex.location); + addToPool('integer', 34, regex); } - addToPool('string', flags, regex.location); + addToPool('string', flags, regex); }, //`node@.method()` transpiles to `node.callfunc("method")` CallfuncExpression: (callfunc) => { @@ -944,7 +944,7 @@ export class BrsFileValidator { if (!callfunc.tokens.methodName || callfunc.findAncestor(isFunctionExpression) !== func) { return; } - addToPool('string', callfunc.tokens.methodName.text, callfunc.tokens.methodName.location ?? callfunc.location); + addToPool('string', callfunc.tokens.methodName.text, util.firstLocatable(callfunc.tokens.methodName, callfunc)); } }); @@ -1067,14 +1067,14 @@ export class BrsFileValidator { * Nested functions have their own limit and are not included. */ private validateFunctionGotoLabelCount(func: FunctionExpression) { - const labelLocations = new Map(); + const labelLocations = new Map(); const continueLoops = new Set(); const continueIsTranspiled = this.event.file.needsTranspiled && !this.event.program.firmwareCapabilities.continueStatement; func.body?.walk(createVisitor({ GotoStatement: (statement) => { const labelName = statement.tokens.label?.text?.toLowerCase(); if (labelName && !labelLocations.has(labelName)) { - labelLocations.set(labelName, statement.tokens.label.location ?? statement.location); + labelLocations.set(labelName, util.firstLocatable(statement.tokens.label, statement)); } }, ContinueStatement: (statement) => { @@ -1087,7 +1087,7 @@ export class BrsFileValidator { } if (enclosingLoop && enclosingLoop !== func && !continueLoops.has(enclosingLoop)) { continueLoops.add(enclosingLoop); - labelLocations.set(`continue-loop:${continueLoops.size}`, statement.location); + labelLocations.set(`continue-loop:${continueLoops.size}`, statement); } } }), { walkMode: WalkMode.visitAll }); @@ -1096,7 +1096,7 @@ export class BrsFileValidator { const firstOverLimitLocation = [...labelLocations.values()][FunctionExpression.MaximumGotoLabels]; this.event.program.diagnostics.register({ ...DiagnosticMessages.tooManyGotoLabels(labelLocations.size, FunctionExpression.MaximumGotoLabels), - location: firstOverLimitLocation ?? func.tokens.functionType?.location ?? func.location + location: firstOverLimitLocation ?? util.firstLocatable(func.tokens.functionType, func) }); } } @@ -1115,7 +1115,7 @@ export class BrsFileValidator { if (memberNames.has(memberNameLower)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.duplicateIdentifier(member.name), - location: member.location + location: member }); } else { memberNames.add(memberNameLower); @@ -1136,7 +1136,7 @@ export class BrsFileValidator { memberValueKind = (member.value as LiteralExpression)?.tokens?.value?.kind; memberValue = member.value; } - const range = (memberValue ?? member)?.location?.range; + const valueLocatable: Locatable = memberValue ?? member; if ( //is integer enum, has value, that value type is not integer (enumValueKind === TokenKind.IntegerLiteral && memberValueKind && memberValueKind !== enumValueKind) || @@ -1147,7 +1147,7 @@ export class BrsFileValidator { ...DiagnosticMessages.enumValueMustBeType( enumValueKind.replace(/literal$/i, '').toLowerCase() ), - location: util.createLocationFromFileRange(this.event.file, range) + location: valueLocatable }); } @@ -1161,7 +1161,7 @@ export class BrsFileValidator { ...DiagnosticMessages.enumValueMustBeType( enumValueKind.replace(/literal$/i, '').toLowerCase() ), - location: util.createLocationFromFileRange(this.event.file, range) + location: valueLocatable }); } @@ -1171,7 +1171,7 @@ export class BrsFileValidator { ...DiagnosticMessages.enumValueIsRequired( enumValueKind.replace(/literal$/i, '').toLowerCase() ), - location: util.createLocationFromFileRange(this.event.file, range) + location: valueLocatable }); } } @@ -1186,12 +1186,12 @@ export class BrsFileValidator { if (!isLiteralBoolean(assignment.value)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.invalidHashConstValue(), - location: assignment.value.location + location: assignment.value }); } else if (statement.isDuplicate) { this.event.program.diagnostics.register({ ...DiagnosticMessages.duplicateConstDeclaration(assignment.tokens.name.text), - location: assignment.tokens.name.location + location: assignment.tokens.name }); } } @@ -1240,36 +1240,45 @@ export class BrsFileValidator { * The diagnostics are indexed once per validator, so each call is a binary search instead of a scan. */ private hasParseDiagnosticWithin(func: FunctionExpression) { - const functionRange = func.location?.range; - if (!functionRange) { + if (func.startPosition === undefined || !func.sourceInfo) { return false; } - const index = this.getParseDiagnosticIndex(); + const index = this.getParseDiagnosticIndex(func.sourceInfo); let low = 0; let high = index.length; while (low < high) { const middle = Math.floor((low + high) / 2); - if (util.comparePosition(index[middle].start, functionRange.end) < 0) { + if (index[middle].startPosition < func.endPosition) { low = middle + 1; } else { high = middle; } } - return low > 0 && util.comparePosition(index[low - 1].furthestEnd, functionRange.start) > 0; + return low > 0 && index[low - 1].furthestEndPosition > func.startPosition; } - private getParseDiagnosticIndex() { + private getParseDiagnosticIndex(sourceInfo: SourceInfo) { if (!this.parseDiagnosticIndex) { - const ranges = this.event.file.parser.diagnostics - .map(diagnostic => diagnostic.location?.range) - .filter(range => !!range) - .sort((rangeA, rangeB) => util.comparePosition(rangeA.start, rangeB.start)); - let furthestEnd: Position; - this.parseDiagnosticIndex = ranges.map(range => { - if (!furthestEnd || util.comparePosition(range.end, furthestEnd) > 0) { - furthestEnd = range.end; - } - return { start: range.start, furthestEnd: furthestEnd }; + const spans = this.event.file.parser.diagnostics + //parser diagnostics are usually Locatables (tokens/nodes). Convert any resolved `Location` into positions too + .map(diagnostic => { + const location = diagnostic.location; + if (location && ('startPosition' in location || !('uri' in location))) { + return location as Locatable; + } + const range = (location as Location)?.range; + return { + startPosition: util.getOffset(sourceInfo, range?.start), + endPosition: util.getOffset(sourceInfo, range?.end) + }; + }) + //synthetic locatables have no positions + .filter(span => span?.startPosition !== undefined && span.endPosition !== undefined) + .sort((spanA, spanB) => spanA.startPosition - spanB.startPosition); + let furthestEndPosition = -1; + this.parseDiagnosticIndex = spans.map(span => { + furthestEndPosition = Math.max(furthestEndPosition, span.endPosition); + return { startPosition: span.startPosition, furthestEndPosition: furthestEndPosition }; }); } return this.parseDiagnosticIndex; @@ -1304,9 +1313,9 @@ export class BrsFileValidator { if (branch.isActive === false) { this.flagInactiveBranchBodies(branch, overlappingRunLocation); } - } else if (branch?.isActive === false && branch.location) { + } else if (branch?.isActive === false && branch.sourceInfo) { //the branch runs from the end of its opening directive to the start of its closing directive - const { start, end } = branch.location.range; + const { start, end } = util.getLocation(branch).range; if (end.line - start.line > 1) { const location = util.createLocation(start.line + 1, 0, end.line, 0, this.event.file.srcPath); if (overlappingRunLocation && util.rangesIntersect(location.range, overlappingRunLocation.range)) { @@ -1328,7 +1337,7 @@ export class BrsFileValidator { if (statement.isConditionDeclared === false && statement.tokens.condition && !this.event.program.firmwareCapabilities.undeclaredHashConstIsFalse) { this.event.program.diagnostics.register({ ...DiagnosticMessages.hashConstDoesNotExist(), - location: statement.tokens.condition.location + location: statement.tokens.condition }); } } @@ -1361,7 +1370,7 @@ export class BrsFileValidator { ) { this.event.program.diagnostics.register({ ...DiagnosticMessages.unexpectedStatementOutsideFunction(), - location: statement.location + location: statement }); } } @@ -1404,17 +1413,17 @@ export class BrsFileValidator { if (isLibraryStatement(result)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.unexpectedStatementLocation('library', 'at the top of the file'), - location: result.location + location: result }); } else if (isImportStatement(result)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.unexpectedStatementLocation('import', 'at the top of the file'), - location: result.location + location: result }); } else if (isAliasStatement(result)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.unexpectedStatementLocation('alias', 'at the top of the file'), - location: result.location + location: result }); } } @@ -1442,7 +1451,7 @@ export class BrsFileValidator { if (isBadTypecastObj) { this.event.program.diagnostics.register({ ...DiagnosticMessages.invalidTypecastStatementApplication(resultVarStr, hasFunctionAncestor), - location: typecastStmt.typecastExpression.obj.location + location: typecastStmt.typecastExpression.obj }); } @@ -1466,18 +1475,17 @@ export class BrsFileValidator { if (!isFirst) { this.event.program.diagnostics.register({ ...DiagnosticMessages.unexpectedStatementLocation('typecast', 'at the top of the file or beginning of block or namespace'), - location: typecastStmt.location + location: typecastStmt }); } } } private validateSelectCaseStatement(statement: SelectCaseStatement) { - const headerLocation = util.createBoundingLocation(statement.tokens.select, statement.tokens.case); if (statement.cases.length === 0) { this.event.program.diagnostics.register({ ...DiagnosticMessages.selectCaseHasNoCases(), - location: headerLocation + location: util.setBounds({} as Locatable, statement.tokens.select, statement.tokens.case) }); return; } @@ -1489,14 +1497,14 @@ export class BrsFileValidator { for (const elseCase of elseCases.slice(1)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.duplicateCaseElse(), - location: util.createBoundingLocation(elseCase.tokens.case, elseCase.tokens.else) + location: util.setBounds({} as Locatable, elseCase.tokens.case, elseCase.tokens.else) }); } //only flag the first one. Any others were already flagged as duplicates if (statement.cases.indexOf(elseCases[0]) < statement.cases.length - elseCases.length) { this.event.program.diagnostics.register({ ...DiagnosticMessages.caseElseMustBeLast(), - location: util.createBoundingLocation(elseCases[0].tokens.case, elseCases[0].tokens.else) + location: util.setBounds({} as Locatable, elseCases[0].tokens.case, elseCases[0].tokens.else) }); } } @@ -1512,7 +1520,7 @@ export class BrsFileValidator { ) { this.event.program.diagnostics.register({ ...DiagnosticMessages.emptyCaseDoesNotFallThrough(), - location: caseStatement.tokens.case.location + location: caseStatement.tokens.case }); } } @@ -1536,10 +1544,10 @@ export class BrsFileValidator { if (firstValue) { this.event.program.diagnostics.register({ ...DiagnosticMessages.duplicateCaseValue( - util.getTextForRange(this.event.file.fileContents, value.location?.range) ?? key, - firstValue.location?.range.start.line + 1 + util.getTextForRange(this.event.file.fileContents, util.getLocation(value)?.range) ?? key, + util.getStartPosition(firstValue)?.line + 1 ), - location: value.location + location: value }); } else { seenValues.set(key, value); @@ -1553,7 +1561,7 @@ export class BrsFileValidator { } else if (valueType !== expectedType) { this.event.program.diagnostics.register({ ...DiagnosticMessages.caseValueTypeMismatch(valueType, expectedType), - location: value.location + location: value }); } } @@ -1612,7 +1620,7 @@ export class BrsFileValidator { const actualLoopType = statement.tokens.loopType; if (actualLoopType && expectedLoopType?.toLowerCase() !== actualLoopType.text?.toLowerCase()) { this.event.program.diagnostics.register({ - location: statement.tokens.loopType.location, + location: statement.tokens.loopType, ...DiagnosticMessages.expectedToken(expectedLoopType) }); } @@ -1634,7 +1642,7 @@ export class BrsFileValidator { //flag continue statements found outside of a loop if (!parent) { this.event.program.diagnostics.register({ - location: statement.location, + location: statement, ...DiagnosticMessages.illegalContinueStatement() }); } @@ -1658,7 +1666,7 @@ export class BrsFileValidator { CONTINUE_MIN_FIRMWARE_VERSION, this.event.program.getMinFirmwareVersion() ), - location: statement.location + location: statement }); } } @@ -1674,7 +1682,7 @@ export class BrsFileValidator { if (isSpreadExpression(element)) { this.event.program.diagnostics.register({ ...DiagnosticMessages.spreadOperatorNotAllowedHere(), - location: element.tokens.dotDotDot.location + location: element.tokens.dotDotDot }); } } @@ -1694,18 +1702,18 @@ export class BrsFileValidator { (isIndexedSetStatement(node) && node.tokens.openingSquare?.kind === TokenKind.QuestionLeftSquare) ) { //try to highlight the entire left-hand-side expression if possible - let range: Range; + let locatable: Locatable; if (isDottedSetStatement(parent)) { - range = util.createBoundingRange(parent.obj?.location, parent.tokens.dot, parent.tokens.name); + locatable = util.setBounds({} as Locatable, parent.obj, parent.tokens.dot, parent.tokens.name); } else if (isIndexedSetStatement(parent)) { - range = util.createBoundingRange(parent.obj?.location, parent.tokens.openingSquare, ...parent.indexes, parent.tokens.closingSquare); + locatable = util.setBounds({} as Locatable, parent.obj, parent.tokens.openingSquare, ...parent.indexes, parent.tokens.closingSquare); } else { - range = node.location?.range; + locatable = node; } this.event.program.diagnostics.register({ ...DiagnosticMessages.noOptionalChainingInLeftHandSideOfAssignment(), - location: util.createLocationFromFileRange(this.event.file, range) + location: locatable }); } @@ -1745,17 +1753,17 @@ export class BrsFileValidator { */ private validateRegexLiteral(node: RegexLiteralExpression) { const token = node.tokens.regexLiteral; - const start = token?.location?.range?.start; - if (!start) { + if (!token?.sourceInfo) { return; } - //regex literals are always on a single line, so problem offsets map directly onto the token's starting line + //problem offsets are relative to the start of the token for (const problem of findRegexLiteralProblems(token.text)) { this.event.program.diagnostics.register({ ...problem.diagnostic, location: { - uri: token.location.uri, - range: util.createRange(start.line, start.character + problem.start, start.line, start.character + problem.end) + startPosition: token.startPosition + problem.start, + endPosition: token.startPosition + problem.end, + sourceInfo: token.sourceInfo } }); } @@ -1767,7 +1775,7 @@ export class BrsFileValidator { * This applies to both .brs and .bs files because optional chaining is not transpiled — * it is emitted as-is, so the target device must natively support it. */ - private validateMinFirmwareVersionForOptionalChaining(range: Range | undefined) { + private validateMinFirmwareVersionForOptionalChaining(locatable: Locatable | undefined) { if (!this.event.program.firmwareCapabilities.optionalChaining) { this.event.program.diagnostics.register({ ...DiagnosticMessages.featureRequiresMinFirmwareVersion( @@ -1775,7 +1783,7 @@ export class BrsFileValidator { OPTIONAL_CHAINING_MIN_FIRMWARE_VERSION, this.event.program.getMinFirmwareVersion() ), - location: util.createLocationFromFileRange(this.event.file, range) + location: locatable }); } } @@ -1808,7 +1816,7 @@ export class BrsFileValidator { if (diagnostic) { this.event.program.diagnostics.register({ ...diagnostic, - location: node.callee.location + location: node.callee }); } } @@ -1865,7 +1873,7 @@ type LiteralPoolName = 'integer' | 'string' | 'float' | 'double' | 'longinteger' interface LiteralPool { values: Set; - firstOverLimitLocation: Location | undefined; + firstOverLimitLocation: Locatable | undefined; } interface LabelsAndGotos { diff --git a/src/bscPlugin/validation/ControlFlowReachability.ts b/src/bscPlugin/validation/ControlFlowReachability.ts index fb3fecb19..09640b840 100644 --- a/src/bscPlugin/validation/ControlFlowReachability.ts +++ b/src/bscPlugin/validation/ControlFlowReachability.ts @@ -361,7 +361,7 @@ class ReachabilityAnalysis { } private extendRun(statement: Statement, reason: string) { - if (!statement.location) { + if (!statement.sourceInfo) { return; } this.unreachableNodes.add(statement); @@ -386,7 +386,7 @@ class ReachabilityAnalysis { } private classifyChainCoverage(chain: ConditionalCompileStatement, runLocation: Location) { - const chainRange = chain.location?.range; + const chainRange = util.getLocation(chain)?.range; if (!chainRange) { return; } diff --git a/src/bscPlugin/validation/ScopeValidator.spec.ts b/src/bscPlugin/validation/ScopeValidator.spec.ts index dd9563883..d0fbe9d5a 100644 --- a/src/bscPlugin/validation/ScopeValidator.spec.ts +++ b/src/bscPlugin/validation/ScopeValidator.spec.ts @@ -1,7 +1,6 @@ import * as sinonImport from 'sinon'; import { DiagnosticMessages } from '../../DiagnosticMessages'; import { Program } from '../../Program'; -import { util } from '../../util'; import { expectDiagnostics, expectDiagnosticsIncludes, expectTypeToBe, expectZeroDiagnostics, trim } from '../../testHelpers.spec'; import { expect } from 'chai'; import type { TypeCompatibilityData } from '../../interfaces'; @@ -19,6 +18,8 @@ import { ScopeValidator } from './ScopeValidator'; import type { FunctionStatement, ReturnStatement } from '../../parser/Statement'; import { Logger } from '@rokucommunity/logger'; import { ParseMode } from '../..'; +import { Range } from 'vscode-languageserver'; +import { util, standardizePath as s } from '../../util'; describe('ScopeValidator', () => { @@ -7568,4 +7569,325 @@ describe('ScopeValidator', () => { expect((unvalidatedSegments[0] as FunctionStatement).getName(ParseMode.BrighterScript)).to.equal('doStuff'); }); }); + + /* eslint-disable no-template-curly-in-string */ + describe('location boundaries', () => { + function uriOf(pkgPath: string) { + return util.pathToUri(s`${rootDir}/${pkgPath}`); + } + + function range(startLine: number, startChar: number, endLine: number, endChar: number) { + return Range.create(startLine, startChar, endLine, endChar); + } + + it('flags an identifier in a .brs file', () => { + program.setFile('source/main.brs', 'sub main()\n print notDefined\nend sub\n'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.brs'), + range: range(1, 10, 1, 20) + } + }]); + }); + + it('flags an identifier in a .bs file', () => { + program.setFile('source/main.bs', 'sub main()\n print notDefined\nend sub\n'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.bs'), + range: range(1, 10, 1, 20) + } + }]); + }); + + it('flags a token at column 0 right after a newline', () => { + program.setFile('source/main.brs', 'sub main()\nnotDefined()\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindFunction('notDefined').code, + location: { + uri: uriOf('source/main.brs'), + range: range(1, 0, 1, 10) + } + }]); + }); + + it('flags a token at the end of a line and at the end of a file with no trailing newline', () => { + program.setFile('source/main.brs', 'sub main()\nend sub\nprint notDefined'); + program.validate(); + expectDiagnostics(program, [{ + //the whole statement (starting at column 0) + code: DiagnosticMessages.unexpectedStatementOutsideFunction().code, + location: { + uri: uriOf('source/main.brs'), + range: range(2, 0, 2, 16) + } + }, { + //the very last token in the file + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.brs'), + range: range(2, 6, 2, 16) + } + }]); + }); + + it('flags a multi-line statement in a .brs file', () => { + program.setFile('source/main.brs', 'sub main()\n return {\n a: 1\n }\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + //the whole return statement + code: DiagnosticMessages.voidFunctionMayNotReturnValue('sub').code, + location: { + uri: uriOf('source/main.brs'), + range: range(1, 4, 3, 5) + } + }, { + //just the returned expression + code: DiagnosticMessages.returnTypeMismatch('roAssociativeArray', 'void').code, + location: { + uri: uriOf('source/main.brs'), + range: range(1, 11, 3, 5) + } + }]); + }); + + it('flags a multi-line statement in a .bs file', () => { + program.setFile('source/main.bs', 'sub main()\n x as integer = {\n a: 1\n }\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.assignmentTypeMismatch('roAssociativeArray', 'integer').code, + location: { + uri: uriOf('source/main.bs'), + range: range(1, 4, 3, 5) + } + }]); + }); + + it('flags a multi-line statement in a file with CRLF line endings', () => { + program.setFile('source/main.brs', 'sub main()\r\n return {\r\n a: 1\r\n }\r\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.voidFunctionMayNotReturnValue('sub').code, + location: { + uri: uriOf('source/main.brs'), + range: range(1, 4, 3, 5) + } + }, { + code: DiagnosticMessages.returnTypeMismatch('roAssociativeArray', 'void').code, + location: { + uri: uriOf('source/main.brs'), + range: range(1, 11, 3, 5) + } + }]); + }); + + it('flags identifiers in a file with CRLF line endings', () => { + program.setFile('source/main.bs', 'sub main()\r\n a = 1\r\n print notDefined\r\nnotDefined2()\r\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.bs'), + range: range(2, 10, 2, 20) + } + }, { + code: DiagnosticMessages.cannotFindFunction('notDefined2').code, + location: { + uri: uriOf('source/main.bs'), + range: range(3, 0, 3, 11) + } + }]); + }); + + it('flags identifiers in a file mixing LF and CRLF line endings', () => { + program.setFile('source/main.brs', 'sub main()\r\n a = 1\n print notDefined\r\n\n\r\n print a + notDefined2\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.brs'), + range: range(2, 10, 2, 20) + } + }, { + code: DiagnosticMessages.cannotFindName('notDefined2').code, + location: { + uri: uriOf('source/main.brs'), + range: range(5, 14, 5, 25) + } + }]); + }); + + it('counts a surrogate-pair emoji before the flagged token as two characters', () => { + program.setFile('source/main.brs', 'sub main()\n print "😀" + notDefined\n print "😀😀" + notDefined2\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.brs'), + range: range(1, 17, 1, 27) + } + }, { + code: DiagnosticMessages.cannotFindName('notDefined2').code, + location: { + uri: uriOf('source/main.brs'), + range: range(2, 19, 2, 30) + } + }]); + }); + + it('counts a surrogate-pair emoji before the flagged token on a CRLF line', () => { + program.setFile('source/main.bs', 'sub main()\r\n print "😀" + notDefined\r\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.bs'), + range: range(1, 17, 1, 27) + } + }]); + }); + + it('flags a token after a multi-line template string', () => { + program.setFile('source/main.bs', 'sub main()\n x = `a\n${1}\n`: print notDefined\n print notDefined2\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.bs'), + range: range(3, 9, 3, 19) + } + }, { + code: DiagnosticMessages.cannotFindName('notDefined2').code, + location: { + uri: uriOf('source/main.bs'), + range: range(4, 10, 4, 21) + } + }]); + }); + + it('flags a token after a multi-line template string with CRLF line endings', () => { + program.setFile('source/main.bs', 'sub main()\r\n x = `a\r\n${1}\r\n`: print notDefined\r\n print notDefined2\r\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.bs'), + range: range(3, 9, 3, 19) + } + }, { + code: DiagnosticMessages.cannotFindName('notDefined2').code, + location: { + uri: uriOf('source/main.bs'), + range: range(4, 10, 4, 21) + } + }]); + }); + + it('flags a token inside a template string expression on a later line of the template', () => { + program.setFile('source/main.bs', 'sub main()\n x = `😀 ${1}\n and ${notDefined}\n`\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.bs'), + range: range(2, 8, 2, 18) + } + }]); + }); + + it('flags a token inside a template string expression on a later CRLF line of the template', () => { + program.setFile('source/main.bs', 'sub main()\r\n x = `😀 ${1}\r\n and ${notDefined}\r\n`\r\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.cannotFindName('notDefined').code, + location: { + uri: uriOf('source/main.bs'), + range: range(2, 8, 2, 18) + } + }]); + }); + + it('includes correct related information uris and ranges across files', () => { + program.setFile('source/a.brs', 'sub main()\nend sub\n\nsub doIt()\nend sub'); + program.setFile('source/b.brs', 'sub doIt()\r\nend sub'); + program.validate(); + expectDiagnostics(program, [{ + code: DiagnosticMessages.duplicateFunctionImplementation('doIt').code, + location: { + uri: uriOf('source/a.brs'), + range: range(3, 4, 3, 8) + }, + relatedInformation: [{ + location: { + uri: uriOf('source/b.brs'), + range: range(0, 4, 0, 8) + }, + message: 'Function declared here' + }, { + location: { + uri: uriOf('source/a.brs'), + range: range(3, 4, 3, 8) + }, + message: `In scope 'source'` + }] + }, { + code: DiagnosticMessages.duplicateFunctionImplementation('doIt').code, + location: { + uri: uriOf('source/b.brs'), + range: range(0, 4, 0, 8) + }, + relatedInformation: [{ + location: { + uri: uriOf('source/a.brs'), + range: range(3, 4, 3, 8) + }, + message: 'Function declared here' + }, { + location: { + uri: uriOf('source/b.brs'), + range: range(0, 4, 0, 8) + }, + message: `In scope 'source'` + }] + }]); + }); + + it('includes correct locations for xml diagnostics in a CRLF file', () => { + program.setFile('components/Comp.xml', '\r\n\r\n