Skip to content

Conversation

@dmytr
Copy link
Contributor

@dmytr dmytr commented Sep 30, 2025

Currently code generator produces code that uses fully qualified type names conditionally. This can cause shadowing of packages by local identifiers.

This is somewhat related to #1911, but there are more cases when this problem occurs.

For example:

syntax = "proto3";

package echo;

service EchoService {
  rpc Echo(EchoMessage) returns (EchoMessage);
}

message EchoMessage {
  string echo = 1;
}

Causes the following compilation error:

[error] -- [E046] Cyclic Error: /home/me/example/target/scala-3.3.6/src_managed/main/echo/echo/EchoServiceGrpc.scala:28:22
[error] 28 |    def echo(request: echo.echo.EchoMessage): scala.concurrent.Future[echo.echo.EchoMessage]
[error]    |                      ^
[error]    |                      Cyclic reference involving method echo
[error]    |
[error]    |                       Run with -explain-cyclic for more details.
[error]    |
[error]    | longer explanation available when compiling with `-explain`

This PR changes code generator to always use fully qualified type names, which should prevent majority of cases where shadowing could cause a problem.

@dmytr
Copy link
Contributor Author

dmytr commented Oct 6, 2025

Job build (2_12, examples_and_formatting) was failing because generated code had to be updated. I've run ./examples_and_formatting.sh and committed the resulting changes in Format generated sources.

@dmytr
Copy link
Contributor Author

dmytr commented Oct 8, 2025

Hi @thesamet, maybe you could take a look when you have time, please? 🙏 Only the first commit contains actual changes. The second one contains only changes of generated code. Effective changes are quite small.

@thesamet thesamet merged commit 6f09c19 into scalapb:master Oct 8, 2025
12 checks passed
@dmytr
Copy link
Contributor Author

dmytr commented Oct 8, 2025

@thesamet thank you for merging this PR. Maybe you could publish it as an alpha3 release as well, please? Once it's published I could take it for a spin in our project. Thanks again!

@thesamet
Copy link
Contributor

thesamet commented Oct 9, 2025

Done

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.

2 participants