Skip to content

Commit e4a7b02

Browse files
author
Andy Hanson
committed
Clean up amalgamatedDuplicates
1 parent 5fb3976 commit e4a7b02

7 files changed

+178
-57
lines changed

‎src/compiler/checker.ts

Lines changed: 55 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,19 @@ namespace ts {
464464
const enumNumberIndexInfo = createIndexInfo(stringType, /*isReadonly*/ true);
465465

466466
const globals = createSymbolTable();
467-
let amalgamatedDuplicates: Map<{ firstFile: SourceFile, secondFile: SourceFile, firstFileInstances: Map<{ instances: Node[], blockScoped: boolean }>, secondFileInstances: Map<{ instances: Node[], blockScoped: boolean }> }> | undefined;
467+
interface DuplicateInfoForSymbol {
468+
readonly firstFileLocations: Node[];
469+
readonly secondFileLocations: Node[];
470+
readonly isBlockScoped: boolean;
471+
}
472+
interface DuplicateInfoForFiles {
473+
readonly firstFile: SourceFile;
474+
readonly secondFile: SourceFile;
475+
/** Key is symbol name. */
476+
readonly conflictingSymbols: Map<DuplicateInfoForSymbol>;
477+
}
478+
/** Key is "/path/to/a.ts|/path/to/b.ts". */
479+
let amalgamatedDuplicates: Map<DuplicateInfoForFiles> | undefined;
468480
const reverseMappedCache = createMap<Type | undefined>();
469481
let ambientModulesCache: Symbol[] | undefined;
470482
/**
@@ -883,51 +895,43 @@ namespace ts {
883895
: Diagnostics.Duplicate_identifier_0;
884896
const sourceSymbolFile = source.declarations && getSourceFileOfNode(source.declarations[0]);
885897
const targetSymbolFile = target.declarations && getSourceFileOfNode(target.declarations[0]);
898+
const symbolName = symbolToString(source);
886899

887900
// Collect top-level duplicate identifier errors into one mapping, so we can then merge their diagnostics if there are a bunch
888901
if (sourceSymbolFile && targetSymbolFile && amalgamatedDuplicates && !isEitherEnum && sourceSymbolFile !== targetSymbolFile) {
889902
const firstFile = comparePaths(sourceSymbolFile.path, targetSymbolFile.path) === Comparison.LessThan ? sourceSymbolFile : targetSymbolFile;
890903
const secondFile = firstFile === sourceSymbolFile ? targetSymbolFile : sourceSymbolFile;
891-
const cacheKey = `${firstFile.path}|${secondFile.path}`;
892-
const existing = amalgamatedDuplicates.get(cacheKey) || { firstFile, secondFile, firstFileInstances: createMap(), secondFileInstances: createMap() };
893-
const symbolName = symbolToString(source);
894-
const firstInstanceList = existing.firstFileInstances.get(symbolName) || { instances: [], blockScoped: isEitherBlockScoped };
895-
const secondInstanceList = existing.secondFileInstances.get(symbolName) || { instances: [], blockScoped: isEitherBlockScoped };
896-
897-
forEach(source.declarations, node => {
898-
const errorNode = (getExpandoInitializer(node, /*isPrototypeAssignment*/ false) ? getNameOfExpando(node) : getNameOfDeclaration(node)) || node;
899-
const targetList = sourceSymbolFile === firstFile ? firstInstanceList : secondInstanceList;
900-
targetList.instances.push(errorNode);
901-
});
902-
forEach(target.declarations, node => {
903-
const errorNode = (getExpandoInitializer(node, /*isPrototypeAssignment*/ false) ? getNameOfExpando(node) : getNameOfDeclaration(node)) || node;
904-
const targetList = targetSymbolFile === firstFile ? firstInstanceList : secondInstanceList;
905-
targetList.instances.push(errorNode);
906-
});
907-
908-
existing.firstFileInstances.set(symbolName, firstInstanceList);
909-
existing.secondFileInstances.set(symbolName, secondInstanceList);
910-
amalgamatedDuplicates.set(cacheKey, existing);
911-
return target;
904+
const filesDuplicates = getOrUpdate<DuplicateInfoForFiles>(amalgamatedDuplicates, `${firstFile.path}|${secondFile.path}`, () =>
905+
({ firstFile, secondFile, conflictingSymbols: createMap() }));
906+
const conflictingSymbolInfo = getOrUpdate<DuplicateInfoForSymbol>(filesDuplicates.conflictingSymbols, symbolName, () =>
907+
({ isBlockScoped: isEitherBlockScoped, firstFileLocations: [], secondFileLocations: [] }));
908+
for (const { locs, symbol } of [{ locs: conflictingSymbolInfo.firstFileLocations, symbol: source }, { locs: conflictingSymbolInfo.secondFileLocations, symbol: target }]) {
909+
for (const decl of symbol.declarations) {
910+
pushIfUnique(locs, (getExpandoInitializer(decl, /*isPrototypeAssignment*/ false) ? getNameOfExpando(decl) : getNameOfDeclaration(decl)) || decl);
911+
}
912+
}
913+
}
914+
else {
915+
addDuplicateDeclarationErrorsForSymbols(source, message, symbolName, target);
916+
addDuplicateDeclarationErrorsForSymbols(target, message, symbolName, source);
912917
}
913-
const symbolName = symbolToString(source);
914-
addDuplicateDeclarationErrorsForSymbols(source, message, symbolName, target);
915-
addDuplicateDeclarationErrorsForSymbols(target, message, symbolName, source);
916918
}
917919
return target;
918920
}
919921

