Skip to content

Commit 3f248ec

Browse files
rbucktonrbuckton
authored
Merge pull request microsoft#19452 from Microsoft/compareStrings
Clean up outdated string comparison logic
2 parents 77b24ae + ed914a8 commit 3f248ec

24 files changed

Lines changed: 469 additions & 221 deletions

‎src/compiler/checker.ts‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7359,28 +7359,12 @@ namespace ts {
73597359
unionIndex?: number;
73607360
}
73617361

7362-
function binarySearchTypes(types: Type[], type: Type): number {
7363-
let low = 0;
7364-
let high = types.length - 1;
7365-
const typeId = type.id;
7366-
while (low <= high) {
7367-
const middle = low + ((high - low) >> 1);
7368-
const id = types[middle].id;
7369-
if (id === typeId) {
7370-
return middle;
7371-
}
7372-
else if (id > typeId) {
7373-
high = middle - 1;
7374-
}
7375-
else {
7376-
low = middle + 1;
7377-
}
7378-
}
7379-
return ~low;
7362+
function getTypeId(type: Type) {
7363+
return type.id;
73807364
}
73817365

73827366
function containsType(types: Type[], type: Type): boolean {
7383-
return binarySearchTypes(types, type) >= 0;
7367+
return binarySearch(types, type, getTypeId, compareValues) >= 0;
73847368
}
73857369

73867370
// Return true if the given intersection type contains (a) more than one unit type or (b) an object
@@ -7421,7 +7405,7 @@ namespace ts {
74217405
if (flags & TypeFlags.Number) typeSet.containsNumber = true;
74227406
if (flags & TypeFlags.StringOrNumberLiteral) typeSet.containsStringOrNumberLiteral = true;
74237407
const len = typeSet.length;
7424-
const index = len && type.id > typeSet[len - 1].id ? ~len : binarySearchTypes(typeSet, type);
7408+
const index = len && type.id > typeSet[len - 1].id ? ~len : binarySearch(typeSet, type, getTypeId, compareValues);
74257409
if (index < 0) {
74267410
if (!(flags & TypeFlags.Object && (<ObjectType>type).objectFlags & ObjectFlags.Anonymous &&
74277411
type.symbol && type.symbol.flags & (SymbolFlags.Function | SymbolFlags.Method) && containsIdenticalType(typeSet, type))) {

‎src/compiler/core.ts‎

Lines changed: 361 additions & 125 deletions
Large diffs are not rendered by default.

‎src/compiler/parser.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6146,7 +6146,7 @@ namespace ts {
61466146
const checkJsDirectiveMatchResult = checkJsDirectiveRegEx.exec(comment);
61476147
if (checkJsDirectiveMatchResult) {
61486148
checkJsDirective = {
6149-
enabled: compareStrings(checkJsDirectiveMatchResult[1], "@ts-check", /*ignoreCase*/ true) === Comparison.EqualTo,
6149+
enabled: equateStringsCaseInsensitive(checkJsDirectiveMatchResult[1], "@ts-check"),
61506150
end: range.end,
61516151
pos: range.pos
61526152
};

‎src/compiler/program.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,11 +1093,12 @@ namespace ts {
10931093

10941094
// If '--lib' is not specified, include default library file according to '--target'
10951095
// otherwise, using options specified in '--lib' instead of '--target' default library file
1096+
const equalityComparer = host.useCaseSensitiveFileNames() ? equateStringsCaseSensitive : equateStringsCaseInsensitive;
10961097
if (!options.lib) {
1097-
return compareStrings(file.fileName, getDefaultLibraryFileName(), /*ignoreCase*/ !host.useCaseSensitiveFileNames()) === Comparison.EqualTo;
1098+
return equalityComparer(file.fileName, getDefaultLibraryFileName());
10981099
}
10991100
else {
1100-
return forEach(options.lib, libFileName => compareStrings(file.fileName, combinePaths(defaultLibraryPath, libFileName), /*ignoreCase*/ !host.useCaseSensitiveFileNames()) === Comparison.EqualTo);
1101+
return forEach(options.lib, libFileName => equalityComparer(file.fileName, combinePaths(defaultLibraryPath, libFileName)));
11011102
}
11021103
}
11031104

‎src/compiler/scanner.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ namespace ts {
355355
* We assume the first line starts at position 0 and 'position' is non-negative.
356356
*/
357357
export function computeLineAndCharacterOfPosition(lineStarts: ReadonlyArray<number>, position: number): LineAndCharacter {
358-
let lineNumber = binarySearch(lineStarts, position);
358+
let lineNumber = binarySearch(lineStarts, position, identity, compareValues);
359359
if (lineNumber < 0) {
360360
// If the actual position was not found,
361361
// the binary search returns the 2's-complement of the next line start

‎src/compiler/tsc.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ namespace ts {
294294

295295
// Sort our options by their names, (e.g. "--noImplicitAny" comes before "--watch")
296296
const optsList = showAllOptions ?
297-
optionDeclarations.slice().sort((a, b) => compareValues<string>(a.name.toLowerCase(), b.name.toLowerCase())) :
297+
sort(optionDeclarations, (a, b) => compareStringsCaseInsensitive(a.name, b.name)) :
298298
filter(optionDeclarations.slice(), v => v.showInSimplifiedHelpView);
299299

300300
// We want our descriptions to align at the same column in our output,

‎src/compiler/types.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,19 @@ namespace ts {
3636
push(...values: T[]): void;
3737
}
3838

39+
/* @internal */
40+
export type EqualityComparer<T> = (a: T, b: T) => boolean;
41+
42+
/* @internal */
43+
export type Comparer<T> = (a: T, b: T) => Comparison;
44+
45+
/* @internal */
46+
export const enum Comparison {
47+
LessThan = -1,
48+
EqualTo = 0,
49+
GreaterThan = 1
50+
}
51+
3952
// branded string type used to store absolute, normalized and canonicalized paths
4053
// arbitrary file name can be converted to Path via toPath function
4154
export type Path = string & { __pathBrand: any };

‎src/compiler/utilities.ts‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -321,16 +321,16 @@ namespace ts {
321321
return getSourceTextOfNodeFromSourceFile(getSourceFileOfNode(node), node, includeTrivia);
322322
}
323323

324+
function getPos(range: Node) {
325+
return range.pos;
326+
}
327+
324328
/**
325329
* Note: it is expected that the `nodeArray` and the `node` are within the same file.
326330
* For example, searching for a `SourceFile` in a `SourceFile[]` wouldn't work.
327331
*/
328332
export function indexOfNode(nodeArray: ReadonlyArray<Node>, node: Node) {
329-
return binarySearch(nodeArray, node, compareNodePos);
330-
}
331-
332-
function compareNodePos({ pos: aPos }: Node, { pos: bPos}: Node) {
333-
return aPos < bPos ? Comparison.LessThan : bPos < aPos ? Comparison.GreaterThan : Comparison.EqualTo;
333+
return binarySearch(nodeArray, node, getPos, compareValues);
334334
}
335335

336336
/**
@@ -2292,6 +2292,7 @@ namespace ts {
22922292
let nonFileDiagnostics: Diagnostic[] = [];
22932293
const fileDiagnostics = createMap<Diagnostic[]>();
22942294

2295+
let hasReadNonFileDiagnostics = false;
22952296
let diagnosticsModified = false;
22962297
let modificationCount = 0;
22972298

@@ -2321,6 +2322,12 @@ namespace ts {
23212322
}
23222323
}
23232324
else {
2325+
// If we've already read the non-file diagnostics, do not modify the existing array.
2326+
if (hasReadNonFileDiagnostics) {
2327+
hasReadNonFileDiagnostics = false;
2328+
nonFileDiagnostics = nonFileDiagnostics.slice();
2329+
}
2330+
23242331
diagnostics = nonFileDiagnostics;
23252332
}
23262333

@@ -2331,6 +2338,7 @@ namespace ts {
23312338

23322339
function getGlobalDiagnostics(): Diagnostic[] {
23332340
sortAndDeduplicate();
2341+
hasReadNonFileDiagnostics = true;
23342342
return nonFileDiagnostics;
23352343
}
23362344

@@ -3950,6 +3958,9 @@ namespace ts {
39503958
trySetLanguageAndTerritory(language, /*territory*/ undefined, errors);
39513959
}
39523960

3961+
// Set the UI locale for string collation
3962+
setUILocale(locale);
3963+
39533964
function trySetLanguageAndTerritory(language: string, territory: string, errors?: Push<Diagnostic>): boolean {
39543965
const compilerFilePath = normalizePath(sys.getExecutingFilePath());
39553966
const containingDirectoryPath = getDirectoryPath(compilerFilePath);

‎src/harness/fourslash.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -564,15 +564,31 @@ namespace FourSlash {
564564
}
565565

566566
for (const { start, length, messageText, file } of errors) {
567-
Harness.IO.log(" from: " + showPosition(file, start) +
568-
", to: " + showPosition(file, start + length) +
567+
Harness.IO.log(" " + this.formatRange(file, start, length) +
569568
", message: " + ts.flattenDiagnosticMessageText(messageText, Harness.IO.newLine()) + "\n");
570569
}
570+
}
571+
572+
private formatRange(file: ts.SourceFile, start: number, length: number) {
573+
if (file) {
574+
return `from: ${this.formatLineAndCharacterOfPosition(file, start)}, to: ${this.formatLineAndCharacterOfPosition(file, start + length)}`;
575+
}
576+
return "global";
577+
}
571578

572-
function showPosition(file: ts.SourceFile, pos: number) {
579+
private formatLineAndCharacterOfPosition(file: ts.SourceFile, pos: number) {
580+
if (file) {
573581
const { line, character } = ts.getLineAndCharacterOfPosition(file, pos);
574582
return `${line}:${character}`;
575583
}
584+
return "global";
585+
}
586+
587+
private formatPosition(file: ts.SourceFile, pos: number) {
588+
if (file) {
589+
return file.fileName + "@" + pos;
590+
}
591+
return "global";
576592
}
577593

578594
public verifyNoErrors() {
@@ -582,7 +598,7 @@ namespace FourSlash {
582598
if (errors.length) {
583599
this.printErrorLog(/*expectErrors*/ false, errors);
584600
const error = errors[0];
585-
this.raiseError(`Found an error: ${error.file.fileName}@${error.start}: ${error.messageText}`);
601+
this.raiseError(`Found an error: ${this.formatPosition(error.file, error.start)}: ${error.messageText}`);
586602
}
587603
});
588604
}

‎src/harness/harness.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,7 +1324,7 @@ namespace Harness {
13241324
export const diagnosticSummaryMarker = "__diagnosticSummary";
13251325
export const globalErrorsMarker = "__globalErrors";
13261326
export function *iterateErrorBaseline(inputFiles: ReadonlyArray<TestFile>, diagnostics: ReadonlyArray<ts.Diagnostic>, pretty?: boolean): IterableIterator<[string, string, number]> {
1327-
diagnostics = diagnostics.slice().sort(ts.compareDiagnostics);
1327+
diagnostics = ts.sort(diagnostics, ts.compareDiagnostics);
13281328
let outputLines = "";
13291329
// Count up all errors that were found in files other than lib.d.ts so we don't miss any
13301330
let totalErrorsReportedInNonLibraryFiles = 0;
@@ -1707,7 +1707,7 @@ namespace Harness {
17071707

17081708
export function *iterateOutputs(outputFiles: Harness.Compiler.GeneratedFile[]): IterableIterator<[string, string]> {
17091709
// Collect, test, and sort the fileNames
1710-
outputFiles.sort((a, b) => ts.compareStrings(cleanName(a.fileName), cleanName(b.fileName)));
1710+
outputFiles.sort((a, b) => ts.compareStringsCaseSensitive(cleanName(a.fileName), cleanName(b.fileName)));
17111711
const dupeCase = ts.createMap<number>();
17121712
// Yield them
17131713
for (const outputFile of outputFiles) {

0 commit comments

Comments
 (0)