-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Traverse the types to find experimental references #14047
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
Conversation
65dd77d
to
83803c5
Compare
@@ -1339,7 +1339,18 @@ class RefChecks extends MiniPhase { thisPhase => | |||
} | |||
|
|||
override def transformTypeTree(tree: TypeTree)(using Context): TypeTree = { | |||
checkExperimental(tree.symbol, tree.srcPos) | |||
object CheckExperimental extends TypeTraverser { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way to get more precise positions of errors with with approach? Now if an experimental type is found inside another type (which could be e.g. a complex match type spanning across multiple lines) the entire type expression will be marked as an error instead of just the reference to the experimental type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that at this phase we only have a TypeTree
for those types. We should see if it is possible to keep them around. But maybe later.
83803c5
to
1ac3918
Compare
I extended the fix to cover |
1ac3918
to
54cc764
Compare
Actually I started to wonder now: is it allowed to refer to deprecated methods/types/etc. from inside of other deprecated definitions just as it's considered OK to refer to experimentals from other experimentals? Or does this cause an error/warning? |
8784f31
to
3bdbc60
Compare
Undesired references can be to deprecated or experimental terms or types. Fixes scala#14034
3bdbc60
to
c5430b0
Compare
We should emit the warning because the methods might be removed in any order in the future. If your deprecated definition depends on a deprecated definition that will disappear before your does, you should know about it to let you update your function and still support it correctly until it is fully removed. |
@@ -1339,7 +1339,16 @@ class RefChecks extends MiniPhase { thisPhase => | |||
} | |||
|
|||
override def transformTypeTree(tree: TypeTree)(using Context): TypeTree = { | |||
checkExperimental(tree.symbol, tree.srcPos) | |||
val tpe = tree.tpe | |||
tpe.dealias.foreachPart { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's consider a situation like this:
object LibApi:
@deprecated
trait OldName
type NewName = OldName
when the author of a library decided to rename a trait so they created an nondeprecated type alias pointing to it. Should we raise a warning in this case as well?
Probably it would be better for the author to have
object LibApi:
@deprecated
type OldName = NewName
trait NewName
instead but can/should we enforce such a solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good example. We should not dealias here.
1a58f80
to
a1b7e17
Compare
Fixes #14034