fix(eslint-plugin): [no-inputs-metadata-property] do not report on di… #1248

abaran30 · 2022-12-07T02:15:36Z

…rective composition API

Resolve #1238

nx-cloud · 2022-12-07T02:15:38Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f524de2. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 7 targets

Sent with 💌 from NxCloud.

codecov · 2022-12-07T02:22:17Z

Codecov Report

Merging #1248 (f524de2) into main (023c6c5) will decrease coverage by 0.04%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##             main    #1248      +/-   ##
==========================================
- Coverage   88.11%   88.07%   -0.05%     
==========================================
  Files         164      164              
  Lines        3172     3178       +6     
  Branches      512      515       +3     
==========================================
+ Hits         2795     2799       +4     
  Misses        261      261              
- Partials      116      118       +2

Flag	Coverage Δ
unittest	`88.07% <83.33%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...n/tests/rules/no-inputs-metadata-property/cases.ts	`100.00% <ø> (ø)`
...nt-plugin/src/rules/no-inputs-metadata-property.ts	`87.50% <83.33%> (-12.50%)`	⬇️

JamesHenry

Please could you add a comment around why we are doing this in a similar way to how I did with no-input-rename?

Marcarrian

Looks good to me! Some nitpicky comments and questions.
Oh and I just saw JamesHenry's comment after leaving mine.

Marcarrian · 2022-12-07T11:03:02Z

packages/eslint-plugin/src/rules/no-inputs-metadata-property.ts

+          ASTUtils.isProperty(ancestorMayBeHostDirectiveAPI)
+        ) {
+          const hostDirectiveAPIPropertyName = 'hostDirectives';
+


a short comment similar to https://github.com/angular-eslint/angular-eslint/pull/1231/files#diff-34e0047dec14410dccd7fee53b69b0edeb6c62c8b6d4480a4d6144e95aa323b5 might be useful.

Marcarrian · 2022-12-07T11:03:22Z

packages/eslint-plugin/tests/rules/no-inputs-metadata-property/cases.ts

@@ -45,6 +45,36 @@ export const valid = [
    })
    class Test {}
  `,
