feat: no classfield shadowing (#180)

introduces a new rule: no-classifield-shadowing

.gitignore

@@ -1,5 +1,6 @@
+.idea
 node_modules/
 lib/
 *.swp
 .nyc_output/
-coverage/
+coverage/

docs/rules/no-classfield-shadowing.md

@@ -0,0 +1,34 @@
+# Disallows class fields with same name as static properties
+
+Class fields set with same names as static properties will not trigger updates as expected. They will overwrite
+accessors used for detecting changes. See https://lit.dev/msg/class-field-shadowing for more information.
+
+## Rule Details
+
+This rule disallows class fields with same name as static properties.
+
+The following patterns are considered warnings:
+
+```ts
+class MyEl extends LitElement {
+  foo;
+
+  static properties = {
+    foo: {}
+  }
+}
+```
+
+The following patterns are not warnings:
+
+```ts
+class MyEl extends LitElement {
+  static properties = {
+    foo: {}
+  }
+}
+```
+
+## When Not To Use It
+
+If you don't care about class fields with same name as static properties.

src/rules/no-classfield-shadowing.ts

@@ -0,0 +1,51 @@
+/**
+ * @fileoverview Disallows properties shadowed as class fields
+ * @author Michel Langeveld <https://github.com/michellangeveld>
+ */
+
+import {Rule} from 'eslint';
+import * as ESTree from 'estree';
+import {getClassFields, getPropertyMap, isLitClass} from '../util';
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+const rule: Rule.RuleModule = {
+  meta: {
+    docs: {
+      description: 'Disallows properties shadowed as class fields',
+      recommended: true,
+      url: 'https://github.com/43081j/eslint-plugin-lit/blob/master/docs/rules/no-classfield-shadowing.md'
+    },
+    schema: [],
+    messages: {
+      noClassfieldShadowing:
+        'The {{ prop }} property is a class field which has the same name as ' +
+        'static property which could have unintended side-effects.'
+    }
+  },
+
+  create(context): Rule.RuleListener {
+    return {
+      ClassDeclaration: (node: ESTree.Class): void => {
+        if (isLitClass(node)) {
+          const propertyMap = getPropertyMap(node);
+          const classMembers = getClassFields(node);
+
+          for (const [prop, {key}] of propertyMap.entries()) {
+            if (classMembers.has(prop)) {
+              context.report({
+                node: key,
+                messageId: 'noClassfieldShadowing',
+                data: {prop}
+              });
+            }
+          }
+        }
+      }
+    };
+  }
+};
+
+export = rule;

src/rules/prefer-static-styles.ts

@@ -6,6 +6,7 @@
 import {Rule} from 'eslint';
 import * as ESTree from 'estree';
 import {TemplateAnalyzer} from '../template-analyzer';
+import {isLitClass} from '../util';
 
 //------------------------------------------------------------------------------
 // Rule Definition
@@ -35,12 +36,7 @@ const rule: Rule.RuleModule = {
 
     return {
       'ClassExpression,ClassDeclaration': (node: ESTree.Class): void => {
-        if (
-          !prefer &&
-          node.superClass &&
-          node.superClass.type === 'Identifier' &&
-          node.superClass.name === 'LitElement'
-        ) {
+        if (!prefer && isLitClass(node)) {
           for (const member of node.body.body) {
             if (
               member.type === 'MethodDefinition' &&

src/test/rules/no-classfield-shadowing_test.ts

@@ -0,0 +1,145 @@
+/**
+ * @fileoverview Disallows properties shadowed as class field
+ * @author Michel Langeveld <https://github.com/michellangeveld>
+ */
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+import rule = require('../../rules/no-classfield-shadowing');
+import {RuleTester} from 'eslint';
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+  parserOptions: {
+    sourceType: 'module',
+    ecmaVersion: 'latest'
+  }
+});
+
+ruleTester.run('no-classfield-shadowing', rule, {
+  valid: [
+    `class MyElement extends LitElement {
+      static properties = {
+        foo: { type: String }
+      }
+    }`,
+    `class MyElement extends LitElement {
+      foo;
+      properties = {
+        foo: { type: String }
+      }
+    }`,
+    `class MyElement {
+      foo;
+      properties = {
+        foo: { type: String }
+      }
+    }`
+  ],
+
+  invalid: [
+    {
+      code: `class MyElement extends LitElement {
+        foo;
+        static properties = {foo: {}}
+      }`,
+      errors: [
+        {
+          messageId: 'noClassfieldShadowing',
+          data: {prop: 'foo'},
+          line: 3,
+          column: 30
+        }
+      ]
+    },
+    {
+      code: `class MyElement extends LitElement {
+        static properties = {foo: {}}
+        foo;
+      }`,
+      errors: [
+        {
+          messageId: 'noClassfieldShadowing',
+          data: {prop: 'foo'},
+          line: 2,
+          column: 30
+        }
+      ]
+    },
+    {
+      code: `class MyElement extends LitElement {
+        foo;
+        static get properties() { return { foo: {}}};
+      }`,
+      errors: [
+        {
+          messageId: 'noClassfieldShadowing',
+          data: {prop: 'foo'},
+          line: 3,
+          column: 44
+        }
+      ]
+    },
+    {
+      code: `class MyElement extends LitElement {
+        static get properties() { return { foo: {}}};
+        foo;
+      }`,
+      errors: [
+        {
+          messageId: 'noClassfieldShadowing',
+          data: {prop: 'foo'},
+          line: 2,
+          column: 44
+        }
+      ]
+    },
+    {
+      code: `class Foo extends A(LitElement) {
+        foo;
+        static properties = { foo: {} };
+      }`,
+      errors: [
+        {
+          messageId: 'noClassfieldShadowing',
+          data: {prop: 'foo'},
+          line: 3,
+          column: 31
+        }
+      ]
+    },
+    {
+      code: `class Foo extends A(B(LitElement)) {
+        foo;
+        static properties = { foo: {} };
+      }`,
+      errors: [
+        {
+          messageId: 'noClassfieldShadowing',
+          data: {prop: 'foo'},
+          line: 3,
+          column: 31
+        }
+      ]
+    },
+    {
+      code: `class Foo extends A(B(C(LitElement))) {
+        foo;
+        static properties = { foo: {} };
+      }`,
+      errors: [
+        {
+          messageId: 'noClassfieldShadowing',
+          data: {prop: 'foo'},
+          line: 3,
+          column: 31
+        }
+      ]
+    }
+  ]
+});

src/util.ts

@@ -9,6 +9,46 @@ export interface BabelProperty extends ESTree.MethodDefinition {
   decorators?: BabelDecorator[];
 }
 
+/**
+ * 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
+ * @return {boolean}
+ */
+function isLitByExpression(node: ESTree.Node): boolean {
+  if (node) {
+    if (hasLitIdentifier(node)) {
+      return true;
+    }
+    if (node.type === 'CallExpression') {
+      return node.arguments.some(isLitByExpression);
+    }
+  }
+  return false;
+}
+
+/**
+ * Returns if the given node is a lit class
+ * @param {ESTree.Class} clazz
+ * @return { boolean }
+ */
+export function isLitClass(clazz: ESTree.Class): boolean {
+  if (clazz.superClass) {
+    return (
+      hasLitIdentifier(clazz.superClass) || isLitByExpression(clazz.superClass)
+    );
+  }
+  return hasLitIdentifier(clazz);
+}
+
 /**
  * Get the name of a node
  *
@@ -67,6 +107,29 @@ export function extractPropertyEntry(
   };
 }
 
+/**
+ * Returns the class fields of a class
+ * @param {ESTree.Class} node Class to retrieve class fields for
+ * @return {ReadonlyMap<string, ESTreeObjectExpression>}
+ */
+export function getClassFields(
+  node: ESTree.Class
+): ReadonlyMap<string, ESTree.PropertyDefinition> {
+  const result = new Map<string, ESTree.PropertyDefinition>();
+
+  for (const member of node.body.body) {
+    if (
+      member.type === 'PropertyDefinition' &&
+      member.key.type === 'Identifier' &&
+      // TODO: we should cast as the equivalent tsestree PropertyDefinition
+      !(member as ESTree.PropertyDefinition & {declare?: boolean}).declare
+    ) {
+      result.set(member.key.name, member);
+    }
+  }
+  return result;
+}
+
 /**
  * Get the properties object of an element class
  *