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 6 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 -->
4 changes: 4 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -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',
},
};

Expand All @@ -107,13 +109,15 @@ 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',
},
},
},
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,
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',
});
}
}
},
}
: {};
},
});
95 changes: 95 additions & 0 deletions test/rules/no-default-depends-on-flags.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* 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<ScratchCreateResponse> {
public static flags = {
alias: Flags.string({
summary: 'foo',
char: 'a'
})
}
}
`,
},
{
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<ScratchCreateResponse> {
public static flags = {
alias: Flags.string({
summary: 'foo',
default: 'a',
dependsOn: ['balias'],
char: 'a'
}),
balias: Flags.string({
summary: 'baz',
default: 'b',
char: 'b'
})
}
}

`,
},
],
invalid: [
{
name: 'uses dependsOn and default',
filename: path.normalize('src/commands/foo.ts'),
errors: [{ messageId: 'message' }],
code: `
export default class EnvCreateScratch extends SfCommand<Foo> {
public static flags = {
json: Flags.boolean({
char: 'h',
default: true,
dependsOn: ['myOtherFlag']
})
}
}
`,
},

{
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<Foo> {
public static flags = {
alias: Flags.string({
summary: 'foo',
default: 'a',
dependsOn: ['balias'],
char: 'a'
}),
balias: Flags.string({
summary: 'baz',
char: 'b'
})
}
}
`,
},
],
});
Loading