From 361e6a41c06cbbb3b8d730b0388a89d63f35b7d0 Mon Sep 17 00:00:00 2001 From: Fanis Prodromou Date: Tue, 1 Aug 2023 11:01:57 +0300 Subject: [PATCH 1/3] fix(eslint-plugin): [no-output-rename] do not report on directive composition API --- .../docs/rules/no-output-rename.md | 99 +++++++++++++++++++ .../src/rules/no-output-rename.ts | 22 +++++ .../tests/rules/no-output-rename/cases.ts | 34 +++++++ 3 files changed, 155 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/no-output-rename.md b/packages/eslint-plugin/docs/rules/no-output-rename.md index 733aa2a79..ac8617f3a 100644 --- a/packages/eslint-plugin/docs/rules/no-output-rename.md +++ b/packages/eslint-plugin/docs/rules/no-output-rename.md @@ -725,6 +725,105 @@ class Test { #### ✅ Valid Code +```ts +@Component({ + selector: 'foo', + hostDirectives: [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-output-rename": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'foo', + 'hostDirectives': [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-output-rename": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'foo', + ['hostDirectives']: [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-output-rename": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + ```ts @Directive({ selector: 'foo' diff --git a/packages/eslint-plugin/src/rules/no-output-rename.ts b/packages/eslint-plugin/src/rules/no-output-rename.ts index 23f71258a..30033ba5d 100644 --- a/packages/eslint-plugin/src/rules/no-output-rename.ts +++ b/packages/eslint-plugin/src/rules/no-output-rename.ts @@ -98,6 +98,28 @@ export default createESLintRule({ [Selectors.OUTPUTS_METADATA_PROPERTY_LITERAL]( node: TSESTree.Literal | TSESTree.TemplateElement, ) { + const ancestorMaybeHostDirectiveAPI = node.parent?.parent?.parent; + if ( + ancestorMaybeHostDirectiveAPI && + ASTUtils.isProperty(ancestorMaybeHostDirectiveAPI) + ) { + /** + * Angular v15 introduced the directive composition API: https://angular.io/guide/directive-composition-api + * Renaming host directive outputs using this API is not a bad practice and should not be reported + */ + const hostDirectiveAPIPropertyName = 'hostDirectives'; + if ( + (ASTUtils.isLiteral(ancestorMaybeHostDirectiveAPI.key) && + ancestorMaybeHostDirectiveAPI.key.value === + hostDirectiveAPIPropertyName) || + (TSESLintASTUtils.isIdentifier(ancestorMaybeHostDirectiveAPI.key) && + ancestorMaybeHostDirectiveAPI.key.name === + hostDirectiveAPIPropertyName) + ) { + return; + } + } + const [propertyName, aliasName] = withoutBracketsAndWhitespaces( ASTUtils.getRawText(node), ).split(':'); diff --git a/packages/eslint-plugin/tests/rules/no-output-rename/cases.ts b/packages/eslint-plugin/tests/rules/no-output-rename/cases.ts index fc9539e3f..692e0a166 100644 --- a/packages/eslint-plugin/tests/rules/no-output-rename/cases.ts +++ b/packages/eslint-plugin/tests/rules/no-output-rename/cases.ts @@ -108,6 +108,40 @@ export const valid = [ @Output('foo') label: string; } `, + /** + * Renaming outputs when using the directive composition API is not a bad practice + * https://angular.io/guide/directive-composition-api + */ + ` + @Component({ + selector: 'foo', + hostDirectives: [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] + }) + class Test {} + `, + ` + @Component({ + selector: 'foo', + 'hostDirectives': [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] + }) + class Test {} + `, + ` + @Component({ + selector: 'foo', + ['hostDirectives']: [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] + }) + class Test {} + `, ` @Directive({ selector: 'foo' From ebae7fc76b73694f0147b325f2e9c85ec79b2a29 Mon Sep 17 00:00:00 2001 From: Fanis Prodromou Date: Tue, 1 Aug 2023 11:02:33 +0300 Subject: [PATCH 2/3] fix(eslint-plugin): [no-outputs-metadata-property] do not report on directive composition API --- .../rules/no-outputs-metadata-property.md | 99 +++++++++++++++++++ .../src/rules/no-outputs-metadata-property.ts | 27 ++++- .../no-outputs-metadata-property/cases.ts | 34 +++++++ 3 files changed, 159 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-outputs-metadata-property.md b/packages/eslint-plugin/docs/rules/no-outputs-metadata-property.md index e8b5b73a4..600025b9c 100644 --- a/packages/eslint-plugin/docs/rules/no-outputs-metadata-property.md +++ b/packages/eslint-plugin/docs/rules/no-outputs-metadata-property.md @@ -489,6 +489,105 @@ class Test {} class Test {} ``` +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-outputs-metadata-property": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'foo', + hostDirectives: [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-outputs-metadata-property": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'foo', + 'hostDirectives': [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-outputs-metadata-property": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'foo', + ['hostDirectives']: [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] +}) +class Test {} +``` +
diff --git a/packages/eslint-plugin/src/rules/no-outputs-metadata-property.ts b/packages/eslint-plugin/src/rules/no-outputs-metadata-property.ts index 881ccd5be..98f7f15c1 100644 --- a/packages/eslint-plugin/src/rules/no-outputs-metadata-property.ts +++ b/packages/eslint-plugin/src/rules/no-outputs-metadata-property.ts @@ -1,5 +1,6 @@ -import { Selectors } from '@angular-eslint/utils'; +import { ASTUtils, Selectors } from '@angular-eslint/utils'; import type { TSESTree } from '@typescript-eslint/utils'; +import { ASTUtils as TSESLintASTUtils } from '@typescript-eslint/utils'; import { createESLintRule } from '../utils/create-eslint-rule'; type Options = []; @@ -29,6 +30,30 @@ export default createESLintRule({ } ${Selectors.metadataProperty(METADATA_PROPERTY_NAME)}`]( node: TSESTree.Property, ) { + /** + * Angular v15 introduced the directive composition API: https://angular.io/guide/directive-composition-api + * Using host directive outputs using this API is not a bad practice and should not be reported + */ + const ancestorMayBeHostDirectiveAPI = node.parent?.parent?.parent; + + if ( + ancestorMayBeHostDirectiveAPI && + ASTUtils.isProperty(ancestorMayBeHostDirectiveAPI) + ) { + const hostDirectiveAPIPropertyName = 'hostDirectives'; + + if ( + (ASTUtils.isLiteral(ancestorMayBeHostDirectiveAPI.key) && + ancestorMayBeHostDirectiveAPI.key.value === + hostDirectiveAPIPropertyName) || + (TSESLintASTUtils.isIdentifier(ancestorMayBeHostDirectiveAPI.key) && + ancestorMayBeHostDirectiveAPI.key.name === + hostDirectiveAPIPropertyName) + ) { + return; + } + } + context.report({ node, messageId: 'noOutputsMetadataProperty', diff --git a/packages/eslint-plugin/tests/rules/no-outputs-metadata-property/cases.ts b/packages/eslint-plugin/tests/rules/no-outputs-metadata-property/cases.ts index daa6aed39..e09dd4ee7 100644 --- a/packages/eslint-plugin/tests/rules/no-outputs-metadata-property/cases.ts +++ b/packages/eslint-plugin/tests/rules/no-outputs-metadata-property/cases.ts @@ -45,6 +45,40 @@ export const valid = [ }) class Test {} `, + /** + * Renaming outputs when using the directive composition API is not a bad practice + * https://angular.io/guide/directive-composition-api + */ + ` + @Component({ + selector: 'foo', + hostDirectives: [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] + }) + class Test {} + `, + ` + @Component({ + selector: 'foo', + 'hostDirectives': [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] + }) + class Test {} + `, + ` + @Component({ + selector: 'foo', + ['hostDirectives']: [{ + directive: CdkMenuItem, + outputs: ['cdkMenuItemTriggered: triggered'], + }] + }) + class Test {} + `, ]; export const invalid = [ From 69c90a8470d2da171ed825e398ee5fc5ebd594a1 Mon Sep 17 00:00:00 2001 From: Fanis Prodromou Date: Mon, 21 Aug 2023 10:46:44 +0300 Subject: [PATCH 3/3] fix(angular-eslint): fix unit tests --- packages/eslint-plugin/src/rules/no-output-rename.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-output-rename.ts b/packages/eslint-plugin/src/rules/no-output-rename.ts index 30033ba5d..6d446d085 100644 --- a/packages/eslint-plugin/src/rules/no-output-rename.ts +++ b/packages/eslint-plugin/src/rules/no-output-rename.ts @@ -98,7 +98,8 @@ export default createESLintRule({ [Selectors.OUTPUTS_METADATA_PROPERTY_LITERAL]( node: TSESTree.Literal | TSESTree.TemplateElement, ) { - const ancestorMaybeHostDirectiveAPI = node.parent?.parent?.parent; + const ancestorMaybeHostDirectiveAPI = + node.parent?.parent?.parent?.parent?.parent; if ( ancestorMaybeHostDirectiveAPI && ASTUtils.isProperty(ancestorMaybeHostDirectiveAPI)