organizeImports makes no changes if there are parse errors in the s…

…ourceFile (#58903)
microsoft · Jul 25, 2024 · 107a007 · 107a007
1 parent 941d154
commit 107a007
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 53 deletions.
diff --git a/src/services/services.ts b/src/services/services.ts
@@ -2674,8 +2674,9 @@ export function createLanguageService(
         synchronizeHostData();
         Debug.assert(args.type === "file");
         const sourceFile = getValidSourceFile(args.fileName);
-        const formatContext = formatting.getFormatContext(formatOptions, host);
+        if (containsParseError(sourceFile)) return emptyArray;
 
+        const formatContext = formatting.getFormatContext(formatOptions, host);
         const mode = args.mode ?? (args.skipDestructiveCodeActions ? OrganizeImportsMode.SortAndCombine : OrganizeImportsMode.All);
         return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, preferences, mode);
     }

diff --git a/src/testRunner/unittests/services/organizeImports.ts b/src/testRunner/unittests/services/organizeImports.ts
@@ -470,6 +470,28 @@ console.log(A, B, a, b);`,
 console.log(A, B, a, b);`,
         });
 
+        testOrganizeImports("parseErrors", /*skipDestructiveCodeActions*/ false, {
+            path: "/test.js",
+            content: `declare module 'mod1' {
+    declare export type P = {|
+    |};
+    declare export type F = {|
+        ...$Exact<Node>,
+        await?: Span,
+    |};
+    declare export type S = {|
+    |};
+    declare export type C = {|
+    |};
+}
+
+declare module 'mod2' {
+    import type {
+        U,
+    } from 'mod1';
+}`,
+        });
+
         testOrganizeImports("Renamed_used", /*skipDestructiveCodeActions*/ false, {
             path: "/test.ts",
             content: `
@@ -822,8 +844,7 @@ import { React, Other } from "react";
 `,
         }, reactLibFile);
 
