From 0b76a524b5bf3784271ca01f15e51d0db1ce81d5 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 2 May 2020 12:22:24 +0200 Subject: [PATCH 1/2] Added jsx-key lint rule. --- packages/@romejs/diagnostics/categories.ts | 1 + packages/@romejs/diagnostics/descriptions.ts | 4 + .../js-compiler/lint/rules/react/index.ts | 3 +- .../lint/rules/react/jsxKey.test.md | 176 ++++++++++++++++++ .../lint/rules/react/jsxKey.test.ts | 64 +++++++ .../js-compiler/lint/rules/react/jsxKey.ts | 78 ++++++++ 6 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.md create mode 100644 packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts create mode 100644 packages/@romejs/js-compiler/lint/rules/react/jsxKey.ts diff --git a/packages/@romejs/diagnostics/categories.ts b/packages/@romejs/diagnostics/categories.ts index 2e8f92e101d..c38da4101f5 100644 --- a/packages/@romejs/diagnostics/categories.ts +++ b/packages/@romejs/diagnostics/categories.ts @@ -68,6 +68,7 @@ export type DiagnosticCategory = | 'lint/unsafeNegation' | 'lint/unusedVariables' | 'lint/react/jsxNoCommentText' + | 'lint/react/jsxKey' | 'lsp/parse' | 'parse/js' | 'parse/json' diff --git a/packages/@romejs/diagnostics/descriptions.ts b/packages/@romejs/diagnostics/descriptions.ts index 46aeaed64af..74feda7f70c 100644 --- a/packages/@romejs/diagnostics/descriptions.ts +++ b/packages/@romejs/diagnostics/descriptions.ts @@ -354,6 +354,10 @@ export const descriptions = createMessages({ category: 'lint/react/jsxNoCommentText', message: 'Comments inside children should be placed in braces', }, + REACT_JSX_KEY: (origin: string) => ({ + category: 'lint/react/jsxKey', + message: markup`Missing the "key" prop for element in ${origin}`, + }), UNSAFE_NEGATION: { category: 'lint/unsafeNegation', message: 'Unsafe usage of negation operator in left side of binary expression', diff --git a/packages/@romejs/js-compiler/lint/rules/react/index.ts b/packages/@romejs/js-compiler/lint/rules/react/index.ts index 977a4b08fd7..3c52c7d701d 100644 --- a/packages/@romejs/js-compiler/lint/rules/react/index.ts +++ b/packages/@romejs/js-compiler/lint/rules/react/index.ts @@ -5,7 +5,8 @@ * LICENSE file in the root directory of this source tree. */ +import jsxKey from './jsxKey'; import jsxNoCommentText from './jsxNoCommentText'; // Add transforms in alphabetical order. -export const reactLintTransforms = [jsxNoCommentText]; +export const reactLintTransforms = [jsxKey, jsxNoCommentText]; diff --git a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.md b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.md new file mode 100644 index 00000000000..03109f2d834 --- /dev/null +++ b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.md @@ -0,0 +1,176 @@ +# `jsxKey.test.ts` + +**DO NOT MODIFY**. This file has been autogenerated. Run `rome test packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts --update-snapshots` to update. + +## `jsx key` + +### `0` + +``` + + unknown:1:11 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ✖ Missing the "key" prop for element in array + + const a = [
,
] + ^^^^^^^ + + unknown:1:20 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ✖ Missing the "key" prop for element in array + + const a = [
,
] + ^^^^^^^ + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +✖ Found 2 problems + +``` + +### `0: formatted` + +``` +const a = [
,
]; + +``` + +### `1` + +``` + + unknown:1:26 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ✖ Missing the "key" prop for element in iterator + + const a = [1, 2].map(x =>
{x}
); + ^^^^^^^^^^^^^^ + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +✖ Found 1 problem + +``` + +### `1: formatted` + +``` +const a = [1, 2].map((x) =>
{x}
); + +``` + +### `2` + +``` + + unknown:2:9 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ✖ Missing the "key" prop for element in iterator + + 1 │ const a = [1, 2].map(x => { + > 2 │ return
{x}
; + │ ^^^^^^^^^^^^^^ + 3 │ }); + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +✖ Found 1 problem + +``` + +### `2: formatted` + +``` +const a = [1, 2].map((x) => { + return
{x}
; +}); + +``` + +### `3` + +``` + + unknown:2:9 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ✖ Missing the "key" prop for element in iterator + + 1 │ const a = [1, 2].map(function(x) { + > 2 │ return
{x}
; + │ ^^^^^^^^^^^^^^ + 3 │ }); + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +✖ Found 1 problem + +``` + +### `3: formatted` + +``` +const a = [1, 2].map(function(x) { + return
{x}
; +}); + +``` + +### `4` + +``` +✔ No known problems! + +``` + +### `4: formatted` + +``` +const a = [
,
]; + +``` + +### `5` + +``` +✔ No known problems! + +``` + +### `5: formatted` + +``` +const a = [1, 2].map((x) =>
{x}
); + +``` + +### `6` + +``` +✔ No known problems! + +``` + +### `6: formatted` + +``` +const a = [1, 2].map((x) => { + return
{x}
; +}); + +``` + +### `7` + +``` +✔ No known problems! + +``` + +### `7: formatted` + +``` +const a = [1, 2].map(function(x) { + return
{x}
; +}); + +``` diff --git a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts new file mode 100644 index 00000000000..ff589aaa913 --- /dev/null +++ b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts @@ -0,0 +1,64 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {test} from 'rome'; +import {dedent} from '@romejs/string-utils'; +import {testLintMultiple} from '../../testHelpers'; + +test( + 'jsx key', + async (t) => { + await testLintMultiple( + t, + [ + // INVALID + 'const a = [
,
]', + dedent( + ` + const a = [1, 2].map(x =>
{x}
); + `, + ), + dedent( + ` + const a = [1, 2].map(x => { + return
{x}
; + }); + `, + ), + dedent( + ` + const a = [1, 2].map(function(x) { + return
{x}
; + }); + `, + ), + // VALID + 'const a = [
,
]', + dedent( + ` + const a = [1, 2].map(x =>
{x}
); + `, + ), + dedent( + ` + const a = [1, 2].map(x => { + return
{x}
; + }); + `, + ), + dedent( + ` + const a = [1, 2].map(function(x) { + return
{x}
; + }); + `, + ), + ], + {category: 'lint/react/jsxKey'}, + ); + }, +); diff --git a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.ts b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.ts new file mode 100644 index 00000000000..e43ed6aa9bb --- /dev/null +++ b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.ts @@ -0,0 +1,78 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {Path} from '@romejs/js-compiler'; +import {AnyNode, JSXElement} from '@romejs/js-ast'; +import {descriptions} from '@romejs/diagnostics'; + +function containsKeyAttr(node: JSXElement): boolean { + const ATTR_NAME = 'key'; + return !!node.attributes.find((attr) => + attr.type === 'JSXAttribute' && attr.name.name === ATTR_NAME + ); +} + +export default { + name: 'jsxKey', + enter(path: Path): AnyNode { + const {node, context} = path; + + // JSXElement in array literal + if ( + node.type === 'JSXElement' && + !containsKeyAttr(node) && + path.parentPath.node.type === 'ArrayExpression' + ) { + context.addNodeDiagnostic(node, descriptions.LINT.REACT_JSX_KEY('array')); + } + + // Array.prototype.map + if ( + node.type === 'CallExpression' && + node.callee?.type === 'MemberExpression' && + (node.callee.property?.value)?.type === 'Identifier' && + node.callee.property.value.name === 'map' + ) { + const fn = node.arguments[0]; + + // Short hand arrow function + if ( + fn?.type === 'ArrowFunctionExpression' && + fn.body?.type === 'JSXElement' && + !containsKeyAttr(fn.body) + ) { + context.addNodeDiagnostic( + fn.body, + descriptions.LINT.REACT_JSX_KEY('iterator'), + ); + } + + // Function or arrow function with block statement + if ( + fn && + (fn.type === 'FunctionExpression' || + fn.type === 'ArrowFunctionExpression') && + fn.body?.type === 'BlockStatement' + ) { + fn.body.body.forEach((statement) => { + if ( + statement.type === 'ReturnStatement' && + statement.argument?.type === 'JSXElement' && + !containsKeyAttr(statement.argument) + ) { + context.addNodeDiagnostic( + statement.argument, + descriptions.LINT.REACT_JSX_KEY('iterator'), + ); + } + }); + } + } + + return node; + }, +}; From becef7148a3a3b2e220ed3193da7b766afa7ec74 Mon Sep 17 00:00:00 2001 From: Matthias Van Parijs Date: Sun, 3 May 2020 20:33:58 +0200 Subject: [PATCH 2/2] Removed optional chaining where possible and migrated to the auto generated lint index. --- packages/@romejs/diagnostics/categories.ts | 1 + packages/@romejs/diagnostics/descriptions.ts | 2 +- packages/@romejs/js-compiler/lint/rules/index.ts | 3 +++ .../js-compiler/lint/rules/react/jsxKey.test.md | 10 +++++----- .../js-compiler/lint/rules/react/jsxKey.test.ts | 4 ++-- .../@romejs/js-compiler/lint/rules/react/jsxKey.ts | 10 +++++----- 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/@romejs/diagnostics/categories.ts b/packages/@romejs/diagnostics/categories.ts index 662e5916762..81aabf32a39 100644 --- a/packages/@romejs/diagnostics/categories.ts +++ b/packages/@romejs/diagnostics/categories.ts @@ -99,6 +99,7 @@ type LintDiagnosticCategory = | 'lint/emptyBlocks' | 'lint/getterReturn' | 'lint/inconsiderateLanguage' + | 'lint/jsxKey' | 'lint/jsxNoCommentText' | 'lint/noArguments' | 'lint/noAsyncPromiseExecutor' diff --git a/packages/@romejs/diagnostics/descriptions.ts b/packages/@romejs/diagnostics/descriptions.ts index 69f0c79145e..e6f15d47f5f 100644 --- a/packages/@romejs/diagnostics/descriptions.ts +++ b/packages/@romejs/diagnostics/descriptions.ts @@ -355,7 +355,7 @@ export const descriptions = createMessages({ message: 'Comments inside children should be placed in braces', }, REACT_JSX_KEY: (origin: string) => ({ - category: 'lint/react/jsxKey', + category: 'lint/jsxKey', message: markup`Missing the "key" prop for element in ${origin}`, }), UNSAFE_NEGATION: { diff --git a/packages/@romejs/js-compiler/lint/rules/index.ts b/packages/@romejs/js-compiler/lint/rules/index.ts index 6811be18ed0..4ac76321f47 100644 --- a/packages/@romejs/js-compiler/lint/rules/index.ts +++ b/packages/@romejs/js-compiler/lint/rules/index.ts @@ -6,6 +6,7 @@ */ // EVERYTHING BELOW IS AUTOGENERATED. SEE SCRIPTS FOLDER FOR UPDATE SCRIPTS + import camelCase from './regular/camelCase'; import caseSingleStatement from './regular/caseSingleStatement'; import defaultExportSameBasename from './regular/defaultExportSameBasename'; @@ -14,6 +15,7 @@ import duplicateImportSource from './regular/duplicateImportSource'; import emptyBlocks from './regular/emptyBlocks'; import getterReturn from './regular/getterReturn'; import inconsiderateLanguage from './regular/inconsiderateLanguage'; +import jsxKey from './react/jsxKey'; import jsxNoCommentText from './react/jsxNoCommentText'; import noArguments from './regular/noArguments'; import noAsyncPromiseExecutor from './regular/noAsyncPromiseExecutor'; @@ -62,6 +64,7 @@ export const lintTransforms = [ emptyBlocks, getterReturn, inconsiderateLanguage, + jsxKey, jsxNoCommentText, noArguments, noAsyncPromiseExecutor, diff --git a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.md b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.md index 03109f2d834..d2ef40255d1 100644 --- a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.md +++ b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.md @@ -8,14 +8,14 @@ ``` - unknown:1:11 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + unknown:1:11 lint/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ✖ Missing the "key" prop for element in array const a = [
,
] ^^^^^^^ - unknown:1:20 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + unknown:1:20 lint/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ✖ Missing the "key" prop for element in array @@ -39,7 +39,7 @@ const a = [
,
]; ``` - unknown:1:26 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + unknown:1:26 lint/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ✖ Missing the "key" prop for element in iterator @@ -63,7 +63,7 @@ const a = [1, 2].map((x) =>
{x}
); ``` - unknown:2:9 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + unknown:2:9 lint/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ✖ Missing the "key" prop for element in iterator @@ -91,7 +91,7 @@ const a = [1, 2].map((x) => { ``` - unknown:2:9 lint/react/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + unknown:2:9 lint/jsxKey ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ✖ Missing the "key" prop for element in iterator diff --git a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts index ff589aaa913..dc3bdaeb8a8 100644 --- a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts +++ b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.test.ts @@ -7,7 +7,7 @@ import {test} from 'rome'; import {dedent} from '@romejs/string-utils'; -import {testLintMultiple} from '../../testHelpers'; +import {testLintMultiple} from '../testHelpers'; test( 'jsx key', @@ -58,7 +58,7 @@ test( `, ), ], - {category: 'lint/react/jsxKey'}, + {category: 'lint/jsxKey'}, ); }, ); diff --git a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.ts b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.ts index e43ed6aa9bb..d3b36516b80 100644 --- a/packages/@romejs/js-compiler/lint/rules/react/jsxKey.ts +++ b/packages/@romejs/js-compiler/lint/rules/react/jsxKey.ts @@ -33,16 +33,16 @@ export default { // Array.prototype.map if ( node.type === 'CallExpression' && - node.callee?.type === 'MemberExpression' && - (node.callee.property?.value)?.type === 'Identifier' && + node.callee.type === 'MemberExpression' && + node.callee.property.value.type === 'Identifier' && node.callee.property.value.name === 'map' ) { const fn = node.arguments[0]; // Short hand arrow function if ( - fn?.type === 'ArrowFunctionExpression' && - fn.body?.type === 'JSXElement' && + fn.type === 'ArrowFunctionExpression' && + fn.body.type === 'JSXElement' && !containsKeyAttr(fn.body) ) { context.addNodeDiagnostic( @@ -56,7 +56,7 @@ export default { fn && (fn.type === 'FunctionExpression' || fn.type === 'ArrowFunctionExpression') && - fn.body?.type === 'BlockStatement' + fn.body.type === 'BlockStatement' ) { fn.body.body.forEach((statement) => { if (