From df5e029a3679efae980227aeb48ceed6fe21788e Mon Sep 17 00:00:00 2001 From: mshanemc Date: Thu, 25 Jul 2024 17:26:28 -0500 Subject: [PATCH 1/2] refactor: pr review --- src/rules/dash-h.ts | 10 ++-- src/rules/dash-o.ts | 10 ++-- src/rules/flag-min-max-default.ts | 4 +- src/rules/flag-summary.ts | 12 ++--- src/rules/id-flag-suggestions.ts | 4 +- .../migration/encourage-alias-deprecation.ts | 49 +++++++++---------- src/rules/no-default-depends-on-flags.ts | 11 ++--- src/rules/no-depends-on-boolean-flags.ts | 19 ++++--- src/rules/no-duplicate-short-characters.ts | 8 +-- src/shared/flags.ts | 6 ++- 10 files changed, 62 insertions(+), 71 deletions(-) diff --git a/src/rules/dash-h.ts b/src/rules/dash-h.ts index 48c5c90..9042635 100644 --- a/src/rules/dash-h.ts +++ b/src/rules/dash-h.ts @@ -33,13 +33,9 @@ export const dashH = RuleCreator.withoutDocs({ node.value?.type === AST_NODE_TYPES.CallExpression && node.value.arguments?.[0]?.type === AST_NODE_TYPES.ObjectExpression ) { - const hChar = node.value.arguments[0].properties.find( - (property) => - property.type === AST_NODE_TYPES.Property && - flagPropertyIsNamed(property, 'char') && - property.value.type === AST_NODE_TYPES.Literal && - property.value.value === 'h' - ); + const hChar = node.value.arguments[0].properties + .filter(flagPropertyIsNamed('char')) + .find((property) => property.value.type === AST_NODE_TYPES.Literal && property.value.value === 'h'); if (hChar) { context.report({ node: hChar, diff --git a/src/rules/dash-o.ts b/src/rules/dash-o.ts index 43b3baf..f739e6e 100644 --- a/src/rules/dash-o.ts +++ b/src/rules/dash-o.ts @@ -38,13 +38,9 @@ export const dashO = RuleCreator.withoutDocs({ !node.value.callee.property.name.toLowerCase().includes('hub') && node.value.arguments?.[0]?.type === AST_NODE_TYPES.ObjectExpression ) { - const hChar = node.value.arguments[0].properties.find( - (property) => - property.type === AST_NODE_TYPES.Property && - flagPropertyIsNamed(property, 'char') && - property.value.type === AST_NODE_TYPES.Literal && - property.value.value === 'o' - ); + const hChar = node.value.arguments[0].properties + .filter(flagPropertyIsNamed('char')) + .find((property) => property.value.type === AST_NODE_TYPES.Literal && property.value.value === 'o'); if (hChar) { context.report({ node: hChar, diff --git a/src/rules/flag-min-max-default.ts b/src/rules/flag-min-max-default.ts index 81a2fe7..ff723c9 100644 --- a/src/rules/flag-min-max-default.ts +++ b/src/rules/flag-min-max-default.ts @@ -35,13 +35,13 @@ export const flagMinMaxDefault = RuleCreator.withoutDocs({ node.value.arguments[0].properties.some( (property) => property.type === AST_NODE_TYPES.Property && - (flagPropertyIsNamed(property, 'min') || flagPropertyIsNamed(property, 'max')) + (flagPropertyIsNamed('min')(property) || flagPropertyIsNamed('max')(property)) ) && !node.value.arguments[0].properties.some( (property) => property.type === AST_NODE_TYPES.Property && // defaultValue for DurationFlags - (flagPropertyIsNamed(property, 'default') || flagPropertyIsNamed(property, 'defaultValue')) + (flagPropertyIsNamed('default')(property) || flagPropertyIsNamed('defaultValue')(property)) ) ) { context.report({ diff --git a/src/rules/flag-summary.ts b/src/rules/flag-summary.ts index 795da52..02fbe35 100644 --- a/src/rules/flag-summary.ts +++ b/src/rules/flag-summary.ts @@ -38,10 +38,8 @@ export const flagSummary = RuleCreator.withoutDocs({ ASTUtils.isNodeOfType(AST_NODE_TYPES.Property) ); - if (!propertyArguments.some((property) => flagPropertyIsNamed(property, 'summary'))) { - const descriptionProp = propertyArguments.find((property) => - flagPropertyIsNamed(property, 'description') - ); + if (!propertyArguments.some(flagPropertyIsNamed('summary'))) { + const descriptionProp = propertyArguments.find(flagPropertyIsNamed('description')); const range = descriptionProp && 'key' in descriptionProp ? descriptionProp?.key.range : undefined; return context.report({ @@ -55,11 +53,9 @@ export const flagSummary = RuleCreator.withoutDocs({ : {}), }); } - if (!propertyArguments.some((property) => flagPropertyIsNamed(property, 'description'))) { + if (!propertyArguments.some(flagPropertyIsNamed('description'))) { // if there is no description, but there is a longDescription, turn that into the description - const longDescriptionProp = propertyArguments.find((property) => - flagPropertyIsNamed(property, 'longDescription') - ); + const longDescriptionProp = propertyArguments.find(flagPropertyIsNamed('longDescription')); if (!longDescriptionProp) { return; } diff --git a/src/rules/id-flag-suggestions.ts b/src/rules/id-flag-suggestions.ts index f6c2194..54a79bb 100644 --- a/src/rules/id-flag-suggestions.ts +++ b/src/rules/id-flag-suggestions.ts @@ -44,8 +44,8 @@ export const idFlagSuggestions = RuleCreator.withoutDocs({ const argProps = node.value.arguments[0].properties.filter( ASTUtils.isNodeOfType(AST_NODE_TYPES.Property) ); - const hasStartsWith = argProps.some((property) => flagPropertyIsNamed(property, 'startsWith')); - const hasLength = argProps.some((property) => flagPropertyIsNamed(property, 'length')); + const hasStartsWith = argProps.some(flagPropertyIsNamed('startsWith')); + const hasLength = argProps.some(flagPropertyIsNamed('length')); if (!hasStartsWith || !hasLength) { const existing = context.sourceCode.getText(node); diff --git a/src/rules/migration/encourage-alias-deprecation.ts b/src/rules/migration/encourage-alias-deprecation.ts index 6008260..b4e8c72 100644 --- a/src/rules/migration/encourage-alias-deprecation.ts +++ b/src/rules/migration/encourage-alias-deprecation.ts @@ -32,30 +32,29 @@ export const encourageAliasDeprecation = RuleCreator.withoutDocs({ return isInCommandDirectory(context) ? { PropertyDefinition(node): void { - if (ancestorsContainsSfCommand(context)) { - if (node.key.type === AST_NODE_TYPES.Identifier && node.key.name === 'aliases') { - // but you don't have deprecateAliases = true then add id - if ( - node.parent?.type === AST_NODE_TYPES.ClassBody && - !node.parent.body.some( - (n) => - n.type === AST_NODE_TYPES.PropertyDefinition && - n.key.type === AST_NODE_TYPES.Identifier && - n.key.name === 'deprecateAliases' - ) - ) { - context.report({ - node, + if ( + ancestorsContainsSfCommand(context) && + node.key.type === AST_NODE_TYPES.Identifier && + node.key.name === 'aliases' && + node.parent?.type === AST_NODE_TYPES.ClassBody && + !node.parent.body.some( + (n) => + n.type === AST_NODE_TYPES.PropertyDefinition && + n.key.type === AST_NODE_TYPES.Identifier && + n.key.name === 'deprecateAliases' + ) + ) { + // but you don't have deprecateAliases = true then add id + context.report({ + node, + messageId: 'command', + suggest: [ + { messageId: 'command', - suggest: [ - { - messageId: 'command', - fix: (fixer) => fixer.insertTextBefore(node, 'public static readonly deprecateAliases = true;'), - }, - ], - }); - } - } + fix: (fixer) => fixer.insertTextBefore(node, 'public static readonly deprecateAliases = true;'), + }, + ], + }); } }, Property(node): void { @@ -69,8 +68,8 @@ export const encourageAliasDeprecation = RuleCreator.withoutDocs({ ASTUtils.isNodeOfType(AST_NODE_TYPES.Property) ); - const aliasesProperty = argProps.find((property) => flagPropertyIsNamed(property, 'aliases')); - if (aliasesProperty && !argProps.some((property) => flagPropertyIsNamed(property, 'deprecateAliases'))) { + const aliasesProperty = argProps.find(flagPropertyIsNamed('aliases')); + if (aliasesProperty && !argProps.some(flagPropertyIsNamed('deprecateAliases'))) { context.report({ node: aliasesProperty, messageId: 'flag', diff --git a/src/rules/no-default-depends-on-flags.ts b/src/rules/no-default-depends-on-flags.ts index ef15907..e2ecf6f 100644 --- a/src/rules/no-default-depends-on-flags.ts +++ b/src/rules/no-default-depends-on-flags.ts @@ -4,7 +4,7 @@ * 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 { AST_NODE_TYPES, ASTUtils } from '@typescript-eslint/utils'; import { RuleCreator } from '@typescript-eslint/utils/eslint-utils'; import { ancestorsContainsSfCommand, isInCommandDirectory } from '../shared/commands'; import { flagPropertyIsNamed, isFlag } from '../shared/flags'; @@ -33,12 +33,9 @@ export const noDefaultDependsOnFlags = RuleCreator.withoutDocs({ 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') - ); + const props = node.value.arguments[0].properties.filter(ASTUtils.isNodeOfType(AST_NODE_TYPES.Property)); + const dependsOnProperty = props.find(flagPropertyIsNamed('dependsOn')); + const defaultValueProperty = props.find(flagPropertyIsNamed('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 diff --git a/src/rules/no-depends-on-boolean-flags.ts b/src/rules/no-depends-on-boolean-flags.ts index c49f82e..bce365f 100644 --- a/src/rules/no-depends-on-boolean-flags.ts +++ b/src/rules/no-depends-on-boolean-flags.ts @@ -4,10 +4,10 @@ * 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 { AST_NODE_TYPES, ASTUtils } from '@typescript-eslint/utils'; import { RuleCreator } from '@typescript-eslint/utils/eslint-utils'; import { ancestorsContainsSfCommand, isInCommandDirectory } from '../shared/commands'; -import { isFlag } from '../shared/flags'; +import { flagPropertyIsNamed, isFlag } from '../shared/flags'; export const noDependsOnBooleanFlags = RuleCreator.withoutDocs({ meta: { @@ -32,13 +32,18 @@ export const noDependsOnBooleanFlags = RuleCreator.withoutDocs({ node.value?.type === AST_NODE_TYPES.CallExpression && node.value.arguments?.[0]?.type === AST_NODE_TYPES.ObjectExpression ) { - const dependsOnFlagsProp = node.value.arguments[0].properties.find( - (p) => p.type === 'Property' && p.key.type === 'Identifier' && p.key.name === 'dependsOn' - ); + const dependsOnFlagsProp = node.value.arguments[0].properties + .filter(ASTUtils.isNodeOfType(AST_NODE_TYPES.Property)) + .find(flagPropertyIsNamed('dependsOn')); if (dependsOnFlagsProp) { - // @ts-ignore we know `dependsOn` exists/is an array - const dependedOnFlags = dependsOnFlagsProp.value.elements.map((l) => l.value); + const dependedOnFlags = + 'value' in dependsOnFlagsProp && + ASTUtils.isNodeOfType(AST_NODE_TYPES.ArrayExpression)(dependsOnFlagsProp.value) + ? dependsOnFlagsProp.value.elements + .filter(ASTUtils.isNodeOfType(AST_NODE_TYPES.Literal)) + .map((l) => l.value) + : []; for (const flag of dependedOnFlags) { if (node.parent.type === 'ObjectExpression') { diff --git a/src/rules/no-duplicate-short-characters.ts b/src/rules/no-duplicate-short-characters.ts index f91af9e..f727fab 100644 --- a/src/rules/no-duplicate-short-characters.ts +++ b/src/rules/no-duplicate-short-characters.ts @@ -61,7 +61,7 @@ export const noDuplicateShortCharacters = RuleCreator.withoutDocs({ ); // 2. has the char already been used? If so, mark the char as a problem const charNode = flagProperties.find( - (p) => flagPropertyIsNamed(p, 'char') && p.value.type === AST_NODE_TYPES.Literal + (p) => flagPropertyIsNamed('char')(p) && p.value.type === AST_NODE_TYPES.Literal ); if (charNode?.value.type === AST_NODE_TYPES.Literal) { const char = charNode.value.value; @@ -81,9 +81,9 @@ export const noDuplicateShortCharacters = RuleCreator.withoutDocs({ } // 3. is anything in this this flag's aliases already seen (alias or char)? If so, mark that alias as a problem - const aliasesNode = flagProperties.find( - (p) => flagPropertyIsNamed(p, 'aliases') && p.value.type === AST_NODE_TYPES.ArrayExpression - ); + const aliasesNode = flagProperties + .filter(flagPropertyIsNamed('aliases')) + .find((p) => p.value.type === AST_NODE_TYPES.ArrayExpression); if (aliasesNode?.value.type === AST_NODE_TYPES.ArrayExpression) { aliasesNode.value.elements.forEach((alias) => { if (alias?.type === AST_NODE_TYPES.Literal) diff --git a/src/shared/flags.ts b/src/shared/flags.ts index 0a05ea5..2741ddf 100644 --- a/src/shared/flags.ts +++ b/src/shared/flags.ts @@ -34,8 +34,10 @@ export const isBaseFlagsStaticProperty = (node: TSESTree.Node): node is TSESTree node.key.name === 'baseFlags' && ['public', 'protected'].includes(node.accessibility); -export const flagPropertyIsNamed = (node: TSESTree.Property, name: string): node is TSESTree.Property => - resolveFlagName(node) === name; +export const flagPropertyIsNamed = + (name: string) => + (node: TSESTree.Property): node is TSESTree.Property => + resolveFlagName(node) === name; /** pass in a flag Property and it gives back the key name/value depending on type */ export const resolveFlagName = ( From 9829eb50f983ea48d6a4931f8837985c55e89a4b Mon Sep 17 00:00:00 2001 From: mshanemc Date: Thu, 25 Jul 2024 17:34:17 -0500 Subject: [PATCH 2/2] refactor: min/max/default --- src/rules/flag-min-max-default.ts | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/rules/flag-min-max-default.ts b/src/rules/flag-min-max-default.ts index ff723c9..22d4f04 100644 --- a/src/rules/flag-min-max-default.ts +++ b/src/rules/flag-min-max-default.ts @@ -4,7 +4,7 @@ * 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 { AST_NODE_TYPES, ASTUtils } from '@typescript-eslint/utils'; import { RuleCreator } from '@typescript-eslint/utils/eslint-utils'; import { ancestorsContainsSfCommand, isInCommandDirectory } from '../shared/commands'; @@ -30,24 +30,18 @@ export const flagMinMaxDefault = RuleCreator.withoutDocs({ if (isFlag(node) && ancestorsContainsSfCommand(context)) { if ( node.value?.type === AST_NODE_TYPES.CallExpression && - node.value.arguments?.[0]?.type === AST_NODE_TYPES.ObjectExpression && - // has min/max - node.value.arguments[0].properties.some( - (property) => - property.type === AST_NODE_TYPES.Property && - (flagPropertyIsNamed('min')(property) || flagPropertyIsNamed('max')(property)) - ) && - !node.value.arguments[0].properties.some( - (property) => - property.type === AST_NODE_TYPES.Property && - // defaultValue for DurationFlags - (flagPropertyIsNamed('default')(property) || flagPropertyIsNamed('defaultValue')(property)) - ) + node.value.arguments?.[0]?.type === AST_NODE_TYPES.ObjectExpression ) { - context.report({ - node, - messageId: 'message', - }); + const props = node.value.arguments[0].properties.filter(ASTUtils.isNodeOfType(AST_NODE_TYPES.Property)); + if ( + props.some((p) => flagPropertyIsNamed('min')(p) || flagPropertyIsNamed('max')(p)) && + !props.some((p) => flagPropertyIsNamed('default')(p) || flagPropertyIsNamed('defaultValue')(p)) + ) { + context.report({ + node, + messageId: 'message', + }); + } } } },