diff --git a/server/src/capabilities/capabilities.ts b/server/src/capabilities/capabilities.ts index b02d0b0..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 { @@ -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; @@ -287,24 +291,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 +533,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; @@ -787,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; @@ -800,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. @@ -808,7 +811,7 @@ export class ScopeItemCapability { scope.link.backlinks); // Clean the invaludated scope. - scope.invalidate(uri, range); + scope.invalidate(uri, scope.range); return; } @@ -849,10 +852,24 @@ 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 */ private getVisibility(item: ScopeItemCapability): boolean { // Classes and modules are always public. @@ -900,7 +917,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. @@ -972,7 +989,7 @@ export class ScopeItemCapability { link.locationUri, link.element.context.range, link.element.identifierCapability.range, - this.element?.context.range + this.range ); } @@ -1045,7 +1062,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 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