From d4af3bd5cc894d5c36d360500bb9c372c1f2a59a Mon Sep 17 00:00:00 2001 From: toby Date: Thu, 10 Feb 2022 01:40:54 +1100 Subject: [PATCH] Add support for namespaces and fix imports --- .../__tests__/missing-make-observable.js | 235 ++++++++++++------ .../src/missing-make-observable.js | 178 ++++++++----- packages/eslint-plugin-mobx/src/utils.js | 59 +++-- 3 files changed, 314 insertions(+), 158 deletions(-) diff --git a/packages/eslint-plugin-mobx/__tests__/missing-make-observable.js b/packages/eslint-plugin-mobx/__tests__/missing-make-observable.js index fe3abacd68..1556ee3f00 100644 --- a/packages/eslint-plugin-mobx/__tests__/missing-make-observable.js +++ b/packages/eslint-plugin-mobx/__tests__/missing-make-observable.js @@ -1,38 +1,42 @@ -import { RuleTester } from "eslint"; +import { RuleTester } from "eslint" -import rule from "../src/missing-make-observable.js"; +import rule from "../src/missing-make-observable.js" const tester = new RuleTester({ - parser: require.resolve('@typescript-eslint/parser'), - parserOptions: {} -}); + parser: require.resolve("@typescript-eslint/parser"), + parserOptions: {} +}) const fields = [ - '@observable o = 5', - '@observable.ref or = []', - '@observable.shallow os = []', - '@observable.deep od = {}', - '@computed get c() {}', - '@computed.struct get cs() {}', - '@computed({ equals }) get co() {}', - '@action a() {}', - '@action.bound ab() {}', - '@flow *f() {}', - '@flow.bound *fb() {}', -]; - -const valid1 = fields.map(field => ` + "observable o = 5", + "observable.ref or = []", + "observable.shallow os = []", + "observable.deep od = {}", + "computed get c() {}", + "computed.struct get cs() {}", + "computed({ equals }) get co() {}", + "action a() {}", + "action.bound ab() {}", + "flow *f() {}", + "flow.bound *fb() {}" +] + +const valid1 = fields + .map( + field => ` class C { - ${field} + @${field} constructor() { makeObservable(this) } } -`).map(code => ({ code })) +` + ) + .map(code => ({ code })) const valid2 = { - code: ` + code: ` class C { o = 5; get c() {}; @@ -46,19 +50,25 @@ class C { ` } -const valid3 = fields.map(field => ` +const valid3 = fields + .map( + field => ` class C { - ${field} + @${field} constructor() { makeObservable(this, null, { name: 'foo' }) } } -`).map(code => ({ code })) +` + ) + .map(code => ({ code })) -const valid4 = fields.map(field => ` +const valid4 = fields + .map( + field => ` class C { - ${field} + @${field} constructor(aString: string); constructor(aNum: number); @@ -66,111 +76,178 @@ class C { makeObservable(this, null, { name: 'foo' }) } } -`).map(code => ({ code })) +` + ) + .map(code => ({ code })) const invalid1 = fields.map(field => ({ - code: ` + code: ` class C { - ${field} + @${field} } `, - errors: [ - { messageId: 'missingMakeObservable' }, - ], - output: ` + errors: [{ messageId: "missingMakeObservable" }], + output: ` class C { constructor() { makeObservable(this); } - ${field} + @${field} } ` })) const invalid2 = fields.map(field => ({ - code: ` + code: ` class C { - ${field} + @${field} constructor() {} } `, - errors: [ - { messageId: 'missingMakeObservable' }, - ], - output: ` + errors: [{ messageId: "missingMakeObservable" }], + output: ` class C { - ${field} + @${field} constructor() {;makeObservable(this);} } -`, +` })) const invalid3 = fields.map(field => ({ - code: ` + code: ` class C { - ${field} + @${field} constructor() { makeObservable({ a: 5 }); } } `, - errors: [ - { messageId: 'missingMakeObservable' }, - ], - output: ` + errors: [{ messageId: "missingMakeObservable" }], + output: ` class C { - ${field} + @${field} constructor() { makeObservable({ a: 5 }); ;makeObservable(this);} } -`, +` })) const invalid4 = fields.map(field => ({ - code: ` + code: ` class C { - ${field} + @${field} constructor() } `, - errors: [ - { messageId: 'missingMakeObservable' }, - ], - output: ` + errors: [{ messageId: "missingMakeObservable" }], + output: ` class C { - ${field} + @${field} constructor() { makeObservable(this); } } -`, +` })) - const invalid5 = fields.map(field => ({ - code: ` + code: ` class C { - ${field} + @${field} constructor() { makeObservable(this, { o: observable.ref }); } } `, - errors: [ - { messageId: 'secondArgMustBeNullish' }, - ], + errors: [{ messageId: "secondArgMustBeNullish" }] +})) + +const invalid6 = fields.map(field => ({ + code: ` +import * as mobx from 'mobx'; + +class C { + @mobx.${field} +} +`, + errors: [{ messageId: "missingMakeObservable" }], + output: ` +import * as mobx from 'mobx'; + +class C { +constructor() { mobx.makeObservable(this); } + @mobx.${field} +} +` +})) + +const invalid7 = fields.map(field => ({ + code: ` +import { foo } from 'mobx'; + +class C { + @${field} +} +`, + errors: [{ messageId: "missingMakeObservable" }], + output: ` +import { foo, makeObservable } from 'mobx'; + +class C { +constructor() { makeObservable(this); } + @${field} +} +` })) +const invalid8 = fields.map(field => ({ + code: ` +import { foo } from 'mobx'; +import * as mobx from 'mobx'; + +class C { + @mobx.${field} +} +`, + errors: [{ messageId: "missingMakeObservable" }], + output: ` +import { foo } from 'mobx'; +import * as mobx from 'mobx'; + +class C { +constructor() { mobx.makeObservable(this); } + @mobx.${field} +} +` +})) + +const invalid9 = fields.map(field => ({ + code: ` +import { makeObservable as makeBanana } from 'mobx'; + +class C { + @${field} +} +`, + errors: [{ messageId: "missingMakeObservable" }], + output: ` +import { makeObservable as makeBanana } from 'mobx'; + +class C { +constructor() { makeBanana(this); } + @${field} +} +` +})) tester.run("missing-make-observable", rule, { - valid: [ - ...valid1, - valid2, - ...valid3, - ...valid4, - ], - invalid: [ - ...invalid1, - ...invalid2, - ...invalid3, - ...invalid4, - ...invalid5, - ], -}); \ No newline at end of file + valid: [...valid1, valid2, ...valid3, ...valid4], + invalid: [ + ...invalid1, + ...invalid2, + ...invalid3, + ...invalid4, + ...invalid5, + ...invalid6, + ...invalid7, + ...invalid8, + ...invalid9 + ] +}) diff --git a/packages/eslint-plugin-mobx/src/missing-make-observable.js b/packages/eslint-plugin-mobx/src/missing-make-observable.js index 45d40ca782..bfb8fedaa5 100644 --- a/packages/eslint-plugin-mobx/src/missing-make-observable.js +++ b/packages/eslint-plugin-mobx/src/missing-make-observable.js @@ -1,71 +1,127 @@ -'use strict'; +"use strict" -const { findAncestor, isMobxDecorator } = require('./utils.js'); +const { findAncestor, isMobxDecorator } = require("./utils.js") function create(context) { - const sourceCode = context.getSourceCode(); + const sourceCode = context.getSourceCode() - return { - 'Decorator': decorator => { - if (!isMobxDecorator(decorator)) return; - const clazz = findAncestor(decorator, node => node.type === 'ClassDeclaration' || node.type === 'ClassExpression'); - if (!clazz) return; - // ClassDeclaration > ClassBody > [] - const constructor = clazz.body.body.find(node => node.kind === 'constructor' && node.value.type === 'FunctionExpression') ?? - clazz.body.body.find(node => node.kind === 'constructor'); - // MethodDefinition > FunctionExpression > BlockStatement > [] - const isMakeObservable = node => node.expression?.callee?.name === 'makeObservable' && node.expression?.arguments[0]?.type === 'ThisExpression'; - const makeObservable = constructor?.value.body?.body.find(isMakeObservable)?.expression; + let namespaceImportName = undefined // is 'mobxFoo' when import * as mobxFoo from 'mobx' + let makeObserverImportName = undefined // is 'mobxFoo' when import * as mobxFoo from 'mobx' + let lastSpecifierImport = undefined - if (makeObservable) { - // make sure second arg is nullish - const secondArg = makeObservable.arguments[1]; - if (secondArg && secondArg.value !== null && secondArg.name !== 'undefined') { - context.report({ - node: makeObservable, - messageId: 'secondArgMustBeNullish', - }) - } - } else { - const fix = fixer => { - if (constructor?.value.type === 'TSEmptyBodyFunctionExpression') { - // constructor() - yes this a thing - const closingBracket = sourceCode.getLastToken(constructor.value); - return fixer.insertTextAfter(closingBracket, ' { makeObservable(this); }') - } else if (constructor) { - // constructor() {} - const closingBracket = sourceCode.getLastToken(constructor.value.body); - return fixer.insertTextBefore(closingBracket, ';makeObservable(this);') - } else { - // class C {} - const openingBracket = sourceCode.getFirstToken(clazz.body); - return fixer.insertTextAfter(openingBracket, '\nconstructor() { makeObservable(this); }') - } - }; + return { + ImportDeclaration: node => { + if (node.source.value !== "mobx") return - context.report({ - node: clazz, - messageId: 'missingMakeObservable', - fix, - }) - } - }, - }; + // Collect the imports + + for (const specifier of node.specifiers) { + if (specifier.type === "ImportNamespaceSpecifier") { + namespaceImportName = specifier.local.name + } + + if (specifier.type === "ImportSpecifier") { + lastSpecifierImport = specifier + if (specifier.imported.name === "makeObservable") { + makeObserverImportName = specifier.local.name + } + } + } + }, + Decorator: decorator => { + if (!isMobxDecorator(decorator, namespaceImportName)) return + const clazz = findAncestor( + decorator, + node => node.type === "ClassDeclaration" || node.type === "ClassExpression" + ) + if (!clazz) return + // ClassDeclaration > ClassBody > [] + const constructor = + clazz.body.body.find( + node => node.kind === "constructor" && node.value.type === "FunctionExpression" + ) ?? clazz.body.body.find(node => node.kind === "constructor") + // MethodDefinition > FunctionExpression > BlockStatement > [] + const isMakeObservable = node => + node.expression?.callee?.name === "makeObservable" && + node.expression?.arguments[0]?.type === "ThisExpression" + const makeObservable = constructor?.value.body?.body.find(isMakeObservable)?.expression + + if (makeObservable) { + // make sure second arg is nullish + const secondArg = makeObservable.arguments[1] + if (secondArg && secondArg.value !== null && secondArg.name !== "undefined") { + context.report({ + node: makeObservable, + messageId: "secondArgMustBeNullish" + }) + } + } else { + const fix = fixer => { + const fixes = [] + let makeObservableExpr = "makeObservable" + + // Insert the makeObservable import if required + if (!namespaceImportName && !makeObserverImportName && lastSpecifierImport) { + fixes.push(fixer.insertTextAfter(lastSpecifierImport, ", makeObservable")) + } else if (namespaceImportName) { + makeObservableExpr = `${namespaceImportName}.makeObservable` + } else if (makeObserverImportName) { + makeObservableExpr = makeObserverImportName + } + + if (constructor?.value.type === "TSEmptyBodyFunctionExpression") { + // constructor() - yes this a thing + const closingBracket = sourceCode.getLastToken(constructor.value) + fixes.push( + fixer.insertTextAfter( + closingBracket, + ` { ${makeObservableExpr}(this); }` + ) + ) + } else if (constructor) { + // constructor() {} + const closingBracket = sourceCode.getLastToken(constructor.value.body) + fixes.push( + fixer.insertTextBefore(closingBracket, `;${makeObservableExpr}(this);`) + ) + } else { + // class C {} + const openingBracket = sourceCode.getFirstToken(clazz.body) + fixes.push( + fixer.insertTextAfter( + openingBracket, + `\nconstructor() { ${makeObservableExpr}(this); }` + ) + ) + } + + return fixes + } + + context.report({ + node: clazz, + messageId: "missingMakeObservable", + fix + }) + } + } + } } module.exports = { - meta: { - type: 'problem', - fixable: 'code', - docs: { - description: 'prevents missing `makeObservable(this)` when using decorators', - recommended: true, - suggestion: false, - }, - messages: { - missingMakeObservable: "Constructor is missing `makeObservable(this)`.", - secondArgMustBeNullish: "`makeObservable`'s second argument must be nullish or not provided when using decorators." + meta: { + type: "problem", + fixable: "code", + docs: { + description: "prevents missing `makeObservable(this)` when using decorators", + recommended: true, + suggestion: false + }, + messages: { + missingMakeObservable: "Constructor is missing `makeObservable(this)`.", + secondArgMustBeNullish: + "`makeObservable`'s second argument must be nullish or not provided when using decorators." + } }, - }, - create, -}; \ No newline at end of file + create +} diff --git a/packages/eslint-plugin-mobx/src/utils.js b/packages/eslint-plugin-mobx/src/utils.js index ffc23ef684..b3d61e7cd2 100644 --- a/packages/eslint-plugin-mobx/src/utils.js +++ b/packages/eslint-plugin-mobx/src/utils.js @@ -1,29 +1,52 @@ -'use strict'; +"use strict" -const mobxDecorators = new Set(['observable', 'computed', 'action', 'flow', 'override']); +const mobxDecorators = new Set(["observable", "computed", "action", "flow", "override"]) -function isMobxDecorator(decorator) { - return mobxDecorators.has(decorator.expression.name) // @foo - || mobxDecorators.has(decorator.expression.callee?.name) // @foo() - || mobxDecorators.has(decorator.expression.object?.name) // @foo.bar +function isMobxDecorator(decorator, namespace) { + if (namespace !== undefined) { + let memberExpression + if (decorator.expression.type === "MemberExpression") { + memberExpression = decorator.expression + } + + if ( + decorator.expression.type === "CallExpression" && + decorator.expression.callee.type === "MemberExpression" + ) { + memberExpression = decorator.expression.callee + } + + if ( + memberExpression.object.name === namespace || + memberExpression.object.object?.name === namespace + ) { + return true + } + } + + return ( + mobxDecorators.has(decorator.expression.name) || // @foo + mobxDecorators.has(decorator.expression.callee?.name) || // @foo() + mobxDecorators.has(decorator.expression.object?.name) + ) // @foo.bar } function findAncestor(node, match) { - const { parent } = node; - if (!parent) return; - if (match(parent)) return parent; - return findAncestor(parent, match); + const { parent } = node + if (!parent) return + if (match(parent)) return parent + return findAncestor(parent, match) } function assert(expr, error) { - if (!expr) { - error ??= 'Assertion failed'; - error = error instanceof Error ? error : new Error(error) - throw error; - } + if (!expr) { + error ??= "Assertion failed" + error = error instanceof Error ? error : new Error(error) + throw error + } } module.exports = { - findAncestor, - isMobxDecorator, -} \ No newline at end of file + findAncestor, + isMobxDecorator +}