Skip to content
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

fix(#17255): cannot find Scala companion module from Java #19773

Merged

Conversation

i10416
Copy link
Contributor

@i10416 i10416 commented Feb 24, 2024

To find Scala companion mudule from Java, we should strip module suffix $.
This provides workaround fixes #17255 as well as forward-port scala/scala#10644. , 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.

Related to scala/scala#10644

@i10416 i10416 force-pushed the fix/cannot-find-companion-module-in-java branch 2 times, most recently from 5ab2e1a to e17c97d Compare February 24, 2024 06:56
@i10416 i10416 changed the title fix: cannot find Scala companion module from Java fix(17255): cannot find Scala companion module from Java Feb 24, 2024
@i10416 i10416 changed the title fix(17255): cannot find Scala companion module from Java fix(#17255): cannot find Scala companion module from Java Feb 24, 2024
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
@i10416 i10416 force-pushed the fix/cannot-find-companion-module-in-java branch from e17c97d to 22d98d6 Compare February 24, 2024 09:33
Avoid skipping searchin a member by the original name even when name is likely
companion module name.
@i10416
Copy link
Contributor Author

i10416 commented Feb 25, 2024

CI is failing because of #19772

@i10416 i10416 requested a review from bishabosha February 25, 2024 14:29
@bishabosha bishabosha self-assigned this Feb 27, 2024
@bishabosha
Copy link
Member

Hello, thank you for opening this! I am getting to it

public static p.J f() {
return p.J.j;
}
public static p.Module$ module() {

This comment was marked as outdated.

Copy link
Member

@bishabosha bishabosha Feb 27, 2024

Choose a reason for hiding this comment

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

When the tree reaches TASTy, this looks like

    87:       DEFDEF(12) 16 [module]
    90:         EMPTYCLAUSE
    91:         SELECTtpt 17 [Module$]
    93:           SHAREDtype 3
    95:         ELIDED
    96:           TYPEREF 19 [Module[ModuleClass]]
    98:             SHAREDtype 3
   100:         STATIC

really, tree 91 should be SELECTtpt 19 [Module[ModuleClass]].

It would be very nice to perhaps also replace typeName("Module$") with typeName("Module").moduleClassName in typedSelect in Typer

Copy link
Member

Choose a reason for hiding this comment

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

can leave this to follow-up, as it is still better than status quo. However this does rely on the compiler (aka all TASTy clients) intrinsically knowing how to resolve Module$ when the definition is actually Module[ModuleClass]

Copy link
Member

Choose a reason for hiding this comment

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

new issue #19806

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for reviewing. I will investigate #19806 too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not familiar with TASTy, so it may take a while...

Copy link
Member

@bishabosha bishabosha Feb 28, 2024

Choose a reason for hiding this comment

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

The way to solve this is in typedSelect replacing the name in the Select tree, in the same way you did in javaFindMember

Copy link
Contributor Author

@i10416 i10416 Feb 28, 2024

Choose a reason for hiding this comment

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

How can I test TASTy?(or make sure SELECTtpt 17 [Module$] is replaced with
SELECTtpt 19 [Module[ModuleClass]] because testp/pos does not seem to detect the difference...)

Copy link
Member

@bishabosha bishabosha Feb 28, 2024

Choose a reason for hiding this comment

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

you can do it manually with the -Yprint-tasty flag which will print to the console any generated TASTy after pickler phase, make sure you use the required -Yjava-tasty also so that you generate tasty for java sources

Test imported type as well as fully-qualified type to check `findRef` codepath.

Co-authored-by: Jamie Thompson <[email protected]>
@bishabosha bishabosha merged commit de25aa3 into scala:main Feb 28, 2024
19 checks passed
@bishabosha
Copy link
Member

bishabosha commented Feb 28, 2024

Thank you very much for contributing this fix! I noticed its been a long time since you opened that issue originally, its probably annoying it didn't get any attention

@Kordyjan Kordyjan added this to the 3.4.2 milestone Mar 28, 2024
WojciechMazur added a commit that referenced this pull request Jul 3, 2024
…to LTS (#20959)

Backports #19773 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
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.

Companion object cannot be found from Java in Scala 3, but it compiles in Scala 2.x.
3 participants