From 6b11f144ed36d60ef8d0d57d85a4e709acd6ef8e Mon Sep 17 00:00:00 2001 From: jpradelle Date: Mon, 22 Jul 2024 17:00:20 +0200 Subject: [PATCH 1/2] feat: improve LitElement identification --- src/rules/lifecycle-super.ts | 7 ++-- src/rules/no-property-change-update.ts | 8 ++--- src/rules/no-this-assign-in-render.ts | 7 ++-- src/test/rules/attribute-names_test.ts | 34 +++++++++++++++++++ src/test/rules/lifecycle-super_test.ts | 26 ++++++++++++++ .../rules/no-property-change-update_test.ts | 21 ++++++++++++ .../rules/no-this-assign-in-render_test.ts | 25 ++++++++++++++ src/util.ts | 32 +++++++++++++++-- 8 files changed, 141 insertions(+), 19 deletions(-) diff --git a/src/rules/lifecycle-super.ts b/src/rules/lifecycle-super.ts index b90e030..f8451a6 100644 --- a/src/rules/lifecycle-super.ts +++ b/src/rules/lifecycle-super.ts @@ -5,6 +5,7 @@ import {Rule} from 'eslint'; import * as ESTree from 'estree'; +import {isLitClass} from "../util"; const methodNames = ['connectedCallback', 'disconnectedCallback', 'update']; @@ -44,11 +45,7 @@ const rule: Rule.RuleModule = { * @return {void} */ function classEnter(node: ESTree.Class): void { - if ( - !node.superClass || - node.superClass.type !== 'Identifier' || - node.superClass.name !== 'LitElement' - ) { + if (!isLitClass(node)) { return; } diff --git a/src/rules/no-property-change-update.ts b/src/rules/no-property-change-update.ts index 2062724..b55bd22 100644 --- a/src/rules/no-property-change-update.ts +++ b/src/rules/no-property-change-update.ts @@ -5,7 +5,7 @@ import {Rule} from 'eslint'; import * as ESTree from 'estree'; -import {getPropertyMap, PropertyMapEntry} from '../util'; +import {getPropertyMap, isLitClass, PropertyMapEntry} from '../util'; const superUpdateQuery = 'CallExpression' + @@ -49,11 +49,7 @@ const rule: Rule.RuleModule = { * @return {void} */ function classEnter(node: ESTree.Class): void { - if ( - !node.superClass || - node.superClass.type !== 'Identifier' || - node.superClass.name !== 'LitElement' - ) { + if (!isLitClass(node)) { return; } diff --git a/src/rules/no-this-assign-in-render.ts b/src/rules/no-this-assign-in-render.ts index 401a22f..70c6b73 100644 --- a/src/rules/no-this-assign-in-render.ts +++ b/src/rules/no-this-assign-in-render.ts @@ -5,6 +5,7 @@ import {Rule} from 'eslint'; import * as ESTree from 'estree'; +import {isLitClass} from "../util"; //------------------------------------------------------------------------------ // Rule Definition @@ -38,11 +39,7 @@ const rule: Rule.RuleModule = { * @return {void} */ function classEnter(node: ESTree.Class): void { - if ( - !node.superClass || - node.superClass.type !== 'Identifier' || - node.superClass.name !== 'LitElement' - ) { + if (!isLitClass(node)) { return; } diff --git a/src/test/rules/attribute-names_test.ts b/src/test/rules/attribute-names_test.ts index 80ee3f3..8869a21 100644 --- a/src/test/rules/attribute-names_test.ts +++ b/src/test/rules/attribute-names_test.ts @@ -125,6 +125,40 @@ ruleTester.run('attribute-names', rule, { messageId: 'casedPropertyWithoutAttribute' } ] + }, + { + code: `@customElement('foo-bar') + class FooBar extends FooElement { + @property({ type: String }) + camelCase = 'foo'; + }`, + parser, + parserOptions, + errors: [ + { + line: 4, + column: 9, + messageId: 'casedPropertyWithoutAttribute' + } + ] + }, + { + code: `@foo + @customElement('foo-bar') + @bar + class FooBar extends FooElement { + @property({ type: String }) + camelCase = 'foo'; + }`, + parser, + parserOptions, + errors: [ + { + line: 6, + column: 9, + messageId: 'casedPropertyWithoutAttribute' + } + ] } ] }); diff --git a/src/test/rules/lifecycle-super_test.ts b/src/test/rules/lifecycle-super_test.ts index 2ab64ef..deb4b48 100644 --- a/src/test/rules/lifecycle-super_test.ts +++ b/src/test/rules/lifecycle-super_test.ts @@ -21,6 +21,14 @@ const ruleTester = new RuleTester({ } }); +const parser = require.resolve('@babel/eslint-parser'); +const parserOptions = { + requireConfigFile: false, + babelOptions: { + plugins: [['@babel/plugin-proposal-decorators', {version: '2023-11'}]] + } +}; + ruleTester.run('lifecycle-super', rule, { valid: [ 'class Foo { }', @@ -184,6 +192,24 @@ ruleTester.run('lifecycle-super', rule, { column: 9 } ] + }, + { + code: `@customElement('foo') + class Foo extends FooElement { + connectedCallback() { + super.foo.connectedCallback(); + } + }`, + parser, + parserOptions, + errors: [ + { + messageId: 'callSuper', + data: {method: 'connectedCallback'}, + line: 3, + column: 9 + } + ] } ] }); diff --git a/src/test/rules/no-property-change-update_test.ts b/src/test/rules/no-property-change-update_test.ts index 9c30026..5a9c605 100644 --- a/src/test/rules/no-property-change-update_test.ts +++ b/src/test/rules/no-property-change-update_test.ts @@ -228,6 +228,27 @@ ruleTester.run('no-property-change-update', rule, { column: 11 } ] + }, + { + code: `@customElement('foo') + class Foo extends FooElement { + static get properties() { + return { prop: { type: String } }; + } + update(change) { + super.update(); + this.prop = 'foo'; + } + }`, + parser, + parserOptions, + errors: [ + { + messageId: 'propertyChange', + line: 8, + column: 11 + } + ] } ] }); diff --git a/src/test/rules/no-this-assign-in-render_test.ts b/src/test/rules/no-this-assign-in-render_test.ts index 40f0372..21f3b33 100644 --- a/src/test/rules/no-this-assign-in-render_test.ts +++ b/src/test/rules/no-this-assign-in-render_test.ts @@ -21,6 +21,14 @@ const ruleTester = new RuleTester({ } }); +const parser = require.resolve('@babel/eslint-parser'); +const parserOptions = { + requireConfigFile: false, + babelOptions: { + plugins: [['@babel/plugin-proposal-decorators', {version: '2023-11'}]] + } +}; + ruleTester.run('no-this-assign-in-render', rule, { valid: [ 'const x = 808;', @@ -126,6 +134,23 @@ ruleTester.run('no-this-assign-in-render', rule, { column: 11 } ] + }, + { + code: `@customElement('foo') + class Foo extends FooElement { + render() { + this['prop'] = 'foo'; + } + }`, + parser, + parserOptions, + errors: [ + { + messageId: 'noThis', + line: 4, + column: 11 + } + ] } ] }); diff --git a/src/util.ts b/src/util.ts index 88e81c8..9516de7 100644 --- a/src/util.ts +++ b/src/util.ts @@ -13,13 +13,39 @@ export type DecoratedNode = ESTree.Node & { decorators?: BabelDecorator[]; }; +export type NodeWithParent = ESTree.Node & { + parent?: ESTree.Node; +} + /** * Returns if given node has a lit identifier - * @param {ESTree.Node} node + * @param {NodeWithParent} node * @return {boolean} */ -function hasLitIdentifier(node: ESTree.Node): boolean { - return node.type === 'Identifier' && node.name === 'LitElement'; +function hasLitIdentifier(node: NodeWithParent): boolean { + if (node.type === 'Identifier' && node.name === 'LitElement') { + return true; + } + + if (node.parent && node.parent.type === 'ClassDeclaration') { + const decoratedNode = node.parent as DecoratedNode; + + if (!decoratedNode.decorators || !Array.isArray(decoratedNode.decorators)) { + return false; + } + + for (const decorator of decoratedNode.decorators) { + if ( + decorator.expression.type === 'CallExpression' && + decorator.expression.callee.type === 'Identifier' && + decorator.expression.callee.name === 'customElement' + ) { + return true; + } + } + } + + return false; } /** From c5c9305ac173015dafd3521faa309feaf1ada4d2 Mon Sep 17 00:00:00 2001 From: jpradelle Date: Tue, 27 Aug 2024 15:18:11 +0200 Subject: [PATCH 2/2] Fix LitElement identification PR #206 --- src/rules/lifecycle-super.ts | 2 +- src/rules/no-this-assign-in-render.ts | 2 +- src/util.ts | 50 ++++++++++++++------------- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/rules/lifecycle-super.ts b/src/rules/lifecycle-super.ts index f8451a6..80de48b 100644 --- a/src/rules/lifecycle-super.ts +++ b/src/rules/lifecycle-super.ts @@ -5,7 +5,7 @@ import {Rule} from 'eslint'; import * as ESTree from 'estree'; -import {isLitClass} from "../util"; +import {isLitClass} from '../util'; const methodNames = ['connectedCallback', 'disconnectedCallback', 'update']; diff --git a/src/rules/no-this-assign-in-render.ts b/src/rules/no-this-assign-in-render.ts index 70c6b73..fbc69fa 100644 --- a/src/rules/no-this-assign-in-render.ts +++ b/src/rules/no-this-assign-in-render.ts @@ -5,7 +5,7 @@ import {Rule} from 'eslint'; import * as ESTree from 'estree'; -import {isLitClass} from "../util"; +import {isLitClass} from '../util'; //------------------------------------------------------------------------------ // Rule Definition diff --git a/src/util.ts b/src/util.ts index 9516de7..124ece5 100644 --- a/src/util.ts +++ b/src/util.ts @@ -13,41 +13,40 @@ export type DecoratedNode = ESTree.Node & { decorators?: BabelDecorator[]; }; -export type NodeWithParent = ESTree.Node & { - parent?: ESTree.Node; -} - /** - * Returns if given node has a lit identifier - * @param {NodeWithParent} node + * Returns if given node has a customElement decorator + * @param {ESTree.Class} node * @return {boolean} */ -function hasLitIdentifier(node: NodeWithParent): boolean { - if (node.type === 'Identifier' && node.name === 'LitElement') { - return true; - } - - if (node.parent && node.parent.type === 'ClassDeclaration') { - const decoratedNode = node.parent as DecoratedNode; +function hasCustomElementDecorator(node: ESTree.Class): boolean { + const decoratedNode = node as DecoratedNode; - if (!decoratedNode.decorators || !Array.isArray(decoratedNode.decorators)) { - return false; - } + if (!decoratedNode.decorators || !Array.isArray(decoratedNode.decorators)) { + return false; + } - for (const decorator of decoratedNode.decorators) { - if ( - decorator.expression.type === 'CallExpression' && - decorator.expression.callee.type === 'Identifier' && - decorator.expression.callee.name === 'customElement' - ) { - return true; - } + for (const decorator of decoratedNode.decorators) { + if ( + decorator.expression.type === 'CallExpression' && + decorator.expression.callee.type === 'Identifier' && + decorator.expression.callee.name === 'customElement' + ) { + return true; } } return false; } +/** + * Returns if given node has a lit identifier + * @param {ESTree.Node} node + * @return {boolean} + */ +function hasLitIdentifier(node: ESTree.Node): boolean { + return node.type === 'Identifier' && node.name === 'LitElement'; +} + /** * Returns if the given node is a lit element by expression * @param {ESTree.Node} node @@ -71,6 +70,9 @@ function isLitByExpression(node: ESTree.Node): boolean { * @return { boolean } */ export function isLitClass(clazz: ESTree.Class): boolean { + if (hasCustomElementDecorator(clazz)) { + return true; + } if (clazz.superClass) { return ( hasLitIdentifier(clazz.superClass) || isLitByExpression(clazz.superClass)