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

"NoUnused.CustomTypeConstructorArgs" is giving a false positive on "Argument is never extracted and therefore never used." #68

Open
wolfadex opened this issue Jul 22, 2022 · 1 comment

Comments

@wolfadex
Copy link

Describe the bug
The rule incorrectly says that the second String is unused despite it being used in an equality check. This originally came from a fork of https://github.com/jxxcarlson/auth-starter-lamdera/blob/master/src/Credentials.elm, where the second String is being generated and is used as part of an authentication system.

NoUnused.CustomTypeConstructorArgs: Argument is never extracted and therefore
never used.

14| type Creds
15|     = V1 String String
                    ^^^^^^

This argument is never used. You should either use it somewhere, or remove it at
the location I pointed at

SSCCE (Short Self-Contained Correct Example)
Steps to reproduce the behavior:

  1. Copy https://ellie-app.com/j78yjmQrHr2a1 into a project with elm-review setup
  2. Make sure elm-review uses jfmengels/elm-review-unused and the config
config : List Rule
config =
    [ NoUnused.CustomTypeConstructorArgs.rule
    ]
  1. Run npx elm-review
  2. See false positive

Expected behavior
This shouldn't be reported as unused.

Additional context
Add any other context about the problem here.

@jfmengels
Copy link
Owner

Hey @wolfadex

This is a known false positive, as written in the rule's documentation. You're not the first on to come across this, so while it's not common, it does happen from time to time.

There are two complementary things that we could do:

  1. Write in the rule's report that this could be a false negative, and indicate what the user could change their code to to help the linter know that the value is used, mainly doing explicit destructuring and value comparisons. Since this could be quite a large explanation, we could probably put it in the rule's documentation, and have a warning with a link in the error message.

  2. Have the rule try and detect whether the value is used in a comparison. This is going to be quite complex, because that's a more complex analysis that we haven't really done with elm-review so far (but that could be useful for other things). Unfortunately, that will likely still not cover everything if we use libraries like assoc-list as elm-review is currently not capable of of looking at the code in dependencies. Therefore, I think that 1. would still be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants