Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add support for namespaces and fix imports
Browse files Browse the repository at this point in the history
WearyMonkey committed Feb 9, 2022
1 parent b4fc9c4 commit d4af3bd
Showing 3 changed files with 314 additions and 158 deletions.
235 changes: 156 additions & 79 deletions packages/eslint-plugin-mobx/__tests__/missing-make-observable.js
Original file line number Diff line number Diff line change
@@ -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,131 +50,204 @@ 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);
constructor(stringOrNum: string | number) {
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,
],
});
valid: [...valid1, valid2, ...valid3, ...valid4],
invalid: [
...invalid1,
...invalid2,
...invalid3,
...invalid4,
...invalid5,
...invalid6,
...invalid7,
...invalid8,
...invalid9
]
})
178 changes: 117 additions & 61 deletions packages/eslint-plugin-mobx/src/missing-make-observable.js
Original file line number Diff line number Diff line change
@@ -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,
};
create
}
59 changes: 41 additions & 18 deletions packages/eslint-plugin-mobx/src/utils.js
Original file line number Diff line number Diff line change
@@ -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,
}
findAncestor,
isMobxDecorator
}

0 comments on commit d4af3bd

Please sign in to comment.