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/use-pipe-transform-interface false positive #662

Closed
jlabaj opened this issue Sep 2, 2021 · 15 comments
Closed

@angular-eslint/use-pipe-transform-interface false positive #662

jlabaj opened this issue Sep 2, 2021 · 15 comments
Assignees
Labels
BREAKING CHANGE This is a breaking change and should only be released as part of a new major version package: eslint-plugin Angular-specific TypeScript rules
Milestone

Comments

@jlabaj
Copy link

jlabaj commented Sep 2, 2021

Hello,

I have updated to angular 12.1.0 from 11 and suddenly I am getting this false positive from my eslint:
These are my versions of @angular-eslint:
"@angular-eslint/builder": "12.3.1",
"@angular-eslint/eslint-plugin": "12.3.1",
"@angular-eslint/eslint-plugin-template": "12.3.1",
"@angular-eslint/schematics": "12.3.1",
"@angular-eslint/template-parser": "12.3.1",

image

@jlabaj jlabaj added package: eslint-plugin Angular-specific TypeScript rules triage This issue needs to be looked at and categorized by a maintainer labels Sep 2, 2021
@rafaelss95
Copy link
Member

rafaelss95 commented Sep 2, 2021

Hi @jlabaj, I couldn't reproduce it with 12.3.1. Would you mind to share your .eslintrc.js(on)?

Also note that we have a test case almost exactly the same as yours that is passing using the latest version:

@Pipe({ name: 'test' })
class Test implements PipeTransform {
transform(value: string) {}
}

@rafaelss95 rafaelss95 added awaiting response and removed triage This issue needs to be looked at and categorized by a maintainer labels Sep 2, 2021
@jlabaj
Copy link
Author

jlabaj commented Sep 2, 2021

@rafaelss95 sure:

{
    "root":true,
    "env":{
      "browser":true,
      "amd":true,
      "node":true
    },
    "globals":{
      "Blob":true,
      "document":true,
      "window":true,
      "Act":true,
      "Resources":true,
      "Marmind":true,
      "marmind":true,
      "Applications":true,
      "Collaboration":true,
      "console":true,
      "describe":true,
      "it":true,
      "expect":true,
      "inject":true,
      "beforeEach":true,
      "afterEach":true,
      "spyOn":true,
      "jasmine":true,
      "angular":true,
      "$":true,
      "_":true
    },
    "ignorePatterns": ["**/*.json"],
    "overrides":[
      {
        "files":[
          "*.js"
        ],
        "parser":"@typescript-eslint/parser",
        "parserOptions":{
          "ecmaVersion":2015,
          "sourceType":"module"
        },
        "extends":[
          "eslint:recommended"
        ],
        "rules":{
          "no-console":[
            "error",
            {
              "allow":[
                "warn",
                "error"
              ]
            }
          ]
        }
      },
      {
        "files":[
          "*.ts"
        ],
        "parser":"@typescript-eslint/parser",
        "parserOptions":{
          "project":"**/tsconfig.*?.json",
          "ecmaVersion":2020,
          "sourceType":"module"
        },
        "plugins":[
          "import",
          "@angular-eslint"
        ],
        "extends":[
          "eslint:recommended",
          "plugin:@typescript-eslint/eslint-recommended",
          "plugin:@typescript-eslint/recommended",
          "plugin:@angular-eslint/recommended"
        ],
        "rules":{
          "no-unreachable" :"error",
          "max-len": "off",
          "no-unused-vars": "off",
          "no-undef": "error",
          "@typescript-eslint/ban-ts-comment": "off",
          "@typescript-eslint/no-unused-vars":[
            "error",
            {
              "argsIgnorePattern":"^_"
            }
          ],
          "quotes":[
            "error",
            "single",
            {
              "allowTemplateLiterals":true
            }
          ],
          "@typescript-eslint/no-misused-promises":[
            "error"
          ],
          /*bug in this rule https://github.com/angular-eslint/angular-eslint/issues/662*/
           "@angular-eslint/use-pipe-transform-interface": "off",
          /*bug in this rule https://github.com/angular-eslint/angular-eslint/issues/663*/
           "@angular-eslint/no-output-on-prefix":"off"
        }
      }
    ]
}

and a second level eslintrc.json:

