Included check for react components only

rome · Jun 7, 2020 · 3c8d797 · 3c8d797
1 parent c17bc7e
commit 3c8d797
Show file tree

Hide file tree

Showing 3 changed files with 233 additions and 51 deletions.
diff --git a/packages/@romejs/compiler/lint/rules/react/noDirectMutationState.test.md b/packages/@romejs/compiler/lint/rules/react/noDirectMutationState.test.md
@@ -753,35 +753,20 @@ class Hello extends React.Component {
 ### `21`
 
 ```
-
- unknown:4:16 lint/react/noDirectMutationState ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
-
-  ✖ Avoid mutating this.state directly.
-
-    2 │             class Hello {
-    3 │               getFoo() {
-  > 4 │                 this.state.foo = 'bar';
-      │                 ^^^^^^^^^^^^^^^^^^^^^^
-    5 │                 return this.state.foo;
-    6 │               }
-
-  ℹ Calling setState() after mutating this.state directly may replace the mutation you made.
-
-━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
-
-✖ Found 1 problem
+✔ No known problems!
 
 ```
 
 ### `21: formatted`
 
 ```
-class Hello {
-	getFoo() {
-		this.state.foo = "bar";
-		return this.state.foo;
-	}
-}
+var Hello = createReactClass({
+	render: function() {
+		var obj = {state: {}};
+		obj.state.name = "foo";
+		return <div>Hello {obj.state.name}</div>;
+	},
+});
 
 ```
 
@@ -815,13 +800,8 @@ var Hello = createReactClass({
 ### `23: formatted`
 
 ```
-var Hello = createReactClass({
-	render: function() {
-		var obj = {state: {}};
-		obj.state.name = "foo";
-		return <div>Hello {obj.state.name}</div>;
-	},
-});
+var Hello = "foo";
+module.exports = {};
 
 ```
 
@@ -835,8 +815,12 @@ var Hello = createReactClass({
 ### `24: formatted`
 
 ```
-var Hello = "foo";
-module.exports = {};
+class Hello {
+	getFoo() {
+		this.state.foo = "bar";
+		return this.state.foo;
+	}
+}
 
 ```
 
@@ -918,3 +902,129 @@ var Hello = createReactClass({
 });
 
 ```
+
+### `29`
+
+```
+✔ No known problems!
+
+```
+
+### `29: formatted`
+
+```
+class Example extends React.Component {
+	constructor(props) {
+		super(props);
+		this.state = {
+			count: 0,
+		};
+	}
+}
+
+```
+
+### `30`
+
+```
+✔ No known problems!
+
+```
+
+### `30: formatted`
+
+```
+class Example {
+	constructor(props) {
+		super(props);
+		this.state = {
+			count: 0,
+		};
+	}
+}
+
+```
+
+### `31`
+
+```
+✔ No known problems!
+
+```
+
+### `31: formatted`
+
+```
+class Example extends SuperExample {
+	constructor(props) {
+		super(props);
+		this.state = {
+			count: 0,
+		};
+	}
+}
+
+```
+
+### `32`
+
+```
+✔ No known problems!
+
+```
+
+### `32: formatted`
+
+```
+class Example extends React.SuperExample {
+	constructor(props) {
+		super(props);
+		this.state = {
+			count: 0,
+		};
+	}
+}
+
+```
+
+### `33`
+
+```
+✔ No known problems!
+
+```
+
+### `33: formatted`
+
+```
+class Example extends SuperExample.Component {
+	constructor(props) {
+		super(props);
+		this.state = {
+			count: 0,
+		};
+	}
+}
+
+```
+
+### `34`
+
+```
+✔ No known problems!
+
+```
+
+### `34: formatted`
+
+```
+class Example extends SuperExample.Component {
+	someOtherFunction(props) {
+		super(props);
+		this.state = {
+			count: 0,
+		};
+	}
+}
+
+```
diff --git a/packages/@romejs/compiler/lint/rules/react/noDirectMutationState.test.ts b/packages/@romejs/compiler/lint/rules/react/noDirectMutationState.test.ts
@@ -173,14 +173,6 @@ test(
                 this.state.foo = "bar";
               }
             }
-          `,
-					`
-            class Hello {
-              getFoo() {
-                this.state.foo = \'bar\';
-                return this.state.foo;
-              }
-            }
           `,
 				],
 				valid: [
@@ -206,14 +198,14 @@ test(
             var Hello = "foo";
             module.exports = {};
           `,
-					// `
-					//   class Hello {
-					//     getFoo() {
-					//       this.state.foo = \'bar\';
-					//       return this.state.foo;
-					//     }
-					//   }
-					// `,
+					`
+            class Hello {
+              getFoo() {
+                this.state.foo = \'bar\';
+                return this.state.foo;
+              }
+            }
+					`,
 					`
             class Hello extends React.Component {
               constructor() {
@@ -248,6 +240,66 @@ test(
                 return <div>Hello {this.props.name}</div>;
               }
             });
+          `,
+					`
+            class Example extends React.Component {
+              constructor(props) {
+                super(props);
+                this.state = {
+                  count: 0
+                };
+              }
+            }
+          `,
+					`
+            class Example {
+              constructor(props) {
+                super(props);
+                this.state = {
+                  count: 0
+                };
+              }
+            }
+          `,
+					`
+            class Example extends SuperExample {
+              constructor(props) {
+                super(props);
+                this.state = {
+                  count: 0
+                };
+              }
+            }
+          `,
+					`
+            class Example extends React.SuperExample {
+              constructor(props) {
+                super(props);
+                this.state = {
+                  count: 0
+                };
+              }
+            }
+          `,
+					`
+            class Example extends SuperExample.Component {
+              constructor(props) {
+                super(props);
+                this.state = {
+                  count: 0
+                };
+              }
+            }
+          `,
+					`
+            class Example extends SuperExample.Component {
+              someOtherFunction(props) {
+                super(props);
+                this.state = {
+                  count: 0
+                };
+              }
+            }
           `,
 				],
 			},

diff --git a/packages/@romejs/compiler/lint/rules/react/noDirectMutationState.ts b/packages/@romejs/compiler/lint/rules/react/noDirectMutationState.ts
@@ -3,6 +3,22 @@ import {AnyNode} from "@romejs/ast";
 import {descriptions} from "@romejs/diagnostics";
 import {doesNodeMatchPattern} from "@romejs/js-ast-utils";
 
+// Checks if the current class extends React.Component
+function isReactComponent(path: Path): boolean {
+	const ancestor = path.findAncestry(({node}) =>
+	// Check if it is in createReactClass
+		(node.type === "JSCallExpression" &&
+		doesNodeMatchPattern(node.callee, "createReactClass")) ||
+		// Check if it extends React.Component or Component
+		(node.type === "JSClassHead" &&
+		node.superClass !== undefined &&
+		(doesNodeMatchPattern(node.superClass, "React.Component") ||
+		doesNodeMatchPattern(node.superClass, "Component")))
+	);
+
+	return ancestor !== undefined;
+}
+
 // Check if this.state mutation was in the constructor
 function isComponentInConstructor(path: Path): boolean {
 	// Find the first instance of a constructor or a call method
@@ -22,8 +38,8 @@ function isComponentInConstructor(path: Path): boolean {
 function isStateMutated(node: AnyNode): boolean {
 	// Check if node is a binary expression where this.state is the left side
 	if (
-    node.type === "JSAssignmentExpression" &&
-    (doesNodeMatchPattern(node.left, "this.state") || 
+		node.type === "JSAssignmentExpression" &&
+		(doesNodeMatchPattern(node.left, "this.state") ||
 		doesNodeMatchPattern(node.left, "this.state.**"))
 	) {
 		return true;
@@ -57,7 +73,11 @@ export default {
 		const {node} = path;
 
 		// If the state is mutated anywhere except in a constructor, show message
-		if (isStateMutated(node) && !isComponentInConstructor(path)) {
+		if (
+			isStateMutated(node) &&
+			isReactComponent(path) &&
+			!isComponentInConstructor(path)
+		) {
 			path.context.addNodeDiagnostic(
 				node,
 				descriptions.LINT.REACT_NO_DIRECT_MUTATION_STATE,