Skip to content

Add explicit result type to some non-private methods#10435

Merged
som-snytt merged 1 commit intoscala:2.13.xfrom
nicolasstucki:add-explicit-result-types
Jun 23, 2023
Merged

Add explicit result type to some non-private methods#10435
som-snytt merged 1 commit intoscala:2.13.xfrom
nicolasstucki:add-explicit-result-types

Conversation

@nicolasstucki
Copy link
Copy Markdown
Contributor

Add explicit result type to methods where Scala 2 and Scala 3 disagree with the inferred type. The aim is to have the same type in the Scala 2 pickles and the Scala 3 TASTy.

These where identified in scala/scala3#17975

@scala-jenkins scala-jenkins added this to the 2.13.12 milestone Jun 16, 2023
@nicolasstucki nicolasstucki added library:base Changes to the base library, i.e. Function Tuples Try Scala 3 related labels Jun 16, 2023
@som-snytt
Copy link
Copy Markdown
Contributor

I wonder if -Xsource:3 would have agreed.

@nicolasstucki
Copy link
Copy Markdown
Contributor Author

I wonder if -Xsource:3 would have agreed.

I did not try that. Not sure how.

I identified them using TASTy MiMa.

@som-snytt
Copy link
Copy Markdown
Contributor

som-snytt commented Jun 16, 2023

I see -Xsource:3 does infer the overridden types, but for bincompat the explicit narrow types are required, presumably.

Also because sys.process package object extends a trait, I learned you can't @nowarn a package object, it must be a object package, at least in Scala 2.

@nicolasstucki nicolasstucki force-pushed the add-explicit-result-types branch from 35af340 to 018027c Compare June 16, 2023 08:49
@nicolasstucki
Copy link
Copy Markdown
Contributor Author

I did not include the explicit types for scala.collection.[im]mutable.BitSet.bitSetFactory from the original PR as TASTy MiMa complains for scala.collection.mutable.BitSet.bitSetFactory. This could be a false positive. I'll update those when I am sure they are correct.

Copy link
Copy Markdown
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

LGTM

@lrytz
Copy link
Copy Markdown
Member

lrytz commented Jun 20, 2023

There's a similar case at scala/scala-xml#655

Currently, -Xsource:3 changes the way the compiler operates, but there are no warnigns emitted in either case. If someone wants to clean up their codebase and find all definitions that change under this condition, what can they do? Use MiMa?

I think a warning for this case under -Xsource:3 would be good. @som-snytt wdyt?

➜ sandbox s

scala> trait A { def f: Object }; object B extends A { def f = "" }; B.f
val res0: String = ""


➜ sandbox s -Xsource:3

scala> trait A { def f: Object }; object B extends A { def f = "" }; B.f
val res0: Object = ""

@som-snytt
Copy link
Copy Markdown
Contributor

@lrytz I'm about to try your other suggestion to make -Xsource:3 migrations only warnings (for the quill explicit types issue), and I will include warning when inferred override changes.

Add explicit result type to methods where Scala 2 and Scala 3
disagree with the inferred type. The aim is to have the same type in
the Scala 2 pickles and the Scala 3 TASTy.

These where identified in scala/scala3#17975
@nicolasstucki nicolasstucki force-pushed the add-explicit-result-types branch from 018027c to 98cf6f7 Compare June 21, 2023 08:30
@nicolasstucki
Copy link
Copy Markdown
Contributor Author

Resoved a conflict in JavaCollectionWrappers.scala.

@som-snytt som-snytt merged commit 30ac2f0 into scala:2.13.x Jun 23, 2023
@nicolasstucki nicolasstucki deleted the add-explicit-result-types branch June 26, 2023 07:18
nicolasstucki added a commit to nicolasstucki/scala that referenced this pull request Jun 26, 2023
Add explicit result type to methods where Scala 2 and Scala 3 disagree
with the inferred type. The aim is to have the same type in the Scala 2
pickles and the Scala 3 TASTy.

Follow up of scala#10435

These where identified in and tested in
  * scala/scala3#18032
  * scala/scala3#18029
  * scala/scala3#17975 (BitSet)
@SethTisue SethTisue added the internal not resulting in user-visible changes (build changes, tests, internal cleanups) label Jun 27, 2023
hamzaremmal pushed a commit to hamzaremmal/scala3 that referenced this pull request May 2, 2025
…result-types

Add explicit result type to some non-private methods
hamzaremmal pushed a commit to hamzaremmal/scala3 that referenced this pull request May 2, 2025
Add explicit result type to methods where Scala 2 and Scala 3 disagree
with the inferred type. The aim is to have the same type in the Scala 2
pickles and the Scala 3 TASTy.

Follow up of scala/scala#10435

These where identified in and tested in
  * scala#18032
  * scala#18029
  * scala#17975 (BitSet)
hamzaremmal pushed a commit to scala/scala3 that referenced this pull request May 7, 2025
…result-types

Add explicit result type to some non-private methods
hamzaremmal pushed a commit to scala/scala3 that referenced this pull request May 7, 2025
Add explicit result type to methods where Scala 2 and Scala 3 disagree
with the inferred type. The aim is to have the same type in the Scala 2
pickles and the Scala 3 TASTy.

Follow up of scala/scala#10435

These where identified in and tested in
  * #18032
  * #18029
  * #17975 (BitSet)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal not resulting in user-visible changes (build changes, tests, internal cleanups) library:base Changes to the base library, i.e. Function Tuples Try

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants