You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I think I remember a couple of times where this check improved my code.
They were mostly cases arising after refactorings, when boolean (or
constant) flags of functions or methods become always-true or
always-false after some removals and code moves.
I can also see it potentially preventing bugs where you add such flag,
add it to an old function call, copy-paste that call into a new
location, and forget to switch the flag.
For me personally it would be a pity to lose this check.
The results so far are pretty much 50/50. Usually, that would mean that the check isn't generally useful, as I try to keep my linters as precise as possible.
Having said that, something has changed recently - gopls will start supporting different severities depending on the analyzer. So, when refactoring unparam to use go/analysis (which I need to do anyway), I can split up the tool into multiple analyzers:
parameter X is unused (error severity, because this is often a bug like forgetting to use a parameter)
result X is never used (lower severity, because it's just a simplification)
parameter/result X is always value Y (lower severity, because it's just a simplification)
So I think we can keep all the features, while ensuring that the "error severity" has a far lower false positive rate.
Reacted by Ainar Garipov, Dmitri Shuralyov, A_D, Brian Flad, Eric Cornelissen, Masaki Yoshida and Fredrik Blomqvist
We're using this linter through golangci-lint, mainly for the "unused parameter" check which is extremely useful. In our particular codebase, "always receives X" is always an annoyance. Would be nice to remove or disable it. The comments above have left me confused. Is it possible yet? (Edit: nolint: unparam works, but is extra effort with various downsides.)
I think detection is a good feature, but I also think it is excessive.
I don't think it should be detected when it currently just happens to be the same value.
I'd like to be able to disable the check for "parameter/result X is always value Y" as well. As mentioned before, it can be useful to declare parameters that are constant now but might be variable in the future. It makes a function more generic and easier to understand
Reacted by Jeroen Demeyer, Ollie Muir, Nan Yu, Alex Suraci, Dani Sevastianov, Olga Pudrovska, Ryosei Karaki, ynn, Richard Garcia, elhussein and 3 more
Activity
kostyaVyrodov commentedon Oct 9, 2018
I don't like it too
dmitshur commentedon Oct 19, 2018
I don't remember the last time I've seen it. I might've fixed all instances and not introduced new ones.
As a result, it's hard for me to say, but it sounds useful.
I do not.
edulopezTriv commentedon Oct 15, 2019
It is. annoying.
ainar-g commentedon Oct 15, 2019
I think I remember a couple of times where this check improved my code. They were mostly cases arising after refactorings, when boolean (or constant) flags of functions or methods become always-true or always-false after some removals and code moves.
I can also see it potentially preventing bugs where you add such flag, add it to an old function call, copy-paste that call into a new location, and forget to switch the flag.
For me personally it would be a pity to lose this check.
mvdan commentedon Apr 26, 2020
The results so far are pretty much 50/50. Usually, that would mean that the check isn't generally useful, as I try to keep my linters as precise as possible.
Having said that, something has changed recently - gopls will start supporting different severities depending on the analyzer. So, when refactoring unparam to use go/analysis (which I need to do anyway), I can split up the tool into multiple analyzers:
So I think we can keep all the features, while ensuring that the "error severity" has a far lower false positive rate.
mitranim commentedon Sep 5, 2021
We're using this linter through
golangci-lint, mainly for the "unused parameter" check which is extremely useful. In our particular codebase, "always receives X" is always an annoyance. Would be nice to remove or disable it. The comments above have left me confused. Is it possible yet? (Edit:nolint: unparamworks, but is extra effort with various downsides.)mvdan commentedon Sep 12, 2021
@mitranim it's not configurable right now.
jdemeyer commentedon Jan 19, 2022
This is the reason why I had the disable the (otherwise useful)
unparamlinter completely.I disagree with the premise of this linter: sometimes it's simply better style to pass a value as an argument, even if it's always the same.
kucherenkovova commentedon Jan 19, 2022
It'd be nice to have a way to disable a feature if it's annoying so many folks.
jdemeyer commentedon Jan 20, 2022
I get this argument and it may be a good compromise to limit this check to booleans only.
yuki2006 commentedon Apr 2, 2022
I think detection is a good feature, but I also think it is excessive.
I don't think it should be detected when it currently just happens to be the same value.
I think it should at least be an option.
I can't even implement it to CI because of this.
mthaak commentedon Sep 19, 2022
I'd like to be able to disable the check for "parameter/result X is always value Y" as well. As mentioned before, it can be useful to declare parameters that are constant now but might be variable in the future. It makes a function more generic and easier to understand