Fix accidental relative imports from non namespace barrels (#59544)

microsoft · Aug 8, 2024 · 5e9e6ad · 5e9e6ad
1 parent 1f54d0a
commit 5e9e6ad
Show file tree

Hide file tree

Showing 6 changed files with 113 additions and 26 deletions.
diff --git a/eslint.config.mjs b/eslint.config.mjs
@@ -210,12 +210,20 @@ export default tseslint.config(
                 { name: "clearImmediate" },
                 { name: "performance" },
             ],
+            "local/no-direct-import": "error",
         },
     },
     {
         files: ["src/harness/**", "src/testRunner/**"],
         rules: {
             "no-restricted-globals": "off",
+            "local/no-direct-import": "off",
+        },
+    },
+    {
+        files: ["src/**/_namespaces/**"],
+        rules: {
+            "local/no-direct-import": "off",
         },
     },
     {

diff --git a/scripts/eslint/rules/no-direct-import.cjs b/scripts/eslint/rules/no-direct-import.cjs
@@ -0,0 +1,81 @@
+const { createRule } = require("./utils.cjs");
+const path = require("path");
+
+/** @import { TSESTree } from "@typescript-eslint/utils" */
+void 0;
+
+module.exports = createRule({
+    name: "no-direct-import",
+    meta: {
+        docs: {
+            description: ``,
+        },
+        messages: {
+            noDirectImport: `This import relatively references another project; did you mean to import from a local _namespace module?`,
+        },
+        schema: [],
+        type: "problem",
+    },
+    defaultOptions: [],
+
+    create(context) {
+        const containingFileName = context.filename;
+        const containingDirectory = path.dirname(containingFileName);
+
+        /** @type {(node: TSESTree.ImportDeclaration | TSESTree.TSImportEqualsDeclaration) => void} */
+        const check = node => {
+            let source;
+            if (node.type === "TSImportEqualsDeclaration") {
+                const moduleReference = node.moduleReference;
+                if (
+                    moduleReference.type === "TSExternalModuleReference"
+                    && moduleReference.expression.type === "Literal"
+                    && typeof moduleReference.expression.value === "string"
+                ) {
+                    source = moduleReference.expression;
+                }
+            }
+            else {
+                source = node.source;
+            }
+
+            if (!source) return;
+
+            const p = source.value;
+
+            // These appear in place of public API imports.
+            if (p.endsWith("../typescript/typescript.js")) return;
+
+            // The below is similar to https://github.com/microsoft/DefinitelyTyped-tools/blob/main/packages/eslint-plugin/src/rules/no-bad-reference.ts
+
+            // Any relative path that goes to the wrong place will contain "..".
+            if (!p.includes("..")) return;
+
+            const parts = p.split("/");
+            let cwd = containingDirectory;
+            for (const part of parts) {
+                if (part === "" || part === ".") {
+                    continue;
+                }
+                if (part === "..") {
+                    cwd = path.dirname(cwd);
+                }
+                else {
+                    cwd = path.join(cwd, part);
+                }
+
+                if (path.basename(cwd) === "src") {
+                    context.report({
+                        messageId: "noDirectImport",
+                        node: source,
+                    });
+                }
+            }
+        };
+
+        return {
+            ImportDeclaration: check,
+            TSImportEqualsDeclaration: check,
+        };
+    },
+});
diff --git a/src/services/codefixes/fixMissingTypeAnnotationOnExports.ts b/src/services/codefixes/fixMissingTypeAnnotationOnExports.ts
@@ -1,3 +1,12 @@
+import {
+    createCodeFixAction,
+    createCombinedCodeActions,
+    createImportAdder,
+    eachDiagnostic,
+    registerCodeFix,
+    typePredicateToAutoImportableTypeNode,
+    typeToAutoImportableTypeNode,
+} from "../_namespaces/ts.codefix.js";
 import {
     ArrayBindingPattern,
     ArrayLiteralExpression,
@@ -97,17 +106,7 @@ import {
     VariableStatement,
     walkUpParenthesizedExpressions,
 } from "../_namespaces/ts.js";
-
-import {
-    createCodeFixAction,
-    createCombinedCodeActions,
-    createImportAdder,
-    eachDiagnostic,
-    registerCodeFix,
-    typePredicateToAutoImportableTypeNode,
-    typeToAutoImportableTypeNode,
-} from "../_namespaces/ts.codefix.js";
-import { getIdentifierForNode } from "../refactors/helpers.js";
+import { getIdentifierForNode } from "../_namespaces/ts.refactor.js";
 
 const fixId = "fixMissingTypeAnnotationOnExports";
 

diff --git a/src/services/pasteEdits.ts b/src/services/pasteEdits.ts
@@ -1,22 +1,20 @@
-import { findIndex } from "../compiler/core.js";
 import {
     CancellationToken,
-    Program,
-    SourceFile,
-    Statement,
-    SymbolFlags,
-    TextRange,
-    UserPreferences,
-} from "../compiler/types.js";
-import {
     codefix,
     Debug,
     fileShouldUseJavaScriptRequire,
+    findIndex,
     forEachChild,
     formatting,
     getQuotePreference,
     isIdentifier,
+    Program,
+    SourceFile,
+    Statement,
+    SymbolFlags,
     textChanges,
+    TextRange,
+    UserPreferences,
 } from "./_namespaces/ts.js";
 import { addTargetFileImports } from "./refactors/helpers.js";
 import {

diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts
@@ -1,4 +1,3 @@
-import { getModuleSpecifier } from "../../compiler/_namespaces/ts.moduleSpecifiers.js";
 import {
     ApplicableRefactorInfo,
     arrayFrom,
@@ -118,6 +117,7 @@ import {
     ModifierLike,
     ModuleDeclaration,
     ModuleKind,
+    moduleSpecifiers,
     moduleSpecifierToValidIdentifier,
     NamedImportBindings,
     Node,
@@ -157,8 +157,10 @@ import {
     VariableDeclarationList,
     VariableStatement,
 } from "../_namespaces/ts.js";
-import { addTargetFileImports } from "../_namespaces/ts.refactor.js";
-import { registerRefactor } from "../refactorProvider.js";
+import {
+    addTargetFileImports,
+    registerRefactor,
+} from "../_namespaces/ts.refactor.js";
 
 const refactorNameForMoveToFile = "Move to file";
 const description = getLocaleSpecificMessage(Diagnostics.Move_to_file);
@@ -358,7 +360,7 @@ function updateImportsInOtherFiles(
 
                 if (getStringComparer(!program.useCaseSensitiveFileNames())(pathToTargetFileWithExtension, sourceFile.fileName) === Comparison.EqualTo) return;
 
-                const newModuleSpecifier = getModuleSpecifier(program.getCompilerOptions(), sourceFile, sourceFile.fileName, pathToTargetFileWithExtension, createModuleSpecifierResolutionHost(program, host));
+                const newModuleSpecifier = moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, sourceFile.fileName, pathToTargetFileWithExtension, createModuleSpecifierResolutionHost(program, host));
                 const newImportDeclaration = filterImport(importNode, makeStringLiteral(newModuleSpecifier, quotePreference), shouldMove);
                 if (newImportDeclaration) changes.insertNodeAfter(sourceFile, statement, newImportDeclaration);
 

diff --git a/src/services/stringCompletions.ts b/src/services/stringCompletions.ts
@@ -1,4 +1,3 @@
-import { getModuleSpecifierPreferences } from "../compiler/moduleSpecifiers.js";
 import {
     CompletionKind,
     createCompletionDetails,
@@ -823,7 +822,7 @@ function getFilenameWithExtensionOption(name: string, program: Program, extensio
         return { name, extension: tryGetExtensionFromPath(name) };
     }
 
-    let allowedEndings = getModuleSpecifierPreferences(
+    let allowedEndings = moduleSpecifiers.getModuleSpecifierPreferences(
         { importModuleSpecifierEnding: extensionOptions.endingPreference },
         program,
         program.getCompilerOptions(),