Skip to content

Commit

Permalink
Add support for namespaces and fix imports
Browse files Browse the repository at this point in the history
  • Loading branch information
WearyMonkey committed Feb 9, 2022
1 parent b4fc9c4 commit 4c7d762
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 29 deletions.
139 changes: 116 additions & 23 deletions packages/eslint-plugin-mobx/__tests__/missing-make-observable.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,22 @@ const tester = new RuleTester({
});

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() {}',
'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)
Expand All @@ -48,7 +48,7 @@ class C {

const valid3 = fields.map(field => `
class C {
${field}
@${field}
constructor() {
makeObservable(this, null, { name: 'foo' })
Expand All @@ -58,7 +58,7 @@ class C {

const valid4 = fields.map(field => `
class C {
${field}
@${field}
constructor(aString: string);
constructor(aNum: number);
Expand All @@ -71,7 +71,7 @@ class C {
const invalid1 = fields.map(field => ({
code: `
class C {
${field}
@${field}
}
`,
errors: [
Expand All @@ -80,15 +80,15 @@ class C {
output: `
class C {
constructor() { makeObservable(this); }
${field}
@${field}
}
`
}))

const invalid2 = fields.map(field => ({
code: `
class C {
${field}
@${field}
constructor() {}
}
`,
Expand All @@ -97,7 +97,7 @@ class C {
],
output: `
class C {
${field}
@${field}
constructor() {;makeObservable(this);}
}
`,
Expand All @@ -106,7 +106,7 @@ class C {
const invalid3 = fields.map(field => ({
code: `
class C {
${field}
@${field}
constructor() {
makeObservable({ a: 5 });
}
Expand All @@ -117,7 +117,7 @@ class C {
],
output: `
class C {
${field}
@${field}
constructor() {
makeObservable({ a: 5 });
;makeObservable(this);}
Expand All @@ -128,7 +128,7 @@ class C {
const invalid4 = fields.map(field => ({
code: `
class C {
${field}
@${field}
constructor()
}
`,
Expand All @@ -137,7 +137,7 @@ class C {
],
output: `
class C {
${field}
@${field}
constructor() { makeObservable(this); }
}
`,
Expand All @@ -147,7 +147,7 @@ class C {
const invalid5 = fields.map(field => ({
code: `
class C {
${field}
@${field}
constructor() {
makeObservable(this, { o: observable.ref });
}
Expand All @@ -158,6 +158,95 @@ class C {
],
}))

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: [
Expand All @@ -172,5 +261,9 @@ tester.run("missing-make-observable", rule, {
...invalid3,
...invalid4,
...invalid5,
...invalid6,
...invalid7,
...invalid8,
...invalid9,
],
});
47 changes: 42 additions & 5 deletions packages/eslint-plugin-mobx/src/missing-make-observable.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,35 @@
'use strict';

const { findAncestor, isMobxDecorator } = require('./utils.js');
const {findAncestor, isMobxDecorator} = require('./utils.js');

function create(context) {
const sourceCode = context.getSourceCode();

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;

return {
'ImportDeclaration': node => {
if (node.source.value !== 'mobx') return;

// 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)) return;
if (!isMobxDecorator(decorator, namespaceImportName)) return;
const clazz = findAncestor(decorator, node => node.type === 'ClassDeclaration' || node.type === 'ClassExpression');
if (!clazz) return;
// ClassDeclaration > ClassBody > []
Expand All @@ -28,19 +50,34 @@ function create(context) {
}
} 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);
return fixer.insertTextAfter(closingBracket, ' { makeObservable(this); }')
fixes.push(fixer.insertTextAfter(closingBracket, ` { ${makeObservableExpr}(this); }`));
} else if (constructor) {
// constructor() {}
const closingBracket = sourceCode.getLastToken(constructor.value.body);
return fixer.insertTextBefore(closingBracket, ';makeObservable(this);')
fixes.push(fixer.insertTextBefore(closingBracket, `;${makeObservableExpr}(this);`));
} else {
// class C {}
const openingBracket = sourceCode.getFirstToken(clazz.body);
return fixer.insertTextAfter(openingBracket, '\nconstructor() { makeObservable(this); }')
fixes.push(fixer.insertTextAfter(openingBracket, `\nconstructor() { ${makeObservableExpr}(this); }`));
}

return fixes;
};

context.report({
Expand Down
17 changes: 16 additions & 1 deletion packages/eslint-plugin-mobx/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,22 @@

const mobxDecorators = new Set(['observable', 'computed', 'action', 'flow', 'override']);

function isMobxDecorator(decorator) {
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
Expand Down

0 comments on commit 4c7d762

Please sign in to comment.