Only fail if the assignment is to a reactive property

43081j · Sep 11, 2024 · a604e2d · a604e2d
1 parent 7d80d74
commit a604e2d
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 9 deletions.
diff --git 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<string, PropertyMapEntry> | 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
@@ -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,108 +65,171 @@ 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';
         }
       }`,
       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';
         }
       }`,
       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';
         }
       }`,
       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';
         }
       }`,
       errors: [
         {
           messageId: 'noThis',
-          line: 3,
+          line: 6,
           column: 11
         }
       ]
     },
     {
       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',