Skip to content

Commit 2cd6f20

Browse files
committed
Enable duplicate property declaration diagnostics
1 parent f357e15 commit 2cd6f20

File tree

6 files changed

+80
-22
lines changed

6 files changed

+80
-22
lines changed

server/src/capabilities/diagnostics.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ export class DuplicateAttributeDiagnostic extends BaseDiagnostic {
6060
}
6161
}
6262

63+
export class DuplicateDeclarationDiagnostic extends BaseDiagnostic {
64+
message = "Duplicate declaration in current scope";
65+
severity = DiagnosticSeverity.Error;
66+
constructor(range: Range) {
67+
super(range);
68+
}
69+
}
70+
6371
export class IgnoredAttributeDiagnostic extends BaseDiagnostic {
6472
message = "This attribute will be ignored.";
6573
severity = DiagnosticSeverity.Information;

server/src/project/document.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { CancellationToken, Diagnostic, PublishDiagnosticsParams, SymbolInformation, SymbolKind } from 'vscode-languageserver';
22
import { Workspace } from './workspace';
33
import { FoldableElement } from './elements/special';
4-
import { BaseSyntaxElement, HasDiagnosticCapability, HasSemanticToken, HasSymbolInformation, ScopeElement } from './elements/base';
4+
import { BaseSyntaxElement, HasDiagnosticCapability, HasSemanticToken, HasSymbolInformation, IdentifiableSyntaxElement, ScopeElement } from './elements/base';
55
import { Range, TextDocument } from 'vscode-languageserver-textdocument';
66
import { SyntaxParser } from './parser/vbaSyntaxParser';
77
import { FoldingRange } from '../capabilities/folding';
@@ -147,7 +147,8 @@ export abstract class BaseProjectDocument {
147147
return this;
148148
};
149149

150-
registerNamedElement(element: BaseSyntaxElement) {
150+
registerNamedElement(element: IdentifiableSyntaxElement) {
151+
this.currentScopeElement?.pushDeclaredName(element);
151152
return this;
152153
}
153154

server/src/project/elements/base.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ParserRuleContext } from 'antlr4ng';
22
import { Diagnostic, Range, SemanticTokenModifiers, SemanticTokenTypes, SymbolInformation, SymbolKind } from 'vscode-languageserver';
33
import { Position, TextDocument } from 'vscode-languageserver-textdocument';
44
import { FoldingRangeKind } from '../../capabilities/folding';
5-
import { IdentifierElement } from './memory';
5+
import { IdentifierElement, PropertyDeclarationElement } from './memory';
66
import '../../extensions/parserExtensions';
77

88
export interface ContextOptionalSyntaxElement {
@@ -31,6 +31,7 @@ export interface NamedSyntaxElement extends SyntaxElement {
3131

3232
export interface IdentifiableSyntaxElement extends NamedSyntaxElement {
3333
identifier: IdentifierElement;
34+
isPropertyElement(): this is PropertyDeclarationElement
3435
}
3536

3637
export interface HasSymbolInformation extends NamedSyntaxElement {
@@ -55,6 +56,7 @@ export interface FoldingRangeElement {
5556

5657
export interface ScopeElement {
5758
declaredNames: Map<string, IdentifiableSyntaxElement[]>;
59+
pushDeclaredName(element: IdentifiableSyntaxElement): void
5860
}
5961

6062
export abstract class BaseSyntaxElement implements ContextOptionalSyntaxElement {

server/src/project/elements/memory.ts

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import { TextDocument } from 'vscode-languageserver-textdocument';
2-
import { SemanticTokenModifiers, SemanticTokenTypes, SymbolInformation, SymbolKind } from 'vscode-languageserver';
2+
import { Diagnostic, SemanticTokenModifiers, SemanticTokenTypes, SymbolInformation, SymbolKind } from 'vscode-languageserver';
33
import { AmbiguousIdentifierContext, ConstItemContext, EnumDeclarationContext, EnumMemberContext, FunctionDeclarationContext, ProcedureDeclarationContext, PropertyGetDeclarationContext, PropertySetDeclarationContext, ReservedMemberNameContext, SubroutineDeclarationContext, UdtDeclarationContext, UdtElementContext, UntypedNameContext, VariableDclContext } from '../../antlr/out/vbaParser';
44

5-
import { BaseContextSyntaxElement, HasSemanticToken, HasSymbolInformation, IdentifiableSyntaxElement, NamedSyntaxElement } from './base';
5+
import { BaseContextSyntaxElement, HasDiagnosticCapability, HasSemanticToken, HasSymbolInformation, IdentifiableSyntaxElement, NamedSyntaxElement } from './base';
66

77
import { ScopeElement } from './special';
8-
import { VbaClassDocument, VbaModuleDocument } from '../document';
8+
import { BaseProjectDocument, VbaClassDocument, VbaModuleDocument } from '../document';
99
import { SymbolInformationFactory } from '../../capabilities/symbolInformation';
1010
import '../../extensions/parserExtensions';
11+
import { DuplicateDeclarationDiagnostic } from '../../capabilities/diagnostics';
1112

1213

1314

@@ -17,44 +18,59 @@ export class IdentifierElement extends BaseContextSyntaxElement {
1718
}
1819
}
1920

20-
export abstract class DeclarationElement extends ScopeElement {
21+
export abstract class DeclarationElement extends ScopeElement implements HasDiagnosticCapability {
22+
abstract diagnostics: Diagnostic[];
2123
abstract identifier: IdentifierElement;
2224

2325
constructor(context: ProcedureDeclarationContext, document: TextDocument) {
2426
super(context, document);
2527
}
28+
29+
evaluateDiagnostics(): void {
30+
return;
31+
}
32+
2633
get name(): string {
2734
throw new Error('Method not implemented.');
2835
}
2936

3037
static create(context: ProcedureDeclarationContext, document: VbaClassDocument | VbaModuleDocument) {
3138
let methodContext: SubroutineDeclarationContext | FunctionDeclarationContext | PropertyGetDeclarationContext | null;
39+
40+
// Create a sub if we have one.
3241
methodContext = context.subroutineDeclaration();
3342
if (methodContext) {
3443
return new SubDeclarationElement(context, document.textDocument, methodContext);
3544
}
3645

46+
// Create a function if we have one.
3747
methodContext = context.functionDeclaration();
3848
if (methodContext) {
3949
return new FunctionDeclarationElement(context, document.textDocument, methodContext);
4050
}
4151

52+
// Check if we already have a property with this name.
4253
const propertyDeclaration = new PropertyDeclarationElement(context, document.textDocument);
43-
const predeclaredElements = document.currentScopeElement?.declaredNames.get(propertyDeclaration.identifier.text);
44-
predeclaredElements?.forEach(predeclaredElement => {
45-
if (predeclaredElement && isPropertyDeclarationElement(predeclaredElement)) {
46-
predeclaredElement.addPropertyDeclaration(context, document.textDocument);
47-
return predeclaredElement;
54+
const identifierText = propertyDeclaration.identifier.text;
55+
const predeclaredElements = document.currentScopeElement?.declaredNames.get(identifierText) ?? [];
56+
57+
// Add to an existing property rather than creating.
58+
for (const element of predeclaredElements) {
59+
if (element.isPropertyElement() && element.identifier.text === identifierText) {
60+
element.addPropertyDeclaration(context, document.textDocument);
61+
return element;
4862
}
49-
});
63+
}
64+
65+
// Return a new property.
5066
return propertyDeclaration;
5167
}
52-
5368
}
5469

5570
export class SubDeclarationElement extends DeclarationElement implements HasSymbolInformation {
5671
identifier: IdentifierElement;
5772
symbolInformation: SymbolInformation;
73+
diagnostics: Diagnostic[] = [];
5874

5975
constructor(context: ProcedureDeclarationContext, document: TextDocument, methodContext: SubroutineDeclarationContext) {
6076
super(context, document);
@@ -73,6 +89,7 @@ export class SubDeclarationElement extends DeclarationElement implements HasSymb
7389
export class FunctionDeclarationElement extends DeclarationElement implements HasSymbolInformation {
7490
identifier: IdentifierElement;
7591
symbolInformation: SymbolInformation;
92+
diagnostics: Diagnostic[] = [];
7693

7794
constructor(context: ProcedureDeclarationContext, document: TextDocument, methodContext: FunctionDeclarationContext) {
7895
super(context, document);
@@ -89,11 +106,18 @@ export class FunctionDeclarationElement extends DeclarationElement implements Ha
89106

90107
export class PropertyDeclarationElement extends DeclarationElement implements HasSymbolInformation {
91108
identifier: IdentifierElement;
109+
diagnostics: Diagnostic[] = [];
92110
symbolInformation: SymbolInformation;
93111
getDeclarations: PropertyGetDeclarationElement[] = [];
94112
letDeclarations: PropertyLetDeclarationElement[] = [];
95113
setDeclarations: PropertyLetDeclarationElement[] = [];
96114

115+
get countDeclarations(): number {
116+
return this.getDeclarations.length
117+
+ this.letDeclarations.length
118+
+ this.setDeclarations.length;
119+
}
120+
97121
constructor(context: ProcedureDeclarationContext, document: TextDocument) {
98122
super(context, document);
99123
this.identifier = this.addPropertyDeclaration(context, document);
@@ -105,6 +129,10 @@ export class PropertyDeclarationElement extends DeclarationElement implements Ha
105129
);
106130
}
107131

132+
evaluateDiagnostics(): void {
133+
this._evaluateDuplicateDeclarationsDiagnostics();
134+
}
135+
108136
addPropertyDeclaration(context: ProcedureDeclarationContext, document: TextDocument) {
109137
switch (true) {
110138
case !!context.propertyGetDeclaration():
@@ -121,10 +149,19 @@ export class PropertyDeclarationElement extends DeclarationElement implements Ha
121149
return this.setDeclarations[0].identifier;
122150
}
123151
}
152+
153+
private _evaluateDuplicateDeclarationsDiagnostics(): void {
154+
[this.getDeclarations, this.letDeclarations, this.setDeclarations].forEach(declarations => {
155+
declarations.forEach((declaration, i) => {
156+
if (i > 0) this.diagnostics.push(new DuplicateDeclarationDiagnostic(declaration.identifier.range));
157+
});
158+
});
159+
}
124160
}
125161

126162
class PropertyGetDeclarationElement extends DeclarationElement {
127163
identifier: IdentifierElement;
164+
diagnostics: Diagnostic[] = [];
128165

129166
constructor(context: ProcedureDeclarationContext, document: TextDocument, getContext: PropertyGetDeclarationContext) {
130167
super(context, document);
@@ -134,6 +171,7 @@ class PropertyGetDeclarationElement extends DeclarationElement {
134171

135172
class PropertyLetDeclarationElement extends DeclarationElement {
136173
identifier: IdentifierElement;
174+
diagnostics: Diagnostic[] = [];
137175

138176
constructor(context: ProcedureDeclarationContext, document: TextDocument, setContext: PropertySetDeclarationContext) {
139177
super(context, document);
@@ -143,17 +181,14 @@ class PropertyLetDeclarationElement extends DeclarationElement {
143181

144182
class PropertySetDeclarationElement extends DeclarationElement {
145183
identifier: IdentifierElement;
184+
diagnostics: Diagnostic[] = [];
146185

147186
constructor(context: ProcedureDeclarationContext, document: TextDocument, setContext: PropertySetDeclarationContext) {
148187
super(context, document);
149188
this.identifier = new IdentifierElement(setContext.subroutineName()!.ambiguousIdentifier()!, document);
150189
}
151190
}
152191

153-
function isPropertyDeclarationElement(element: IdentifiableSyntaxElement): element is PropertyDeclarationElement {
154-
return 'getDeclarations' in element;
155-
}
156-
157192
abstract class BaseEnumDeclarationElement extends ScopeElement implements HasSemanticToken, HasSymbolInformation {
158193
identifier: IdentifierElement;
159194
tokenModifiers: SemanticTokenModifiers[] = [];
@@ -181,7 +216,7 @@ export class EnumDeclarationElement extends BaseEnumDeclarationElement implement
181216
this.tokenType = SemanticTokenTypes.enum;
182217
this.identifier = new IdentifierElement(context.untypedName().ambiguousIdentifier()!, document);
183218
context.enumMemberList().enumElement().forEach(enumElementContext =>
184-
this._pushDeclaredName(new EnumMemberDeclarationElement(enumElementContext.enumMember()!, document))
219+
this.pushDeclaredName(new EnumMemberDeclarationElement(enumElementContext.enumMember()!, document))
185220
);
186221
}
187222

@@ -228,6 +263,10 @@ abstract class BaseVariableDeclarationStatementElement extends BaseContextSyntax
228263
);
229264
}
230265

266+
isPropertyElement(): this is PropertyDeclarationElement {
267+
return false;
268+
}
269+
231270
constructor(context: VariableDclContext | ConstItemContext | UdtElementContext, document: TextDocument, tokenType: SemanticTokenTypes, symbolKind: SymbolKind) {
232271
super(context, document);
233272
this.tokenType = tokenType;
@@ -262,7 +301,7 @@ export class TypeDeclarationElement extends ScopeElement implements HasSemantic
262301
this.tokenType = SemanticTokenTypes.struct;
263302
this.identifier = new IdentifierElement(context.untypedName(), document);
264303
context.udtMemberList().udtElement().forEach(member =>
265-
this._pushDeclaredName(new TypeMemberDeclarationElement(member, document))
304+
this.pushDeclaredName(new TypeMemberDeclarationElement(member, document))
266305
);
267306
}
268307

server/src/project/elements/special.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { ParserRuleContext } from 'antlr4ng';
22
import { FoldingRangeKind } from '../../capabilities/folding';
33
import { BaseContextSyntaxElement, FoldingRangeElement, IdentifiableSyntaxElement } from './base';
44
import { Range, TextDocument } from 'vscode-languageserver-textdocument';
5+
import { PropertyDeclarationElement } from './memory';
56

67

78
export class FoldableElement extends BaseContextSyntaxElement implements FoldingRangeElement {
@@ -21,10 +22,14 @@ export class ScopeElement extends FoldableElement implements ScopeElement {
2122
super(ctx, doc);
2223
}
2324

24-
protected _pushDeclaredName(element: IdentifiableSyntaxElement) {
25+
pushDeclaredName(element: IdentifiableSyntaxElement): void {
2526
const name = element.identifier.text;
2627
const names: IdentifiableSyntaxElement[] = this.declaredNames.get(name) ?? [];
2728
names.push(element);
2829
this.declaredNames.set(name, names);
2930
}
31+
32+
isPropertyElement(): this is PropertyDeclarationElement {
33+
return 'getDeclarations' in this;
34+
}
3035
}

server/src/project/parser/vbaSyntaxParser.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,15 @@ class VbaListener extends vbaListener {
144144
};
145145

146146
enterProcedureDeclaration = (ctx: ProcedureDeclarationContext) => {
147-
// TODO: figure out how to handle scope for properties.
148147
const element = DeclarationElement.create(ctx, this.document);
149148
this.document.registerSymbolInformation(element)
150149
.registerFoldableElement(element)
151150
.registerNamedElement(element)
152151
.registerScopedElement(element);
152+
153+
if (element.isPropertyElement() && element.countDeclarations === 1) {
154+
this.document.registerDiagnosticElement(element);
155+
}
153156
};
154157

155158
exitProcedureDeclaration = (ctx: ProcedureDeclarationContext) => {

0 commit comments

Comments
 (0)