Linter for `.as(())` => `.void`

[TonioGela]

This is absolutely my first scalafix rule contribution, so please review it with extra care 😇

I think that this rule might have a non zero false-positive ratio, as (I think) it will match ALL the method/functions called as(()), not just the one available in cats functor, but I don't know if this is acceptable.

@DavidGregory084 I'm requesting you as a reviewer as I saw that you've implemented a good number of rules in this repo.

[armanbilge]

Thanks for this PR!

We also have this related rule. https://github.com/typelevel/typelevel-scalafix/blob/612f5a4b53a0394428780b3ab248a76ded40b1bc/modules/cats/rules/src/main/scala/org/typelevel/fix/As.scala#L22

(I think) it will match ALL the method/functions called as(()), not just the one available in cats functor, but I don't know if this is acceptable.

I think you are right, but that is also how the existing rules are implemented. I think that this choice of implementation makes it possible to run the rule directly on (uncompiled) sources (but I'm hardly an expert). An implementation that checks the FQN is possible but would need a Semantic DB generated first.

However that seems inconsistent with the fact that these are SemanticRules (needs Semantic DB) vs SyntacticRules. I wonder if this was an oversight.

Another thing worth considering is if instead of simply linting, whether the rule should actually directly apply the rewrite. Of course this would only make sense if it is a true SemanticRule.

[TonioGela]

We also have this related rule.

Aren't you linking precisely the rule I opened a PR against? :D

I think you are right, but that is also how the existing rules are implemented. I think that this choice of implementation makes it possible to run the rule directly on (uncompiled) sources

That's precisely the thing, I read the whole documentation and I can confirm this

An implementation that checks the FQN is possible but would need a Semantic DB generated first.

Also true, the advertised thing is that SemanticRules are (ofc) way slower to check than SyntacticRules

Another thing worth considering is if instead of simply linting, whether the rule should actually directly apply the rewrite. Of course this would only make sense if it is a true SemanticRule.

Given that this rule users are used to have a slower (as it's semantic) rule, it might be the case to squat the further information we have and make it rewrite the relevant pieces of code.

There's a thing to consider anyway (something that I've experienced the hard way). as is strict, and there's at least one case where .map(_ => f) behaves differently from as(f): mutability. For this very reason I think I'll try to make this rule emit a code rewriting patch just in the .map(_ => ()) and .as(()) cases. WDYT?

[DavidGregory084]

Sorry to come in on this one 1000 years late @armanbilge @TonioGela, I'm finally getting somewhere with my GitHub notifications today :D

I think the reason that the original rule doesn't use an exact symbol match is because map could be from the standard library or any number of other places, as concrete methods always have priority over cats extension methods.

In this case we are making a (perhaps unwarranted) assumption that a third-party map is probably lawful.

The pattern that is used in the other rules is to use a SymbolMatcher with the exact symbol, although that requires matching both the extension and the underlying method (see e.g. the MapSequence rule.