diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index e5b34b8d5081d..245306ac10954 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -887,7 +887,7 @@ namespace ts.textChanges { namespace changesToText { export function getTextChangesFromChanges(changes: readonly Change[], newLineCharacter: string, formatContext: formatting.FormatContext, validate: ValidateNonFormattedText | undefined): FileTextChanges[] { - return group(changes, c => c.sourceFile.path).map(changesInFile => { + return mapDefined(group(changes, c => c.sourceFile.path), changesInFile => { const sourceFile = changesInFile[0].sourceFile; // order changes by start position // If the start position is the same, put the shorter range first, since an empty range (x, x) may precede (x, y) but not vice-versa. @@ -897,9 +897,20 @@ namespace ts.textChanges { Debug.assert(normalized[i].range.end <= normalized[i + 1].range.pos, "Changes overlap", () => `${JSON.stringify(normalized[i].range)} and ${JSON.stringify(normalized[i + 1].range)}`); } - const textChanges = normalized.map(c => - createTextChange(createTextSpanFromRange(c.range), computeNewText(c, sourceFile, newLineCharacter, formatContext, validate))); - return { fileName: sourceFile.fileName, textChanges }; + + const textChanges = mapDefined(normalized, c => { + const span = createTextSpanFromRange(c.range); + const newText = computeNewText(c, sourceFile, newLineCharacter, formatContext, validate); + + // Filter out redundant changes. + if (span.length === newText.length && stringContainsAt(sourceFile.text, newText, span.start)) { + return undefined; + } + + return createTextChange(span, newText); + }); + + return textChanges.length > 0 ? { fileName: sourceFile.fileName, textChanges } : undefined; }); } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 4eb452658005e..58f53198f7905 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2827,6 +2827,35 @@ namespace ts { return symbol.name; } + /** + * Useful to check whether a string contains another string at a specific index + * without allocating another string or traversing the entire contents of the outer string. + * + * This function is useful in place of either of the following: + * + * ```ts + * // Allocates + * haystack.substr(startIndex, needle.length) === needle + * + * // Full traversal + * haystack.indexOf(needle, startIndex) === startIndex + * ``` + * + * @param haystack The string that potentially contains `needle`. + * @param needle The string whose content might sit within `haystack`. + * @param startIndex The index within `haystack` to start searching for `needle`. + */ + export function stringContainsAt(haystack: string, needle: string, startIndex: number) { + const needleLength = needle.length; + if (needleLength + startIndex > haystack.length) { + return false; + } + for (let i = 0; i < needleLength; i++) { + if (needle.charCodeAt(i) !== haystack.charCodeAt(i + startIndex)) return false; + } + return true; + } + export function startsWithUnderscore(name: string): boolean { return name.charCodeAt(0) === CharacterCodes._; } diff --git a/src/testRunner/unittests/services/organizeImports.ts b/src/testRunner/unittests/services/organizeImports.ts index 6627616a67dfe..627928c03e89c 100644 --- a/src/testRunner/unittests/services/organizeImports.ts +++ b/src/testRunner/unittests/services/organizeImports.ts @@ -285,6 +285,7 @@ namespace ts { }); }); + describe("Baselines", () => { const libFile = { @@ -327,6 +328,16 @@ export const Other = 1; assert.isEmpty(changes); }); + it("doesn't return any changes when the text would be identical", () => { + const testFile = { + path: "/a.ts", + content: `import { f } from 'foo';\nf();` + }; + const languageService = makeLanguageService(testFile); + const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatSettings, emptyOptions); + assert.isEmpty(changes); + }); + testOrganizeImports("Renamed_used", { path: "/test.ts", @@ -366,6 +377,16 @@ D(); }, libFile); + it("doesn't return any changes when the text would be identical", () => { + const testFile = { + path: "/a.ts", + content: `import { f } from 'foo';\nf();` + }; + const languageService = makeLanguageService(testFile); + const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatSettings, emptyOptions); + assert.isEmpty(changes); + }); + testOrganizeImports("Unused_All", { path: "/test.ts", @@ -377,14 +398,17 @@ import D from "lib"; }, libFile); - testOrganizeImports("Unused_Empty", - { + it("Unused_Empty", () => { + const testFile = { path: "/test.ts", content: ` import { } from "lib"; `, - }, - libFile); + }; + const languageService = makeLanguageService(testFile); + const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatSettings, emptyOptions); + assert.isEmpty(changes); + }); testOrganizeImports("Unused_false_positive_module_augmentation", { @@ -414,25 +438,33 @@ declare module 'caseless' { test(name: KeyType): boolean; } }` - }); + }); - testOrganizeImports("Unused_false_positive_shorthand_assignment", - { + it("Unused_false_positive_shorthand_assignment", () => { + const testFile = { path: "/test.ts", content: ` import { x } from "a"; const o = { x }; ` - }); + }; + const languageService = makeLanguageService(testFile); + const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatSettings, emptyOptions); + assert.isEmpty(changes); + }); - testOrganizeImports("Unused_false_positive_export_shorthand", - { + it("Unused_false_positive_export_shorthand", () => { + const testFile = { path: "/test.ts", content: ` import { x } from "a"; export { x }; ` - }); + }; + const languageService = makeLanguageService(testFile); + const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatSettings, emptyOptions); + assert.isEmpty(changes); + }); testOrganizeImports("MoveToTop", {