Skip to content

Commit

Permalink
feat: username property conversion and removal of unnecessary default…
Browse files Browse the repository at this point in the history
… props

* feat: username property conversion

* chore: docs

* fix: better targetting of import

* fix: output node selection and undefined handling

* feat: remove unnecessary default=false props
  • Loading branch information
mshanemc authored Jan 3, 2023
1 parent 95d790f commit ceebd69
Show file tree
Hide file tree
Showing 15 changed files with 482 additions and 25 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,20 @@ module.exports = {
| [no-builtin-flags](docs/rules/no-builtin-flags.md) | Handling for sfdxCommand's flags.builtin | ✈️ | | 🔧 | |
| [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-filepath-flags](docs/rules/no-filepath-flags.md) | Change filepath flag to file flag | | | 🔧 | |
| [no-h-short-char](docs/rules/no-h-short-char.md) | Do not allow creation of a flag with short char -h | ✈️ ✅ | | | |
| [no-hardcoded-messages-commands](docs/rules/no-hardcoded-messages-commands.md) | Use loaded messages and separate files for messages | | ✈️ ✅ | | |
| [no-hardcoded-messages-flags](docs/rules/no-hardcoded-messages-flags.md) | Use loaded messages and separate files for messages | | ✈️ ✅ | | |
| [no-id-flags](docs/rules/no-id-flags.md) | Change Id flag to salesforceId | ✈️ | | 🔧 | |
| [no-json-flag](docs/rules/no-json-flag.md) | Do not allow creation of json flag | ✈️ ✅ | | | |
| [no-number-flags](docs/rules/no-number-flags.md) | Change number flag to integer | | | 🔧 | |
| [no-oclif-flags-command-import](docs/rules/no-oclif-flags-command-import.md) | Change import of flags and Command from oclif to use sf-plugins-core | ✈️ ✅ | | 🔧 | |
| [no-sfdx-command-import](docs/rules/no-sfdx-command-import.md) | Change import and base class from SfdxCommand to sfCommand | ✈️ | | 🔧 | |
| [no-this-flags](docs/rules/no-this-flags.md) | Fix references to this.org (property on SfdxCommand) | ✈️ | | 🔧 | 💡 |
| [no-this-org](docs/rules/no-this-org.md) | Fix references to this.org (property on SfdxCommand) | ✈️ | | 🔧 | 💡 |
| [no-this-ux](docs/rules/no-this-ux.md) | SfCommand does not have a ux property | ✈️ | | 🔧 | |
| [no-time-flags](docs/rules/no-time-flags.md) | Migrate time flags to Flags.duration | ✈️ | | 🔧 | |
| [no-username-properties](docs/rules/no-username-properties.md) | Convert requiresUsername and supportusername to username flags | ✈️ | | 🔧 | |
| [read-only-properties](docs/rules/read-only-properties.md) | Class-level static properties, like flags or descriptions, should be marked read-only | | ✈️ ✅ | 🔧 | |
| [run-matches-class-type](docs/rules/run-matches-class-type.md) | The return type of the run method should match the Type passed to sfCommand | ✈️ ✅ | | 🔧 | |
| [sfdx-flags-property](docs/rules/sfdx-flags-property.md) | Change flag definitions to SfCommand version | ✈️ | | 🔧 | |
Expand Down
5 changes: 5 additions & 0 deletions docs/rules/no-filepath-flags.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Change filepath flag to file flag (`sf-plugin/no-filepath-flags`)

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->
5 changes: 5 additions & 0 deletions docs/rules/no-number-flags.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Change number flag to integer (`sf-plugin/no-number-flags`)

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->
7 changes: 7 additions & 0 deletions docs/rules/no-split-examples.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Arrays of messags should use getMessages instead of getMessage followed by EOL splitting (`sf-plugin/no-split-examples`)

💼 This rule is enabled 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).

<!-- end auto-generated rule header -->
7 changes: 7 additions & 0 deletions docs/rules/no-username-properties.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Convert requiresUsername and supportusername to username flags (`sf-plugin/no-username-properties`)

💼 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).

<!-- end auto-generated rule header -->
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@
"lint": "yarn eslint .",
"test": "jest --coverage",
"test:watch": "jest --watch",
"docs": "eslint-doc-generator --config-emoji migration,✈️"
"docs": "eslint-doc-generator --config-emoji migration,✈️",
"docs:init": "eslint-doc-generator --config-emoji migration,✈️ --init-rule-docs"
},
"homepage": "https://github.com/salesforcecli/eslint-plugin-sf-plugin",
"publishConfig": {
Expand Down
6 changes: 6 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import { idFlagSuggestions } from './rules/id-flag-suggestions';
import { noIdFlags } from './rules/migration/no-id-flags';
import { noFilepathFlags } from './rules/migration/no-filepath-flags';
import { noNumberFlags } from './rules/migration/no-number-flags';
import { noUsernameProperties } from './rules/migration/no-username-properties';
import { noUnnecessaryProperties } from './rules/no-unnecessary-properties';

const recommended = {
plugins: ['sf-plugin'],
Expand All @@ -55,6 +57,7 @@ const recommended = {
'sf-plugin/no-oclif-flags-command-import': 'error',
'sf-plugin/read-only-properties': 'warn',
'sf-plugin/id-flag-suggestions': 'warn',
'sf-plugin/no-unnecessary-properties': 'warn',
},
};
export = {
Expand All @@ -75,6 +78,7 @@ export = {
'sf-plugin/no-builtin-flags': 'error',
'sf-plugin/no-time-flags': 'error',
'sf-plugin/no-id-flags': 'error',
'sf-plugin/no-username-properties': 'error',
},
},
},
Expand Down Expand Up @@ -109,5 +113,7 @@ export = {
'no-id-flags': noIdFlags,
'no-filepath-flags': noFilepathFlags,
'no-number-flags': noNumberFlags,
'no-username-properties': noUsernameProperties,
'no-unnecessary-properties': noUnnecessaryProperties,
},
};
9 changes: 1 addition & 8 deletions src/rules/migration/no-deprecated-properties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,7 @@ export const noDeprecatedProperties = ESLintUtils.RuleCreator.withoutDocs({
if (ancestorsContainsSfCommand(context.getAncestors())) {
if (
node.key.type === AST_NODE_TYPES.Identifier &&
[
'requiresUsername',
'supportUsername',
'supportsDevhubUsername',
'requiresDevhubUsername',
'varargs',
'args',
].includes(node.key.name)
['supportsDevhubUsername', 'varargs', 'args'].includes(node.key.name)
) {
context.report({
node,
Expand Down
122 changes: 122 additions & 0 deletions src/rules/migration/no-username-properties.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* 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,
getSfCommand,
getSfImportFromProgram,
} from '../../shared/commands';
import { getFlagsStaticPropertyFromCommandClass } from '../../shared/flags';

const propertyMap = new Map<
string,
{ flag: string; message: 'requires' | 'supports' | 'requiresHub'; flagName: string }
>([
[
'requiresUsername',
{
flag: 'requiredOrgFlagWithDeprecations',
flagName: 'target-org',
message: 'requires',
},
],
['supportsUsername', { flag: 'optionalOrgFlagWithDeprecations', flagName: 'target-org', message: 'supports' }],
[
'requiresDevhubUsername',
{
flag: 'requiredHubFlagWithDeprecations',
flagName: 'target-dev-hub',

message: 'requiresHub',
},
],
]);

export const noUsernameProperties = ESLintUtils.RuleCreator.withoutDocs({
meta: {
docs: {
description: 'Convert requiresUsername and supportusername to username flags',
recommended: 'error',
},
messages: {
requires:
'Class property requiresUsername is not available on SfCommand and should be removed. Import `requiredOrgFlagWithDeprecations` from `@salesforce/sf-plugins-core` and add it to the flags object.',
requiresHub:
'Class property requiresDevhubUsername is not available on SfCommand and should be removed. Import `requiredHubFlagWithDeprecations` from `@salesforce/sf-plugins-core` and add it to the flags object.',
supports:
'Class property supportUsername is not available on SfCommand and should be removed. Import `optionalOrgFlagWithDeprecations` from `@salesforce/sf-plugins-core` and add it to the flags object.',
},
type: 'problem',
schema: [],
fixable: 'code',
},
defaultOptions: [],
create(context) {
return isInCommandDirectory(context)
? {
PropertyDefinition(node): void {
if (ancestorsContainsSfCommand(context.getAncestors())) {
if (node.key.type === AST_NODE_TYPES.Identifier && propertyMap.has(node.key.name)) {
const mappedMetadata = propertyMap.get(node.key.name);

// ensure the import exists
const ancestors = context.getAncestors();
const source = context.getSourceCode();
const importDeclaration = getSfImportFromProgram(ancestors[0]);
if (importDeclaration && !source.getText(importDeclaration).includes(mappedMetadata.flag)) {
const fixedImport = source
.getText(importDeclaration)
.replace(/{(.*)}/g, `{$1, ${mappedMetadata.flag}}`);
context.report({
node: importDeclaration,
messageId: mappedMetadata.message,
fix: (fixer) => {
return fixer.replaceText(importDeclaration, fixedImport);
},
});
}

// add the flag if not already present

const outerClass = getSfCommand(ancestors);
const flagsProperty = getFlagsStaticPropertyFromCommandClass(outerClass);

if (flagsProperty && !source.getText(flagsProperty).includes(mappedMetadata.flag)) {
const addedFlag = `'${mappedMetadata.flagName}': ${mappedMetadata.flag},`;
context.report({
node,
messageId: mappedMetadata.message,
fix: (fixer) => {
return fixer.insertTextAfterRange(
[flagsProperty.value.range[0] + 1, flagsProperty.value.range[0] + 1],
addedFlag
);
},
});
}

if (source.getText(importDeclaration).includes(mappedMetadata.flag)) {
// remove the property only after the other two fixes have been applied
context.report({
node,
messageId: mappedMetadata.message,
data: {
property: node.key.name,
},
fix: (fixer) => {
return fixer.remove(node);
},
});
}
}
}
},
}
: {};
},
});
52 changes: 52 additions & 0 deletions src/rules/no-unnecessary-properties.ts
Original file line number Diff line number Diff line change
@@ -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 { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils';
import { extendsSfCommand, isInCommandDirectory } from '../shared/commands';

const props = ['requiresProject', 'hidden', 'deprecateAliases'];

export const noUnnecessaryProperties = ESLintUtils.RuleCreator.withoutDocs({
meta: {
docs: {
description: 'Boolean properties are false by default, so they should not be set to false',
recommended: 'warn',
},
messages: {
message: 'The {{prop}} property can be omitted since it is false by default',
},
type: 'problem',
fixable: 'code',
schema: [],
},
defaultOptions: [],
create(context) {
return isInCommandDirectory(context)
? {
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),
});
}
},
}
: {};
},
});
11 changes: 11 additions & 0 deletions src/shared/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,14 @@ export const isRunMethod = (node: TSESTree.Node): boolean =>

export const getRunMethod = (node: TSESTree.ClassDeclaration): TSESTree.ClassElement =>
node.body.body.find((b) => isRunMethod(b));

export const getSfImportFromProgram = (node: TSESTree.Node): TSESTree.ImportDeclaration | undefined => {
if (node.type === AST_NODE_TYPES.Program) {
return node.body.find(
(item): item is TSESTree.ImportDeclaration =>
item.type === AST_NODE_TYPES.ImportDeclaration &&
item.source.type === AST_NODE_TYPES.Literal &&
item.source.value === '@salesforce/sf-plugins-core'
);
}
};
10 changes: 9 additions & 1 deletion src/shared/flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const isFlag = (node: TSESTree.Node): boolean =>
node.value?.callee?.object?.name === 'Flags';

/** Current node is public static flags = */
export const isFlagsStaticProperty = (node: TSESTree.Node): boolean =>
export const isFlagsStaticProperty = (node: TSESTree.Node): node is TSESTree.PropertyDefinition =>
node.type === AST_NODE_TYPES.PropertyDefinition &&
node.static &&
node.value?.type === AST_NODE_TYPES.ObjectExpression &&
Expand All @@ -49,3 +49,11 @@ export const resolveFlagName = (flag: TSESTree.PropertyComputedName | TSESTree.P
return flag.key.value;
}
};

