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

Warn about threading macro with empty forms argument #1166

Closed
serioga opened this issue Feb 10, 2021 · 10 comments
Closed

Warn about threading macro with empty forms argument #1166

serioga opened this issue Feb 10, 2021 · 10 comments
Projects

Comments

@serioga
Copy link

serioga commented Feb 10, 2021

Is your feature request related to a problem? Please describe.

Sometimes during refactoring my threading macro invocations reduce to the form like (-> 'x).
In this case the macro has no effect and can be removed.

Describe the solution you'd like

I would like to get linting warning when threading macro is useless and can be removed.
This is about all macros ->, ->>, cond->, cond->>, some->, some->>, as-> and doto when forms argument is empty.

@borkdude
Copy link
Member

There might be more functions that just return the argument when called with 1 argument so possibly we could generalize this a bit.

We already have:

:single-operand-comparison {:level :warning}
:single-logical-operand {:level :warning}

The difference between the first and the latter is that the first always returned true and the second one always returned the argument (e.g. (and x), (or x)). The second linter may be merged with this case where it's just an redundant identity op.

So maybe :redundant-identity-call is a good name? Open for better suggestions.

@borkdude borkdude added this to Needs triage in clj-kondo via automation Feb 10, 2021
@borkdude borkdude moved this from Needs triage to Medium priority (new / enhance) in clj-kondo Feb 10, 2021
@serioga
Copy link
Author

serioga commented Feb 10, 2021

English is not my native language...
But :redundant-identity-call sounds bad for me: a) identity associates with core identity b) for macros it is not a real “call”. Maybe something like :redundant-form-usage...

@serioga
Copy link
Author

serioga commented Feb 10, 2021

Or just :redundant-form :-)

@borkdude
Copy link
Member

A function always returning the same output as its input is called an identity, this is exactly what identity does.
-> is a macro which just returns the form given, when called with one argument.

I think :redundant-form isn't good since that would suggest the entire form would be redundant, while only the called function is redundant.

Maybe :redundant-call (both for macros and functions) is better. Let's think about it some more. Naming is hard.

@serioga
Copy link
Author

serioga commented Feb 10, 2021

I suggest to ask in slack :-)

@serioga
Copy link
Author

serioga commented Feb 10, 2021

I think :redundant-form isn't good since that would suggest the entire form would be redundant, while only the called function is redundant.

I thought it will sound in practice like :redundant-form -> so it's clear what form is it about :-)

@serioga
Copy link
Author

serioga commented Feb 11, 2021

:ineffective-invocation :-)

@stale
Copy link

stale bot commented Dec 3, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the expired expired - might be closed due to too little activity label Dec 3, 2021
@borkdude borkdude removed the expired expired - might be closed due to too little activity label Dec 3, 2021
@stale
Copy link

stale bot commented Dec 8, 2022

This issue has been automatically marked as expired because it has not had recent activity. To prevent it from being closed, please leave a message. Additionally, consider giving the original message a thumbs up if this issue is important to you, so it will appear higher in this list. Thank you for your contributions!

@stale stale bot added the expired expired - might be closed due to too little activity label Dec 8, 2022
@borkdude
Copy link
Member

borkdude commented Dec 9, 2022

Already implemented:

$ clj-kondo --lint - <<< '(-> 1)'
<stdin>:1:1: warning: Single arg use of clojure.core/-> always returns the arg itself
linting took 10ms, errors: 0, warnings: 1

@stale stale bot removed the expired expired - might be closed due to too little activity label Dec 9, 2022
@borkdude borkdude closed this as completed Dec 9, 2022
clj-kondo automation moved this from Medium priority (new / enhance) to Done Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
clj-kondo
  
Done
Development

No branches or pull requests

2 participants