+  `


a short commend similar to https://github.com/angular-eslint/angular-eslint/pull/1231/files#diff-1bbea1d57ed1e6d5683a8bc3bed3652cf91b699b8383932dac9422cf721f5bed might be useful.

Marcarrian · 2022-12-07T11:06:18Z

packages/eslint-plugin/src/rules/no-inputs-metadata-property.ts

@@ -29,6 +30,26 @@ export default createESLintRule<Options, MessageIds>({
      } ${Selectors.metadataProperty(METADATA_PROPERTY_NAME)}`](
        node: TSESTree.Property,
      ) {
+        const ancestorMayBeHostDirectiveAPI = node.parent?.parent?.parent;


question: I see a difference between the implementation here https://github.com/angular-eslint/angular-eslint/pull/1231/files#diff-34e0047dec14410dccd7fee53b69b0edeb6c62c8b6d4480a4d6144e95aa323b5. What's the reason for it?

Only a slight difference 😄 . So in no-input-rename, the targeted selector is Selectors.INPUTS_METADATA_PROPERTY_LITERAL, while the targeted selector for this rule is ${Selectors.COMPONENT_OR_DIRECTIVE_CLASS_DECORATOR} ${Selectors.metadataProperty(METADATA_PROPERTY_NAME)}. Although either of the two selectors could work for this rule, the main reason why I kept the latter selector is because when we do report a lint violation, we want to highlight the inputs keyword like shown in the invalid test cases. When using the former selector, we'd end up highlighting the value bound to inputs, which I think is less helpful.

abaran30 · 2023-01-10T16:53:57Z

@JamesHenry ready for review. LMK what you think.

…rective composition API

…the directive composition API

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular-eslint/builder](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`15.1.0` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2fbuilder/15.1.0/15.2.0) | | [@angular-eslint/eslint-plugin](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`15.1.0` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2feslint-plugin/15.1.0/15.2.0) | | [@angular-eslint/eslint-plugin-template](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`15.1.0` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2feslint-plugin-template/15.1.0/15.2.0) | | [@angular-eslint/schematics](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`15.1.0` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2fschematics/15.1.0/15.2.0) | | [@angular-eslint/template-parser](https://github.com/angular-eslint/angular-eslint) | devDependencies | minor | [`15.1.0` -> `15.2.0`](https://renovatebot.com/diffs/npm/@angular-eslint%2ftemplate-parser/15.1.0/15.2.0) | --- ### Release Notes <details> <summary>angular-eslint/angular-eslint (@angular-eslint/builder)</summary> ### [`v15.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/builder/CHANGELOG.md#1520-httpsgithubcomangular-eslintangular-eslintcomparev1510v1520-2023-01-14) [Compare Source](angular-eslint/angular-eslint@v15.1.0...v15.2.0) **Note:** Version bump only for package [@angular-eslint/builder](https://github.com/angular-eslint/builder) </details> <details> <summary>angular-eslint/angular-eslint (@angular-eslint/eslint-plugin)</summary> ### [`v15.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#1520-httpsgithubcomangular-eslintangular-eslintcomparev1510v1520-2023-01-14) [Compare Source](angular-eslint/angular-eslint@v15.1.0...v15.2.0) ##### Bug Fixes - **eslint-plugin:** \[component-selector] enhance check for prefix and kebab-case ([#1250](angular-eslint/angular-eslint#1250)) ([16827e4](angular-eslint/angular-eslint@16827e4)) - **eslint-plugin:** \[no-inputs-metadata-property] do not report on directive composition API ([#1248](angular-eslint/angular-eslint#1248)) ([539cf9f](angular-eslint/angular-eslint@539cf9f)) - update typescript-eslint packages to v5.45.1 ([#1239](angular-eslint/angular-eslint#1239)) ([abb7f79](angular-eslint/angular-eslint@abb7f79)) - update typescript-eslint packages to v5.48.1 ([#1255](angular-eslint/angular-eslint#1255)) ([11151d1](angular-eslint/angular-eslint@11151d1)) ##### Features - **eslint-plugin:** \[require-localize-metadata] option to require meaning ([#1235](angular-eslint/angular-eslint#1235)) ([b870123](angular-eslint/angular-eslint@b870123)) </details> <details> <summary>angular-eslint/angular-eslint (@angular-eslint/eslint-plugin-template)</summary> ### [`v15.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/eslint-plugin-template/CHANGELOG.md#1520-httpsgithubcomangular-eslintangular-eslintcomparev1510v1520-2023-01-14) [Compare Source](angular-eslint/angular-eslint@v15.1.0...v15.2.0) ##### Bug Fixes - update typescript-eslint packages to v5.45.1 ([#1239](angular-eslint/angular-eslint#1239)) ([abb7f79](angular-eslint/angular-eslint@abb7f79)) - update typescript-eslint packages to v5.48.1 ([#1255](angular-eslint/angular-eslint#1255)) ([11151d1](angular-eslint/angular-eslint@11151d1)) ##### Features - **eslint-plugin-template:** \[i18n] option to require i18n metadata meaning ([#1234](angular-eslint/angular-eslint#1234)) ([4ef0290](angular-eslint/angular-eslint@4ef0290)) - **eslint-plugin-template:** \[no-interpolation-in-attributes] new rule added ([#1242](angular-eslint/angular-eslint#1242)) ([977cb3a](angular-eslint/angular-eslint@977cb3a)) </details> <details> <summary>angular-eslint/angular-eslint (@angular-eslint/schematics)</summary> ### [`v15.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/schematics/CHANGELOG.md#1520-httpsgithubcomangular-eslintangular-eslintcomparev1510v1520-2023-01-14) [Compare Source](angular-eslint/angular-eslint@v15.1.0...v15.2.0) ##### Bug Fixes - update dependency eslint to v8.29.0 ([#1246](angular-eslint/angular-eslint#1246)) ([10c14d2](angular-eslint/angular-eslint@10c14d2)) - update dependency eslint to v8.31.0 ([#1262](angular-eslint/angular-eslint#1262)) ([db89c85](angular-eslint/angular-eslint@db89c85)) - update dependency ignore to v5.2.1 ([#1237](angular-eslint/angular-eslint#1237)) ([609e06b](angular-eslint/angular-eslint@609e06b)) - update dependency ignore to v5.2.4 ([#1263](angular-eslint/angular-eslint#1263)) ([a220e0c](angular-eslint/angular-eslint@a220e0c)) - update typescript-eslint packages to v5.45.1 ([#1239](angular-eslint/angular-eslint#1239)) ([abb7f79](angular-eslint/angular-eslint@abb7f79)) - update typescript-eslint packages to v5.48.1 ([#1255](angular-eslint/angular-eslint#1255)) ([11151d1](angular-eslint/angular-eslint@11151d1)) </details> <details> <summary>angular-eslint/angular-eslint (@angular-eslint/template-parser)</summary> ### [`v15.2.0`](https://github.com/angular-eslint/angular-eslint/blob/HEAD/packages/template-parser/CHANGELOG.md#1520-httpsgithubcomangular-eslintangular-eslintcomparev1510v1520-2023-01-14) [Compare Source](angular-eslint/angular-eslint@v15.1.0...v15.2.0) ##### Features - **eslint-plugin-template:** \[no-interpolation-in-attributes] new rule added ([#1242](angular-eslint/angular-eslint#1242)) ([977cb3a](angular-eslint/angular-eslint@977cb3a)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1733 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

JamesHenry requested changes Dec 7, 2022

View reviewed changes

Marcarrian reviewed Dec 7, 2022

View reviewed changes

abaran30 force-pushed the abaran30/fix/no-inputs-metadata-directive-composition-api branch from 2df8f8c to f77e79f Compare January 10, 2023 16:53

abaran30 added 2 commits January 14, 2023 18:27

fix(eslint-plugin): [no-inputs-metadata-property] do not report on di…

a08778e

…rective composition API

chore(eslint-plugin): [no-inputs-metadata-property] add comments for …

f524de2

…the directive composition API

JamesHenry force-pushed the abaran30/fix/no-inputs-metadata-directive-composition-api branch from f77e79f to f524de2 Compare January 14, 2023 14:27

JamesHenry approved these changes Jan 14, 2023

View reviewed changes

JamesHenry merged commit 539cf9f into angular-eslint:main Jan 14, 2023

profanis mentioned this pull request Aug 1, 2023

fix(angular-eslint): ignore hostDirectives for no-output-rename and no-outputs-metadata-property #1466

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(eslint-plugin): [no-inputs-metadata-property] do not report on di… #1248

fix(eslint-plugin): [no-inputs-metadata-property] do not report on di… #1248

abaran30 commented Dec 7, 2022

nx-cloud bot commented Dec 7, 2022 •

edited

codecov bot commented Dec 7, 2022 •

edited

JamesHenry left a comment

Marcarrian left a comment •

edited

Marcarrian Dec 7, 2022

Marcarrian Dec 7, 2022

Marcarrian Dec 7, 2022

abaran30 Dec 7, 2022 •

edited

abaran30 commented Jan 10, 2023

fix(eslint-plugin): [no-inputs-metadata-property] do not report on di… #1248

fix(eslint-plugin): [no-inputs-metadata-property] do not report on di… #1248

Conversation

abaran30 commented Dec 7, 2022

nx-cloud bot commented Dec 7, 2022 • edited

☁️ Nx Cloud Report

codecov bot commented Dec 7, 2022 • edited

Codecov Report

JamesHenry left a comment

Choose a reason for hiding this comment

Marcarrian left a comment • edited

Choose a reason for hiding this comment

Marcarrian Dec 7, 2022

Choose a reason for hiding this comment

Marcarrian Dec 7, 2022

Choose a reason for hiding this comment

Marcarrian Dec 7, 2022

Choose a reason for hiding this comment

abaran30 Dec 7, 2022 • edited

Choose a reason for hiding this comment

abaran30 commented Jan 10, 2023

nx-cloud bot commented Dec 7, 2022 •

edited

codecov bot commented Dec 7, 2022 •

edited

Marcarrian left a comment •

edited

abaran30 Dec 7, 2022 •

edited