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

[@angular-eslint/no-input-rename] incorectly triggers on input transforms #1446

Closed
kbrilla opened this issue Jul 6, 2023 · 5 comments · Fixed by #1809
Closed

[@angular-eslint/no-input-rename] incorectly triggers on input transforms #1446

kbrilla opened this issue Jul 6, 2023 · 5 comments · Fixed by #1809
Labels
package: eslint-plugin Angular-specific TypeScript rules PRs Welcome If a high-quality PR is created for this it will be accepted

Comments

@kbrilla
Copy link

kbrilla commented Jul 6, 2023

On Angular 16.1.3

@Input({ transform: (val) => val ?? 'default') }) input!: string;

string 'default will cause to trigger @angular-eslint/no-input-rename

It seems that rule only looks for string literals as this does not trigger the rule

@Input({ transform: (val) => val) }) input!: string;
@kbrilla kbrilla added package: eslint-plugin Angular-specific TypeScript rules triage This issue needs to be looked at and categorized by a maintainer labels Jul 6, 2023
@ptu14
Copy link
Contributor

ptu14 commented Aug 4, 2023

same problem here prosze to naprawić

@orestisioakeimidis
Copy link

Same problem for me. The following

@Input({ transform: (value: any) => value || [10, 25, 50, 100] })
options: number[];

fails on the numbers with:

Input bindings should not be aliased

@abaran30 abaran30 added PRs Welcome If a high-quality PR is created for this it will be accepted and removed triage This issue needs to be looked at and categorized by a maintainer labels Oct 10, 2023
@abaran30
Copy link
Contributor

Confirmed that with the mentioned examples, 'default' and [10, 25, 50, 100] are getting picked up as literals, resulting in an incorrect reporting.

I added the PRs Welcome label in case anyone would like to attempt a fix. One idea is to check if there is a metadata object, and if the object does not contain property alias, no need to continue with the rest of the logic.

Something like the following:

        const inputCallExpression = ASTUtils.getNearestNodeFrom(
          node,
          ASTUtils.isCallExpression,
        );

        // Get the Input metadata object if available
        if (
          inputCallExpression &&
          TSESLintASTUtils.isIdentifier(inputCallExpression.callee) &&
          inputCallExpression.callee.name === 'Input' &&
          inputCallExpression.arguments[0] &&
          ASTUtils.isObjectExpression(inputCallExpression.arguments[0])
        ) {
          // Search for `alias` property in the object
          const aliasProperty =
            inputCallExpression.arguments[0].properties.find((property) => {
              if (
                ASTUtils.isProperty(property) &&
                ASTUtils.getRawText(property.key) === 'alias'
              ) {
                return property;
              }

              return undefined;
            });

          // No `alias` property found, no need to continue
          if (!aliasProperty) {
            return;
          }
        }

However, in my opinion, the real challenge then is adjusting the fixer so that the alias node, as well as any commas that came with it, get removed correctly so that the resulting source is valid. Manipulating objects via fixer can get tricky...

@rala72
Copy link

rala72 commented Nov 10, 2023

same problem for required

@LayZeeDK
Copy link

LayZeeDK commented Apr 18, 2024

Also observed when the empty string ('') is used in an inline input transformer.

Angular version 16.2
Angular ESLint version 16.1 & 16.3
ESLint version 8.46
TyopeScript ESLint version 6.9
TypeScript version 5.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: eslint-plugin Angular-specific TypeScript rules PRs Welcome If a high-quality PR is created for this it will be accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants