Skip to content

Commit ac55b29

Browse files
authored
Upgrade "boolean-trivia" lint to new "argument-trivia" lint that uses type info, has quick fixes, etc. (#53002)
1 parent 3a3146e commit ac55b29

File tree

89 files changed

+615
-528
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

89 files changed

+615
-528
lines changed

.eslintrc.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@
9797
"allowDeclarations": true
9898
}],
9999
"local/no-double-space": "error",
100-
"local/boolean-trivia": "error",
100+
"local/argument-trivia": "error",
101101
"local/no-in-operator": "error",
102102
"local/simple-indent": "error",
103103
"local/debug-assert": "error",
+203
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
const { AST_NODE_TYPES, TSESTree, ESLintUtils } = require("@typescript-eslint/utils");
2+
const { createRule } = require("./utils.cjs");
3+
const ts = require("typescript");
4+
5+
const unset = Symbol();
6+
/**
7+
* @template T
8+
* @param {() => T} fn
9+
* @returns {() => T}
10+
*/
11+
function memoize(fn) {
12+
/** @type {T | unset} */
13+
let value = unset;
14+
return () => {
15+
if (value === unset) {
16+
value = fn();
17+
}
18+
return value;
19+
};
20+
}
21+
22+
23+
module.exports = createRule({
24+
name: "argument-trivia",
25+
meta: {
26+
docs: {
27+
description: ``,
28+
recommended: "error",
29+
},
30+
messages: {
31+
argumentTriviaArgumentError: `Tag argument with parameter name`,
32+
argumentTriviaArgumentSpaceError: `There should be 1 space between an argument and its comment`,
33+
argumentTriviaArgumentNameError: `Argument name "{{ got }}" does not match expected name "{{ want }}"`,
34+
},
35+
schema: [],
36+
type: "problem",
37+
fixable: "code",
38+
},
39+
defaultOptions: [],
40+
41+
create(context) {
42+
const sourceCode = context.getSourceCode();
43+
const sourceCodeText = sourceCode.getText();
44+
45+
/** @type {(name: string) => boolean} */
46+
const isSetOrAssert = (name) => name.startsWith("set") || name.startsWith("assert");
47+
/** @type {(node: TSESTree.Node) => boolean} */
48+
const isTrivia = (node) => {
49+
if (node.type === AST_NODE_TYPES.Identifier) {
50+
return node.name === "undefined";
51+
}
52+
53+
if (node.type === AST_NODE_TYPES.Literal) {
54+
// eslint-disable-next-line no-null/no-null
55+
return node.value === null || node.value === true || node.value === false;
56+
}
57+
58+
return false;
59+
};
60+
61+
/** @type {(node: TSESTree.CallExpression | TSESTree.NewExpression) => boolean} */
62+
const shouldIgnoreCalledExpression = (node) => {
63+
if (node.callee && node.callee.type === AST_NODE_TYPES.MemberExpression) {
64+
const methodName = node.callee.property.type === AST_NODE_TYPES.Identifier
65+
? node.callee.property.name
66+
: "";
67+
68+
if (isSetOrAssert(methodName)) {
69+
return true;
70+
}
71+
72+
switch (methodName) {
73+
case "apply":
74+
case "call":
75+
case "equal":
76+
case "stringify":
77+
case "push":
78+
return true;
79+
}
80+
81+
return false;
82+
}
83+
84+
if (node.callee && node.callee.type === AST_NODE_TYPES.Identifier) {
85+
const functionName = node.callee.name;
86+
87+
if (isSetOrAssert(functionName)) {
88+
return true;
89+
}
90+
91+
switch (functionName) {
92+
case "contains":
93+
return true;
94+
}
95+
96+
return false;
97+
}
98+
99+
return false;
100+
};
101+
102+
103+
/** @type {(node: TSESTree.Node, i: number, getSignature: () => ts.Signature | undefined) => void} */
104+
const checkArg = (node, i, getSignature) => {
105+
if (!isTrivia(node)) {
106+
return;
107+
}
108+
109+
const getExpectedName = memoize(() => {
110+
const signature = getSignature();
111+
if (signature) {
112+
const expectedName = signature.parameters[i]?.escapedName;
113+
if (expectedName) {
114+
const name = ts.unescapeLeadingUnderscores(expectedName);
115+
// If a parameter is unused, we prepend an underscore. Ignore this
116+
// so that we can switch between used and unused without modifying code,
117+
// requiring that arugments are tagged with the non-underscored name.
118+
return name.startsWith("_") ? name.slice(1) : name;
119+
}
120+
}
121+
return undefined;
122+
});
123+
124+
const comments = sourceCode.getCommentsBefore(node);
125+
/** @type {TSESTree.Comment | undefined} */
126+
const comment = comments[comments.length - 1];
127+
128+
if (!comment || comment.type !== "Block") {
129+
const expectedName = getExpectedName();
130+
if (expectedName) {
131+
context.report({
132+
messageId: "argumentTriviaArgumentError",
133+
node,
134+
fix: (fixer) => {
135+
return fixer.insertTextBefore(node, `/*${expectedName}*/ `);
136+
}
137+
});
138+
}
139+
else {
140+
context.report({ messageId: "argumentTriviaArgumentError", node });
141+
}
142+
return;
143+
}
144+
145+
const argRangeStart = node.range[0];
146+
const commentRangeEnd = comment.range[1];
147+
const expectedName = getExpectedName();
148+
if (expectedName) {
149+
const got = comment.value;
150+
if (got !== expectedName) {
151+
context.report({
152+
messageId: "argumentTriviaArgumentNameError",
153+
data: { got, want: expectedName },
154+
node: comment,
155+
fix: (fixer) => {
156+
return fixer.replaceText(comment, `/*${expectedName}*/`);
157+
},
158+
});
159+
return;
160+
}
161+
}
162+
163+
const hasNewLine = sourceCodeText.slice(commentRangeEnd, argRangeStart).indexOf("\n") >= 0;
164+
if (argRangeStart !== commentRangeEnd + 1 && !hasNewLine) {
165+
// TODO(jakebailey): range should be whitespace
166+
context.report({
167+
messageId: "argumentTriviaArgumentSpaceError",
168+
node,
169+
fix: (fixer) => {
170+
return fixer.replaceTextRange([commentRangeEnd, argRangeStart], " ");
171+
}
172+
});
173+
}
174+
};
175+
176+
/** @type {(node: TSESTree.CallExpression | TSESTree.NewExpression) => void} */
177+
const checkArgumentTrivia = (node) => {
178+
if (shouldIgnoreCalledExpression(node)) {
179+
return;
180+
}
181+
182+
const getSignature = memoize(() => {
183+
if (context.parserServices?.hasFullTypeInformation) {
184+
const parserServices = ESLintUtils.getParserServices(context);
185+
const checker = parserServices.program.getTypeChecker();
186+
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
187+
return checker.getResolvedSignature(tsNode);
188+
}
189+
return undefined;
190+
});
191+
192+
for (let i = 0; i < node.arguments.length; i++) {
193+
const arg = node.arguments[i];
194+
checkArg(arg, i, getSignature);
195+
}
196+
};
197+
198+
return {
199+
CallExpression: checkArgumentTrivia,
200+
NewExpression: checkArgumentTrivia,
201+
};
202+
},
203+
});

scripts/eslint/rules/boolean-trivia.cjs

-110
This file was deleted.

scripts/eslint/tests/boolean-trivia.test.cjs renamed to scripts/eslint/tests/argument-trivia.test.cjs

+14-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const { RuleTester } = require("./support/RuleTester.cjs");
2-
const rule = require("../rules/boolean-trivia.cjs");
2+
const rule = require("../rules/argument-trivia.cjs");
33

44
const ruleTester = new RuleTester({
55
parserOptions: {
@@ -8,7 +8,7 @@ const ruleTester = new RuleTester({
88
parser: require.resolve("@typescript-eslint/parser"),
99
});
1010

11-
ruleTester.run("boolean-trivia", rule, {
11+
ruleTester.run("argument-trivia", rule, {
1212
valid: [
1313
{
1414
code: `
@@ -48,6 +48,12 @@ const fn = (prop: boolean) => {};
4848
fn.apply(null, true);
4949
`,
5050
},
51+
{
52+
code: `
53+
const fn = (prop: boolean) => {};
54+
fn(/* first comment */ /* second comment */ false);
55+
`,
56+
},
5157
],
5258

5359
invalid: [
@@ -56,28 +62,25 @@ fn.apply(null, true);
5662
const fn = (prop: null) => {};
5763
fn(null);
5864
`,
59-
errors: [{ messageId: "booleanTriviaArgumentError" }],
65+
errors: [{ messageId: "argumentTriviaArgumentError" }],
6066
},
6167
{
6268
code: `
6369
const fn = (prop: boolean) => {};
6470
fn(false);
6571
`,
66-
errors: [{ messageId: "booleanTriviaArgumentError" }],
72+
errors: [{ messageId: "argumentTriviaArgumentError" }],
6773
},
6874
{
6975
code: `
7076
const fn = (prop: boolean) => {};
7177
fn(/* boolean arg */false);
7278
`,
73-
errors: [{ messageId: "booleanTriviaArgumentSpaceError" }],
74-
},
75-
{
76-
code: `
79+
errors: [{ messageId: "argumentTriviaArgumentSpaceError" }],
80+
output:`
7781
const fn = (prop: boolean) => {};
78-
fn(/* first comment */ /* second comment */ false);
79-
`,
80-
errors: [{ messageId: "booleanTriviaArgumentError" }],
82+
fn(/* boolean arg */ false);
83+
`
8184
},
8285
],
8386
});

0 commit comments

Comments
 (0)