feat(eslint-plugin-mobx): configurable default autofix annotation for…

… makeObservable (#3881)
mobxjs · May 29, 2024 · 44a5fe0 · 44a5fe0
1 parent 4666972
commit 44a5fe0
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 82 deletions.
diff --git a/.changeset/lovely-lemons-joke.md b/.changeset/lovely-lemons-joke.md
@@ -0,0 +1,7 @@
+---
+"eslint-plugin-mobx": patch
+---
+
+Adds an option for the `mobx/exhaustive-make-observable` eslint rule to configure whether fields are annotated with `true` or `false` with the autofixer.
+
+This option defaults to `true` if not present or an invalid value is received to maintain existing behavior.
diff --git a/packages/eslint-plugin-mobx/README.md b/packages/eslint-plugin-mobx/README.md
@@ -35,10 +35,22 @@ module.exports = {
 ### mobx/exhaustive-make-observable
 
 Makes sure that `makeObservable` annotates all fields defined on class or object literal.<br>
-**Autofix** adds `field: true` for each missing field.<br>
 To exclude a field, annotate it using `field: false`.<br>
 Does not support fields introduced by constructor (`this.foo = 5`).<br>
-Does not warn about annotated non-existing fields (there is a runtime check, but the autofix removing the field could be handy...).
+Does not warn about annotated non-existing fields (there is a runtime check, but the autofix removing the field could be handy...).<br>
+**Autofix** adds `field: true` for each missing field by default. You can change this behaviour by specifying options in your eslint config:
+
+```json
+{
+    "rules": {
+        "mobx/exhaustive-make-observable": ["error", { "autofixAnnotation": false }]
+    }
+}
+```
+
+This is a boolean value that controls if the field is annotated with `true` or `false`.
+If you are migrating an existing project using `makeObservable` and do not want this rule to override
+your current usage (even if it may be wrong), you should run the autofix with the annotation set to `false` to maintain existing behaviour: `eslint --no-eslintrc --fix --rule='mobx/exhaustive-make-observable: [2, { "autofixAnnotation": false }]' .`
 
 ### mobx/missing-make-observable
 

diff --git a/packages/eslint-plugin-mobx/src/exhaustive-make-observable.js b/packages/eslint-plugin-mobx/src/exhaustive-make-observable.js
@@ -1,55 +1,62 @@
-'use strict';
+"use strict"
 
-const { findAncestor, isMobxDecorator } = require('./utils.js');
+const { findAncestor, isMobxDecorator } = require("./utils.js")
 
 // TODO support this.foo = 5; in constructor
 // TODO? report on field as well
 function create(context) {
-  const sourceCode = context.getSourceCode();
+    const sourceCode = context.getSourceCode()
+    const autofixAnnotation = context.options[0]?.autofixAnnotation ?? true
 
-  function fieldToKey(field) {
-    // TODO cache on field?
-    const key = sourceCode.getText(field.key);
-    return field.computed ? `[${key}]` : key;
-  }
+    function fieldToKey(field) {
+        // TODO cache on field?
+        const key = sourceCode.getText(field.key)
+        return field.computed ? `[${key}]` : key
+    }
 
-  return {
-    'CallExpression[callee.name="makeObservable"]': makeObservable => {
-      // Only interested about makeObservable(this, ...) in constructor or makeObservable({}, ...)
-      // ClassDeclaration 
-      //   ClassBody
-      //     MethodDefinition[kind="constructor"]
-      //       FunctionExpression
-      //         BlockStatement
-      //           ExpressionStatement
-      //             CallExpression[callee.name="makeObservable"]        
-      const [firstArg, secondArg] = makeObservable.arguments;
-      if (!firstArg) return;
-      let members;
-      if (firstArg.type === 'ThisExpression') {
-        const closestFunction = findAncestor(makeObservable, node => node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration')
-        if (closestFunction?.parent?.kind !== 'constructor') return;
-        members = closestFunction.parent.parent.parent.body.body;
-      } else if (firstArg.type === 'ObjectExpression') {
-        members = firstArg.properties;
-      } else {
-        return;
-      }
+    return {
+        'CallExpression[callee.name="makeObservable"]': makeObservable => {
+            // Only interested about makeObservable(this, ...) in constructor or makeObservable({}, ...)
+            // ClassDeclaration
+            //   ClassBody
+            //     MethodDefinition[kind="constructor"]
+            //       FunctionExpression
+            //         BlockStatement
+            //           ExpressionStatement
+            //             CallExpression[callee.name="makeObservable"]
+            const [firstArg, secondArg] = makeObservable.arguments
+            if (!firstArg) return
+            let members
+            if (firstArg.type === "ThisExpression") {
+                const closestFunction = findAncestor(
+                    makeObservable,
+                    node =>
+                        node.type === "FunctionExpression" || node.type === "FunctionDeclaration"
+                )
+                if (closestFunction?.parent?.kind !== "constructor") return
+                members = closestFunction.parent.parent.parent.body.body
+            } else if (firstArg.type === "ObjectExpression") {
+                members = firstArg.properties
+            } else {
+                return
+            }
 
-      const annotationProps = secondArg?.properties || [];
-      const nonAnnotatedMembers = [];
-      let hasAnyDecorator = false;
+            const annotationProps = secondArg?.properties || []
+            const nonAnnotatedMembers = []
+            let hasAnyDecorator = false
 
-      members.forEach(member => {
-        if (member.static) return;
-        if (member.kind === "constructor") return;
-        //if (member.type !== 'MethodDefinition' && member.type !== 'ClassProperty') return;                    
-        hasAnyDecorator = hasAnyDecorator || member.decorators?.some(isMobxDecorator) || false;
-        if (!annotationProps.some(prop => fieldToKey(prop) === fieldToKey(member))) { // TODO optimize?
-          nonAnnotatedMembers.push(member);
-        }
-      })
-      /*
+            members.forEach(member => {
+                if (member.static) return
+                if (member.kind === "constructor") return
+                //if (member.type !== 'MethodDefinition' && member.type !== 'ClassProperty') return;
+                hasAnyDecorator =
+                    hasAnyDecorator || member.decorators?.some(isMobxDecorator) || false
+                if (!annotationProps.some(prop => fieldToKey(prop) === fieldToKey(member))) {
+                    // TODO optimize?
+                    nonAnnotatedMembers.push(member)
+                }
+            })
+            /*
       // With decorators, second arg must be null/undefined or not provided
       if (hasAnyDecorator && secondArg && secondArg.name !== "undefined" && secondArg.value !== null) {
         context.report({
@@ -66,46 +73,59 @@ function create(context) {
       }
       */
 
-      if (!hasAnyDecorator && nonAnnotatedMembers.length) {
-        // Set avoids reporting twice for setter+getter pair or actual duplicates
-        const keys = [...new Set(nonAnnotatedMembers.map(fieldToKey))];
-        const keyList = keys.map(key => `\`${key}\``).join(', ');
+            if (!hasAnyDecorator && nonAnnotatedMembers.length) {
+                // Set avoids reporting twice for setter+getter pair or actual duplicates
+                const keys = [...new Set(nonAnnotatedMembers.map(fieldToKey))]
+                const keyList = keys.map(key => `\`${key}\``).join(", ")
 
-        const fix = fixer => {
-          const annotationList = keys.map(key => `${key}: true`).join(', ') + ',';
-          if (!secondArg) {
-            return fixer.insertTextAfter(firstArg, `, { ${annotationList} }`);
-          } else if (secondArg.type !== 'ObjectExpression') {
-            return fixer.replaceText(secondArg, `{ ${annotationList} }`);
-          } else {
-            const openingBracket = sourceCode.getFirstToken(secondArg)
-            return fixer.insertTextAfter(openingBracket, ` ${annotationList} `);
-          }
-        };
+                const fix = fixer => {
+                    const annotationList =
+                        keys.map(key => `${key}: ${autofixAnnotation}`).join(", ") + ","
+                    if (!secondArg) {
+                        return fixer.insertTextAfter(firstArg, `, { ${annotationList} }`)
+                    } else if (secondArg.type !== "ObjectExpression") {
+                        return fixer.replaceText(secondArg, `{ ${annotationList} }`)
+                    } else {
+                        const openingBracket = sourceCode.getFirstToken(secondArg)
+                        return fixer.insertTextAfter(openingBracket, ` ${annotationList} `)
+                    }
+                }
 
-        context.report({
-          node: makeObservable,
-          messageId: 'missingAnnotation',
-          data: { keyList },
-          fix,
-        })
-      }
-    },
-  };
+                context.report({
+                    node: makeObservable,
+                    messageId: "missingAnnotation",
+                    data: { keyList },
+                    fix
+                })
+            }
+        }
+    }
 }
 
 module.exports = {
-  meta: {
-    type: 'suggestion',
-    fixable: 'code',
-    docs: {
-      description: 'enforce all fields being listen in `makeObservable`',
-      recommended: true,
-      suggestion: false,
-    },
-    messages: {
-      'missingAnnotation': 'Missing annotation for {{ keyList }}. To exclude a field, use `false` as annotation.',
+    meta: {
+        type: "suggestion",
+        fixable: "code",
+        schema: [
+            {
+                type: "object",
+                properties: {
+                    autofixAnnotation: {
+                        type: "boolean"
+                    }
+                },
+                additionalProperties: false
+            }
+        ],
+        docs: {
+            description: "enforce all fields being listen in `makeObservable`",
+            recommended: true,
+            suggestion: false
+        },
+        messages: {
+            missingAnnotation:
+                "Missing annotation for {{ keyList }}. To exclude a field, use `false` as annotation."
+        }
     },
-  },
-  create,
-};
+    create
+}