feat: remove unnecessary command aliases (#135)

* feat: remove unnecessary command aliases

* test: single and multiple unnecessary aliases

* feat: remove empty aliases

src/index.ts

@@ -38,6 +38,8 @@ import { noSplitExamples } from './rules/no-split-examples';
 import { noUsernameProperties } from './rules/migration/no-username-properties';
 import { noUnnecessaryProperties } from './rules/no-unnecessary-properties';
 import { encourageAliasDeprecation } from './rules/migration/encourage-alias-deprecation';
+import { noUnnecessaryAliases } from './rules/no-unnecessary-aliases';
+
 const recommended = {
   plugins: ['sf-plugin'],
   rules: {
@@ -60,6 +62,7 @@ const recommended = {
     'sf-plugin/id-flag-suggestions': 'warn',
     'sf-plugin/no-split-examples': 'error',
     'sf-plugin/no-unnecessary-properties': 'warn',
+    'sf-plugin/no-unnecessary-aliases': 'error',
   },
 };
 export = {
@@ -120,5 +123,6 @@ export = {
     'no-username-properties': noUsernameProperties,
     'no-unnecessary-properties': noUnnecessaryProperties,
     'encourage-alias-deprecation': encourageAliasDeprecation,
+    'no-unnecessary-aliases': noUnnecessaryAliases,
   },
 };

src/rules/no-unnecessary-aliases.ts

@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2020, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils';
+import { isInCommandDirectory, ancestorsContainsSfCommand, getCommandNameParts } from '../shared/commands';
+
+export const noUnnecessaryAliases = ESLintUtils.RuleCreator.withoutDocs({
+  meta: {
+    docs: {
+      description:
+        'Mark when an alias is unnecessary because its only an order permutation, not really a different name',
+      recommended: 'error',
+    },
+    messages: {
+      summary: 'the Salesforce CLI will match the command words in any order, so this alias is unnecessary',
+    },
+    type: 'problem',
+    schema: [],
+    fixable: 'code',
+  },
+  defaultOptions: [],
+  create(context) {
+    return isInCommandDirectory(context)
+      ? {
+          Literal(node): void {
+            if (
+              node.parent.type === AST_NODE_TYPES.ArrayExpression &&
+              node.parent.parent.type === AST_NODE_TYPES.PropertyDefinition &&
+              node.parent.parent.key.type === AST_NODE_TYPES.Identifier &&
+              node.parent.parent.key.name === 'aliases' &&
+              ancestorsContainsSfCommand(context.getAncestors())
+            ) {
+              const parentLength = node.parent.elements.length;
+              const cmdParts = getCommandNameParts(context.getPhysicalFilename());
+              const aliasParts = typeof node.value === 'string' ? node.value.split(':') : [];
+              if (
+                aliasParts.every((part) => cmdParts.includes(part)) &&
+                cmdParts.every((part) => aliasParts.includes(part))
+              ) {
+                context.report({
+                  node,
+                  messageId: 'summary',
+                  fix: (fixer) => {
+                    const comma = context.getSourceCode().getTokenAfter(node);
+                    if (parentLength === 1) {
+                      return fixer.remove(node.parent.parent);
+                    }
+                    return comma.value === ','
+                      ? fixer.removeRange([node.range[0], node.range[1] + 1])
+                      : fixer.remove(node);
+                  },
+                });
+              }
+            }
+          },
+        }
+      : {};
+  },
+});

src/rules/no-unnecessary-properties.ts

@@ -7,7 +7,8 @@
 import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils';
 import { extendsSfCommand, isInCommandDirectory } from '../shared/commands';
 
-const props = ['requiresProject', 'hidden'];
+const falseProps = ['requiresProject', 'hidden'];
+const emptyProps = ['aliases'];
 
 export const noUnnecessaryProperties = ESLintUtils.RuleCreator.withoutDocs({
   meta: {
@@ -16,7 +17,8 @@ export const noUnnecessaryProperties = ESLintUtils.RuleCreator.withoutDocs({
       recommended: 'warn',
     },
     messages: {
-      message: 'The {{prop}} property can be omitted since it is false by default',
+      messageFalse: 'The {{prop}} property can be omitted since it is false by default',
+      messageEmpty: 'The {{prop}} property can be omitted since it is empty',
     },
     type: 'problem',
     fixable: 'code',
@@ -28,22 +30,38 @@ export const noUnnecessaryProperties = ESLintUtils.RuleCreator.withoutDocs({
       ? {
           PropertyDefinition(node): void {
             if (
-              !node.readonly &&
               node.static &&
               node.key.type === AST_NODE_TYPES.Identifier &&
-              props.includes(node.key.name) &&
               node.parent.type === AST_NODE_TYPES.ClassBody &&
               node.parent.parent.type === AST_NODE_TYPES.ClassDeclaration &&
-              node.value.type === AST_NODE_TYPES.Literal &&
-              node.value.value === false &&
               extendsSfCommand(node.parent.parent)
             ) {
-              context.report({
-                node,
-                messageId: 'message',
-                data: { prop: node.key.name },
-                fix: (fixer) => fixer.remove(node),
-              });
+              // properties that default to false
+              if (
+                node.value.type === AST_NODE_TYPES.Literal &&
+                falseProps.includes(node.key.name) &&
+                node.value.value === false
+              ) {
+                context.report({
+                  node,
+                  messageId: 'messageFalse',
+                  data: { prop: node.key.name },
+                  fix: (fixer) => fixer.remove(node),
+                });
+              }
+              // properties that default to emptyArrays
+              if (
+                node.value.type === AST_NODE_TYPES.ArrayExpression &&
+                emptyProps.includes(node.key.name) &&
+                node.value.elements.length === 0
+              ) {
+                context.report({
+                  node,
+                  messageId: 'messageEmpty',
+                  data: { prop: node.key.name },
+                  fix: (fixer) => fixer.remove(node),
+                });
+              }
             }
           },
         }

src/shared/commands.ts

@@ -5,7 +5,7 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
-import { sep } from 'path';
+import { sep, parse } from 'path';
 import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils';
 import { RuleContext } from '@typescript-eslint/utils/dist/ts-eslint';
 
@@ -55,3 +55,11 @@ export const getSfImportFromProgram = (node: TSESTree.Node): TSESTree.ImportDecl
     );
   }
 };
+
+/** pass a filename, and get back an array of the parts that occur after `commands`
+ * in other words, the command's canonical name
+ */
+export const getCommandNameParts = (filename: string): string[] => {
+  const parts = filename.replace(parse(filename).ext, '').split(sep);
+  return parts.slice(parts.indexOf('commands') + 1);
+};

test/rules/no-uneccessary-properties.test.ts

@@ -21,6 +21,14 @@ export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
   public static requiresProject = true
   public static notInTargetList = false
 }
+`,
+    },
+    {
+      name: 'populated aliases  for a command',
+      code: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static aliases = ['foo']
+}
 `,
     },
     {
@@ -42,16 +50,37 @@ export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
     },
   ],
   invalid: [
+    {
+      name: 'empty aliases',
+      filename: path.normalize('src/commands/foo.ts'),
+      errors: [
+        {
+          messageId: 'messageEmpty',
+          data: { prop: 'aliases' },
+        },
+      ],
+      code: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static readonly aliases = []
+}
+`,
+      output: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  
+}
+`,
+    },
+
     {
       name: '2 properties set to false',
       filename: path.normalize('src/commands/foo.ts'),
       errors: [
         {
-          messageId: 'message',
+          messageId: 'messageFalse',
           data: { prop: 'hidden' },
         },
         {
-          messageId: 'message',
+          messageId: 'messageFalse',
           data: { prop: 'requiresProject' },
         },
       ],

test/rules/no-unnecessary-aliases.test.ts

@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2020, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+import path from 'path';
+import { ESLintUtils } from '@typescript-eslint/utils';
+import { noUnnecessaryAliases } from '../../src/rules/no-unnecessary-aliases';
+
+const ruleTester = new ESLintUtils.RuleTester({
+  parser: '@typescript-eslint/parser',
+});
+
+ruleTester.run('no unnecessary aliases', noUnnecessaryAliases, {
+  valid: [
+    {
+      name: 'aliases are not a permutation',
+      filename: path.normalize('src/commands/foo/bar/baz.ts'),
+      code: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static readonly aliases = ['bar:baz', 'bar:qux'];
+}
+`,
+    },
+
+    {
+      name: 'not in commands dir',
+      filename: path.normalize('src/shared/foo/bar/baz.ts'),
+      code: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static readonly aliases = ['foo:baz:bar'];
+}
+`,
+    },
+  ],
+  invalid: [
+    {
+      name: 'one alias is a permutation, but there is another that is not',
+      errors: [
+        {
+          messageId: 'summary',
+        },
+      ],
+      filename: path.normalize('src/commands/foo/bar/baz.ts'),
+      code: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static readonly aliases = ['bar:baz:foo', 'bar:qux'];
+}
+`,
+      output: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static readonly aliases = [ 'bar:qux'];
+}
+`,
+    },
+    {
+      name: 'only alias is a permutation',
+      errors: [
+        {
+          messageId: 'summary',
+        },
+      ],
+      filename: path.normalize('src/commands/foo/bar/baz.ts'),
+      code: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static readonly aliases = ['bar:baz:foo'];
+}
+`,
+      output: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  
+}
+`,
+    },
+    {
+      name: 'multiple aliases are a permutation',
+      errors: [
+        {
+          messageId: 'summary',
+        },
+        {
+          messageId: 'summary',
+        },
+      ],
+      filename: path.normalize('src/commands/foo/bar/baz.ts'),
+      code: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static readonly aliases = ['bar:baz:foo', 'baz:bar:foo'];
+}
+`,
+      output: `
+export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
+  public static readonly aliases = [ ];
+}
+`,
+    },
+  ],
+});

test/shared/commands.test.ts

@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2020, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import { join } from 'path';
+import { getCommandNameParts } from '../../src/shared/commands';
+
+describe('getCommandNameParts', () => {
+  it('should return the command name parts', () => {
+    expect(getCommandNameParts(join('src', 'commands', 'foo', 'bar.ts'))).toEqual(['foo', 'bar']);
+  });
+});