Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add no default and depends on flag configuration rule #440

Merged
merged 7 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading