From 9b0c46cc98225e2fc63c55ecd78d7ce4eeffb406 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Mon, 15 Jul 2024 13:40:53 -0600 Subject: [PATCH 1/7] feat: add no default and depends on flag configuration rule --- src/index.ts | 4 ++ src/rules/no-default-depends-on-flags.ts | 57 +++++++++++++++++++ .../rules/no-default-depends-on-flags.test.ts | 52 +++++++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 src/rules/no-default-depends-on-flags.ts create mode 100644 test/rules/no-default-depends-on-flags.test.ts diff --git a/src/index.ts b/src/index.ts index b3d604d1..23082ba0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -46,6 +46,7 @@ import { noClassesInCommandReturnType } from './rules/no-classes-in-command-retu import { noExecCmdDoubleQuotes } from './rules/no-execCmd-double-quotes'; import { noMessagesLoad } from './rules/no-messages-load'; import { esmMessageImport } from './rules/esm-message-import'; +import { noDefaultDependsOnFlags } from "./rules/no-default-depends-on-flags"; const library = { plugins: ['sf-plugin'], @@ -84,6 +85,7 @@ const recommended = { 'sf-plugin/no-args-parse-without-strict-false': 'error', 'sf-plugin/no-hyphens-aliases': 'error', 'sf-plugin/no-classes-in-command-return-type': 'error', + 'sf-plugin/no-default-and-depends-on-flags': 'error', }, }; @@ -107,6 +109,7 @@ export = { 'sf-plugin/no-time-flags': 'error', 'sf-plugin/no-id-flags': 'error', 'sf-plugin/no-username-properties': 'error', + 'sf-plugin/no-default-and-depends-on-flags': 'error', 'sf-plugin/encourage-alias-deprecation': 'warn', }, }, @@ -114,6 +117,7 @@ export = { rules: { 'esm-message-import': esmMessageImport, 'no-h-short-char': dashH, + 'no-default-and-depends-on-flags': noDefaultDependsOnFlags, 'no-duplicate-short-characters': noDuplicateShortCharacters, 'run-matches-class-type': runMatchesClassType, 'flag-case': flagCasing, diff --git a/src/rules/no-default-depends-on-flags.ts b/src/rules/no-default-depends-on-flags.ts new file mode 100644 index 00000000..1d60b8fe --- /dev/null +++ b/src/rules/no-default-depends-on-flags.ts @@ -0,0 +1,57 @@ +/* + * 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 } from '@typescript-eslint/utils'; +import { RuleCreator } from '@typescript-eslint/utils/eslint-utils'; +import { ancestorsContainsSfCommand, isInCommandDirectory } from '../shared/commands'; +import { flagPropertyIsNamed, isFlag } from '../shared/flags'; + +export const noDefaultDependsOnFlags = RuleCreator.withoutDocs({ + meta: { + docs: { + description: 'Do not allow creation of a flag with default value and dependsOn', + recommended: 'recommended', + }, + messages: { + message: 'Cannot create a flag with a default value and dependsOn', + }, + type: 'problem', + schema: [], + }, + defaultOptions: [], + create(context) { + return isInCommandDirectory(context) + ? { + Property(node): void { + // is a flag + if ( + isFlag(node) && + ancestorsContainsSfCommand(context.getAncestors()) && + node.value?.type === AST_NODE_TYPES.CallExpression && + node.value.arguments?.[0]?.type === AST_NODE_TYPES.ObjectExpression + ) { + const dependsOn = node.value.arguments[0].properties.find( + (property) => + property.type === AST_NODE_TYPES.Property && + flagPropertyIsNamed(property, 'dependsOn') + ); + const defaultValue = node.value.arguments[0].properties.find( + (property) => + property.type === AST_NODE_TYPES.Property && + flagPropertyIsNamed(property, 'default') + ); + if (dependsOn && defaultValue) { + context.report({ + node: dependsOn, + messageId: 'message', + }); + } + } + }, + } + : {}; + }, +}); diff --git a/test/rules/no-default-depends-on-flags.test.ts b/test/rules/no-default-depends-on-flags.test.ts new file mode 100644 index 00000000..8b1b5b78 --- /dev/null +++ b/test/rules/no-default-depends-on-flags.test.ts @@ -0,0 +1,52 @@ +/* + * 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 { RuleTester } from '@typescript-eslint/rule-tester'; + +import { noDefaultDependsOnFlags } from "../../src/rules/no-default-depends-on-flags"; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('noDefaultDependsOnFlags', noDefaultDependsOnFlags, { + valid: [ + { + name: 'does not use default and dependsOn', + filename: path.normalize('src/commands/foo.ts'), + code: ` +export default class EnvCreateScratch extends SfCommand { + public static flags = { + alias: Flags.string({ + summary: 'foo', + char: 'a' + }) + } +} + +`, + }, + ], + invalid: [ + { + name: 'uses dependsOn and default', + filename: path.normalize('src/commands/foo.ts'), + errors: [{ messageId: 'message' }], + code: ` +export default class EnvCreateScratch extends SfCommand { + public static flags = { + json: Flags.boolean({ + char: 'h', + default: true, + dependsOn: ['myOtherFlag'] + }) + } +} +`, + }, + ], +}); From 1059b5caab6fafb0fb854369e281433b45dbce9e Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 19 Jul 2024 13:30:28 -0600 Subject: [PATCH 2/7] chore: edge case --- src/rules/no-default-depends-on-flags.ts | 56 ++++++++++--------- .../rules/no-default-depends-on-flags.test.ts | 45 ++++++++++++++- 2 files changed, 74 insertions(+), 27 deletions(-) diff --git a/src/rules/no-default-depends-on-flags.ts b/src/rules/no-default-depends-on-flags.ts index 1d60b8fe..f8ee379e 100644 --- a/src/rules/no-default-depends-on-flags.ts +++ b/src/rules/no-default-depends-on-flags.ts @@ -25,33 +25,37 @@ export const noDefaultDependsOnFlags = RuleCreator.withoutDocs({ create(context) { return isInCommandDirectory(context) ? { - Property(node): void { - // is a flag - if ( - isFlag(node) && - ancestorsContainsSfCommand(context.getAncestors()) && - node.value?.type === AST_NODE_TYPES.CallExpression && - node.value.arguments?.[0]?.type === AST_NODE_TYPES.ObjectExpression - ) { - const dependsOn = node.value.arguments[0].properties.find( - (property) => - property.type === AST_NODE_TYPES.Property && - flagPropertyIsNamed(property, 'dependsOn') - ); - const defaultValue = node.value.arguments[0].properties.find( - (property) => - property.type === AST_NODE_TYPES.Property && - flagPropertyIsNamed(property, 'default') - ); - if (dependsOn && defaultValue) { - context.report({ - node: dependsOn, - messageId: 'message', - }); + Property(node): void { + // is a flag + if ( + isFlag(node) && + ancestorsContainsSfCommand(context.getAncestors()) && + node.value?.type === AST_NODE_TYPES.CallExpression && + node.value.arguments?.[0]?.type === AST_NODE_TYPES.ObjectExpression + ) { + const dependsOnProperty = node.value.arguments[0].properties.find( + (property) => property.type === AST_NODE_TYPES.Property && flagPropertyIsNamed(property, 'dependsOn') + ); + const defaultValueProperty = node.value.arguments[0].properties.find( + (property) => property.type === AST_NODE_TYPES.Property && flagPropertyIsNamed(property, 'default') + ); + + // @ts-expect-error from the node (flag), go up a level (parent) and find the dependsOn flag definition, see if it has a default + const dependsOnFlagDefaultValue = node.parent.properties + .find( + // @ts-expect-error value type on dependsOn + (f) => f.type === AST_NODE_TYPES.Property && f.key.name === dependsOn?.value.elements[0].value + ) + ?.value.arguments[0].properties.find((p) => p.key.name === 'default'); + if (dependsOnProperty && defaultValueProperty && !dependsOnFlagDefaultValue) { + context.report({ + node: dependsOnProperty, + messageId: 'message', + }); + } } - } - }, - } + }, + } : {}; }, }); diff --git a/test/rules/no-default-depends-on-flags.test.ts b/test/rules/no-default-depends-on-flags.test.ts index 8b1b5b78..124bf81e 100644 --- a/test/rules/no-default-depends-on-flags.test.ts +++ b/test/rules/no-default-depends-on-flags.test.ts @@ -7,7 +7,7 @@ import path from 'path'; import { RuleTester } from '@typescript-eslint/rule-tester'; -import { noDefaultDependsOnFlags } from "../../src/rules/no-default-depends-on-flags"; +import { noDefaultDependsOnFlags } from '../../src/rules/no-default-depends-on-flags'; const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', @@ -27,6 +27,27 @@ export default class EnvCreateScratch extends SfCommand { }) } } +`, + }, + { + name: 'does not block flag with a default value depending on another flag with a default value', + filename: path.normalize('src/commands/foo.ts'), + code: ` +export default class EnvCreateScratch extends SfCommand { + public static flags = { + alias: Flags.string({ + summary: 'foo', + default: 'a', + dependsOn: ['balias'], + char: 'a' + }), + balias: Flags.string({ + summary: 'baz', + default: 'b', + char: 'b' + }) + } +} `, }, @@ -46,6 +67,28 @@ export default class EnvCreateScratch extends SfCommand { }) } } +`, + }, + + { + name: 'uses dependsOn and dependsOn doesnt have a default', + filename: path.normalize('src/commands/foo.ts'), + errors: [{ messageId: 'message' }], + code: ` +export default class EnvCreateScratch extends SfCommand { + public static flags = { + alias: Flags.string({ + summary: 'foo', + default: 'a', + dependsOn: ['balias'], + char: 'a' + }), + balias: Flags.string({ + summary: 'baz', + char: 'b' + }) + } +} `, }, ], From 680509f8b8471051f60282ad97c874095138e874 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 19 Jul 2024 13:58:34 -0600 Subject: [PATCH 3/7] chore: fix circular error --- src/rules/no-default-depends-on-flags.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-default-depends-on-flags.ts b/src/rules/no-default-depends-on-flags.ts index f8ee379e..fd2c1afe 100644 --- a/src/rules/no-default-depends-on-flags.ts +++ b/src/rules/no-default-depends-on-flags.ts @@ -44,7 +44,7 @@ export const noDefaultDependsOnFlags = RuleCreator.withoutDocs({ const dependsOnFlagDefaultValue = node.parent.properties .find( // @ts-expect-error value type on dependsOn - (f) => f.type === AST_NODE_TYPES.Property && f.key.name === dependsOn?.value.elements[0].value + (f) => f.type === AST_NODE_TYPES.Property && f.key.name === dependsOnProperty?.value.elements[0].value ) ?.value.arguments[0].properties.find((p) => p.key.name === 'default'); if (dependsOnProperty && defaultValueProperty && !dependsOnFlagDefaultValue) { From 93e6411c49b89320f7262b94b5abc28d81e03c1e Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 19 Jul 2024 13:58:44 -0600 Subject: [PATCH 4/7] docs: generate docs --- README.md | 3 ++- docs/rules/encourage-alias-deprecation.md | 2 +- docs/rules/id-flag-suggestions.md | 2 +- docs/rules/no-default-and-depends-on-flags.md | 5 +++++ docs/rules/no-this-flags.md | 2 +- docs/rules/no-this-org.md | 2 +- 6 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 docs/rules/no-default-and-depends-on-flags.md diff --git a/README.md b/README.md index 0823bbb3..ba737f53 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ module.exports = { ✈️ Set in the `migration` configuration.\ ✅ Set in the `recommended` configuration.\ 🔧 Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--fix).\ -💡 Manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). +💡 Manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). | Name                               | Description | 💼 | ⚠️ | 🚫 | 🔧 | 💡 | | :------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------------------- | :------ | :--- | :------ | :- | :- | @@ -71,6 +71,7 @@ module.exports = { | [no-args-parse-without-strict-false](docs/rules/no-args-parse-without-strict-false.md) | If you parse args/argv, the class should have strict set to false | ✈️ ✅ | | | 🔧 | | | [no-builtin-flags](docs/rules/no-builtin-flags.md) | Handling for sfdxCommand's flags.builtin | ✈️ | | | 🔧 | | | [no-classes-in-command-return-type](docs/rules/no-classes-in-command-return-type.md) | The return type of the run method should not contain a class. | ✈️ ✅ | | | 🔧 | | +| [no-default-and-depends-on-flags](docs/rules/no-default-and-depends-on-flags.md) | Do not allow creation of a flag with default value and dependsOn | ✈️ ✅ | | | | | | [no-deprecated-properties](docs/rules/no-deprecated-properties.md) | Removes non-existent properties left over from SfdxCommand | ✈️ | | | 🔧 | | | [no-duplicate-short-characters](docs/rules/no-duplicate-short-characters.md) | Prevent duplicate use of short characters or conflicts between aliases and flags | ✈️ ✅ | | | | | | [no-execcmd-double-quotes](docs/rules/no-execcmd-double-quotes.md) | Do not use double quotes in NUT examples. They will not work on windows | | | 📚 ✈️ ✅ | 🔧 | | diff --git a/docs/rules/encourage-alias-deprecation.md b/docs/rules/encourage-alias-deprecation.md index 352d0e0e..1099d748 100644 --- a/docs/rules/encourage-alias-deprecation.md +++ b/docs/rules/encourage-alias-deprecation.md @@ -2,6 +2,6 @@ ⚠️ This rule _warns_ in the ✈️ `migration` config. -🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). +🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). diff --git a/docs/rules/id-flag-suggestions.md b/docs/rules/id-flag-suggestions.md index a481ee45..3ed7bb1c 100644 --- a/docs/rules/id-flag-suggestions.md +++ b/docs/rules/id-flag-suggestions.md @@ -2,6 +2,6 @@ ⚠️ This rule _warns_ in the following configs: ✈️ `migration`, ✅ `recommended`. -🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). +🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). diff --git a/docs/rules/no-default-and-depends-on-flags.md b/docs/rules/no-default-and-depends-on-flags.md new file mode 100644 index 00000000..ad20f36d --- /dev/null +++ b/docs/rules/no-default-and-depends-on-flags.md @@ -0,0 +1,5 @@ +# Do not allow creation of a flag with default value and dependsOn (`sf-plugin/no-default-and-depends-on-flags`) + +💼 This rule is enabled in the following configs: ✈️ `migration`, ✅ `recommended`. + + diff --git a/docs/rules/no-this-flags.md b/docs/rules/no-this-flags.md index 8d3e486e..599a3c18 100644 --- a/docs/rules/no-this-flags.md +++ b/docs/rules/no-this-flags.md @@ -2,6 +2,6 @@ 💼 This rule is enabled in the ✈️ `migration` config. -🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). +🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). diff --git a/docs/rules/no-this-org.md b/docs/rules/no-this-org.md index 01d3e790..923877e7 100644 --- a/docs/rules/no-this-org.md +++ b/docs/rules/no-this-org.md @@ -2,6 +2,6 @@ 💼 This rule is enabled in the ✈️ `migration` config. -🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). +🔧💡 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix) and manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). From 9e72b89c16ab73bc3cb1d90ca3d1c34e74c7e8c1 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 19 Jul 2024 14:07:03 -0600 Subject: [PATCH 5/7] chore: fix [0] --- src/rules/no-default-depends-on-flags.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rules/no-default-depends-on-flags.ts b/src/rules/no-default-depends-on-flags.ts index fd2c1afe..d37b639a 100644 --- a/src/rules/no-default-depends-on-flags.ts +++ b/src/rules/no-default-depends-on-flags.ts @@ -43,8 +43,9 @@ export const noDefaultDependsOnFlags = RuleCreator.withoutDocs({ // @ts-expect-error from the node (flag), go up a level (parent) and find the dependsOn flag definition, see if it has a default const dependsOnFlagDefaultValue = node.parent.properties .find( - // @ts-expect-error value type on dependsOn - (f) => f.type === AST_NODE_TYPES.Property && f.key.name === dependsOnProperty?.value.elements[0].value + (f) => + // @ts-expect-error value type on dependsOn + f.type === AST_NODE_TYPES.Property && f.key.name === dependsOnProperty?.value.elements?.at(0)?.value ) ?.value.arguments[0].properties.find((p) => p.key.name === 'default'); if (dependsOnProperty && defaultValueProperty && !dependsOnFlagDefaultValue) { From eafd745e42ada4d76057521f7ca5d4d80f2b4eed Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Fri, 19 Jul 2024 14:13:01 -0600 Subject: [PATCH 6/7] chore: fix [0] (2) --- src/rules/no-default-depends-on-flags.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rules/no-default-depends-on-flags.ts b/src/rules/no-default-depends-on-flags.ts index d37b639a..efbe272b 100644 --- a/src/rules/no-default-depends-on-flags.ts +++ b/src/rules/no-default-depends-on-flags.ts @@ -47,7 +47,8 @@ export const noDefaultDependsOnFlags = RuleCreator.withoutDocs({ // @ts-expect-error value type on dependsOn f.type === AST_NODE_TYPES.Property && f.key.name === dependsOnProperty?.value.elements?.at(0)?.value ) - ?.value.arguments[0].properties.find((p) => p.key.name === 'default'); + ?.value.arguments?.at(0) + ?.properties.find((p) => p.key.name === 'default'); if (dependsOnProperty && defaultValueProperty && !dependsOnFlagDefaultValue) { context.report({ node: dependsOnProperty, From aa7902660378dbd51801aa8535fc4463cdd61577 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Mon, 22 Jul 2024 13:56:25 -0600 Subject: [PATCH 7/7] Wr/spread base flags (#441) * feat: add only extend from SfCommand * feat: add spread super command flags prop * chore: update messages to include super class name * chore: identify SfCommand parent class * chore: move from error to warn on rollout * chore: ..flags, ...baseFlags * chore: update comment --- src/index.ts | 11 +- src/rules/only-extend-sfCommand.ts | 41 +++++++ src/rules/spread-base-flags.ts | 74 ++++++++++++ src/shared/flags.ts | 17 +++ test/rules/only-extend-sfCommand.test.ts | 61 ++++++++++ test/rules/spread-base-flags.test.ts | 147 +++++++++++++++++++++++ 6 files changed, 349 insertions(+), 2 deletions(-) create mode 100644 src/rules/only-extend-sfCommand.ts create mode 100644 src/rules/spread-base-flags.ts create mode 100644 test/rules/only-extend-sfCommand.test.ts create mode 100644 test/rules/spread-base-flags.test.ts diff --git a/src/index.ts b/src/index.ts index 23082ba0..e77a324f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -46,7 +46,9 @@ import { noClassesInCommandReturnType } from './rules/no-classes-in-command-retu import { noExecCmdDoubleQuotes } from './rules/no-execCmd-double-quotes'; import { noMessagesLoad } from './rules/no-messages-load'; import { esmMessageImport } from './rules/esm-message-import'; -import { noDefaultDependsOnFlags } from "./rules/no-default-depends-on-flags"; +import { noDefaultDependsOnFlags } from './rules/no-default-depends-on-flags'; +import { onlyExtendSfCommand } from './rules/only-extend-sfCommand'; +import { spreadBaseFlags } from './rules/spread-base-flags'; const library = { plugins: ['sf-plugin'], @@ -86,6 +88,8 @@ const recommended = { 'sf-plugin/no-hyphens-aliases': 'error', 'sf-plugin/no-classes-in-command-return-type': 'error', 'sf-plugin/no-default-and-depends-on-flags': 'error', + 'sf-plugin/only-extend-SfCommand': 'warn', + 'sf-plugin/spread-base-flags': 'warn', }, }; @@ -110,7 +114,8 @@ export = { 'sf-plugin/no-id-flags': 'error', 'sf-plugin/no-username-properties': 'error', 'sf-plugin/no-default-and-depends-on-flags': 'error', - 'sf-plugin/encourage-alias-deprecation': 'warn', + 'sf-plugin/only-extend-SfCommand': 'warn', + 'sf-plugin/spread-base-flags': 'error', }, }, }, @@ -118,6 +123,8 @@ export = { 'esm-message-import': esmMessageImport, 'no-h-short-char': dashH, 'no-default-and-depends-on-flags': noDefaultDependsOnFlags, + 'only-extend-SfCommand': onlyExtendSfCommand, + 'spread-base-flags': spreadBaseFlags, 'no-duplicate-short-characters': noDuplicateShortCharacters, 'run-matches-class-type': runMatchesClassType, 'flag-case': flagCasing, diff --git a/src/rules/only-extend-sfCommand.ts b/src/rules/only-extend-sfCommand.ts new file mode 100644 index 00000000..670695cb --- /dev/null +++ b/src/rules/only-extend-sfCommand.ts @@ -0,0 +1,41 @@ +/* + * 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 { + isInCommandDirectory, + extendsSfCommand, +} from "../shared/commands"; +import { RuleCreator } from '@typescript-eslint/utils/eslint-utils'; + +export const onlyExtendSfCommand = RuleCreator.withoutDocs({ + meta: { + docs: { + description: 'Only allow commands that directly extend SfCommand', + recommended: 'recommended', + }, + messages: { + message: 'In order to inherit default flags correctly, extend from SfCommand directly', + }, + type: 'problem', + schema: [], + }, + defaultOptions: [], + create(context) { + return isInCommandDirectory(context) + ? { + ClassDeclaration(node): void { + // verify it extends SfCommand + if (!extendsSfCommand(node) && node.id) { + context.report({ + node: node.id, + messageId: 'message', + }); + } + }, + } + : {}; + }, +}); diff --git a/src/rules/spread-base-flags.ts b/src/rules/spread-base-flags.ts new file mode 100644 index 00000000..cde58921 --- /dev/null +++ b/src/rules/spread-base-flags.ts @@ -0,0 +1,74 @@ +/* + * 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 { isInCommandDirectory, ancestorsContainsSfCommand, getSfCommand } from '../shared/commands'; +import { RuleCreator } from '@typescript-eslint/utils/eslint-utils'; +import { + getBaseFlagsStaticPropertyFromCommandClass, + getFlagsStaticPropertyFromCommandClass, + isFlagsStaticProperty, +} from '../shared/flags'; +import { ASTUtils, AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; + +export const spreadBaseFlags = RuleCreator.withoutDocs({ + meta: { + docs: { + description: + "When not directly extending SfCommand, the parent's flags must be spread like flags = { ...{{parent}}.{{property}} }", + recommended: 'recommended', + }, + messages: { + message: + "When not directly extending SfCommand, the parent's flags must be spread like flags = { ...{{parent}}.{{property}} }", + }, + type: 'problem', + schema: [], + }, + defaultOptions: [], + create(context) { + return isInCommandDirectory(context) + ? { + ClassDeclaration(node): void { + const flagsProperty = getFlagsStaticPropertyFromCommandClass(node); + if (flagsProperty) { + // @ts-ignore SpreadElement (...) on property==='flags' (BaseCommand.flags) + const flags = flagsProperty?.value?.properties?.find( + (f) => f.type === 'SpreadElement' && f.argument.property?.name === 'flags' + ); + // @ts-ignore name will not be undefined because we're in a command class, which has to at least extend Command + const parent = node.superClass?.name; + + if (parent !== 'SfCommand' && !flags) { + context.report({ + loc: flagsProperty.loc, + messageId: 'message', + data: { parent, property: 'flags' }, + }); + } + } + + const baseFlagsProperty = getBaseFlagsStaticPropertyFromCommandClass(node); + if (baseFlagsProperty) { + // @ts-ignore SpreadElement (...) on property==='baseFlags' (BaseCommand.baseFlags) + const baseFlags = baseFlagsProperty.value?.properties?.find( + (f) => f.type === 'SpreadElement' && f.argument.property?.name === 'baseFlags' + ); + // @ts-ignore name will not be undefined because we're in a command class, which has to at least extend Command + const parent = node.superClass?.name; + + if (parent !== 'SfCommand' && !baseFlags) { + context.report({ + loc: baseFlagsProperty.loc, + messageId: 'message', + data: { parent, property: 'baseFlags' }, + }); + } + } + }, + } + : {}; + }, +}); diff --git a/src/shared/flags.ts b/src/shared/flags.ts index 375d697a..0a05ea5c 100644 --- a/src/shared/flags.ts +++ b/src/shared/flags.ts @@ -25,6 +25,15 @@ export const isFlagsStaticProperty = (node: TSESTree.Node): node is TSESTree.Pro node.key.name === 'flags' && ['public', 'protected'].includes(node.accessibility); +export const isBaseFlagsStaticProperty = (node: TSESTree.Node): node is TSESTree.PropertyDefinition => + node.type === AST_NODE_TYPES.PropertyDefinition && + typeof node.accessibility === 'string' && + node.static && + node.value?.type === AST_NODE_TYPES.ObjectExpression && + node.key.type === AST_NODE_TYPES.Identifier && + node.key.name === 'baseFlags' && + ['public', 'protected'].includes(node.accessibility); + export const flagPropertyIsNamed = (node: TSESTree.Property, name: string): node is TSESTree.Property => resolveFlagName(node) === name; @@ -48,6 +57,14 @@ export const getFlagsStaticPropertyFromCommandClass = ( } }; +export const getBaseFlagsStaticPropertyFromCommandClass = ( + classDeclaration: TSESTree.ClassDeclaration +): TSESTree.PropertyDefinition | undefined => { + if (classDeclaration.body.type === AST_NODE_TYPES.ClassBody) { + return classDeclaration.body.body.find(isBaseFlagsStaticProperty); + } +}; + export const getCalleePropertyByName = ( node: TSESTree.Property, calleePropName: string diff --git a/test/rules/only-extend-sfCommand.test.ts b/test/rules/only-extend-sfCommand.test.ts new file mode 100644 index 00000000..d16b8cfe --- /dev/null +++ b/test/rules/only-extend-sfCommand.test.ts @@ -0,0 +1,61 @@ +/* + * 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 { RuleTester } from '@typescript-eslint/rule-tester'; + +import { onlyExtendSfCommand } from "../../src/rules/only-extend-sfCommand"; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('only-extend-SfCommand', onlyExtendSfCommand, { + valid: [ + { + name: 'extends SfCommand directly', + filename: path.normalize('src/commands/foo.ts'), + code: ` +export default class EnvCreateScratch extends SfCommand { + public static flags = { + alias: Flags.string({ + summary: 'foo', + char: 'a' + }) + } +} + +`, + }, + { + name: 'not a command, can extend something else', + filename: path.normalize('src/base/foo.ts'), + code: ` +export default class MyFormatter extends Formatter { +} + +`, + }, + ], + invalid: [ + { + name: 'does not extend SfCommand directly', + filename: path.normalize('src/commands/foo.ts'), + errors: [{ messageId: 'message' }], + code: ` +export default class EnvCreateScratch extends BaseCommand { + public static flags = { + json: Flags.boolean({ + char: 'h', + default: true, + dependsOn: ['myOtherFlag'] + }) + } +} +`, + }, + ], +}); diff --git a/test/rules/spread-base-flags.test.ts b/test/rules/spread-base-flags.test.ts new file mode 100644 index 00000000..52ba6336 --- /dev/null +++ b/test/rules/spread-base-flags.test.ts @@ -0,0 +1,147 @@ +/* + * 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 { RuleTester } from '@typescript-eslint/rule-tester'; + +import { spreadBaseFlags } from '../../src/rules/spread-base-flags'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('spread-base-flags', spreadBaseFlags, { + valid: [ + { + name: 'spreads flags when required', + filename: path.normalize('src/commands/foo.ts'), + code: ` +export default class Top extends Base { + public static flags = { + ...Base.flags, + alias: Flags.string({ + summary: 'foo', + char: 'a' + }) + } +} +`, + }, + { + name: 'spreads flags and baseFlags when required', + filename: path.normalize('src/commands/foo.ts'), + code: ` +export default class Top extends Base { + public static baseFlags = { + ...Base.baseFlags + } + public static flags = { + ...Base.flags, + alias: Flags.string({ + summary: 'foo', + char: 'a' + }) + } +} +`, + }, + { + name: 'spreads base flags when required', + filename: path.normalize('src/commands/foo.ts'), + code: ` +export default class Top extends Base { + public static baseFlags = { + ...Base.baseFlags, + alias: Flags.string({ + summary: 'foo', + char: 'a' + }) + } +} +`, + }, + { + name: 'does not apply to SfCommand.flags', + filename: path.normalize('src/commands/foo.ts'), + code: ` +export default class Top extends SfCommand { + public static flags = { + alias: Flags.string({ + summary: 'foo', + char: 'a' + }) + } +} +`, + }, + { + name: 'does not apply to SfCommand baseFlags', + filename: path.normalize('src/commands/foo.ts'), + code: ` +export default class Top extends SfCommand { + public static baseFlags = { + alias: Flags.string({ + summary: 'foo', + char: 'a' + }) + } +} +`, + }, + ], + invalid: [ + { + name: 'does not spread flags, spreads baseFlags properly', + filename: path.normalize('src/commands/foo.ts'), + errors: [{ messageId: 'message' }], + code: ` +export default class Top extends BaseCommand { + public static baseFlags = {...BaseCommand.baseFlags} + public static flags = { + json: Flags.boolean({ + char: 'h', + default: true, + dependsOn: ['myOtherFlag'] + }) + } +} +`, + }, + { + name: 'does not spread base flags', + filename: path.normalize('src/commands/foo.ts'), + errors: [{ messageId: 'message' }], + code: ` +export default class Top extends BaseCommand { + public static baseFlags = { + json: Flags.boolean({ + char: 'h', + default: true, + dependsOn: ['myOtherFlag'] + }) + } +} +`, + }, + { + name: 'does not spread base flags, spreads flags', + filename: path.normalize('src/commands/foo.ts'), + errors: [{ messageId: 'message' }], + code: ` +export default class Top extends BaseCommand { + public static flags={...BaseCommand.flags}; + public static baseFlags = { + json: Flags.boolean({ + char: 'h', + default: true, + dependsOn: ['myOtherFlag'] + }) + } +} +`, + }, + ], +});