feature: navigate to Java files materialized from .proto files#8699
feature: navigate to Java files materialized from .proto files#8699damiankus-vl wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b427446 to
6732291
Compare
tgodzik
left a comment
There was a problem hiding this comment.
Looks pretty great! An I can confirm it works. I have some overall suggestions, nothing blocking.
| * - `getFoo(+1).` -> `getFoo().` | ||
| * - `setBar(+3).` -> `setBar().` | ||
| */ | ||
| private def stripDisambiguators(symbol: String): String = |
There was a problem hiding this comment.
Do we need this? Shouldn't we find the correct definition anyway?
There was a problem hiding this comment.
originally, stripDisambiguators was needed because there was no way to tell apart symbols related to overloaded methods (I think I've come across some case where a generated outline had two setters with the same names but different argument types)
I've added some logic to find the exact overload used by the caller based on the argument names and types, so we no longer need stripDisambiguators
f422d7a
There was a problem hiding this comment.
I think the method order correspond to +1,+2 etc. We shouldn't need parameters:
Disambiguator is:
Concatenation of a left parenthesis ((), a tag and a right parenthesis ()). If the definition is not overloaded, the tag is empty. Two definitions are overloaded if they have the same name and both require a disambiguator. If the definition is overloaded, the tag is computed from the order of appearance of overloads in the source code (see "Function declarations and definitions" below for an example):
Empty string for the definition that appears first.
+1 for the definition that appears second.
+2 for the definition that appears third.
from https://scalameta.org/docs/semanticdb/specification#symbol-1
|
Also, looks like this fixes #8653 could you add a rename test as well? |
Synthesize a Java outline from each .proto and materialize it as a read-only file under .metals/readonly, so go-to-definition on a proto-generated class (message, enum, gRPC stub) lands in the generated Java source alongside the .proto declaration. Route the materialized file to the proto's build target and feed the outlines through the turbine classpath so hover and completion resolve proto-generated types too. Works for every build tool with no build output and no configuration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6732291 to
c4562c0
Compare
The SemanticDB overload disambiguator carries no signature info and is computed independently at the call site (mtags-java) and when re-indexing the materialized outline (mtags), so it can disagree and the previous fix had to return every name-matching candidate instead of the one the user actually navigated to. Thread the call site's real resolved parameter names/types (MtagsIndexer.ParameterSignature) end-to-end through mtags-interfaces/mtags-shared/mtags-java into metals' DefinitionResult, and use them in DefinitionProviderProtobufSupport.findRange to narrow multiple name-matching candidates down to the one whose signature actually matches, comparing by dotted type-name suffix (the outline refers to sibling types by simple name, the call site always resolves fully qualified). Falls back to returning every name match, unchanged, whenever the new info is unavailable or ambiguous. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The parser only recognized `syntax = "proto2"/"proto3";` at the top of a .proto file, so any file using Protobuf Editions (`edition = "2023";`) threw a ProtoError and failed to generate a Java outline entirely, breaking Java-file navigation for those files.
c4562c0 to
2925ada
Compare
| syntax = Optional.of(parseSyntax()); | ||
| syntax = Optional.of(parseSyntax("syntax")); | ||
| break; | ||
| case "edition": |
There was a problem hiding this comment.
in the Bazel project there's a proto file that uses the edition keyword
https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/execution_graph_writer.proto
It breaks Metals with the following error
2026-07-24 10:46:57.230 [info] 2026.07.24 10:46:57 WARN Failed to generate Java outline for proto /Users/dkus/Documents/Projects/bazel/src/main/protobuf/execution_graph_writer.proto
scala.meta.internal.proto.diag.ProtoError: file:///Users/dkus/Documents/Projects/bazel/src/main/protobuf/execution_graph_writer.proto:15:1: Unexpected token: edition
at scala.meta.internal.proto.parse.Parser.error(Parser.java:886)
at scala.meta.internal.proto.parse.Parser.parseFile(Parser.java:92)
at scala.meta.internal.proto.parse.Parser.parse(Parser.java:36)
Commit 2925ada should fix that
Generate read-only Javas file from .proto files and materialize them under .metals/readonly, so go-to-definition on a proto-generated class (message, enum, gRPC stub) lands in the generated Java source alongside the .proto declaration. Route the materialized file to the proto's build target and feed the outlines through the turbine classpath so hover and completion resolve proto-generated types too.
DEMO
An example based on issue #8651 - https://github.com/buildfarm/buildfarm
Before
The return value of waitAndRead is treated as Object
The definition of WorkResponse points to the .proto file
https://github.com/user-attachments/assets/386ddd4f-db38-40dd-8505-c3904d62612d
After
The return value of waitAndRead if a WorkResponse
The definition of WorkResponse points to both the .proto file and the generated Java file
https://github.com/user-attachments/assets/0853b964-c20b-4af9-a803-925dc6ff012f