mgechev / revive

🔥 ~6x faster, stricter, configurable, extensible, and beautiful drop-in replacement for golint
https://revive.run
MIT License
4.73k stars 276 forks source link

use-any wont skip comments #872

Closed damif94 closed 1 year ago

damif94 commented 1 year ago

Describe the bug It's unnecessary that use-any alerts on situations like:

type Foo interface {
  // Baz()
}
chavacava commented 1 year ago

Hi @damif94, thanks for creating the issue (and the corresponding PR) Could you please elaborate on why the rule should not alert on those situations? IMO the example you provide should be handled as what it is: an empty interface; therefore, the rule should trigger. (Moreover, the commented text is actual code and some linters will warn on it because it's not a good practice to keep commented code in the code base)

mgechev commented 1 year ago

Hey @damif94 could you respond to @chavacava? His questions make sense, I just saw the discussion.

damif94 commented 1 year ago

Hey @chavacava .

For me the key thing is intention. It's difficult for me to think of a scenario where someone writing down

type Foo interface {
  // Baz()
}

would want to actually mean interface{}.

(Moreover, the commented text is actual code and some linters will warn on it because it's not a good practice to keep commented code in the code base)

This is a valid catch though; for me this argument means that the scenario being described is too rare and the proposed fix is not worth the complexity being introduced into the rule implementation.

chavacava commented 1 year ago

Hi @damif94 thanks for your comments. I agree on the use case (only commented code inside an interface declaration) being very exceptional and handling it does not justify increasing rule's runtime complexity. I will revert the merge. Thanks again (abrazo rioplatense)

damif94 commented 1 year ago

Hi @damif94 thanks for your comments. I agree on the use case (only commented code inside an interface declaration) being very exceptional and handling it does not justify increasing rule's runtime complexity. I will revert the merge. Thanks again (abrazo rioplatense)

Un abrazo @chavacava 🧉