feat(eslint-plugin): Add a new rule api-no-slashes that forbids using string literals with slashes to API.one()

packages/eslint-plugin/base.config.js

@@ -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,

packages/eslint-plugin/eslint-plugin.js

@@ -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'),

packages/eslint-plugin/rules/api-no-slashes.js

@@ -0,0 +1,123 @@
+'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;
+      }
+
+      // API.one('foo/bad')
+      //          ^^^^^^^
+      // API.all('ok').one('ok', 'foo/bad', 'ok')
+      //                          ^^^^^^^
+      const literalArgs = args.filter((arg) => arg.type === 'Literal' && arg.raw.includes('/'));
+
+      // 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('no slashes');
+        return;
+      }
+
+      // API.all('ok').one('ok', 'foo/bad', 'ok')
+      // ^^^
+      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:
+        //   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:
+        //   let varDeclaration = 'foo/bad';
+        //   API.one(varDeclaration)
+        // replaces argument:
+        //   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,
+};

packages/eslint-plugin/test/api-no-slashes.spec.js

@@ -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');`,
+    },
+  ],
+});