From 6db17ede84979b1d54894d5f4c219f885e53f505 Mon Sep 17 00:00:00 2001 From: sslinky <39886505+SSlinky@users.noreply.github.com> Date: Mon, 16 Jun 2025 11:25:53 +0800 Subject: [PATCH 1/3] Moved link validation to method for readability --- server/src/capabilities/capabilities.ts | 40 ++++++++++++++----------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/server/src/capabilities/capabilities.ts b/server/src/capabilities/capabilities.ts index b02d0b0..eea3d78 100644 --- a/server/src/capabilities/capabilities.ts +++ b/server/src/capabilities/capabilities.ts @@ -287,24 +287,7 @@ export class ScopeItemCapability { if (this.type === ScopeType.REFERENCE) { // Link to declaration if it exists. this.resolveLinks(); - if (!this.link) { - // TODO: - // References to variables should get a diagnostic if they aren't declared. - // -- No option explicit: Hint with code action to declare. - // GET before declared gets a warning. - // -- Option explicit: Error with code action to declare. - // -- Subsequent explicit declaration should raise duplicate declaration (current bahaviour). - // -- All declarations with no GET references get a warning. - // References to function or sub calls should raise an error if they aren't declared. - // -- Must always throw even when option explicit not present. - // -- Nothing required on first reference as declaration may come later. - const severity = this.isOptionExplicitScope - ? DiagnosticSeverity.Error - : DiagnosticSeverity.Hint; - const _ = this.assignmentType & AssignmentType.CALL - ? this.pushDiagnostic(SubOrFunctionNotDefinedDiagnostic, this, this.name) - : this.pushDiagnostic(VariableNotDefinedDiagnostic, this, this.name, severity); - } + this.validateLink(); } else { // Diagnostic checks on declarations. const ancestors = this.getParentChain(); @@ -546,6 +529,27 @@ export class ScopeItemCapability { this.linkThisToItem(foundDeclarations[0]); } + private validateLink() { + if (!this.link) { + // TODO: + // References to variables should get a diagnostic if they aren't declared. + // -- No option explicit: Hint with code action to declare. + // GET before declared gets a warning. + // -- Option explicit: Error with code action to declare. + // -- Subsequent explicit declaration should raise duplicate declaration (current bahaviour). + // -- All declarations with no GET references get a warning. + // References to function or sub calls should raise an error if they aren't declared. + // -- Must always throw even when option explicit not present. + // -- Nothing required on first reference as declaration may come later. + const severity = this.isOptionExplicitScope + ? DiagnosticSeverity.Error + : DiagnosticSeverity.Hint; + const _ = this.assignmentType & AssignmentType.CALL + ? this.pushDiagnostic(SubOrFunctionNotDefinedDiagnostic, this, this.name) + : this.pushDiagnostic(VariableNotDefinedDiagnostic, this, this.name, severity); + } + } + private linkThisToItem(linkItem?: ScopeItemCapability): void { if (!linkItem) { return; From b0d2b71ab4be000218895791059f3553aaae9172 Mon Sep 17 00:00:00 2001 From: sslinky <39886505+SSlinky@users.noreply.github.com> Date: Tue, 17 Jun 2025 11:25:20 +0800 Subject: [PATCH 2/3] Shortcut the range property --- server/src/capabilities/capabilities.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/server/src/capabilities/capabilities.ts b/server/src/capabilities/capabilities.ts index eea3d78..31ec079 100644 --- a/server/src/capabilities/capabilities.ts +++ b/server/src/capabilities/capabilities.ts @@ -266,6 +266,10 @@ export class ScopeItemCapability { return result === '' ? 'NONE' : result; } + get range(): Range | undefined { + return this.element?.context.range; + } + // Item Properties locationUri?: string; isPublicScope?: boolean; @@ -904,7 +908,7 @@ export class ScopeItemCapability { // Check all items for whether they have a name overlap or a scope overlap. scope?.maps.forEach(map => map.forEach(items => items.forEach(item => { - const elementRange = item.element?.context.range; + const elementRange = item.range; const identifierRange = item.element?.identifierCapability?.range; if (identifierRange && isPositionInsideRange(position, identifierRange)) { // Position is inside the identifier, push to results. @@ -976,7 +980,7 @@ export class ScopeItemCapability { link.locationUri, link.element.context.range, link.element.identifierCapability.range, - this.element?.context.range + this.range ); } @@ -1049,7 +1053,7 @@ export class ScopeItemCapability { }; const m = new Map(); - results.forEach(s => m.set(rangeString(s.element?.context.range), s)); + results.forEach(s => m.set(rangeString(s.range), s)); return Array.from(m.values()); } } \ No newline at end of file From 6fdc93222a34f69e5a7bb2e11c1ea7e25a3580f3 Mon Sep 17 00:00:00 2001 From: sslinky <39886505+SSlinky@users.noreply.github.com> Date: Tue, 17 Jun 2025 11:38:28 +0800 Subject: [PATCH 3/3] Bug fix #91 --- server/src/capabilities/capabilities.ts | 29 ++++++++++++++++--------- server/src/project/workspace.ts | 3 +-- server/src/utils/helpers.ts | 15 ++++++++++++- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/server/src/capabilities/capabilities.ts b/server/src/capabilities/capabilities.ts index 31ec079..4903232 100644 --- a/server/src/capabilities/capabilities.ts +++ b/server/src/capabilities/capabilities.ts @@ -17,7 +17,7 @@ import { FoldingRange, FoldingRangeKind } from '../capabilities/folding'; import { SemanticToken, SemanticTokenModifiers, SemanticTokenTypes } from '../capabilities/semanticTokens'; import { BaseRuleSyntaxElement, BaseIdentifyableSyntaxElement, BaseSyntaxElement, Context, HasSemanticTokenCapability } from '../project/elements/base'; import { AmbiguousNameDiagnostic, BaseDiagnostic, DuplicateDeclarationDiagnostic, ShadowDeclarationDiagnostic, SubOrFunctionNotDefinedDiagnostic, UnusedDiagnostic, VariableNotDefinedDiagnostic } from './diagnostics'; -import { isPositionInsideRange, isRangeInsideRange } from '../utils/helpers'; +import { isPositionInsideRange, isRangeInsideRange, rangeEquals } from '../utils/helpers'; abstract class BaseCapability { @@ -795,12 +795,7 @@ export class ScopeItemCapability { * Recursively removes all scopes with the passed in uri and * within the range bounds, including where it is linked. */ - invalidate(uri: string, range: Range): void { - const isInvalidScope = (scope: ScopeItemCapability) => - scope.locationUri === uri - && scope.element?.context.range - && isRangeInsideRange(scope.element.context.range, range); - + invalidate(uri: string, range?: Range): void { const cleanScopes = (scopes?: ScopeItemCapability[]) => { if (scopes === undefined) { return undefined; @@ -808,7 +803,7 @@ export class ScopeItemCapability { const result: ScopeItemCapability[] = []; scopes.forEach(scope => { - if (isInvalidScope(scope)) { + if (scope.isLocatedAt(uri, range)) { Services.logger.debug(`Invalidating ${scope.name}`); // Clean the backlinks on the linked item if we have one. @@ -816,7 +811,7 @@ export class ScopeItemCapability { scope.link.backlinks); // Clean the invaludated scope. - scope.invalidate(uri, range); + scope.invalidate(uri, scope.range); return; } @@ -857,8 +852,22 @@ export class ScopeItemCapability { // Do a basic clean on backlinks that doesn't trigger recursion. if (this.backlinks) { - this.backlinks = this.backlinks.filter(scope => !isInvalidScope(scope)); + this.backlinks = this.backlinks + .filter(scope => !scope.isLocatedAt(uri, range)); + } + } + + /** Returns true if the uri matches and, if passed, range is fully inside range. */ + isLocatedAt(uri: string, range?: Range): boolean { + if (uri !== this.locationUri) { + return false; } + + if (!range) { + return true; + } + + return isRangeInsideRange(this.range, range); } /** Returns true for public and false for private */ diff --git a/server/src/project/workspace.ts b/server/src/project/workspace.ts index 7884226..a587b93 100644 --- a/server/src/project/workspace.ts +++ b/server/src/project/workspace.ts @@ -153,8 +153,7 @@ export class Workspace implements IWorkspace { if (previousDocument) { Services.projectScope.invalidate( - previousDocument.uri, - previousDocument.range + previousDocument.uri ); } diff --git a/server/src/utils/helpers.ts b/server/src/utils/helpers.ts index eaeb02a..837115d 100644 --- a/server/src/utils/helpers.ts +++ b/server/src/utils/helpers.ts @@ -94,7 +94,12 @@ export function isPositionInsideRange(position: Position, range: Range): boolean * @param inner The range to test as enveloped. * @param outer The range to test as enveloping. */ -export function isRangeInsideRange(inner: Range, outer: Range): boolean { +export function isRangeInsideRange(inner?: Range, outer?: Range): boolean { + // Test we have ranges. + if (!inner || !outer) { + return false; + } + // Test characters on single-line ranges. const isSingleLine = inner.start.line === inner.end.line && outer.start.line === outer.end.line @@ -107,4 +112,12 @@ export function isRangeInsideRange(inner: Range, outer: Range): boolean { // Test lines on multi-line ranges. return inner.start.line >= outer.start.line && inner.end.line <= outer.end.line; +} + +export function rangeEquals(r1?: Range, r2?: Range): boolean { + return !!r1 && !!r2 + && r1.start.line === r2.start.line + && r1.start.character === r2.start.character + && r1.end.line === r2.end.line + && r1.end.character === r2.end.character; } \ No newline at end of file