Emit related ranges for duplicate identifiers

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;

src/compiler.ts

@@ -1635,8 +1635,10 @@ export class Compiler extends DiagnosticEmitter {
       }
       case NodeKind.ENUMDECLARATION: {
         let element = this.program.getElementByDeclaration(<EnumDeclaration>statement);
-        assert(element.kind == ElementKind.ENUM);
-        if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileEnum(<Enum>element);
+        if (element) {
+          assert(element.kind == ElementKind.ENUM);
+          if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileEnum(<Enum>element);
+        }
         break;
       }
       case NodeKind.NAMESPACEDECLARATION: {
@@ -1650,17 +1652,19 @@ export class Compiler extends DiagnosticEmitter {
         let declarations = (<VariableStatement>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(<Global>element);
+          if (element) {
+            assert(element.kind == ElementKind.GLOBAL);
+            if (
+              !element.is(CommonFlags.AMBIENT) && // delay imports
+              !element.hasDecorator(DecoratorFlags.LAZY)
+            ) this.compileGlobal(<Global>element);
+          }
         }
         break;
       }
       case NodeKind.FIELDDECLARATION: {
         let element = this.program.getElementByDeclaration(<FieldDeclaration>statement);
-        if (element.kind == ElementKind.GLOBAL) { // static
+        if (element !== null && element.kind == ElementKind.GLOBAL) { // static
           if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileGlobal(<Global>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;
           }

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(")");
     }
   }

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;
       }

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,
+              (<DeclaredElement>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,
+              (<DeclaredElement>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,
+            (<DeclaredElement>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;

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,
                 (<FieldPrototype>member).identifierNode.range,
+                existing.declaration.name.range,
                 member.name
               );
               break;

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"
+  ]
+}

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