Skip to content

Commit cb8a1d8

Browse files
JS-366: detect commented JSX attributes in S125 (#6812)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent a4f86b9 commit cb8a1d8

4 files changed

Lines changed: 178 additions & 17 deletions

File tree

its/ruling/src/test/expected/console/typescript-S125.json

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
"console:src/views/FunctionsView/FunctionLogs/Log.tsx": [
1212
6
1313
],
14+
"console:src/views/FunctionsView/FunctionPopup/TestLog.tsx": [
15+
90
16+
],
1417
"console:src/views/PermissionsView/PermissionsList/ModelPermissions/ModelPermissionList.tsx": [
1518
13
1619
],
@@ -28,14 +31,28 @@
2831
264
2932
],
3033
"console:src/views/Settings/Export/Export.tsx": [
31-
15
34+
15,
35+
103,
36+
106,
37+
110
38+
],
39+
"console:src/views/models/DatabrowserView/Cell/IntCell.tsx": [
40+
23
3241
],
3342
"console:src/views/models/DatabrowserView/DatabrowserView.tsx": [
34-
230
43+
230,
44+
290,
45+
291,
46+
292
3547
],
3648
"console:src/views/models/FieldPopup/Constraints.tsx": [
3749
209
3850
],
51+
"console:src/views/models/ModelHeader.tsx": [
52+
229,
53+
230,
54+
233
55+
],
3956
"console:src/views/playground/PlaygroundView/PlaygroundView.tsx": [
4057
127
4158
]

its/ruling/src/test/expected/react-cloud-music/javascript-S125.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
"react-cloud-music:src/application/User/Login/index.jsx": [
1010
36
1111
],
12+
"react-cloud-music:src/baseUI/confirm/index.jsx": [
13+
89
14+
],
1215
"react-cloud-music:src/baseUI/scroll/index.jsx": [
1316
65
1417
]

packages/analysis/src/jsts/rules/S125/rule.ts