-        // TS files are not JSX contexts, so the parser does not treat
-        // `<div/>` as a JSX element.
+        // TS files are not JSX contexts, so the parser does not treat `<div/>` as a JSX element.
         testOrganizeImports("JsxFactoryUsedTs", /*skipDestructiveCodeActions*/ false, {
             path: "/test.ts",
             content: `
@@ -1049,19 +1070,32 @@ export * from "lib";
             const { path: testPath, content: testContent } = testFile;
             const languageService = makeLanguageService(testFile, ...otherFiles);
             const changes = languageService.organizeImports({ skipDestructiveCodeActions, type: "file", fileName: testPath }, ts.testFormatSettings, ts.emptyOptions);
-            assert.equal(changes.length, 1);
-            assert.equal(changes[0].fileName, testPath);
-
-            const newText = ts.textChanges.applyChanges(testContent, changes[0].textChanges);
-            Harness.Baseline.runBaseline(
-                baselinePath,
-                [
-                    "// ==ORIGINAL==",
-                    testContent,
-                    "// ==ORGANIZED==",
-                    newText,
-                ].join(newLineCharacter),
-            );
+
+            if (changes.length !== 0) {
+                assert.equal(changes.length, 1);
+                assert.equal(changes[0].fileName, testPath);
+
+                const newText = ts.textChanges.applyChanges(testContent, changes[0].textChanges);
+                Harness.Baseline.runBaseline(
+                    baselinePath,
+                    [
+                        "// ==ORIGINAL==",
+                        testContent,
+                        "// ==ORGANIZED==",
+                        newText,
+                    ].join(newLineCharacter),
+                );
+            }
+            else {
+                Harness.Baseline.runBaseline(
+                    baselinePath,
+                    [
+                        "// ==ORIGINAL==",
+                        "// ==NO CHANGES==",
+                        testContent,
+                    ].join(newLineCharacter),
+                );
+            }
         }
 
         function testDetectionBaseline(testName: string, skipDestructiveCodeActions: boolean, testFile: File, ...otherFiles: File[]) {

diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUsedTs.ts b/tests/baselines/reference/organizeImports/JsxFactoryUsedTs.ts
@@ -1,10 +1,6 @@
 // ==ORIGINAL==
+// ==NO CHANGES==
 
 import { React, Other } from "react";
 
-<div/>;
-
-// ==ORGANIZED==
-
-
 <div/>;
diff --git a/tests/baselines/reference/organizeImports/Syntax_Error_Body.ts b/tests/baselines/reference/organizeImports/Syntax_Error_Body.ts
@@ -1,15 +1,9 @@
 // ==ORIGINAL==
+// ==NO CHANGES==
 
 import { F1, F2 } from "lib";
 import * as NS from "lib";
 import D from "lib";
 
 class class class;
 D;
-
-// ==ORGANIZED==
-
-import D from "lib";
-
-class class class;
-D;
diff --git a/tests/baselines/reference/organizeImports/Syntax_Error_Body_skipDestructiveCodeActions.ts b/tests/baselines/reference/organizeImports/Syntax_Error_Body_skipDestructiveCodeActions.ts
@@ -1,16 +1,9 @@
 // ==ORIGINAL==
+// ==NO CHANGES==
 
 import { F1, F2 } from "lib";
 import * as NS from "lib";
 import D from "lib";
 
 class class class;
 D;
-
-// ==ORGANIZED==
-
-import * as NS from "lib";
-import D, { F1, F2 } from "lib";
-
-class class class;
-D;
diff --git a/tests/baselines/reference/organizeImports/Syntax_Error_Imports.ts b/tests/baselines/reference/organizeImports/Syntax_Error_Imports.ts
@@ -1,15 +1,9 @@
 // ==ORIGINAL==
+// ==NO CHANGES==
 
 import { F1, F2 class class class; } from "lib";
 import * as NS from "lib";
 class class class;
 import D from "lib";
 
 D;
-
-// ==ORGANIZED==
-
-import D from "lib";
-class class class;
-
-D;
diff --git a/tests/baselines/reference/organizeImports/Syntax_Error_Imports_skipDestructiveCodeActions.ts b/tests/baselines/reference/organizeImports/Syntax_Error_Imports_skipDestructiveCodeActions.ts
@@ -1,16 +1,9 @@
 // ==ORIGINAL==
+// ==NO CHANGES==
 
 import { F1, F2 class class class; } from "lib";
 import * as NS from "lib";
 class class class;
 import D from "lib";
 
 D;
-
-// ==ORGANIZED==
-
-import * as NS from "lib";
-import D, { F1, F2, class, class, class } from "lib";
-class class class;
-
-D;
diff --git a/tests/baselines/reference/organizeImports/parseErrors.ts b/tests/baselines/reference/organizeImports/parseErrors.ts
@@ -0,0 +1,20 @@
+// ==ORIGINAL==
+// ==NO CHANGES==
+declare module 'mod1' {
+    declare export type P = {|
+    |};
+    declare export type F = {|
+        ...$Exact<Node>,
+        await?: Span,
+    |};
+    declare export type S = {|
+    |};
+    declare export type C = {|
+    |};
+}
+
+declare module 'mod2' {
+    import type {
+        U,
+    } from 'mod1';
+}
diff --git a/tests/cases/fourslash/organizeImports17.ts b/tests/cases/fourslash/organizeImports17.ts
@@ -1,10 +1,10 @@
 /// <reference path="fourslash.ts" />
 
 //// import { Both } from "module-specifiers-unsorted";
-//// import { case, Insensitively, sorted } from "aardvark";
+//// import { aa, CaseInsensitively, sorted } from "aardvark";
 
 verify.organizeImports(
-`import { case, Insensitively, sorted } from "aardvark";
+`import { aa, CaseInsensitively, sorted } from "aardvark";
 import { Both } from "module-specifiers-unsorted";
 `,
     ts.OrganizeImportsMode.SortAndCombine,