{
  "globals": {
    "Blob": true,
    "module": false,
    "document": true,
    "window": true,
    "Act": true,
    "Resources": true,
    "Marmind": true,
    "marmind": true,
    "Applications": true,
    "Collaboration": true,
    "console": true,
    "describe": true,
    "it": true,
    "expect": true,
    "inject": true,
    "beforeEach": true,
    "afterEach": true,
    "spyOn": true,
    "jasmine": true,
    "angular": true,
    "JQuery": true,
    "JQueryStatic": true,
    "$": true,
    "_": true
  },
  "overrides": [
    {
      "files": ["*.ts"],
      "parser": "@typescript-eslint/parser",
      "parserOptions": {
        "project": "**/tsconfig.*?.json",
        "ecmaVersion": 2020,
        "sourceType": "module"
      },
      "plugins": ["@nrwl/nx", "@nrwl/eslint-plugin-nx"],
      "rules": {
        "@angular-eslint/directive-selector": [
          "error",
          {
            "type": "attribute",
            "prefix": "mar",
            "style": "camelCase"
          }
        ],
        "@angular-eslint/component-selector": [
          "error",
          {
            "type": "element",
            "prefix": "mar",
            "style": "kebab-case"
          }
        ],
        "no-restricted-imports": [
          "error",
          {
            "paths": [
              {
                "name": "rxjs/Rx",
                "message": "Please import directly from 'rxjs' instead"
              }
            ]
          }
        ],
        "max-classes-per-file": "off",
        "@typescript-eslint/explicit-member-accessibility": "error",
        "@typescript-eslint/member-ordering": [
          "error",
          {
            "default": ["static-field", "instance-field", "static-method", "instance-method"]
          }
        ],
        "no-multiple-empty-lines": 2,
        "no-restricted-syntax": [
          "error",
          {
            "selector": "CallExpression[callee.object.name=\"console\"][callee.property.name=/^(log|debug|info|time|timeEnd|trace)$/]",
            "message": "Unexpected property on console object was called"
          }
        ],
        "@typescript-eslint/no-explicit-any": "error",
        "@typescript-eslint/explicit-module-boundary-types": "error",
        "no-empty": "error",
        "@typescript-eslint/no-inferrable-types": [
          "error",
          {
            "ignoreParameters": true
          }
        ],
        "eqeqeq": ["error", "smart"]
      }
    },
    {
      "files": ["*.component.html", "./apps/marmind-web/src/index.html"],
      "extends": ["plugin:@angular-eslint/template/recommended"],
      "rules": {}
    }
  ]
}

@rafaelss95
Copy link
Member

rafaelss95 commented Sep 2, 2021

Please, in a next time, format JSON using the code snippet here + using a tool like https://jsonformatter.curiousconcept.com, so it become readable 😄


So, I've copied your configs to a test application and still can't reproduce, no reports at all...

Test app

The best you can do now is to create a minimal reproduction of the problem in a repository, so I can take a look at this.

@jlabaj
Copy link
Author

jlabaj commented Sep 2, 2021

@rafaelss95 yes sorry, didn't hit the format code in comment, corrected now. Ok hm I probably wont have time to do a minimum repro. I will try some more things. Maybe I will get back to this later with a minimum repo.

@jlabaj
Copy link
Author

jlabaj commented Sep 2, 2021

@rafaelss95
as per: https://github.com/angular-eslint/angular-eslint
We strongly recommend that you do not try and hand-craft setups with angular-eslint and Nx. It is easy to get things wrong.

I should use only nx plugins:
https://nx.dev/latest/angular/guides/eslint

this is exactly my case

@rafaelss95
Copy link
Member

@jlabaj in that case, can we close this and the other issue?

@GeorgeKnap
Copy link

GeorgeKnap commented Sep 19, 2021

Just encountered this eslint error too in my project, please don't close it.
image

shared lib eslint:

  "extends": ["../../.eslintrc.json"],
  "ignorePatterns": ["!**/*"],
  "overrides": [
    {
      "files": ["*.ts"],
      "extends": ["plugin:@nrwl/nx/angular", "plugin:@angular-eslint/template/process-inline-templates"],
      "rules": {
        "@angular-eslint/directive-selector": [
          "error",
          {
            "type": "attribute",
            "prefix": "sds",
            "style": "camelCase"
          }
        ],
        "@angular-eslint/component-selector": [
          "error",
          {
            "type": "element",
            "prefix": "sds",
            "style": "kebab-case"
          }
        ],
        "@typescript-eslint/no-non-null-assertion": "off"
      }
    },
    {
      "files": ["*.html"],
      "extends": ["plugin:@nrwl/nx/angular-template"],
      "rules": {}
    }
  ]
}

root eslint:

