diff --git a/docs/rules/no-this-assign-in-render.md b/docs/rules/no-this-assign-in-render.md index 6be569f..a2de5d7 100644 --- a/docs/rules/no-this-assign-in-render.md +++ b/docs/rules/no-this-assign-in-render.md @@ -32,5 +32,5 @@ updated() { ## When Not To Use It -If you have non-observed properties you wish to update per render, you may +If you have non-observed properties you wish to update per render via computed property accessors, you may want to disable this rule. diff --git a/src/rules/no-this-assign-in-render.ts b/src/rules/no-this-assign-in-render.ts index fbc69fa..e0810e9 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; } @@ -84,6 +92,20 @@ const rule: Rule.RuleModule = { inRender = false; } + /** + * Walk left side assignment members and return first non-member + * + * @param {ESTree.MemberExpression} member Member entered + * @return {ESTree.Node} + */ + function walkMembers(member: ESTree.MemberExpression): ESTree.Node { + if (member.object.type === 'MemberExpression') { + return walkMembers(member.object); + } else { + return member.object; + } + } + /** * Left side of an assignment expr found * @@ -91,14 +113,30 @@ const rule: Rule.RuleModule = { * @return {void} */ function assignmentFound(node: Rule.Node): void { - if (!inRender) { + if (!inRender || + !propertyMap || + node.type !== 'MemberExpression') { return; } - context.report({ - node: node.parent, - messageId: 'noThis' - }); + const nonMember = walkMembers(node) as Rule.Node; + if (nonMember.type === 'ThisExpression') { + const parent = nonMember.parent as ESTree.MemberExpression; + + 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.computed) { + context.report({ + node: node.parent, + messageId: 'noThis' + }); + } + } } return { 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 21f3b33..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,33 +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'; } @@ -88,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'; } @@ -102,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'; } @@ -116,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'; } @@ -130,7 +199,7 @@ ruleTester.run('no-this-assign-in-render', rule, { errors: [ { messageId: 'noThis', - line: 3, + line: 6, column: 11 } ] @@ -138,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',