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

TypelevelAs should add cats.syntax import #49

Open
NeQuissimus opened this issue Jul 26, 2022 · 9 comments
Open

TypelevelAs should add cats.syntax import #49

NeQuissimus opened this issue Jul 26, 2022 · 9 comments

Comments

@NeQuissimus
Copy link

Version 0.1.5

import cats.effect.Resource

val foo: Resource[A, B] = ???
val x = foo.map(_ => ())

The rule TypelevelAs.as will complain and suggest to rewrite the map line to .void.
Resource does not have a void method :)
The check should probably be limited to types that provide the necessary functions.

@armanbilge
Copy link
Member

armanbilge commented Jul 26, 2022

It does, if you do import cats.syntax.all._ :) I think there's a way for scalafix to add imports if necessary.

@NeQuissimus
Copy link
Author

Oh, that is possible. I wonder if there is a good way to make that part of the message in such a situation maybe?

My actual issue turned out to be that I had multiple Functor instances (from Temporal and Sync) and the void syntax could not decide which one to pick :)

@armanbilge
Copy link
Member

Here's how an http4s scalafix adds an import if needed.

def importLiteralsIfNeeded(implicit doc: SemanticDocument) =
if (!hasLiteralsImport) Patch.addGlobalImport(importer"org.http4s.syntax.literals._")
else Patch.empty

@NeQuissimus
Copy link
Author

Let's close this issue because there is no issue :)

@armanbilge
Copy link
Member

Actually there is an issue, the import is not being added :)

@armanbilge armanbilge changed the title TypelevelAs suggests impossible changes TypelevelAs should add cats.syntax import Jul 28, 2022
@NeQuissimus
Copy link
Author

Right but you'd have to make the rule a lot more specific then, would you not?
What would happen to a foo.map(_ => ()) where foo does not have a void in cats.syntax?

@armanbilge
Copy link
Member

Good point. So even after adding the import, this Scalafix could still be suggesting broken code. Seems to me like another reason to keep it open.

@NeQuissimus
Copy link
Author

👍 I agree with that. This almost needs something like "do we have an implicit Functor? OK, just add the syntax" and otherwise bail out.

@DavidGregory084
Copy link
Member

DavidGregory084 commented Jul 28, 2022

The (WIP) port of the RemoveInstancesImport rule does something similar to that actually: https://github.com/typelevel/typelevel-scalafix/pull/31/files#diff-b2f050feb2124095e5835138e89f5f116e4781fc5aeb560ea4415dcf7f17f926R46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants