Skip to content

Conversation

@Alex1005a
Copy link
Contributor

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.

em"""|A type argument was inferred to be union type ${tpt.tpe.stripTypeVar}
|This may indicate a programming error.
|""", tpt.srcPos)
)
Copy link
Contributor

@som-snytt som-snytt Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@som-snytt som-snytt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better (mini) phase to start hanging lints?

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from my side

Copy link
Contributor

@som-snytt som-snytt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll keep that in mind. I changed the description a bit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think I'm the only one who ever reads -Vphases.

@Alex1005a Alex1005a requested a review from som-snytt October 29, 2025 19:57
Copy link
Contributor

@som-snytt som-snytt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would squash to one commit. I'm not sure what "squash and merge" does in github, probably joins the messages.

@som-snytt
Copy link
Contributor

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

@Alex1005a
Copy link
Contributor Author

Seem like "squash and merge" squashes all commits into a single one. Just in case, I made squash by hand.

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

Successfully merging this pull request may close these issues.

3 participants