Use Java rules for member lookup in .java sources#12884
Use Java rules for member lookup in .java sources#12884smarter merged 5 commits intoscala:masterfrom
Conversation
|
Please may you squash the WIP commits, or give them meaningful names. |
smarter
left a comment
There was a problem hiding this comment.
I have a few comments but this looks like a great addition!
|
|
||
| toSearch.iterator.map { bc => | ||
| val pre1 = bc.thisType | ||
| val found = pre1.findMember(name,pre,required,excluded | Flags.TypeParam) |
There was a problem hiding this comment.
There's an important difference with what Scala 2 does in scala/scala@b1cb58a here: Scala 2 uses decl which will look for declarations in the current class, whereas the code here uses findMember which will also look for members in base classes, so you should be able to do this call only once on either preSym or preSym.companionClass, then in the iterator.map you only need to look at the companions.
Also instead of .map(...).find(...) I think you could use .collectFirst(...)
There was a problem hiding this comment.
Sorry, I couldn’t find a proper partial function of collectFirst to replace .map(...).find(...)
There was a problem hiding this comment.
I think the solution is to replace .map(...).find(_.exists) with just .find(f(_).exists) where f is the current body of map
There was a problem hiding this comment.
Then, the result of find is one element of toSearch, not the result of f(_).
PS the computing in iterator.map is lazy until invoke find.
|
@smarter Very nice comment, let me amend them. |
4eb2203 to
0fbacf7
Compare
0fbacf7 to
76e941c
Compare
| preSym.asClass.baseClasses | ||
|
|
||
| toSearch.iterator.map { bc => | ||
| val pre1 = bc.thisType.typeSymbol.companionClass.thisType |
There was a problem hiding this comment.
Are you sure this line is working as intended? companionClass will find the companion class of an object, but I think this code wants to find the companion object of a class instead, in which case it should use companionModule instead.
There was a problem hiding this comment.
Yes, it works, but using companionModule is better obviously.
96014e0 to
dcccaaa
Compare
| } | ||
|
|
||
| List(makeCompanionObject(cdefNew, statics), cdefNew) | ||
| } |
There was a problem hiding this comment.
I also deleted the import companion object in the java class.
I think this will make both JavaParsers and generated java AST more simplified.
smarter
left a comment
There was a problem hiding this comment.
LGTM, thanks for implementing this!
To find Scala companion mudule from Java in mixed sources, we should strip module suffix `$`. This provides workaround for scala#17255, but it requires some refinment to fix it because not-fully-qualified type like the following example still fails to compile due to missing symbol. ```java package example; public class Bar { private static final Foo$ MOD = Foo$.MODULE; } ``` This is because `pre` in `javaFindMember` for `Foo` in the case above is `<root>`, not `example` and therefore `pre.findMember` looks for `<root>.Foo` instead of `example.Foo`. I'm not sure whether the qualifier is intentionally dropped. References - scala#12884 - scala/scala#7671
To find Scala companion mudule from Java in mixed sources, we should strip module suffix `$`. This provides workaround for #17255, but it requires some refinment to fix it because not-fully-qualified type like the following example still fails to compile due to missing symbol. ```java package example; public class Bar { private static final Foo$ MOD = Foo$.MODULE; } ``` This is because `pre` in `javaFindMember` for `Foo` in the case above is `<root>`, not `example` and therefore `pre.findMember` looks for `<root>.Foo` instead of `example.Foo`. I'm not sure whether the qualifier is intentionally dropped. References - #12884 - scala/scala#7671 [Cherry-picked 22d98d6][modified]
Migrate scala/scala#7671 to Scala3
Fixes #6138 #10956 #12566