920922
function addDuplicateDeclarationErrorsForSymbols(target: Symbol, message: DiagnosticMessage, symbolName: string, source: Symbol) {
921923
forEach(target.declarations, node => {
922924
const errorNode = (getExpandoInitializer(node, /*isPrototypeAssignment*/ false) ? getNameOfExpando(node) : getNameOfDeclaration(node)) || node;
923-
addDuplicateDeclarationError(errorNode, message, symbolName, source.declarations && source.declarations[0]);
925+
addDuplicateDeclarationError(errorNode, message, symbolName, source.declarations);
924926
});
925927
}
926928

927-
function addDuplicateDeclarationError(errorNode: Node, message: DiagnosticMessage, symbolName: string, relatedNode: Node | undefined) {
929+
function addDuplicateDeclarationError(errorNode: Node, message: DiagnosticMessage, symbolName: string, relatedNodes: ReadonlyArray<Node> | undefined) {
928930
const err = lookupOrIssueError(errorNode, message, symbolName);
929-
if (relatedNode && length(err.relatedInformation) < 5) {
930-
addRelatedInfo(err, !length(err.relatedInformation) ? createDiagnosticForNode(relatedNode, Diagnostics._0_was_also_declared_here, symbolName) : createDiagnosticForNode(relatedNode, Diagnostics.and_here));
931+
for (const relatedNode of relatedNodes || emptyArray) {
932+
err.relatedInformation = err.relatedInformation || [];
933+
if (length(err.relatedInformation) >= 5) continue;
934+
addRelatedInfo(err, !length(err.relatedInformation) ? createDiagnosticForNode(relatedNode, Diagnostics._0_was_also_declared_here, symbolName) : createDiagnosticForNode(relatedNode, Diagnostics.and_here))
931935
}
932936
}
933937

@@ -28842,38 +28846,32 @@ namespace ts {
2884228846
}
2884328847
}
2884428848

28845-
amalgamatedDuplicates.forEach(({ firstFile, secondFile, firstFileInstances, secondFileInstances }) => {
28846-
const conflictingKeys = arrayFrom(firstFileInstances.keys());
28849+
amalgamatedDuplicates.forEach(({ firstFile, secondFile, conflictingSymbols }) => {
2884728850
// If not many things conflict, issue individual errors
28848-
if (conflictingKeys.length < 8) {
28849-
addErrorsForDuplicates(firstFileInstances, secondFileInstances);
28850-
addErrorsForDuplicates(secondFileInstances, firstFileInstances);
28851-
return;
28851+
if (conflictingSymbols.size < 8) {
28852+
conflictingSymbols.forEach(({ isBlockScoped, firstFileLocations, secondFileLocations }, symbolName) => {
28853+
const message = isBlockScoped ? Diagnostics.Cannot_redeclare_block_scoped_variable_0 : Diagnostics.Duplicate_identifier_0;
28854+
for (const { firstLocs, secondLocs } of [{ firstLocs: firstFileLocations, secondLocs: secondFileLocations }, { firstLocs: secondFileLocations, secondLocs: firstFileLocations }]) {
28855+
for (const node of firstLocs) {
28856+
addDuplicateDeclarationError(node, message, symbolName, secondLocs);
28857+
}
28858+
}
28859+
});
28860+
}
28861+
else {
28862+
// Otheriwse issue top-level error since the files appear very identical in terms of what they appear
28863+
const list = arrayFrom(conflictingSymbols.keys()).join(", ");
28864+
diagnostics.add(addRelatedInfo(
28865+
createDiagnosticForNode(firstFile, Diagnostics.Definitions_of_the_following_identifiers_conflict_with_those_in_another_file_Colon_0, list),
28866+
createDiagnosticForNode(secondFile, Diagnostics.Conflicts_are_in_this_file)
28867+
));
28868+
diagnostics.add(addRelatedInfo(
28869+
createDiagnosticForNode(secondFile, Diagnostics.Definitions_of_the_following_identifiers_conflict_with_those_in_another_file_Colon_0, list),
28870+
createDiagnosticForNode(firstFile, Diagnostics.Conflicts_are_in_this_file)
28871+
));
2885228872
}
28853-
// Otheriwse issue top-level error since the files appear very identical in terms of what they appear
28854-
const list = conflictingKeys.join(", ");
28855-
diagnostics.add(addRelatedInfo(
28856-
createDiagnosticForNode(firstFile, Diagnostics.Definitions_of_the_following_identifiers_conflict_with_those_in_another_file_Colon_0, list),
28857-
createDiagnosticForNode(secondFile, Diagnostics.Conflicts_are_in_this_file)
28858-
));
28859-
diagnostics.add(addRelatedInfo(
28860-
createDiagnosticForNode(secondFile, Diagnostics.Definitions_of_the_following_identifiers_conflict_with_those_in_another_file_Colon_0, list),
28861-
createDiagnosticForNode(firstFile, Diagnostics.Conflicts_are_in_this_file)
28862-
));
2886328873
});
2886428874
amalgamatedDuplicates = undefined;
28865-
28866-
function addErrorsForDuplicates(secondFileInstances: Map<{ instances: Node[]; blockScoped: boolean; }>, firstFileInstances: Map<{ instances: Node[]; blockScoped: boolean; }>) {
28867-
secondFileInstances.forEach((locations, symbolName) => {
28868-
const firstFileEquivalent = firstFileInstances.get(symbolName)!;
28869-
const message = locations.blockScoped
28870-
? Diagnostics.Cannot_redeclare_block_scoped_variable_0
28871-
: Diagnostics.Duplicate_identifier_0;
28872-
locations.instances.forEach(node => {
28873-
addDuplicateDeclarationError(node, message, symbolName, firstFileEquivalent.instances[0]);
28874-
});
28875-
});
28876-
}
2887728875
}
2887828876

2887928877
function checkExternalEmitHelpers(location: Node, helpers: ExternalEmitHelpers) {

‎src/compiler/utilities.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8369,4 +8369,16 @@ namespace ts {
83698369
export function isJsonEqual(a: unknown, b: unknown): boolean {
83708370
return a === b || typeof a === "object" && a !== null && typeof b === "object" && b !== null && equalOwnProperties(a as MapLike<unknown>, b as MapLike<unknown>, isJsonEqual);
83718371
}
8372+
8373+
export function getOrUpdate<T>(map: Map<T>, key: string, getValue: () => T): T {
8374+
const got = map.get(key);
8375+
if (got === undefined) {
8376+
const value = getValue();
8377+
map.set(key, value);
8378+
return value;
8379+
}
8380+
else {
8381+
return got;
8382+
}
8383+
}
83728384
}

‎tests/baselines/reference/duplicateIdentifierRelatedSpans_moduleAugmentation.errors.txt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/dir/a.ts(1,14): error TS2451: Cannot redeclare block-scoped variable 'x'.
2+
/dir/b.ts(4,18): error TS2451: Cannot redeclare block-scoped variable 'x'.
3+
/dir/b.ts(8,18): error TS2451: Cannot redeclare block-scoped variable 'x'.
4+
5+
6+
==== /dir/a.ts (1 errors) ====
7+
export const x = 0;
8+
~
9+
!!! error TS2451: Cannot redeclare block-scoped variable 'x'.
10+
!!! related TS6203 /dir/b.ts:4:18: 'x' was also declared here.
11+
!!! related TS6204 /dir/b.ts:8:18: and here.
12+
13+
==== /dir/b.ts (2 errors) ====
14+
export {};
15+
16+
declare module "./a" {
17+
export const x = 0;
18+
~
19+
!!! error TS2451: Cannot redeclare block-scoped variable 'x'.
20+
!!! related TS6203 /dir/a.ts:1:14: 'x' was also declared here.
21+
}
22+
23+
declare module "../dir/a" {
24+
export const x = 0;
25+
~
26+
!!! error TS2451: Cannot redeclare block-scoped variable 'x'.
27+
!!! related TS6203 /dir/a.ts:1:14: 'x' was also declared here.
28+
}
29+

‎tests/baselines/reference/duplicateIdentifierRelatedSpans_moduleAugmentation.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//// [tests/cases/compiler/duplicateIdentifierRelatedSpans_moduleAugmentation.ts] ////
2+
3+
//// [a.ts]
4+
export const x = 0;
5+
6+
//// [b.ts]
7+
export {};
8+
9+
declare module "./a" {
10+
export const x = 0;
11+
}
12+
13+
declare module "../dir/a" {
14+
export const x = 0;
15+
}
16+
17+
18+
//// [a.js]
19+
"use strict";
20+
exports.__esModule = true;
21+
exports.x = 0;
22+
//// [b.js]
23+
"use strict";
24+
exports.__esModule = true;

‎tests/baselines/reference/duplicateIdentifierRelatedSpans_moduleAugmentation.symbols

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
=== /dir/a.ts ===
2+
export const x = 0;
3+
>x : Symbol(x, Decl(a.ts, 0, 12))
4+
5+
=== /dir/b.ts ===
6+
export {};
7+
8+
declare module "./a" {
9+
>"./a" : Symbol("/dir/a", Decl(a.ts, 0, 0), Decl(b.ts, 0, 10), Decl(b.ts, 4, 1))
10+
11+
export const x = 0;
12+
>x : Symbol(x, Decl(b.ts, 3, 16))
13+
}
14+
15+
declare module "../dir/a" {
16+
>"../dir/a" : Symbol("/dir/a", Decl(a.ts, 0, 0), Decl(b.ts, 0, 10), Decl(b.ts, 4, 1))
17+
18+
export const x = 0;
19+
>x : Symbol(x, Decl(b.ts, 7, 16))
20+
}
21+

‎tests/baselines/reference/duplicateIdentifierRelatedSpans_moduleAugmentation.types

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
=== /dir/a.ts ===
2+
export const x = 0;
3+
>x : 0
4+
>0 : 0
5+
6+
=== /dir/b.ts ===
7+
export {};
8+
9+
declare module "./a" {
10+
>"./a" : typeof import("/dir/a")
11+
12+
export const x = 0;
13+
>x : 0
14+
>0 : 0
15+
}
16+
17+
declare module "../dir/a" {
18+
>"../dir/a" : typeof import("/dir/a")
19+
20+
export const x = 0;
21+
>x : 0
22+
>0 : 0
23+
}
24+

‎tests/cases/compiler/duplicateIdentifierRelatedSpans_moduleAugmentation.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// @Filename: /dir/a.ts
2+
export const x = 0;
3+
4+
// @Filename: /dir/b.ts
5+
export {};
6+
7+
declare module "./a" {
8+
export const x = 0;
9+
}
10+
11+
declare module "../dir/a" {
12+
export const x = 0;
13+
}

0 commit comments

Comments
 (0)