diff --git a/src/rules/no-this-assign-in-render.ts b/src/rules/no-this-assign-in-render.ts index 1491297..97b4e73 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 {getPropertyMap, isLitClass, PropertyMapEntry} from '../util'; //------------------------------------------------------------------------------ // Rule Definition @@ -31,6 +31,7 @@ const rule: Rule.RuleModule = { create(context): Rule.RuleListener { let inRender = false; let inComponent = false; + let propertyMap: ReadonlyMap | null = null; /** * Class entered @@ -43,6 +44,12 @@ const rule: Rule.RuleModule = { return; } + const props = getPropertyMap(node); + + if (props) { + propertyMap = props; + } + inComponent = true; } @@ -52,6 +59,7 @@ const rule: Rule.RuleModule = { * @return {void} */ function classExit(): void { + propertyMap = null; inComponent = false; } @@ -105,17 +113,31 @@ const rule: Rule.RuleModule = { * @return {void} */ function assignmentFound(node: Rule.Node): void { - if (!inRender || node.type !== 'MemberExpression') { + if (!inRender || + !propertyMap || + node.type !== 'MemberExpression') { return; } - const nonMember = walkMembers(node); - + const nonMember = walkMembers(node) as Rule.Node; if (nonMember.type === 'ThisExpression') { + const parent = nonMember.parent as ESTree.MemberExpression; + console.log(parent); + + let propertyName = ''; + if (parent.property.type === 'Identifier' && !parent.computed) { + propertyName = parent.property.name; + } else if (parent.property.type === 'Literal') { + propertyName = String(parent.property.value); + } + + if (propertyMap.has(propertyName) || + parent.property.type === 'Identifier' && parent.computed) { context.report({ node: node.parent, messageId: 'noThis' }); + } } } @@ -129,7 +151,6 @@ const rule: Rule.RuleModule = { MethodDefinition: (node: ESTree.Node): void => methodEnter(node as ESTree.MethodDefinition), 'MethodDefinition:exit': methodExit, - // eslint-disable-next-line max-len 'AssignmentExpression > .left:has(ThisExpression)': ( node: Rule.Node ): void => assignmentFound(node) 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 01cecf1..26a78cf 100644 --- a/src/test/rules/no-this-assign-in-render_test.ts +++ b/src/test/rules/no-this-assign-in-render_test.ts @@ -43,7 +43,18 @@ ruleTester.run('no-this-assign-in-render', rule, { this.deep.prop = 5; } }`, + `class Foo { + static get properties() { + return { prop: { type: Number } }; + } + render() { + this.deep.prop = 5; + } + }`, `class Foo extends LitElement { + static get properties() { + return { x: { type: Number } }; + } render() { const x = this.prop; } @@ -54,45 +65,82 @@ ruleTester.run('no-this-assign-in-render', rule, { } }`, `class Foo extends LitElement { + static get properties() { + return { foo: { type: Number } }; + } static render() { this.foo = 5; } }`, `class Foo extends LitElement { + static get properties() { + return { prop: { type: Number } }; + } render() { let x; x = this.prop; } }`, `class Foo extends LitElement { + static get properties() { + return { x: { type: Number } }; + } render() { let x; x = 5; } }`, `class Foo extends LitElement { + static get properties() { + return { foo: { type: Number } }; + } render() { let x; x = this.foo || 123; } }`, `class Foo extends LitElement { + static get properties() { + return { prop: { type: Number } }; + } render() { const x = {}; x[this.prop] = 123; } }`, `class Foo extends LitElement { + static get properties() { + return { prop: { type: Number } }; + } render() { const x = () => ({}); x(this.prop).y = 123; } + }`, + `class Foo extends LitElement { + static get properties() { + return { prop: { type: Number } }; + } + render() { + this.unreactive = 123; + } + }`, + `class Foo extends LitElement { + static get properties() { + return { prop: { type: Number } }; + } + render() { + this.unreactive.prop = 123; + } }` ], invalid: [ { code: `class Foo extends LitElement { + static get properties() { + return { prop: { type: String } }; + } render() { this.prop = 'foo'; } @@ -100,13 +148,16 @@ ruleTester.run('no-this-assign-in-render', rule, { errors: [ { messageId: 'noThis', - line: 3, + line: 6, column: 11 } ] }, { code: `class Foo extends LitElement { + static get properties() { + return { deep: { type: Object } }; + } render() { this.deep.prop = 'foo'; } @@ -114,13 +165,16 @@ ruleTester.run('no-this-assign-in-render', rule, { errors: [ { messageId: 'noThis', - line: 3, + line: 6, column: 11 } ] }, { code: `const foo = class extends LitElement { + static get properties() { + return { prop: { type: String } }; + } render() { this.prop = 'foo'; } @@ -128,13 +182,16 @@ ruleTester.run('no-this-assign-in-render', rule, { errors: [ { messageId: 'noThis', - line: 3, + line: 6, column: 11 } ] }, { code: `class Foo extends LitElement { + static get properties() { + return { prop: { type: String } }; + } render() { this['prop'] = 'foo'; } @@ -142,7 +199,7 @@ ruleTester.run('no-this-assign-in-render', rule, { errors: [ { messageId: 'noThis', - line: 3, + line: 6, column: 11 } ] @@ -150,12 +207,29 @@ ruleTester.run('no-this-assign-in-render', rule, { { code: `@customElement('foo') class Foo extends FooElement { + @property({ type: String }) + prop = ''; render() { this['prop'] = 'foo'; } }`, parser, parserOptions, + errors: [ + { + messageId: 'noThis', + line: 6, + column: 11 + } + ] + }, + { + code: `class Foo extends LitElement { + render() { + const x = 'prop'; + this[x] = 'foo'; + } + }`, errors: [ { messageId: 'noThis',