From 3e5d4107a7dfe405abb3c3fbbdb77a6451c609b6 Mon Sep 17 00:00:00 2001 From: dcode Date: Wed, 2 Oct 2019 02:45:25 +0200 Subject: [PATCH] Emit related ranges for duplicate identifiers --- src/ast.ts | 9 ++- src/compiler.ts | 53 +++++++++----- src/diagnostics.ts | 6 +- src/flow.ts | 17 +++-- src/program.ts | 90 ++++++++++++++++-------- src/resolver.ts | 4 +- tests/compiler/duplicate-identifier.json | 14 ++++ tests/compiler/duplicate-identifier.ts | 27 +++++++ 8 files changed, 166 insertions(+), 54 deletions(-) create mode 100644 tests/compiler/duplicate-identifier.json create mode 100644 tests/compiler/duplicate-identifier.ts diff --git a/src/ast.ts b/src/ast.ts index 6dc0ede3ce..2275a4683d 100644 --- a/src/ast.ts +++ b/src/ast.ts @@ -7,7 +7,8 @@ import { CommonFlags, CommonSymbols, PATH_DELIMITER, - LIBRARY_PREFIX + LIBRARY_PREFIX, + LIBRARY_SUBST } from "./common"; import { @@ -1648,6 +1649,12 @@ export class Source extends Node { this.text = text; } + /** Checks if this source represents native code. */ + get isNative(): bool { + return this.internalPath == LIBRARY_SUBST; + } + + /** Checks if this source is part of the (standard) library. */ get isLibrary(): bool { var kind = this.sourceKind; return kind == SourceKind.LIBRARY || kind == SourceKind.LIBRARY_ENTRY; diff --git a/src/compiler.ts b/src/compiler.ts index 3d617dbd9b..3740f3aa2b 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -1635,8 +1635,10 @@ export class Compiler extends DiagnosticEmitter { } case NodeKind.ENUMDECLARATION: { let element = this.program.getElementByDeclaration(statement); - assert(element.kind == ElementKind.ENUM); - if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileEnum(element); + if (element) { + assert(element.kind == ElementKind.ENUM); + if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileEnum(element); + } break; } case NodeKind.NAMESPACEDECLARATION: { @@ -1650,17 +1652,19 @@ export class Compiler extends DiagnosticEmitter { let declarations = (statement).declarations; for (let i = 0, k = declarations.length; i < k; ++i) { let element = this.program.getElementByDeclaration(declarations[i]); - assert(element.kind == ElementKind.GLOBAL); - if ( - !element.is(CommonFlags.AMBIENT) && // delay imports - !element.hasDecorator(DecoratorFlags.LAZY) - ) this.compileGlobal(element); + if (element) { + assert(element.kind == ElementKind.GLOBAL); + if ( + !element.is(CommonFlags.AMBIENT) && // delay imports + !element.hasDecorator(DecoratorFlags.LAZY) + ) this.compileGlobal(element); + } } break; } case NodeKind.FIELDDECLARATION: { let element = this.program.getElementByDeclaration(statement); - if (element.kind == ElementKind.GLOBAL) { // static + if (element !== null && element.kind == ElementKind.GLOBAL) { // static if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileGlobal(element); } break; @@ -2524,9 +2528,12 @@ export class Compiler extends DiagnosticEmitter { let scopedLocals = flow.scopedLocals; if (!scopedLocals) flow.scopedLocals = scopedLocals = new Map(); else if (scopedLocals.has(name)) { - this.error( + let existing = scopedLocals.get(name)!; + this.errorRelated( DiagnosticCode.Duplicate_identifier_0, - declaration.name.range, name + declaration.name.range, + existing.declaration.name.range, + name ); return this.module.unreachable(); } @@ -2550,20 +2557,32 @@ export class Compiler extends DiagnosticEmitter { ) { // here: not top-level let existingLocal = flow.getScopedLocal(name); if (existingLocal) { - this.error( - DiagnosticCode.Duplicate_identifier_0, - declaration.name.range, declaration.name.text - ); + if (!existingLocal.declaration.range.source.isNative) { + this.errorRelated( + DiagnosticCode.Duplicate_identifier_0, + declaration.name.range, + existingLocal.declaration.name.range, + name + ); + } else { // scoped locals are shared temps that don't track declarations + this.error( + DiagnosticCode.Duplicate_identifier_0, + declaration.name.range, name + ); + } local = existingLocal; } else { local = flow.addScopedLocal(name, type); } if (isConst) flow.setLocalFlag(local.index, LocalFlags.CONSTANT); } else { - if (flow.lookupLocal(name)) { - this.error( + let existing = flow.lookupLocal(name); + if (existing) { + this.errorRelated( DiagnosticCode.Duplicate_identifier_0, - declaration.name.range, name + declaration.name.range, + existing.declaration.name.range, + name ); continue; } diff --git a/src/diagnostics.ts b/src/diagnostics.ts index e28ed8116d..356fd1d0ab 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -212,11 +212,11 @@ export function formatDiagnosticMessage( } sb.push("\n"); sb.push(" in "); - sb.push(range.source.normalizedPath); + sb.push(relatedRange.source.normalizedPath); sb.push("("); - sb.push(range.line.toString(10)); + sb.push(relatedRange.line.toString(10)); sb.push(","); - sb.push(range.column.toString(10)); + sb.push(relatedRange.column.toString(10)); sb.push(")"); } } diff --git a/src/flow.ts b/src/flow.ts index d646b76e0f..b3e532418c 100644 --- a/src/flow.ts +++ b/src/flow.ts @@ -416,10 +416,19 @@ export class Flow { let existingLocal = this.scopedLocals.get(name); if (existingLocal) { if (reportNode) { - this.parentFunction.program.error( - DiagnosticCode.Duplicate_identifier_0, - reportNode.range - ); + if (!existingLocal.declaration.range.source.isNative) { + this.parentFunction.program.errorRelated( + DiagnosticCode.Duplicate_identifier_0, + reportNode.range, + existingLocal.declaration.name.range, + name + ); + } else { + this.parentFunction.program.error( + DiagnosticCode.Duplicate_identifier_0, + reportNode.range, name + ); + } } return existingLocal; } diff --git a/src/program.ts b/src/program.ts index 3bdd75a847..f9ff2ea068 100644 --- a/src/program.ts +++ b/src/program.ts @@ -609,10 +609,11 @@ export class Program extends DiagnosticEmitter { } /** Gets the (possibly merged) program element linked to the specified declaration. */ - getElementByDeclaration(declaration: DeclarationStatement): DeclaredElement { + getElementByDeclaration(declaration: DeclarationStatement): DeclaredElement | null { var elementsByDeclaration = this.elementsByDeclaration; - assert(elementsByDeclaration.has(declaration)); - return elementsByDeclaration.get(declaration)!; + return elementsByDeclaration.has(declaration) + ? elementsByDeclaration.get(declaration) + : null; } /** Initializes the program and its elements prior to compilation. */ @@ -1088,19 +1089,28 @@ export class Program extends DiagnosticEmitter { ensureGlobal(name: string, element: DeclaredElement): DeclaredElement { var elementsByName = this.elementsByName; if (elementsByName.has(name)) { - let actual = elementsByName.get(name)!; + let existing = elementsByName.get(name)!; // NOTE: this is effectively only performed when merging native types with // their respective namespaces in std/builtins, but can also trigger when a // user has multiple global elements of the same name in different files, // which might result in unexpected shared symbols accross files. considering // this a wonky feature for now that we might want to revisit later. - if (actual !== element) { - let merged = tryMerge(elementsByName.get(name)!, element); + if (existing !== element) { + let merged = tryMerge(existing, element); if (!merged) { - this.error( - DiagnosticCode.Duplicate_identifier_0, - element.identifierNode.range, name - ); + if (isDeclaredElement(existing.kind)) { + this.errorRelated( + DiagnosticCode.Duplicate_identifier_0, + element.identifierNode.range, + (existing).declaration.name.range, + name + ); + } else { + this.error( + DiagnosticCode.Duplicate_identifier_0, + element.identifierNode.range, name + ); + } return element; } element = merged; @@ -1675,12 +1685,17 @@ export class Program extends DiagnosticEmitter { if (element) { let exports = parent.exports; if (!exports) parent.exports = exports = new Map(); - else if (exports.has("default")) { - this.error( - DiagnosticCode.Duplicate_identifier_0, - declaration.name.range, "default" - ); - return; + else { + if (exports.has("default")) { + let existing = exports.get("default")!; + this.errorRelated( + DiagnosticCode.Duplicate_identifier_0, + declaration.name.range, + existing.declaration.name.range, + "default" + ); + return; + } } exports.set("default", element); } @@ -2119,18 +2134,27 @@ export abstract class Element { var members = this.members; if (!members) this.members = members = new Map(); else if (members.has(name)) { - let actual = members.get(name)!; - if (actual.parent !== this) { + let existing = members.get(name)!; + if (existing.parent !== this) { // override non-own element } else { - let merged = tryMerge(actual, element); + let merged = tryMerge(existing, element); if (merged) { element = merged; // use merged element } else { - this.program.error( - DiagnosticCode.Duplicate_identifier_0, - element.identifierNode.range, element.identifierNode.text - ); + if (isDeclaredElement(existing.kind)) { + this.program.errorRelated( + DiagnosticCode.Duplicate_identifier_0, + element.identifierNode.range, + (existing).declaration.name.range, + element.identifierNode.text + ); + } else { + this.program.error( + DiagnosticCode.Duplicate_identifier_0, + element.identifierNode.range, element.identifierNode.text + ); + } return false; } } @@ -3188,12 +3212,22 @@ export class ClassPrototype extends DeclaredElement { var instanceMembers = this.instanceMembers; if (!instanceMembers) this.instanceMembers = instanceMembers = new Map(); else if (instanceMembers.has(name)) { - let merged = tryMerge(instanceMembers.get(name)!, element); + let existing = instanceMembers.get(name)!; + let merged = tryMerge(existing, element); if (!merged) { - this.program.error( - DiagnosticCode.Duplicate_identifier_0, - element.identifierNode.range, element.identifierNode.text - ); + if (isDeclaredElement(existing.kind)) { + this.program.errorRelated( + DiagnosticCode.Duplicate_identifier_0, + element.identifierNode.range, + (existing).declaration.name.range, + element.identifierNode.text + ); + } else { + this.program.error( + DiagnosticCode.Duplicate_identifier_0, + element.identifierNode.range, element.identifierNode.text + ); + } return false; } element = merged; diff --git a/src/resolver.ts b/src/resolver.ts index 550806c5e2..d22c1dd1be 100644 --- a/src/resolver.ts +++ b/src/resolver.ts @@ -2738,9 +2738,11 @@ export class Resolver extends DiagnosticEmitter { let instanceMembers = instance.members; if (!instanceMembers) instance.members = instanceMembers = new Map(); else if (instanceMembers.has(member.name)) { - this.error( + let existing = instanceMembers.get(member.name)!; + this.errorRelated( DiagnosticCode.Duplicate_identifier_0, (member).identifierNode.range, + existing.declaration.name.range, member.name ); break; diff --git a/tests/compiler/duplicate-identifier.json b/tests/compiler/duplicate-identifier.json new file mode 100644 index 0000000000..de16d18c48 --- /dev/null +++ b/tests/compiler/duplicate-identifier.json @@ -0,0 +1,14 @@ +{ + "asc_flags": [ + "--runtime none" + ], + "stderr": [ + "TS2300: Duplicate identifier 'a'", "var a: f32", "var a: i32", + "TS2300: Duplicate identifier 'b'", "b: f32", "b: i32", + "TS2300: Duplicate identifier 'c'", "static c: f32", " static c: i32", + "TS2300: Duplicate identifier 'd'", "const d: f32", "const d: i32", + "TS2300: Duplicate identifier 'e'", "var e: f32", "var e: i32", + "TS2300: Duplicate identifier 'f'", "let f: f32", + "EOF" + ] +} \ No newline at end of file diff --git a/tests/compiler/duplicate-identifier.ts b/tests/compiler/duplicate-identifier.ts new file mode 100644 index 0000000000..b9482e8369 --- /dev/null +++ b/tests/compiler/duplicate-identifier.ts @@ -0,0 +1,27 @@ +var a: i32; +var a: f32; + +class Foo { + b: i32; + b: f32; + static c: i32; + static c: f32; +} + +namespace Bar { + const d: i32 = 0; + const d: f32 = 1; +} + +function baz(): void { + var e: i32; + var e: f32; + { + let f: i32; + let f: f32; + } +} + +baz(); + +ERROR("EOF"); // mark end and ensure fail