export const getFlagsStaticPropertyFromCommandClass = (
classDeclaration: TSESTree.ClassDeclaration
): TSESTree.PropertyDefinition | undefined => {
if (classDeclaration.body.type === AST_NODE_TYPES.ClassBody) {
return classDeclaration.body.body.find(isFlagsStaticProperty);
}
};
21 changes: 6 additions & 15 deletions test/rules/migration/no-deprecated-properties.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default class EnvCreateScratch extends SfCommand<ScratchCreateResponse> {
filename: path.normalize('foo.ts'),
code: `
export default class EnvCreateScratch extends SfCommand<string> {
public static readonly requiresUsername = true;
public static readonly args = true;
}
`,
Expand All @@ -40,12 +40,12 @@ export default class EnvCreateScratch extends SfCommand<string> {
invalid: [
//
{
name: 'requiresUsername',
name: 'args',
filename: path.normalize('src/commands/foo.ts'),
errors: [{ messageId: 'property', data: { property: 'requiresUsername' } }],
errors: [{ messageId: 'property', data: { property: 'args' } }],
code: `
export default class EnvCreateScratch extends SfCommand<Foo> {
public static readonly requiresUsername = true;
public static readonly args = 'foo';
public static ok = true;
}`,
output: `
Expand All @@ -58,28 +58,19 @@ export default class EnvCreateScratch extends SfCommand<Foo> {
name: 'all the invalid things',
filename: path.normalize('src/commands/foo.ts'),
errors: [
{ messageId: 'property', data: { property: 'requiresUsername' } },
{ messageId: 'property', data: { property: 'supportUsername' } },
{ messageId: 'property', data: { property: 'supportsDevhubUsername' } },
{ messageId: 'property', data: { property: 'requiresDevhubUsername' } },
{ messageId: 'property', data: { property: 'varargs' } },
{ messageId: 'property', data: { property: 'args' } },
],
code: `
export default class EnvCreateScratch extends SfCommand<Foo> {
public static readonly requiresUsername = true;
public static readonly supportUsername = true;
public static readonly supportsDevhubUsername = true;
public static readonly requiresDevhubUsername = true;
public static readonly varargs = true;
public static readonly args = true;
public static readonly requiresProject = true;
}`,
output: `
export default class EnvCreateScratch extends SfCommand<Foo> {
public static readonly requiresProject = true;
}`,
},
Expand Down
Loading

0 comments on commit ceebd69

Please sign in to comment.