From 5e9e6ad2953e8097cb2e90283f656988f2ddd563 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 8 Aug 2024 09:38:10 -0700 Subject: [PATCH] Fix accidental relative imports from non namespace barrels (#59544) --- eslint.config.mjs | 8 ++ scripts/eslint/rules/no-direct-import.cjs | 81 +++++++++++++++++++ .../fixMissingTypeAnnotationOnExports.ts | 21 +++-- src/services/pasteEdits.ts | 16 ++-- src/services/refactors/moveToFile.ts | 10 ++- src/services/stringCompletions.ts | 3 +- 6 files changed, 113 insertions(+), 26 deletions(-) create mode 100644 scripts/eslint/rules/no-direct-import.cjs diff --git a/eslint.config.mjs b/eslint.config.mjs index 13729d3ee83be..f2566c3b631f8 100644 --- 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 new file mode 100644 index 0000000000000..614e67db30be3 --- /dev/null +++ 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 index db117935dea2d..deea661f2e72a 100644 --- 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 index f3b350816781e..e39a51bf45076 100644 --- 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 index da6d806a06f30..85c040013edcb 100644 --- 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 index a26959d57f2dc..667cdf24ca8aa 100644 --- 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(),