Skip to content

Commit e34b4ee

Browse files
dcodeIOWillem Wyndham
dcodeIO
authored and
Willem Wyndham
committed
Emit related ranges for duplicate identifiers (AssemblyScript#882)
1 parent d0647d4 commit e34b4ee

File tree

8 files changed

+166
-54
lines changed

8 files changed

+166
-54
lines changed

‎src/ast.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import {
77
CommonFlags,
88
CommonSymbols,
99
PATH_DELIMITER,
10-
LIBRARY_PREFIX
10+
LIBRARY_PREFIX,
11+
LIBRARY_SUBST
1112
} from "./common";
1213

1314
import {
@@ -1648,6 +1649,12 @@ export class Source extends Node {
16481649
this.text = text;
16491650
}
16501651

1652+
/** Checks if this source represents native code. */
1653+
get isNative(): bool {
1654+
return this.internalPath == LIBRARY_SUBST;
1655+
}
1656+
1657+
/** Checks if this source is part of the (standard) library. */
16511658
get isLibrary(): bool {
16521659
var kind = this.sourceKind;
16531660
return kind == SourceKind.LIBRARY || kind == SourceKind.LIBRARY_ENTRY;

‎src/compiler.ts‎

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1635,8 +1635,10 @@ export class Compiler extends DiagnosticEmitter {
16351635
}
16361636
case NodeKind.ENUMDECLARATION: {
16371637
let element = this.program.getElementByDeclaration(<EnumDeclaration>statement);
1638-
assert(element.kind == ElementKind.ENUM);
1639-
if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileEnum(<Enum>element);
1638+
if (element) {
1639+
assert(element.kind == ElementKind.ENUM);
1640+
if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileEnum(<Enum>element);
1641+
}
16401642
break;
16411643
}
16421644
case NodeKind.NAMESPACEDECLARATION: {
@@ -1650,17 +1652,19 @@ export class Compiler extends DiagnosticEmitter {
16501652
let declarations = (<VariableStatement>statement).declarations;
16511653
for (let i = 0, k = declarations.length; i < k; ++i) {
16521654
let element = this.program.getElementByDeclaration(declarations[i]);
1653-
assert(element.kind == ElementKind.GLOBAL);
1654-
if (
1655-
!element.is(CommonFlags.AMBIENT) && // delay imports
1656-
!element.hasDecorator(DecoratorFlags.LAZY)
1657-
) this.compileGlobal(<Global>element);
1655+
if (element) {
1656+
assert(element.kind == ElementKind.GLOBAL);
1657+
if (
1658+
!element.is(CommonFlags.AMBIENT) && // delay imports
1659+
!element.hasDecorator(DecoratorFlags.LAZY)
1660+
) this.compileGlobal(<Global>element);
1661+
}
16581662
}
16591663
break;
16601664
}
16611665
case NodeKind.FIELDDECLARATION: {
16621666
let element = this.program.getElementByDeclaration(<FieldDeclaration>statement);
1663-
if (element.kind == ElementKind.GLOBAL) { // static
1667+
if (element !== null && element.kind == ElementKind.GLOBAL) { // static
16641668
if (!element.hasDecorator(DecoratorFlags.LAZY)) this.compileGlobal(<Global>element);
16651669
}
16661670
break;
@@ -2524,9 +2528,12 @@ export class Compiler extends DiagnosticEmitter {
25242528
let scopedLocals = flow.scopedLocals;
25252529
if (!scopedLocals) flow.scopedLocals = scopedLocals = new Map();
25262530
else if (scopedLocals.has(name)) {
2527-
this.error(
2531+
let existing = scopedLocals.get(name)!;
2532+
this.errorRelated(
25282533
DiagnosticCode.Duplicate_identifier_0,
2529-
declaration.name.range, name
2534+
declaration.name.range,
2535+
existing.declaration.name.range,
2536+
name
25302537
);
25312538
return this.module.unreachable();
25322539
}
@@ -2550,20 +2557,32 @@ export class Compiler extends DiagnosticEmitter {
25502557
) { // here: not top-level
25512558
let existingLocal = flow.getScopedLocal(name);
25522559
if (existingLocal) {
2553-
this.error(
2554-
DiagnosticCode.Duplicate_identifier_0,
2555-
declaration.name.range, declaration.name.text
2556-
);
2560+
if (!existingLocal.declaration.range.source.isNative) {
2561+
this.errorRelated(
2562+
DiagnosticCode.Duplicate_identifier_0,
2563+
declaration.name.range,
2564+
existingLocal.declaration.name.range,
2565+
name
2566+
);
2567+
} else { // scoped locals are shared temps that don't track declarations
2568+
this.error(
2569+
DiagnosticCode.Duplicate_identifier_0,
2570+
declaration.name.range, name
2571+
);
2572+
}
25572573
local = existingLocal;
25582574
} else {
25592575
local = flow.addScopedLocal(name, type);
25602576
}
25612577
if (isConst) flow.setLocalFlag(local.index, LocalFlags.CONSTANT);
25622578
} else {
2563-
if (flow.lookupLocal(name)) {
2564-
this.error(
2579+
let existing = flow.lookupLocal(name);
2580+
if (existing) {
2581+
this.errorRelated(
25652582
DiagnosticCode.Duplicate_identifier_0,
2566-
declaration.name.range, name
2583+
declaration.name.range,
2584+
existing.declaration.name.range,
2585+
name
25672586
);
25682587
continue;
25692588
}

‎src/diagnostics.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,11 @@ export function formatDiagnosticMessage(
212212
}
213213
sb.push("\n");
214214
sb.push(" in ");
215-
sb.push(range.source.normalizedPath);
215+
sb.push(relatedRange.source.normalizedPath);
216216
sb.push("(");
217-
sb.push(range.line.toString(10));
217+
sb.push(relatedRange.line.toString(10));
218218
sb.push(",");
219-
sb.push(range.column.toString(10));
219+
sb.push(relatedRange.column.toString(10));
220220
sb.push(")");
221221
}
222222
}

‎src/flow.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,19 @@ export class Flow {
416416
let existingLocal = this.scopedLocals.get(name);
417417
if (existingLocal) {
418418
if (reportNode) {
419-
this.parentFunction.program.error(
420-
DiagnosticCode.Duplicate_identifier_0,
421-
reportNode.range
422-
);
419+
if (!existingLocal.declaration.range.source.isNative) {
420+
this.parentFunction.program.errorRelated(
421+
DiagnosticCode.Duplicate_identifier_0,
422+
reportNode.range,
423+
existingLocal.declaration.name.range,
424+
name
425+
);
426+
} else {
427+
this.parentFunction.program.error(
428+
DiagnosticCode.Duplicate_identifier_0,
429+
reportNode.range, name
430+
);
431+
}
423432
}
424433
return existingLocal;
425434
}

‎src/program.ts‎

Lines changed: 62 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -609,10 +609,11 @@ export class Program extends DiagnosticEmitter {
609609
}
610610

611611
/** Gets the (possibly merged) program element linked to the specified declaration. */
612-
getElementByDeclaration(declaration: DeclarationStatement): DeclaredElement {
612+
getElementByDeclaration(declaration: DeclarationStatement): DeclaredElement | null {
613613
var elementsByDeclaration = this.elementsByDeclaration;
614-
assert(elementsByDeclaration.has(declaration));
615-
return elementsByDeclaration.get(declaration)!;
614+
return elementsByDeclaration.has(declaration)
615+
? elementsByDeclaration.get(declaration)
616+
: null;
616617
}
617618

618619
/** Initializes the program and its elements prior to compilation. */
@@ -1088,19 +1089,28 @@ export class Program extends DiagnosticEmitter {
10881089
ensureGlobal(name: string, element: DeclaredElement): DeclaredElement {
10891090
var elementsByName = this.elementsByName;
10901091
if (elementsByName.has(name)) {
1091-
let actual = elementsByName.get(name)!;
1092+
let existing = elementsByName.get(name)!;
10921093
// NOTE: this is effectively only performed when merging native types with
10931094
// their respective namespaces in std/builtins, but can also trigger when a
10941095
// user has multiple global elements of the same name in different files,
10951096
// which might result in unexpected shared symbols accross files. considering
10961097
// this a wonky feature for now that we might want to revisit later.
1097-
if (actual !== element) {
1098-
let merged = tryMerge(elementsByName.get(name)!, element);
1098+
if (existing !== element) {
1099+
let merged = tryMerge(existing, element);
10991100
if (!merged) {
1100-
this.error(
1101-
DiagnosticCode.Duplicate_identifier_0,
1102-
element.identifierNode.range, name
1103-
);
1101+
if (isDeclaredElement(existing.kind)) {
1102+
this.errorRelated(
1103+
DiagnosticCode.Duplicate_identifier_0,
1104+
element.identifierNode.range,
1105+
(<DeclaredElement>existing).declaration.name.range,
1106+
name
1107+
);
1108+
} else {
1109+
this.error(
1110+
DiagnosticCode.Duplicate_identifier_0,
1111+
element.identifierNode.range, name
1112+
);
1113+
}
11041114
return element;
11051115
}
11061116
element = merged;
@@ -1675,12 +1685,17 @@ export class Program extends DiagnosticEmitter {
16751685
if (element) {
16761686
let exports = parent.exports;
16771687
if (!exports) parent.exports = exports = new Map();
1678-
else if (exports.has("default")) {
1679-
this.error(
1680-
DiagnosticCode.Duplicate_identifier_0,
1681-
declaration.name.range, "default"
1682-
);
1683-
return;
1688+
else {
1689+
if (exports.has("default")) {
1690+
let existing = exports.get("default")!;
1691+
this.errorRelated(
1692+
DiagnosticCode.Duplicate_identifier_0,
1693+
declaration.name.range,
1694+
existing.declaration.name.range,
1695+
"default"
1696+
);
1697+
return;
1698+
}
16841699
}
16851700
exports.set("default", element);
16861701
}
@@ -2119,18 +2134,27 @@ export abstract class Element {
21192134
var members = this.members;
21202135
if (!members) this.members = members = new Map();
21212136
else if (members.has(name)) {
2122-
let actual = members.get(name)!;
2123-
if (actual.parent !== this) {
2137+
let existing = members.get(name)!;
2138+
if (existing.parent !== this) {
21242139
// override non-own element
21252140
} else {
2126-
let merged = tryMerge(actual, element);
2141+
let merged = tryMerge(existing, element);
21272142
if (merged) {
21282143
element = merged; // use merged element
21292144
} else {
2130-
this.program.error(
2131-
DiagnosticCode.Duplicate_identifier_0,
2132-
element.identifierNode.range, element.identifierNode.text
2133-
);
2145+
if (isDeclaredElement(existing.kind)) {
2146+
this.program.errorRelated(
2147+
DiagnosticCode.Duplicate_identifier_0,
2148+
element.identifierNode.range,
2149+
(<DeclaredElement>existing).declaration.name.range,
2150+
element.identifierNode.text
2151+
);
2152+
} else {
2153+
this.program.error(
2154+
DiagnosticCode.Duplicate_identifier_0,
2155+
element.identifierNode.range, element.identifierNode.text
2156+
);
2157+
}
21342158
return false;
21352159
}
21362160
}
@@ -3188,12 +3212,22 @@ export class ClassPrototype extends DeclaredElement {
31883212
var instanceMembers = this.instanceMembers;
31893213
if (!instanceMembers) this.instanceMembers = instanceMembers = new Map();
31903214
else if (instanceMembers.has(name)) {
3191-
let merged = tryMerge(instanceMembers.get(name)!, element);
3215+
let existing = instanceMembers.get(name)!;
3216+
let merged = tryMerge(existing, element);
31923217
if (!merged) {
3193-
this.program.error(
3194-
DiagnosticCode.Duplicate_identifier_0,
3195-
element.identifierNode.range, element.identifierNode.text
3196-
);
3218+
if (isDeclaredElement(existing.kind)) {
3219+
this.program.errorRelated(
3220+
DiagnosticCode.Duplicate_identifier_0,
3221+
element.identifierNode.range,
3222+
(<DeclaredElement>existing).declaration.name.range,
3223+
element.identifierNode.text
3224+
);
3225+
} else {
3226+
this.program.error(
3227+
DiagnosticCode.Duplicate_identifier_0,
3228+
element.identifierNode.range, element.identifierNode.text
3229+
);
3230+
}
31973231
return false;
31983232
}
31993233
element = merged;

‎src/resolver.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2738,9 +2738,11 @@ export class Resolver extends DiagnosticEmitter {
27382738
let instanceMembers = instance.members;
27392739
if (!instanceMembers) instance.members = instanceMembers = new Map();
27402740
else if (instanceMembers.has(member.name)) {
2741-
this.error(
2741+
let existing = instanceMembers.get(member.name)!;
2742+
this.errorRelated(
27422743
DiagnosticCode.Duplicate_identifier_0,
27432744
(<FieldPrototype>member).identifierNode.range,
2745+
existing.declaration.name.range,
27442746
member.name
27452747
);
27462748
break;

‎tests/compiler/duplicate-identifier.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"asc_flags": [
3+
"--runtime none"
4+
],
5+
"stderr": [
6+
"TS2300: Duplicate identifier 'a'", "var a: f32", "var a: i32",
7+
"TS2300: Duplicate identifier 'b'", "b: f32", "b: i32",
8+
"TS2300: Duplicate identifier 'c'", "static c: f32", " static c: i32",
9+
"TS2300: Duplicate identifier 'd'", "const d: f32", "const d: i32",
10+
"TS2300: Duplicate identifier 'e'", "var e: f32", "var e: i32",
11+
"TS2300: Duplicate identifier 'f'", "let f: f32",
12+
"EOF"
13+
]
14+
}

‎tests/compiler/duplicate-identifier.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
var a: i32;
2+
var a: f32;
3+
4+
class Foo {
5+
b: i32;
6+
b: f32;
7+
static c: i32;
8+
static c: f32;
9+
}
10+
11+
namespace Bar {
12+
const d: i32 = 0;
13+
const d: f32 = 1;
14+
}
15+
16+
function baz(): void {
17+
var e: i32;
18+
var e: f32;
19+
{
20+
let f: i32;
21+
let f: f32;
22+
}
23+
}
24+
25+
baz();
26+
27+
ERROR("EOF"); // mark end and ensure fail

0 commit comments

Comments
 (0)