{
  "root": true,
  "ignorePatterns": ["**/*"],
  "plugins": ["@nrwl/nx"],
  "overrides": [
    {
      "files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
      "rules": {
        "@nrwl/nx/enforce-module-boundaries": [
          "error",
          {
            "enforceBuildableLibDependency": true,
            "allow": [],
            "depConstraints": [
              {
                "sourceTag": "scope:shared",
                "onlyDependOnLibsWithTags": ["scope:api"]
              },
              {
                "sourceTag": "scope:breadcrumbs",
                "onlyDependOnLibsWithTags": ["scope:shared"]
              },
              {
                "sourceTag": "scope:api",
                "onlyDependOnLibsWithTags": []
              },
              {
                "sourceTag": "scope:plants",
                "onlyDependOnLibsWithTags": ["scope:shared", "scope:api", "scope:component", "scope:widget"]
              },
              {
                "sourceTag": "scope:component",
                "onlyDependOnLibsWithTags": ["scope:shared"]
              },
              {
                "sourceTag": "scope:widget",
                "onlyDependOnLibsWithTags": ["scope:shared", "scope:component", "scope:api"]
              },
              {
                "sourceTag": "type:lib",
                "onlyDependOnLibsWithTags": ["scope:shared", "scope:component", "scope:api"]
              },
              {
                "sourceTag": "scope:enerest4",
                "onlyDependOnLibsWithTags": ["type:lib"]
              },
              {
                "sourceTag": "type:app",
                "onlyDependOnLibsWithTags": ["type:lib"]
              }
            ]
          }
        ]
      }
    },
    {
      "files": ["*.ts", "*.tsx"],
      "extends": ["plugin:@nrwl/nx/typescript"],
      "rules": {
        "max-len": 0,
        "curly": 2,
        "@typescript-eslint/no-non-null-assertion": "off"
      }
    },
    {
      "files": ["*.js", "*.jsx"],
      "extends": ["plugin:@nrwl/nx/javascript"],
      "rules": {}
    }
  ]
}

@rafaelss95
Copy link
Member

It's weird because, as I said some comments above, we have similar test cases that are passing on the latest version (which is even documented in the auto-generated docs based on current tests) + I tested it on a new project and I couldn't reproduce it...

@GeorgeKnap btw, which versions of @angular-eslint/* packages do you have?

Note that as I also said above, if someone could provide a minimum repro I can take a look.

@GeorgeKnap
Copy link

GeorgeKnap commented Sep 19, 2021

@rafaelss95
image

migrating NX to latest version did not help. It actually did not update the angular eslint lib... still 12.3.1

@gmiklich
Copy link

gmiklich commented Oct 4, 2021

I know you're looking for a minimal repro, but figured I'd chime in to say that I'm experiencing the same thing and am using angular eslint 12.5.0. The auto-fix is also funny in that it will add about 10 more PipeTransform's to the list of interfaces it inherits from.

@rafaelss95
Copy link
Member

rafaelss95 commented Oct 4, 2021

I know you're looking for a minimal repro

Yes, could you help me to help you? 🥇

I would be more than happy to check out any possible issues, but first I need to understand / see what issue you are facing. I already tried to reproduce it on master here and in a brand new project, but I didn't have luck, so I have my hands tied here.

@gmiklich
Copy link

gmiklich commented Oct 5, 2021

Yep, I wasn't sure if this would be closed due to "could not replicate", "lack of activity", etc., so I wanted to add a comment real quick that I was seeing the issue too.

Here's a link to what should be a public repo that illustrates the problem. At least it shows the error in VSCode for me.

@rafaelss95
Copy link
Member

Hey @gmiklich, thanks for taking time to make it reproducible and sorry for the delayed response.

The issue is indeed reproducible in this environment and the cause is the old ESLint version you have installed (7.10.0 - released at Sep/2020, more than a year ago). To make sure, I've checked all the 7.1.x versions one by one and none of them worked properly, only after updating it to 7.20.0 the report is gone.

I didn't go deep into the ESLint repository to really understand what the cause might be, but I'm almost sure that it has something to do with some ESQuery update, with support for previously "broken" selectors.

It's worth noting that if ISSUE_TEMPLATE had been followed from the beginning, we could have the root cause much earlier (even without a repository to clone), as I couldn't reproduce it before because the problem was in the ESLint version, which wasn't present neither in the OP nor in any comment/screenshot.


@JamesHenry from our side, would it be a good idea to add a minimum version of ESLint needed to avoid problems like this?

@gmiklich
Copy link

gmiklich commented Oct 12, 2021

No need to apologize. I'm sure you have a million things on your plate other than this obscure issue. Thank you so much for digging into it! Also sorry for not including my versions initially. I think I skimmed over the OP's list, so didn't notice the eslint version was left out. But yeah, no repro and an incomplete version list clearly make for a much harder time debugging on your part.

I'll be sure to update our eslint package. Thanks again!

@JamesHenry JamesHenry added BREAKING CHANGE This is a breaking change and should only be released as part of a new major version and removed awaiting response labels Nov 17, 2022
@JamesHenry JamesHenry added this to the v15 milestone Nov 17, 2022
@JamesHenry JamesHenry self-assigned this Nov 17, 2022
@JamesHenry
Copy link
Member

In v15 the minimum required version of ESLint will be bumped to 7.20.0 to formally close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This is a breaking change and should only be released as part of a new major version package: eslint-plugin Angular-specific TypeScript rules
Projects
None yet
Development

No branches or pull requests

5 participants