From 0aa12702a7d6fbdebc4c1b43770155922fd48a25 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Tue, 23 Feb 2021 10:42:44 +0800 Subject: [PATCH 1/4] Support `PropertyDefinition` --- package.json | 4 ++-- rules/custom-error-definition.js | 11 +++++++---- rules/no-static-only-class.js | 21 ++++++++++++--------- rules/prefer-number-properties.js | 21 +++++++++++---------- rules/prevent-abbreviations.js | 2 +- 5 files changed, 33 insertions(+), 26 deletions(-) diff --git a/package.json b/package.json index dd9a890a1e..037ab7120a 100644 --- a/package.json +++ b/package.json @@ -51,8 +51,8 @@ }, "devDependencies": { "@babel/code-frame": "7.12.13", - "@babel/core": "7.12.17", - "@babel/eslint-parser": "7.12.17", + "@babel/core": "7.13.1", + "@babel/eslint-parser": "7.13.0", "@lubien/fixture-beta-package": "^1.0.0-beta.1", "@typescript-eslint/parser": "^4.15.1", "ava": "^3.15.0", diff --git a/rules/custom-error-definition.js b/rules/custom-error-definition.js index c5e47d69d0..b9081e9b75 100644 --- a/rules/custom-error-definition.js +++ b/rules/custom-error-definition.js @@ -54,12 +54,15 @@ const isAssignmentExpression = (node, name) => { return lhs.property.name === name; }; -const isClassProperty = (node, name) => { - if (node.type !== 'ClassProperty' || node.computed) { +const isPropertyDefinition = (node, name) => { + const {type, computed, key} = node; + if (type !== 'PropertyDefinition' && type !== 'ClassProperty') { return false; } - const {key} = node; + if (computed) { + return false; + } if (key.type !== 'Identifier') { return false; @@ -144,7 +147,7 @@ const customErrorDefinition = (context, node) => { const nameExpression = constructorBody.find(x => isAssignmentExpression(x, 'name')); if (!nameExpression) { - const nameProperty = body.find(node => isClassProperty(node, 'name')); + const nameProperty = body.find(node => isPropertyDefinition(node, 'name')); if (!nameProperty || !nameProperty.value || nameProperty.value.value !== name) { context.report({ diff --git a/rules/no-static-only-class.js b/rules/no-static-only-class.js index dc02da42f6..afdbc63b9b 100644 --- a/rules/no-static-only-class.js +++ b/rules/no-static-only-class.js @@ -23,9 +23,14 @@ const isDeclarationOfExportDefaultDeclaration = node => node.parent.type === 'ExportDefaultDeclaration' && node.parent.declaration === node; +// https://github.com/estree/estree/blob/master/stage3/class-features.md#propertydefinition +const isPropertyDefinition = node => node.type === 'PropertyDefinition' || + // Legacy node type + node.type === 'ClassProperty'; +const isMethodDefinition = node => node.type === 'MethodDefinition'; + function isStaticMember(node) { const { - type, private: isPrivate, static: isStatic, declare: isDeclare, @@ -37,7 +42,7 @@ function isStaticMember(node) { // Avoid matching unexpected node. For example: https://github.com/tc39/proposal-class-static-block /* istanbul ignore next */ - if (type !== 'ClassProperty' && type !== 'MethodDefinition') { + if (!isPropertyDefinition(node) && !isMethodDefinition(node)) { return false; } @@ -60,8 +65,6 @@ function isStaticMember(node) { } function * switchClassMemberToObjectProperty(node, sourceCode, fixer) { - const {type} = node; - const staticToken = sourceCode.getFirstToken(node); assertToken(staticToken, { expected: [ @@ -75,12 +78,12 @@ function * switchClassMemberToObjectProperty(node, sourceCode, fixer) { yield fixer.remove(staticToken); yield removeSpacesAfter(staticToken, sourceCode, fixer); - const maybeSemicolonToken = type === 'ClassProperty' ? + const maybeSemicolonToken = isPropertyDefinition(node) ? sourceCode.getLastToken(node) : sourceCode.getTokenAfter(node); const hasSemicolonToken = isSemicolonToken(maybeSemicolonToken); - if (type === 'ClassProperty') { + if (isPropertyDefinition(node)) { const {key, value} = node; if (value) { @@ -132,11 +135,11 @@ function switchClassToObject(node, sourceCode) { for (const node of body.body) { if ( - node.type === 'ClassProperty' && + isPropertyDefinition(node) && ( node.typeAnnotation || - // This is a stupid way to check if `value` of `ClassProperty` uses `this` - (node.value && sourceCode.getText(node).includes('this')) + // This is a stupid way to check if `value` of `PropertyDefinition` uses `this` + (node.value && sourceCode.getText(node.value).includes('this')) ) ) { return; diff --git a/rules/prefer-number-properties.js b/rules/prefer-number-properties.js index b2431d7625..cf6314f20b 100644 --- a/rules/prefer-number-properties.js +++ b/rules/prefer-number-properties.js @@ -33,16 +33,17 @@ const propertiesSelector = [ ':matches([name="NaN"],[name="Infinity"])', `:not(${ [ - 'MemberExpression[computed=false] > Identifier.property', - 'FunctionDeclaration > Identifier.id', - 'ClassDeclaration > Identifier.id', - 'ClassProperty[computed=false] > Identifier.key', - 'MethodDefinition[computed=false] > Identifier.key', - 'VariableDeclarator > Identifier.id', - 'Property[shorthand=false][computed=false] > Identifier.key', - 'TSDeclareFunction > Identifier.id', - 'TSEnumMember > Identifier.id', - 'TSPropertySignature > Identifier.key' + 'MemberExpression[computed=false] > .property', + 'FunctionDeclaration > .id', + 'ClassDeclaration > .id', + 'ClassProperty[computed=false] > .key', + 'PropertyDefinition[computed=false] > .key', + 'MethodDefinition[computed=false] > .key', + 'VariableDeclarator > .id', + 'Property[shorthand=false][computed=false] > .key', + 'TSDeclareFunction > .id', + 'TSEnumMember > .id', + 'TSPropertySignature > .key' ].join(', ') })` ].join(''); diff --git a/rules/prevent-abbreviations.js b/rules/prevent-abbreviations.js index 303e760e74..ff015af789 100644 --- a/rules/prevent-abbreviations.js +++ b/rules/prevent-abbreviations.js @@ -538,7 +538,7 @@ const shouldReportIdentifierAsProperty = identifier => { } if ( - identifier.parent.type === 'ClassProperty' && + (identifier.parent.type === 'ClassProperty' || identifier.parent.type === 'PropertyDefinition') && identifier.parent.key === identifier && !identifier.parent.computed ) { From 4f2330b3761ffdf94ff44ffdedb164f4fef08705 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Tue, 23 Feb 2021 10:55:49 +0800 Subject: [PATCH 2/4] Test `this` in `key` --- test/no-static-only-class.mjs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/no-static-only-class.mjs b/test/no-static-only-class.mjs index bddf6c3dcf..e654ef1400 100644 --- a/test/no-static-only-class.mjs +++ b/test/no-static-only-class.mjs @@ -175,6 +175,12 @@ test.typescript({ static b = this.a; } `), + // `this` in `key` should fixable + { + code: 'class A {static [this.a] = 1}', + output: 'const A = {[this.a] : 1,};', + errors: 1 + }, // This case should be fixable, but we simply check code of value includes `this` noFixingCase(outdent` class A { From ce1b1a0eb9fc2810724b71d2c4360f34aacee723 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Tue, 23 Feb 2021 11:35:32 +0800 Subject: [PATCH 3/4] Test coverage --- test/custom-error-definition.mjs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/custom-error-definition.mjs b/test/custom-error-definition.mjs index 591e15c5b6..933df282ae 100644 --- a/test/custom-error-definition.mjs +++ b/test/custom-error-definition.mjs @@ -504,6 +504,28 @@ runTest.babel({ } `, errors: [invalidNameError('ValidationError')] + }, + // `computed` + { + code: outdent` + const name = 'computed-name'; + class FooError extends Error { + [name] = 'FooError'; + constructor(message) { + super(message); + } + } + `, + output: outdent` + const name = 'computed-name'; + class FooError extends Error { + [name] = 'FooError'; + constructor(message) { + super(message); + } + } + `, + errors: [invalidNameError('FooError')] } ] }); From 2cebe3891a531d2e77f42fffec75a2d9711240f4 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 9 Apr 2021 17:45:29 +0800 Subject: [PATCH 4/4] Remove comment --- test/utils/test.mjs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/utils/test.mjs b/test/utils/test.mjs index 764674bb97..e86326a07b 100644 --- a/test/utils/test.mjs +++ b/test/utils/test.mjs @@ -47,8 +47,6 @@ class Tester { testerOptions.parserOptions.babelOptions.parserOpts = testerOptions.parserOptions.babelOptions.parserOpts || {}; let babelPlugins = testerOptions.parserOptions.babelOptions.parserOpts.plugins || []; babelPlugins = [ - // `estree` plugin must be the first plugin - // see https://github.com/babel/babel/pull/12891/files#diff-290770df6e6f19b4a301103852c99ec45f2c838d539160cbff7d85042f8dc080R40 ['estree', {classFeatures: true}], 'jsx', 'classProperties',