Skip to content

Commit

Permalink
feat: add spread-flags/only-extend-SfCommand/`no-default-and-depe…
Browse files Browse the repository at this point in the history
…nds-on-flags` rules (#440)

* feat: add no default and depends on flag configuration rule

* chore: edge case

* chore: fix circular error

* docs: generate docs

* chore: fix [0]

* chore: fix [0] (2)

* 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
  • Loading branch information
WillieRuemmele authored Jul 22, 2024
1 parent 07ee26b commit d4e7391
Show file tree
Hide file tree
Showing 14 changed files with 521 additions and 6 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | 💼 | ⚠️ | 🚫 | 🔧 | 💡 |
| :------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------------------- | :------ | :--- | :------ | :- | :- |
Expand All @@ -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 | | | 📚 ✈️ ✅ | 🔧 | |
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/encourage-alias-deprecation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

<!-- end auto-generated rule header -->
2 changes: 1 addition & 1 deletion docs/rules/id-flag-suggestions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

<!-- end auto-generated rule header -->
5 changes: 5 additions & 0 deletions docs/rules/no-default-and-depends-on-flags.md
Original file line number Diff line number Diff line change
@@ -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`.

<!-- end auto-generated rule header -->
2 changes: 1 addition & 1 deletion docs/rules/no-this-flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

<!-- end auto-generated rule header -->
2 changes: 1 addition & 1 deletion docs/rules/no-this-org.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

<!-- end auto-generated rule header -->
13 changes: 12 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +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 { onlyExtendSfCommand } from './rules/only-extend-sfCommand';
import { spreadBaseFlags } from './rules/spread-base-flags';

const library = {
plugins: ['sf-plugin'],
Expand Down Expand Up @@ -84,6 +87,9 @@ 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',
'sf-plugin/only-extend-SfCommand': 'warn',
'sf-plugin/spread-base-flags': 'warn',
},
};

Expand All @@ -107,13 +113,18 @@ export = {
'sf-plugin/no-time-flags': 'error',
'sf-plugin/no-id-flags': 'error',
'sf-plugin/no-username-properties': 'error',
'sf-plugin/encourage-alias-deprecation': 'warn',
'sf-plugin/no-default-and-depends-on-flags': 'error',
'sf-plugin/only-extend-SfCommand': 'warn',
'sf-plugin/spread-base-flags': 'error',
},
},
},
rules: {
'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,
Expand Down
63 changes: 63 additions & 0 deletions src/rules/no-default-depends-on-flags.ts
Original file line number Diff line number Diff line change
@@ -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 } 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 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(
(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?.at(0)
?.properties.find((p) => p.key.name === 'default');
if (dependsOnProperty && defaultValueProperty && !dependsOnFlagDefaultValue) {
context.report({
node: dependsOnProperty,
messageId: 'message',
});
}
}
},
}
: {};
},
});
41 changes: 41 additions & 0 deletions src/rules/only-extend-sfCommand.ts
Original file line number Diff line number Diff line change
@@ -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',
});
}
},
}
: {};
},
});
74 changes: 74 additions & 0 deletions src/rules/spread-base-flags.ts
Original file line number Diff line number Diff line change
@@ -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' },
});
}
}
},
}
: {};
},
});
17 changes: 17 additions & 0 deletions src/shared/flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down
Loading

0 comments on commit d4e7391

Please sign in to comment.