From d7e33c90a2fb6748c39630fc2a9f5d8e8272e9a7 Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Tue, 11 Oct 2016 13:51:53 -0700 Subject: [PATCH 1/4] Guard localeCompare function --- src/compiler/core.ts | 22 ++++++++++++++++++++-- src/harness/harness.ts | 2 +- src/server/session.ts | 2 +- src/services/navigateTo.ts | 4 ++-- src/services/navigationBar.ts | 6 ++---- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 7c68306211a..5ad9fa2c199 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -20,6 +20,24 @@ namespace ts { const createObject = Object.create; + // More efficient to create a collator once and use its `compare` than to call `a.localeCompare(b)` many times. + export const collator: { compare(a: string, b: string): number } = typeof Intl === "object" && typeof Intl.Collator === "function" ? new Intl.Collator() : undefined; + + /** + * Use this function instead of calling "String.prototype.localeCompre". This function will preform appropriate check to make sure that + * "typeof Intl" is correct as there are reported issues #11110 nad #11339. + * @param a reference string to compare + * @param b string to compare against + * @param locales string of BCP 47 language tag or an array of string of BCP 47 language tag + * @param options an object of Intl.CollatorOptions to specify localeCompare properties + */ + export function localeCompare(a: string, b: string, locales?: string | string[], options?: Intl.CollatorOptions): number | undefined { + if (collator && String.prototype.localeCompare) { + return a.localeCompare(b, locales, options); + } + return undefined; + } + export function createMap(template?: MapLike): Map { const map: Map = createObject(null); // tslint:disable-line:no-null-keyword @@ -1016,8 +1034,8 @@ namespace ts { if (a === undefined) return Comparison.LessThan; if (b === undefined) return Comparison.GreaterThan; if (ignoreCase) { - if (String.prototype.localeCompare) { - const result = a.localeCompare(b, /*locales*/ undefined, { usage: "sort", sensitivity: "accent" }); + const result = localeCompare(a, b, /*locales*/ undefined, /*options*/ { usage: "sort", sensitivity: "accent" }); + if (result) { return result < 0 ? Comparison.LessThan : result > 0 ? Comparison.GreaterThan : Comparison.EqualTo; } diff --git a/src/harness/harness.ts b/src/harness/harness.ts index 7c82aeece78..f26a9dd062b 100644 --- a/src/harness/harness.ts +++ b/src/harness/harness.ts @@ -1634,7 +1634,7 @@ namespace Harness { export function collateOutputs(outputFiles: Harness.Compiler.GeneratedFile[]): string { // Collect, test, and sort the fileNames - outputFiles.sort((a, b) => cleanName(a.fileName).localeCompare(cleanName(b.fileName))); + outputFiles.sort((a, b) => ts.localeCompare(cleanName(a.fileName), cleanName(b.fileName))); // Emit them let result = ""; diff --git a/src/server/session.ts b/src/server/session.ts index 3b95bdd6fb9..d4633f119e3 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -959,7 +959,7 @@ namespace ts.server { result.push({ name, kind, kindModifiers, sortText, replacementSpan: convertedSpan }); } return result; - }, []).sort((a, b) => a.name.localeCompare(b.name)); + }, []).sort((a, b) => localeCompare(a.name, b.name)); } else { return completions; diff --git a/src/services/navigateTo.ts b/src/services/navigateTo.ts index 44aa25f5005..8c968db7ebb 100644 --- a/src/services/navigateTo.ts +++ b/src/services/navigateTo.ts @@ -188,8 +188,8 @@ namespace ts.NavigateTo { // We first sort case insensitively. So "Aaa" will come before "bar". // Then we sort case sensitively, so "aaa" will come before "Aaa". return i1.matchKind - i2.matchKind || - i1.name.localeCompare(i2.name, undefined, baseSensitivity) || - i1.name.localeCompare(i2.name); + ts.localeCompare(i1.name, i2.name, /*locales*/ undefined, /*options*/ baseSensitivity) || + ts.localeCompare(i1.name, i2.name); } function createNavigateToItem(rawItem: RawNavigateToItem): NavigateToItem { diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 0397ae73229..f981ad6016d 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -323,10 +323,8 @@ namespace ts.NavigationBar { } } - // More efficient to create a collator once and use its `compare` than to call `a.localeCompare(b)` many times. - const collator: { compare(a: string, b: string): number } = typeof Intl === "object" && typeof Intl.Collator === "function" ? new Intl.Collator() : undefined; // Intl is missing in Safari, and node 0.10 treats "a" as greater than "B". - const localeCompareIsCorrect = collator && collator.compare("a", "B") < 0; + const localeCompareIsCorrect = ts.collator && ts.collator.compare("a", "B") < 0; const localeCompareFix: (a: string, b: string) => number = localeCompareIsCorrect ? collator.compare : function(a, b) { // This isn't perfect, but it passes all of our tests. for (let i = 0; i < Math.min(a.length, b.length); i++) { @@ -337,7 +335,7 @@ namespace ts.NavigationBar { if (chA === "'" && chB === "\"") { return -1; } - const cmp = chA.toLocaleLowerCase().localeCompare(chB.toLocaleLowerCase()); + const cmp = ts.localeCompare(chA.toLocaleLowerCase(), chB.toLocaleLowerCase()); if (cmp !== 0) { return cmp; } From c03b04bd4f278dc338ba15017ee8c47b20d96163 Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Wed, 12 Oct 2016 14:26:30 -0700 Subject: [PATCH 2/4] Only use localeCompare in CompareStrings --- src/compiler/core.ts | 20 +++++++++++++------- src/harness/harness.ts | 2 +- src/server/session.ts | 2 +- src/services/navigateTo.ts | 4 ++-- src/services/navigationBar.ts | 2 +- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 5ad9fa2c199..47eb66db07c 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -23,6 +23,10 @@ namespace ts { // More efficient to create a collator once and use its `compare` than to call `a.localeCompare(b)` many times. export const collator: { compare(a: string, b: string): number } = typeof Intl === "object" && typeof Intl.Collator === "function" ? new Intl.Collator() : undefined; + // This means "compare in a case insensitive manner but consider accentsor other diacritic marks" + // (e.g. a ≠ b, a ≠ á, a = A) + const accentSensitivity: Intl.CollatorOptions = { usage: "sort", sensitivity: "accent" }; + /** * Use this function instead of calling "String.prototype.localeCompre". This function will preform appropriate check to make sure that * "typeof Intl" is correct as there are reported issues #11110 nad #11339. @@ -1029,13 +1033,13 @@ namespace ts { return a < b ? Comparison.LessThan : Comparison.GreaterThan; } - export function compareStrings(a: string, b: string, ignoreCase?: boolean): Comparison { + export function compareStrings(a: string, b: string, locales?: string | string[], options?: Intl.CollatorOptions): Comparison { if (a === b) return Comparison.EqualTo; if (a === undefined) return Comparison.LessThan; if (b === undefined) return Comparison.GreaterThan; - if (ignoreCase) { - const result = localeCompare(a, b, /*locales*/ undefined, /*options*/ { usage: "sort", sensitivity: "accent" }); - if (result) { + if (options) { + if (collator && String.prototype.localeCompare) { + const result = a.localeCompare(b, locales, options); return result < 0 ? Comparison.LessThan : result > 0 ? Comparison.GreaterThan : Comparison.EqualTo; } @@ -1048,7 +1052,7 @@ namespace ts { } export function compareStringsCaseInsensitive(a: string, b: string) { - return compareStrings(a, b, /*ignoreCase*/ true); + return compareStrings(a, b, /*locales*/ undefined, accentSensitivity); } function getDiagnosticFileName(diagnostic: Diagnostic): string { @@ -1418,7 +1422,8 @@ namespace ts { const bComponents = getNormalizedPathComponents(b, currentDirectory); const sharedLength = Math.min(aComponents.length, bComponents.length); for (let i = 0; i < sharedLength; i++) { - const result = compareStrings(aComponents[i], bComponents[i], ignoreCase); + const result = compareStrings(aComponents[i], bComponents[i], + /*locales*/ undefined, /*options*/ ignoreCase ? accentSensitivity : undefined); if (result !== Comparison.EqualTo) { return result; } @@ -1440,7 +1445,8 @@ namespace ts { } for (let i = 0; i < parentComponents.length; i++) { - const result = compareStrings(parentComponents[i], childComponents[i], ignoreCase); + const result = compareStrings(parentComponents[i], childComponents[i], + /*locales*/ undefined, /*options*/ ignoreCase ? accentSensitivity : undefined); if (result !== Comparison.EqualTo) { return false; } diff --git a/src/harness/harness.ts b/src/harness/harness.ts index f26a9dd062b..9d27257c80f 100644 --- a/src/harness/harness.ts +++ b/src/harness/harness.ts @@ -1634,7 +1634,7 @@ namespace Harness { export function collateOutputs(outputFiles: Harness.Compiler.GeneratedFile[]): string { // Collect, test, and sort the fileNames - outputFiles.sort((a, b) => ts.localeCompare(cleanName(a.fileName), cleanName(b.fileName))); + outputFiles.sort((a, b) => ts.compareStrings(cleanName(a.fileName), cleanName(b.fileName))); // Emit them let result = ""; diff --git a/src/server/session.ts b/src/server/session.ts index d4633f119e3..cebdef717bc 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -959,7 +959,7 @@ namespace ts.server { result.push({ name, kind, kindModifiers, sortText, replacementSpan: convertedSpan }); } return result; - }, []).sort((a, b) => localeCompare(a.name, b.name)); + }, []).sort((a, b) => ts.compareStrings(a.name, b.name)); } else { return completions; diff --git a/src/services/navigateTo.ts b/src/services/navigateTo.ts index 8c968db7ebb..2433dbe9795 100644 --- a/src/services/navigateTo.ts +++ b/src/services/navigateTo.ts @@ -188,8 +188,8 @@ namespace ts.NavigateTo { // We first sort case insensitively. So "Aaa" will come before "bar". // Then we sort case sensitively, so "aaa" will come before "Aaa". return i1.matchKind - i2.matchKind || - ts.localeCompare(i1.name, i2.name, /*locales*/ undefined, /*options*/ baseSensitivity) || - ts.localeCompare(i1.name, i2.name); + ts.compareStrings(i1.name, i2.name, /*locales*/ undefined, /*options*/ baseSensitivity) || + ts.compareStrings(i1.name, i2.name); } function createNavigateToItem(rawItem: RawNavigateToItem): NavigateToItem { diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index f981ad6016d..4dacf202ef8 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -335,7 +335,7 @@ namespace ts.NavigationBar { if (chA === "'" && chB === "\"") { return -1; } - const cmp = ts.localeCompare(chA.toLocaleLowerCase(), chB.toLocaleLowerCase()); + const cmp = ts.compareStrings(chA.toLocaleLowerCase(), chB.toLocaleLowerCase()); if (cmp !== 0) { return cmp; } From 5b4dfcc18f441f6af1bcd5485b86e9a3429019c1 Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Thu, 13 Oct 2016 13:22:58 -0700 Subject: [PATCH 3/4] Address PR: remove locales and options; use accent as default --- src/compiler/core.ts | 19 +++++++------------ src/services/navigateTo.ts | 5 +---- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 47eb66db07c..e8fadbff6e9 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -23,10 +23,6 @@ namespace ts { // More efficient to create a collator once and use its `compare` than to call `a.localeCompare(b)` many times. export const collator: { compare(a: string, b: string): number } = typeof Intl === "object" && typeof Intl.Collator === "function" ? new Intl.Collator() : undefined; - // This means "compare in a case insensitive manner but consider accentsor other diacritic marks" - // (e.g. a ≠ b, a ≠ á, a = A) - const accentSensitivity: Intl.CollatorOptions = { usage: "sort", sensitivity: "accent" }; - /** * Use this function instead of calling "String.prototype.localeCompre". This function will preform appropriate check to make sure that * "typeof Intl" is correct as there are reported issues #11110 nad #11339. @@ -1033,13 +1029,14 @@ namespace ts { return a < b ? Comparison.LessThan : Comparison.GreaterThan; } - export function compareStrings(a: string, b: string, locales?: string | string[], options?: Intl.CollatorOptions): Comparison { + export function compareStrings(a: string, b: string, ignoreCase?: boolean): Comparison { if (a === b) return Comparison.EqualTo; if (a === undefined) return Comparison.LessThan; if (b === undefined) return Comparison.GreaterThan; - if (options) { + if (ignoreCase) { if (collator && String.prototype.localeCompare) { - const result = a.localeCompare(b, locales, options); + // accent means a ≠ b, a ≠ á, a = A + const result = a.localeCompare(b, /*locales*/ undefined, { usage: "sort", sensitivity: "accent" }); return result < 0 ? Comparison.LessThan : result > 0 ? Comparison.GreaterThan : Comparison.EqualTo; } @@ -1052,7 +1049,7 @@ namespace ts { } export function compareStringsCaseInsensitive(a: string, b: string) { - return compareStrings(a, b, /*locales*/ undefined, accentSensitivity); + return compareStrings(a, b, /*ignoreCase*/ true); } function getDiagnosticFileName(diagnostic: Diagnostic): string { @@ -1422,8 +1419,7 @@ namespace ts { const bComponents = getNormalizedPathComponents(b, currentDirectory); const sharedLength = Math.min(aComponents.length, bComponents.length); for (let i = 0; i < sharedLength; i++) { - const result = compareStrings(aComponents[i], bComponents[i], - /*locales*/ undefined, /*options*/ ignoreCase ? accentSensitivity : undefined); + const result = compareStrings(aComponents[i], bComponents[i], ignoreCase); if (result !== Comparison.EqualTo) { return result; } @@ -1445,8 +1441,7 @@ namespace ts { } for (let i = 0; i < parentComponents.length; i++) { - const result = compareStrings(parentComponents[i], childComponents[i], - /*locales*/ undefined, /*options*/ ignoreCase ? accentSensitivity : undefined); + const result = compareStrings(parentComponents[i], childComponents[i], ignoreCase); if (result !== Comparison.EqualTo) { return false; } diff --git a/src/services/navigateTo.ts b/src/services/navigateTo.ts index 2433dbe9795..82f4dd49f2b 100644 --- a/src/services/navigateTo.ts +++ b/src/services/navigateTo.ts @@ -6,9 +6,6 @@ namespace ts.NavigateTo { const patternMatcher = createPatternMatcher(searchValue); let rawItems: RawNavigateToItem[] = []; - // This means "compare in a case insensitive manner." - const baseSensitivity: Intl.CollatorOptions = { sensitivity: "base" }; - // Search the declarations in all files and output matched NavigateToItem into array of NavigateToItem[] forEach(sourceFiles, sourceFile => { cancellationToken.throwIfCancellationRequested(); @@ -188,7 +185,7 @@ namespace ts.NavigateTo { // We first sort case insensitively. So "Aaa" will come before "bar". // Then we sort case sensitively, so "aaa" will come before "Aaa". return i1.matchKind - i2.matchKind || - ts.compareStrings(i1.name, i2.name, /*locales*/ undefined, /*options*/ baseSensitivity) || + ts.compareStringsCaseInsensitive(i1.name, i2.name) || ts.compareStrings(i1.name, i2.name); } From 99e2219c4ef8c1f5580d873074aa17cdbedf2cac Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Thu, 13 Oct 2016 15:09:26 -0700 Subject: [PATCH 4/4] Remove unused function --- src/compiler/core.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index e8fadbff6e9..3ca4abc69da 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -23,21 +23,6 @@ namespace ts { // More efficient to create a collator once and use its `compare` than to call `a.localeCompare(b)` many times. export const collator: { compare(a: string, b: string): number } = typeof Intl === "object" && typeof Intl.Collator === "function" ? new Intl.Collator() : undefined; - /** - * Use this function instead of calling "String.prototype.localeCompre". This function will preform appropriate check to make sure that - * "typeof Intl" is correct as there are reported issues #11110 nad #11339. - * @param a reference string to compare - * @param b string to compare against - * @param locales string of BCP 47 language tag or an array of string of BCP 47 language tag - * @param options an object of Intl.CollatorOptions to specify localeCompare properties - */ - export function localeCompare(a: string, b: string, locales?: string | string[], options?: Intl.CollatorOptions): number | undefined { - if (collator && String.prototype.localeCompare) { - return a.localeCompare(b, locales, options); - } - return undefined; - } - export function createMap(template?: MapLike): Map { const map: Map = createObject(null); // tslint:disable-line:no-null-keyword