IDE indexing broken for go_proto_library with custom compilers

[H5-O5]

Description of the bug:

Hi everyone, I wonder if someone could help me with this issue:

I have custom proto plugins, and my BUILD file looks like this:

load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")

proto_library(
    name = "cache_proto",
    srcs = [
        "cache_api.proto",
        "cache_service.proto",
    ],
    visibility = ["//visibility:public"],
    deps = [
        "@com_google_protobuf//:any_proto",
    ],
)

go_proto_library(
    name = "cache_go_proto",
    importpath = "my.example.com/proj/cachepb",
    proto = ":cache_proto",
    visibility = ["//visibility:public"],
)

go_proto_library(
    name = "cache_go_proto_my_plugin",
    compilers = [
        "@my_plugin_repo//foo:my_plugin",
    ],
    importpath = "my.example.com/proj/cachepb/cacheservice",
    proto = ":cache_proto",
    visibility = ["//visibility:public"],
    deps = [
        ":cache_go_proto",
    ],
)

where my_plugin will generate some codes in a different importpath. bazel build is happy (that is, if a target depends on cache_go_proto_my_plugin, it can successfully use the import path without a problem). But GoLand with bazel plugin would only index my.example.com/proj/cachepb but not my.example.com/proj/cachepb/cacheservice.

Interestingly, GoLand shows the import as green text with red wavy underline showing unused import. If I type cacheservice, it pops up completion entries correctly. But if you write foo := cacheservice.SomeFunc(), the SomeFunc() is marked as red. You can't jump to SomeFunc and foo.<TAB> doesn't work.

What could be the problem and how can I debug this?

Which category does this issue belong to?

No response

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Creating an MWE is not easy. Let's see if there are things I could try before I create an MWE.

Which Intellij IDE are you using? Please provide the specific version.

#GO-243.21565.208

What programming languages and tools are you using? Please provide specific versions.

No response

What Bazel plugin version are you using?

tried latest stable version and DEV version

Have you found anything relevant by searching the web?

Nope

Any other information, logs, or outputs that you want to share?

No response

[tpasternak]

Cc @iliakondratev

[H5-O5]

I got an update for this:

It seems that the problem is due to // + build ignore. That is:

Let's say @my_plugin_repo//foo:my_plugin claims 6 files, but the protoc-plugin only generates 4 files. The missing 2 files will be generated by "rules_go" containing

// +build ignore

package ignore

Then, getPackageName somehow recognizes this, and return "ignore" as package name.

For the official rules, the package name would be basename(importpath), which is cachepb in this case.

I hacked getPackageName to return basename(importpath) and everything seems to work now. I haven't fully tested it but this is promising.

[iliakondratev]

Hi @H5-O5, thank you for the details. Would you expect that getPackageName respects the +build ignore flag while resolving packages and falls back to basename(importpath) in this case?

[H5-O5]

No. Given that "build ignore" is generated by rules_go, and its name clearly indicates that this package should be ignored, I'd suggest adding this filter:

Edit: I misunderstood. The answer is yes. getPackageName should respect +build ignore instead of package ignore.

[iliakondratev]

@H5-O5, thank you for your suggestion. While I see where you’re coming from, ignore can be a valid Go package and cause false positives. Filtering out files with the +build ignore tag seems more appropriate and safer in this case, aligning with how build tags are intended to be used in Go tooling.

Let me know if you have additional thoughts or scenarios where this approach might fall short.

[iliakondratev]

Here's an example of how build flags help avoid similar package conflicts in GoLand (without a Bazel plugin): link

[H5-O5]

OK. I got it. The filter should be looking at +build ignore instead of package name.

fyi, this is where the placeholder go file is created:

https://github.com/bazel-contrib/rules_go/blob/63b0dcb1ebf8678997fea7a6b253214aa4a9f447/go/tools/builders/protoc.go#L189

Could you consider this patch instead?

     return files.stream()
         .map(file -> VfsUtils.resolveVirtualFile(file, /* refreshIfNeeded= */ false))
         .filter(Objects::nonNull)
         .map(psiManager::findFile)
         .filter(GoFile.class::isInstance)
         .map(GoFile.class::cast)
+        .filter(goFile -> !Objects.equals(goFile.getBuildFlags(), "ignore"))
         .map(GoFile::getCanonicalPackageName) // strips _test suffix from test packages
         .filter(Objects::nonNull)
         .filter(packageName -> !packageName.equals("ignore"))

If so, I'd like to make a PR of it.

[iliakondratev]

Hi @H5-05, thanks for the patch—it looks good! Please go ahead and open a PR, we’ll review it.