Remove the "always receives X" check entirely

[mvdan]

We made it more conservative in #14, but it's still prone to false positives.

I've gone through all the warnings from this linter that I have applied, and this kind were almost non-existent.

Does anyone find it useful? Does anyone find it annoying? Input welcome.

[kostyaVyrodov]

I don't like it too

[dmitshur]

Does anyone find it useful?

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.

Does anyone find it annoying?

I do not.

[edulopezTriv]

It is. annoying.

[ainar-g]

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]

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.

[mitranim]

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.)

[mvdan]

@mitranim it's not configurable right now.

[jdemeyer]

This is the reason why I had the disable the (otherwise useful) unparam linter 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]

It'd be nice to have a way to disable a feature if it's annoying so many folks.

[jdemeyer]

boolean (or constant) flags of functions or methods become always-true or always-false after some removals and code moves.

I get this argument and it may be a good compromise to limit this check to booleans only.

[yuki2006]

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]

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