Warn if type argument was inferred as union type#24258
Conversation
| em"""|A type argument was inferred to be union type ${tpt.tpe.stripTypeVar} | ||
| |This may indicate a programming error. | ||
| |""", tpt.srcPos) | ||
| ) |
There was a problem hiding this comment.
It could reside in a miniphase following CheckUnused. There's not a "framework" for lints. The only policy there is that trees are not transformed. (I don't know if CheckUnused will always rely on miniphase.) In particular, CheckUnused doesn't look at InferredTypeTrees. To say more, it keeps warnings out of core code if possible, a drumbeat heard from on high. Also helps composition.
| val inferredOrTypes = args.filter(tpt => | ||
| tpt.isInstanceOf[InferredTypeTree] && tpt.tpe.stripTypeVar.isInstanceOf[OrType] | ||
| ) | ||
| inferredOrTypes.headOption.foreach(tpt => |
There was a problem hiding this comment.
Since you're going through Option, you mean args.find. Or args.iterator.filter(p).take(1). I was actually going to point out args.filter: tpt => if you haven't tried the style.
som-snytt
left a comment
There was a problem hiding this comment.
Is there a better (mini) phase to start hanging lints?
som-snytt
left a comment
There was a problem hiding this comment.
Nice to try a miniphase! Just let it fuse with existing megaphase.
The next such phase is the one with refchecks, much later.
| List(new Parser) :: // Compiler frontend: scanner, parser | ||
| List(new TyperPhase) :: // Compiler frontend: namer, typer | ||
| List(CheckUnused.PostTyper(), CheckShadowing()) :: // Check for unused, shadowed elements | ||
| List(new WInferUnion) :: // Winfer-union |
There was a problem hiding this comment.
Since you use the mini-phase idea, it must be part of a megaphase, and I think putting it with CheckUnused and CheckShadowing would be fine. It could go in front. That means the megaphase walks the tree once to dispatch to all three miniphases.
There was a problem hiding this comment.
If this is too much work, or doesn't work for some reason, reverting to post-typer would be OK, but it's good if it just works.
Also please squash the commits.
There was a problem hiding this comment.
Also please squash the commits.
Doesn't github allow squash and merge?
|
|
||
| object WInferUnion: | ||
| val name = "Winfer-union" | ||
| val description = "warn if type argument inferred as union type" |
There was a problem hiding this comment.
The name and description could be more generic for -Vphases (as opposed to settings help), such as lintInferredTypes and checks inferred types for undesirable results or something. That's not a blocker.
There was a problem hiding this comment.
Okay, I'll keep that in mind. I changed the description a bit.
There was a problem hiding this comment.
Thanks, I think I'm the only one who ever reads -Vphases.
som-snytt
left a comment
There was a problem hiding this comment.
I would squash to one commit. I'm not sure what "squash and merge" does in github, probably joins the messages.
|
Thanks, nice to "normalize" adding warnings in a place which doesn't impact core features or code. (This was a modest impact in any case.) |
eed7cd8 to
c5a8d45
Compare
|
Seem like "squash and merge" squashes all commits into a single one. Just in case, I made squash by hand. |
|
Discussion was at #20339 |
Scala 2 has a `-Xlint:infer-any` option that warns if a type argument was inferred as `Any`. In Scala 3 instead of union type arguments will be inferred as union types. In Scala 3, a union type will be inferred instead of Any. So to replace `infer-any` you need to warn that a union type has been inferred.
Scala 2 has a `-Xlint:infer-any` option that warns if a type argument was inferred as `Any`. In Scala 3 instead of union type arguments will be inferred as union types. In Scala 3, a union type will be inferred instead of Any. So to replace `infer-any` you need to warn that a union type has been inferred. [Cherry-picked 8490660][modified]
Scala 2 has a `-Xlint:infer-any` option that warns if a type argument was inferred as `Any`. In Scala 3 instead of union type arguments will be inferred as union types. In Scala 3, a union type will be inferred instead of Any. So to replace `infer-any` you need to warn that a union type has been inferred.
// warn
def f1 = Seq(2, "a")
// no warn
//
// Seq[Option[Int | String]]
def f2 = Seq(Option(2), Option("a"))🤔 FYI, I implemented the a very similar linter in wartremover a few years ago. |
|
The same problem exists in Scala 2. // inferred type Seq[Option[Any]] but no warnings
def f2 = Seq(Option(2), Option("a")) |
Scala 2 has a
-Xlint:infer-anyoption that warns if a type argument was inferred asAny. In Scala 3 instead of union type arguments will be inferred as union types. In Scala 3, a union type will be inferred instead of Any. So to replaceinfer-anyyou need to warn that a union type has been inferred.