Lines changed: 70 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const DOCUMENTATION_PREFIX_PATTERN = /^(e\.g[.:]|for example\b|examples?:)/i;
3535
// Cheap prefilter: any meaningful JS statement must contain at least one of these characters,
3636
// or be an import/export with a string literal (side-effect imports have no punctuation)
3737
const CODE_CHAR_PATTERN = /[;{}()=<>]|\bimport\s+['"]|\bexport\s/;
38+
const JSX_ATTRIBUTE_WRAPPER = '_S125Wrapper';
3839

3940
const recognizer = new CodeRecognizer(0.9, new JavaScriptFootPrint());
4041

@@ -174,23 +175,22 @@ function containsCode(value: string, context: Rule.RuleContext) {
174175
return false;
175176
}
176177

177-
try {
178-
const options = {
179-
...context.languageOptions?.parserOptions,
180-
filePath: `placeholder${path.extname(context.filename)}`,
181-
programs: undefined,
182-
project: undefined,
183-
};
184-
//In case of Vue parser: we will use the JS/TS parser instead of the Vue parser
185-
const parser =
186-
context.languageOptions?.parserOptions?.parser ?? context.languageOptions?.parser;
187-
const result =
188-
'parse' in parser ? parser.parse(value, options) : parser.parseForESLint(value, options).ast;
189-
const program = result as AST.Program;
178+
const options = {
179+
...context.languageOptions?.parserOptions,
180+
filePath: `placeholder${path.extname(context.filename)}`,
181+
programs: undefined,
182+
project: undefined,
183+
};
184+
//In case of Vue parser: we will use the JS/TS parser instead of the Vue parser
185+
const parser = context.languageOptions?.parserOptions?.parser ?? context.languageOptions?.parser;
186+
const program = parseWithRuleParser(value, parser, options);
187+
188+
if (program) {
190189
return program.body.length > 0 && !isExclusion(program.body, value, program, context);
191-
} catch {
192-
return false;
193190
}
191+
192+
const wrappedProgram = parseWithRuleParser(wrapAsJsxTag(value), parser, options);
193+
return hasValidJsxAttributeCode(wrappedProgram);
194194
}
195195

196196
function couldBeJsCode(input: string): boolean {
@@ -252,3 +252,58 @@ function isDocumentationExample(value: string): boolean {
252252
const stripped = firstLine.replace(/^(?:\/\/\s*)+/, '').trim();
253253
return DOCUMENTATION_PREFIX_PATTERN.test(stripped);
254254
}
255+
256+
function parseWithRuleParser(
257+
value: string,
258+
parser: NonNullable<Rule.RuleContext['languageOptions']['parser']>,
259+
options: Rule.RuleContext['languageOptions']['parserOptions'],
260+
): AST.Program | null {
261+
try {
262+
return (
263+
'parse' in parser ? parser.parse(value, options) : parser.parseForESLint(value, options).ast
264+
) as AST.Program;
265+
} catch {
266+
return null;
267+
}
268+
}
269+
270+
function wrapAsJsxTag(value: string): string {
271+
return `<${JSX_ATTRIBUTE_WRAPPER} ${value} />`;
272+
}
273+
274+
function hasValidJsxAttributeCode(program: AST.Program | null): boolean {
275+
if (program?.body.length !== 1) {
276+
return false;
277+
}
278+
279+
const statement = program.body[0] as TSESTree.Statement;
280+
if (statement.type !== 'ExpressionStatement' || statement.expression.type !== 'JSXElement') {
281+
return false;
282+
}
283+
284+
const jsxElement = statement.expression;
285+
if (jsxElement.openingElement.name.type !== 'JSXIdentifier') {
286+
return false;
287+
}
288+
289+
if (jsxElement.openingElement.name.name !== JSX_ATTRIBUTE_WRAPPER) {
290+
return false;
291+
}
292+
293+
const attributes = jsxElement.openingElement.attributes;
294+
return attributes.length > 0 && attributes.every(isValidJsxAttribute);
295+
}
296+
297+
function isValidJsxAttribute(
298+
attribute: TSESTree.JSXAttribute | TSESTree.JSXSpreadAttribute,
299+
): boolean {
300+
if (attribute.type === 'JSXSpreadAttribute') {
301+
return true;
302+
}
303+
304+
if (attribute.name.type !== 'JSXIdentifier') {
305+
return false;
306+
}
307+
308+
return attribute.value !== null;
309+
}

packages/analysis/src/jsts/rules/S125/unit.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,18 @@ describe('S125', () => {
159159
// FP: "e.g:" colon variant is a documentation example
160160
code: `// e.g: step.where(condition);`,
161161
},
162+
{
163+
// FP guard for JSX fallback: prose with "TODO:" and JSX-like text should not be flagged
164+
code: `
165+
const Calendar = () => {
166+
return (
167+
<DateCalendar
168+
// TODO: implement views={options}
169+
value={1234}
170+
/>
171+
);
172+
};`,
173+
},
162174
],
163175
invalid: [
164176
{
@@ -384,6 +396,80 @@ let x = 0;`,
384396
},
385397
],
386398
},
399+
{
400+
// JSX attributes commented in TSX should be reported as commented-out code
401+
code: `
402+
const Calendar: React.FC = () => {
403+
return (
404+
<DateCalendar
405+
data-testid="header-calendar"
406+
value={1234}
407+
openTo="year"
408+
// views={["year"]}
409+
// maxDate={new Date()}
410+
onChange={date => {
411+
console.log(date);
412+
}}
413+
/>
414+
);
415+
};`,
416+
errors: [
417+
{
418+
messageId: 'commentedCode',
419+
suggestions: [
420+
{
421+
desc: 'Remove this commented out code',
422+
output: `
423+
const Calendar: React.FC = () => {
424+
return (
425+
<DateCalendar
426+
data-testid="header-calendar"
427+
value={1234}
428+
openTo="year"
429+
430+
onChange={date => {
431+
console.log(date);
432+
}}
433+
/>
434+
);
435+
};`,
436+
},
437+
],
438+
},
439+
],
440+
},
441+
{
442+
// JSX attributes commented in JS should also be reported
443+
filename: 'placeholder.js',
444+
code: `
445+
const Calendar = () => {
446+
return (
447+
<DateCalendar
448+
// views={["year"]}
449+
value={1234}
450+
/>
451+
);
452+
};`,
453+
errors: [
454+
{
455+
messageId: 'commentedCode',
456+
suggestions: [
457+
{
458+
desc: 'Remove this commented out code',
459+
output: `
460+
const Calendar = () => {
461+
return (
462+
<DateCalendar
463+
464+
value={1234}
465+
/>
466+
);
467+
};`,
468+
},
469+
],
470+
},
471+
],
472+
},
387473
],
388474
});
389475
});

0 commit comments

Comments
 (0)