Skip to content

Commit

Permalink
super
Browse files Browse the repository at this point in the history
  • Loading branch information
WearyMonkey committed Feb 9, 2022
1 parent 4c7d762 commit 0fe7e04
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 13 deletions.
51 changes: 51 additions & 0 deletions packages/eslint-plugin-mobx/__tests__/missing-make-observable.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,54 @@ constructor() { makeBanana(this); }
}
))

const invalid10 = fields.map(field => ({
code: `
class C extends Component {
@${field}
}
`,
errors: [
{ messageId: 'missingMakeObservable' },
],
output: `
class C extends Component {
constructor(props) { super(props); makeObservable(this); }
@${field}
}
`,
}
))

const invalid11 = fields.map(field => ({
code: `
class C extends React.Component<{ foo: string }> {
@${field}
}
`,
errors: [
{ messageId: 'missingMakeObservable' },
],
output: `
class C extends React.Component<{ foo: string }> {
constructor(props: { foo: string }) { super(props); makeObservable(this); }
@${field}
}
`,
}
))

const invalid12 = fields.map(field => ({
code: `
class C extends Banana {
@${field}
}
`,
errors: [
{ messageId: 'missingMakeObservableSuper' },
],
}
))

tester.run("missing-make-observable", rule, {
valid: [
...valid1,
Expand All @@ -265,5 +313,8 @@ tester.run("missing-make-observable", rule, {
...invalid7,
...invalid8,
...invalid9,
...invalid10,
...invalid11,
...invalid12,
],
});
49 changes: 37 additions & 12 deletions packages/eslint-plugin-mobx/src/missing-make-observable.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ 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 makeObservableExpr = undefined; // is 'mobxFoo' when import * as mobxFoo from 'mobx'
let lastSpecifierImport = undefined;

return {
Expand All @@ -18,12 +18,13 @@ function create(context) {
for (const specifier of node.specifiers) {
if (specifier.type === 'ImportNamespaceSpecifier') {
namespaceImportName = specifier.local.name;
makeObservableExpr = `${namespaceImportName}.makeObservable`;
}

if (specifier.type === 'ImportSpecifier') {
lastSpecifierImport = specifier;
if (specifier.imported.name === 'makeObservable') {
makeObserverImportName = specifier.local.name;
makeObservableExpr = specifier.local.name;
}
}
}
Expand All @@ -36,7 +37,9 @@ function create(context) {
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 isReact = isReactComponent(clazz.superClass);
const unsupportedSuper = !isReact && !!clazz.superClass;
const isMakeObservable = node => (node.expression?.callee?.name === 'makeObservable' || node.expression?.callee?.property?.name === 'makeObservable') && node.expression?.arguments[0]?.type === 'ThisExpression';
const makeObservable = constructor?.value.body?.body.find(isMakeObservable)?.expression;

if (makeObservable) {
Expand All @@ -50,17 +53,17 @@ function create(context) {
}
} else {
const fix = fixer => {
// The class extends a another unknown class so we can not safely create a super call.
if (unsupportedSuper && !constructor) {
return;
}

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 (!makeObservableExpr) {
lastSpecifierImport && fixes.push(fixer.insertTextAfter(lastSpecifierImport, ', makeObservable'));
makeObservableExpr = 'makeObservable';
}

if (constructor?.value.type === 'TSEmptyBodyFunctionExpression') {
Expand All @@ -71,7 +74,15 @@ function create(context) {
// constructor() {}
const closingBracket = sourceCode.getLastToken(constructor.value.body);
fixes.push(fixer.insertTextBefore(closingBracket, `;${makeObservableExpr}(this);`));
} else {
} else if (isReact) {
// class C extends Component<{ foo: string }> {}
let propsType = '';
if (clazz.superTypeParameters?.params.length) {
propsType = `: ${sourceCode.getText(clazz.superTypeParameters.params[0])}`;
}
const openingBracket = sourceCode.getFirstToken(clazz.body);
fixes.push(fixer.insertTextAfter(openingBracket, `\nconstructor(props${propsType}) { super(props); ${makeObservableExpr}(this); }`));
} else if (!unsupportedSuper) {
// class C {}
const openingBracket = sourceCode.getFirstToken(clazz.body);
fixes.push(fixer.insertTextAfter(openingBracket, `\nconstructor() { ${makeObservableExpr}(this); }`));
Expand All @@ -82,14 +93,27 @@ function create(context) {

context.report({
node: clazz,
messageId: 'missingMakeObservable',
messageId: unsupportedSuper ? 'missingMakeObservableSuper' : 'missingMakeObservable',
fix,
})
}
},
};
}

function isReactComponent(superClass) {
const classes = ['Component', 'PureComponent'];
if (!superClass) {
return false;
}

if (classes.includes(superClass.name)) {
return true;
}

return superClass.object?.name === 'React' && classes.includes(superClass.property.name);
}

module.exports = {
meta: {
type: 'problem',
Expand All @@ -101,6 +125,7 @@ module.exports = {
},
messages: {
missingMakeObservable: "Constructor is missing `makeObservable(this)`.",
missingMakeObservableSuper: "Constructor is missing `makeObservable(this)`. Can not fix because of missing super call.",
secondArgMustBeNullish: "`makeObservable`'s second argument must be nullish or not provided when using decorators."
},
},
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-mobx/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function isMobxDecorator(decorator, namespace) {
memberExpression = decorator.expression.callee;
}

if (memberExpression.object.name === namespace || memberExpression.object.object?.name === namespace) {
if (memberExpression?.object.name === namespace || memberExpression?.object.object?.name === namespace) {
return true;
}
}
Expand Down

0 comments on commit 0fe7e04

Please sign in to comment.