Skip to content

Commit

Permalink
feat(eslint-plugin): Add a new rule api-no-slashes that forbids using…
Browse files Browse the repository at this point in the history
… passing string literals to API.one() with slashes
  • Loading branch information
christopherthielen committed Oct 7, 2020
1 parent 5cdc7fa commit 3cad0d3
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 176 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/base.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
plugins: ['@typescript-eslint', '@spinnaker/eslint-plugin', 'react-hooks'],
extends: ['eslint:recommended', 'prettier', 'prettier/@typescript-eslint', 'plugin:@typescript-eslint/recommended'],
rules: {
'@spinnaker/api-no-slashes': 2,
'@spinnaker/import-from-alias-not-npm': 2,
'@spinnaker/import-from-npm-not-alias': 2,
'@spinnaker/import-from-npm-not-relative': 2,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/eslint-plugin.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module.exports = {
rules: {
'api-no-slashes': require('./rules/api-no-slashes'),
'import-from-alias-not-npm': require('./rules/import-from-alias-not-npm'),
'import-from-npm-not-alias': require('./rules/import-from-npm-not-alias'),
'import-from-npm-not-relative': require('./rules/import-from-npm-not-relative'),
Expand Down
122 changes: 122 additions & 0 deletions packages/eslint-plugin/rules/api-no-slashes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
'use strict';
const _ = require('lodash');

/**
* Recursively grab the callee until an Identifier is found.
*
* API.all().all().one('foo/bar');
*
* var calleeOne = ...
* getCallingIdentifier(calleeOne).name === 'API'
*/
function getCallingIdentifier(calleeObject) {
if (calleeObject.type && calleeObject.type === 'Identifier') {
return calleeObject;
} else if (calleeObject.callee && calleeObject.callee.object) {
return getCallingIdentifier(calleeObject.callee.object);
}
return null;
}

/** given an identifier, finds its Variable in the enclosing scope */
function getVariableInScope(context, identifier) {
const { variables } = context.getScope();
return variables.find((v) => v.name === identifier.name);
}

/**
* No slashes in string literals passed to API.one() / API.all()
*
* @version 0.1.0
* @category
*/
const rule = function (context) {
return {
CallExpression: function (node) {
const { callee = {}, arguments: args = [] } = node;

// .one() or .all()
const propertyName = (callee.property && callee.property.name) || '';
if (propertyName !== 'one' && propertyName !== 'all') {
// console.log('not one or all');
return;
}

// finds 'foo/bad' from:
// ('foo/bad')
// ('ok', 'foo/bad')
const literalArgs = args.filter((arg) => arg.type === 'Literal' && arg.raw.includes('/'));

// finds (badvar) from:
// var badvar = 'foo/bad'; API.one(badvar);
const variableArgs = args.filter((argument) => {
if (argument.type !== 'Identifier') {
return false;
}
const variable = getVariableInScope(context, argument);
const literalValue = _.get(variable, 'defs[0].node.init.raw', '');
return literalValue.includes('/');
});

if (literalArgs.length === 0 && variableArgs.length === 0) {
// console.log(args[0]);
// console.log('no slashes');
return;
}

// API.*
if ((getCallingIdentifier(node) || {}).name !== 'API') {
// console.log(getCallingIdentifier(callee));
// console.log('calling identifier not API');
return;
}

const message =
`Do not include slashes in API.one() or API.all() calls because arguments to .one() and .all() get url encoded.` +
`Instead, of API.one('foo/bar'), split into multiple arguments: API.one('foo', 'bar').`;

const fix = (fixer) => {
// within the code:
// API.one('foo/bad')
// replaces:
// 'foo/bad'
// with:
// 'foo', 'bad'
const literalArgFixes = literalArgs.map((arg) => {
const varArgs = arg.value
.split('/')
.map((segment) => "'" + segment + "'")
.join(', ');

return fixer.replaceText(arg, varArgs);
});

// within the code:
// let varDeclaration = 'foo/bad';
// API.one(varDeclaration)
// replaces:
// (varDeclaration)
// with:
// (...varDeclaration.split('/'))
const variableArgFixes = variableArgs.map((arg) => {
const spread = `...${arg.name}.split('/')`;
return fixer.replaceText(arg, spread);
});

return literalArgFixes.concat(variableArgFixes);
};
context.report({ fix, node, message });
},
};
};

module.exports = {
meta: {
type: 'problem',
docs: {
description: ``,
},
fixable: 'code',
},
create: rule,
};
45 changes: 45 additions & 0 deletions packages/eslint-plugin/test/api-no-slashes.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
'use strict';

const ruleTester = require('../utils/ruleTester');
const rule = require('../rules/api-no-slashes');
const errorMessage =
`Do not include slashes in API.one() or API.all() calls because arguments to .one() and .all() get url encoded.` +
`Instead, of API.one('foo/bar'), split into multiple arguments: API.one('foo', 'bar').`;

ruleTester.run('api-no-slashes', rule, {
valid: [
{
code: `API.one('foo', 'bar');`,
},
],

invalid: [
{
code: `API.one('foo/bad');`,
errors: [errorMessage],
output: `API.one('foo', 'bad');`,
},
{
code: `API.one('ok').one('ok').all('ok').one('foo/bad');`,
errors: [errorMessage],
output: `API.one('ok').one('ok').all('ok').one('foo', 'bad');`,
},
{
code: `API.one('ok').one('ok', 'foo/bad');`,
output: `API.one('ok').one('ok', 'foo', 'bad');`,
errors: [errorMessage],
},
// Variables which are initialized to a string literal with a slash
{
code: `let foo = "foo/bad"; API.one(foo);`,
errors: [errorMessage],
output: `let foo = "foo/bad"; API.one(...foo.split('/'));`,
},
// Mix of everything
{
code: `const foo = "foo/bad"; API.all('api').one(foo).one('ok', 'bar/baz');`,
errors: [errorMessage, errorMessage], // two errors
output: `const foo = "foo/bad"; API.all('api').one(...foo.split('/')).one('ok', 'bar', 'baz');`,
},
],
});
Loading

0 comments on commit 3cad0d3

Please sign in to comment.