Skip to content

Commit 23c07b2

Browse files
jakebaileyjakebailey
committed
Switch to module and make lint rule
1 parent ca64946 commit 23c07b2

File tree

18 files changed

+1265
-1003
lines changed

18 files changed

+1265
-1003
lines changed

‎eslint.config.mjs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ export default tseslint.config(
156156
"local/jsdoc-format": "error",
157157
"local/js-extensions": "error",
158158
"local/no-array-mutating-method-expressions": "error",
159+
"local/prefer-direct-import": "error",
159160
},
160161
},
161162
{
@@ -221,6 +222,12 @@ export default tseslint.config(
221222
"local/no-direct-import": "off",
222223
},
223224
},
225+
{
226+
files: ["src/harness/**", "src/testRunner/**", "src/tsserver/**", "src/typingsInstaller/**"],
227+
rules: {
228+
"local/prefer-direct-import": "off",
229+
},
230+
},
224231
{
225232
files: ["src/lib/*.d.ts"],
226233
...tseslint.configs.disableTypeChecked,

‎scripts/eslint/rules/no-direct-import.cjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ module.exports = createRule({
4545

4646
// These appear in place of public API imports.
4747
if (p.endsWith("../typescript/typescript.js")) return;
48+
// Okay to direct import, for now.
49+
// TODO(jakebailey): we should consider where this is safe to ignore
50+
if (p.endsWith("/debug.js")) return;
4851

4952
// The below is similar to https://github.com/microsoft/DefinitelyTyped-tools/blob/main/packages/eslint-plugin/src/rules/no-bad-reference.ts
5053

‎scripts/eslint/rules/prefer-direct-import.cjs‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
const { AST_NODE_TYPES } = require("@typescript-eslint/utils");
2+
const { createRule } = require("./utils.cjs");
3+
const path = require("path");
4+
5+
const srcRoot = path.resolve(__dirname, "../../../src");
6+
7+
const tsNamespaceBarrelRegex = /_namespaces\/ts(?:\.ts|\.js|\.mts|\.mjs|\.cts|\.cjs)?$/;
8+
9+
/**
10+
* @type {Array<{ name: string; path: string; }>}
11+
*/
12+
const modules = [
13+
{
14+
name: "Debug",
15+
path: "compiler/debug",
16+
},
17+
];
18+
19+
module.exports = createRule({
20+
name: "prefer-direct-import",
21+
meta: {
22+
docs: {
23+
description: ``,
24+
},
25+
messages: {
26+
importError: `{{ name }} should be imported directly from {{ path }}`,
27+
},
28+
schema: [],
29+
type: "problem",
30+
fixable: "code",
31+
},
32+
defaultOptions: [],
33+
34+
create(context) {
35+
/**
36+
* @param {string} p
37+
*/
38+
function getImportPath(p) {
39+
let out = path.relative(path.dirname(context.filename), path.join(srcRoot, p)).replace(/\\/g, "/");
40+
if (!out.startsWith(".")) out = "./" + out;
41+
return out;
42+
}
43+
44+
/** @type {any} */
45+
let program;
46+
let addedImport = false;
47+
48+
return {
49+
Program: node => {
50+
program = node;
51+
},
52+
ImportDeclaration: node => {
53+
if (node.importKind !== "value" || !tsNamespaceBarrelRegex.test(node.source.value)) return;
54+
55+
for (const specifier of node.specifiers) {
56+
if (specifier.type !== AST_NODE_TYPES.ImportSpecifier || specifier.importKind !== "value") continue;
57+
58+
for (const mod of modules) {
59+
if (specifier.imported.name !== mod.name) continue;
60+
61+
context.report({
62+
messageId: "importError",
63+
data: { name: mod.name, path: mod.path },
64+
node: specifier,
65+
fix: fixer => {
66+
const newCode = `import * as ${mod.name} from "${getImportPath(mod.path)}";`;
67+
const fixes = [];
68+
if (node.specifiers.length === 1) {
69+
if (addedImport) {
70+
fixes.push(fixer.remove(node));
71+
}
72+
else {
73+
fixes.push(fixer.replaceText(node, newCode));
74+
addedImport = true;
75+
}
76+
}
77+
else {
78+
const comma = context.sourceCode.getTokenAfter(specifier, token => token.value === ",");
79+
if (!comma) throw new Error("comma is null");
80+
const prevNode = context.sourceCode.getTokenBefore(specifier);
81+
if (!prevNode) throw new Error("prevNode is null");
82+
fixes.push(
83+
fixer.removeRange([prevNode.range[1], specifier.range[0]]),
84+
fixer.remove(specifier),
85+
fixer.remove(comma),
86+
);
87+
if (!addedImport) {
88+
fixes.push(fixer.insertTextBefore(node, newCode + "\r\n"));
89+
addedImport = true;
90+
}
91+
}
92+
93+
return fixes;
94+
},
95+
});
96+
}
97+
}
98+
},
99+
MemberExpression: node => {
100+
if (node.object.type !== AST_NODE_TYPES.Identifier || node.object.name !== "ts") return;
101+
102+
for (const mod of modules) {
103+
if (node.property.type !== AST_NODE_TYPES.Identifier || node.property.name !== mod.name) continue;
104+
105+
context.report({
106+
messageId: "importError",
107+
data: { name: mod.name, path: mod.path },
108+
node,
109+
fix: fixer => {
110+
const fixes = [fixer.replaceText(node, mod.name)];
111+
112+
if (!addedImport) {
113+
fixes.push(fixer.insertTextBefore(program, `import * as ${mod.name} from "${getImportPath(mod.path)}";\r\n`));
114+
addedImport = true;
115+
}
116+
117+
return fixes;
118+
},
119+
});
120+
}
121+
},
122+
};
123+
},
124+
});

‎scripts/eslint/tests/prefer-direct-import.cjs‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
const { RuleTester } = require("./support/RuleTester.cjs");
2+
const rule = require("../rules/prefer-direct-import.cjs");
3+
4+
const ruleTester = new RuleTester({
5+
parserOptions: {
6+
warnOnUnsupportedTypeScriptVersion: false,
7+
},
8+
parser: require.resolve("@typescript-eslint/parser"),
9+
});
10+
11+
ruleTester.run("no-ts-debug", rule, {
12+
valid: [
13+
{
14+
code: `
15+
import * as Debug from "./debug";
16+
`,
17+
},
18+
{
19+
code: `
20+
import type { Debug } from "./_namespaces/ts";
21+
`,
22+
},
23+
{
24+
code: `
25+
import { type Debug } from "./_namespaces/ts";
26+
`,
27+
},
28+
],
29+
30+
invalid: [
31+
{
32+
filename: "src/compiler/checker.ts",
33+
code: `
34+
import { Debug } from "./_namespaces/ts";
35+
`.replace(/\r?\n/g, "\r\n"),
36+
errors: [{ messageId: "importError", data: { name: "Debug", path: "compiler/debug" } }],
37+
output: `
38+
import * as Debug from "./debug";
39+
`.replace(/\r?\n/g, "\r\n"),
40+
},
41+
{
42+
filename: "src/compiler/transformers/ts.ts",
43+
code: `
44+
import { Debug } from "../_namespaces/ts";
45+
`.replace(/\r?\n/g, "\r\n"),
46+
errors: [{ messageId: "importError", data: { name: "Debug", path: "compiler/debug" } }],
47+
output: `
48+
import * as Debug from "../debug";
49+
`.replace(/\r?\n/g, "\r\n"),
50+
},
51+
// TODO(jakebailey): the rule probably needs to handle .js extensions
52+
{
53+
filename: "src/compiler/checker.ts",
54+
code: `
55+
import { Debug } from "./_namespaces/ts.js";
56+
`.replace(/\r?\n/g, "\r\n"),
57+
errors: [{ messageId: "importError", data: { name: "Debug", path: "compiler/debug" } }],
58+
output: `
59+
import * as Debug from "./debug";
60+
`.replace(/\r?\n/g, "\r\n"),
61+
},
62+
{
63+
filename: "src/compiler/checker.ts",
64+
code: `
65+
import * as ts from "./_namespaces/ts";
66+
67+
ts.Debug.assert(true);
68+
`.replace(/\r?\n/g, "\r\n"),
69+
errors: [{ messageId: "importError", data: { name: "Debug", path: "compiler/debug" } }],
70+
output: `
71+
import * as Debug from "./debug";
72+
import * as ts from "./_namespaces/ts";
73+
74+
Debug.assert(true);
75+
`.replace(/\r?\n/g, "\r\n"),
76+
},
77+
],
78+
});

‎src/compiler/_namespaces/ts.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
export * from "../corePublic.js";
44
export * from "../core.js";
5-
export * from "../debug.js";
5+
import * as Debug from "../debug.js";
6+
export { Debug };
67
export * from "../semver.js";
78
export * from "../performanceCore.js";
89
export * from "../tracing.js";

0 commit comments

Comments
 (0)