Skip to content

Commit

Permalink
Fix imports for go modules named "external" (#6325)
Browse files Browse the repository at this point in the history
* Change the external paths regexp to match the first 'external' occurrence

* Add test case for packages named 'external'

---------

Co-authored-by: Borja Lorente <[email protected]>
  • Loading branch information
muldrik and Borja Lorente authored Mar 28, 2024
1 parent e115a13 commit cc2760e
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 2 deletions.
6 changes: 6 additions & 0 deletions examples/go/with_proto/MODULE.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
###############################################################################
# Bazel now uses Bzlmod by default to manage external dependencies.
# Please consider migrating your external dependencies from WORKSPACE to MODULE.bazel.
#
# For more details, please check https://github.com/bazelbuild/bazel/issues/18958
###############################################################################
8 changes: 8 additions & 0 deletions examples/go/with_proto/go/external/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "external",
srcs = ["external.go"],
importpath = "github.com/bazelbuild/intellij/examples/go/with_proto/go/external",
visibility = ["//visibility:public"],
)
4 changes: 4 additions & 0 deletions examples/go/with_proto/go/external/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Packages named `external`

This directory exists to make sure that packages named `external` are imported correctly, as per https://github.com/bazelbuild/intellij/issues/6324.
Targets depending on this package (such as `//lib:lib.go`) should resolve without red squigglies.
5 changes: 5 additions & 0 deletions examples/go/with_proto/go/external/external.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package external

func ExternalCats() []string {
return []string{"Ember", "Cinder"}
}
10 changes: 9 additions & 1 deletion examples/go/with_proto/go/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"fmt"
"github.com/bazelbuild/intellij/examples/go/with_proto/go/external"
"github.com/bazelbuild/intellij/examples/go/with_proto/go/lib"
"github.com/bazelbuild/intellij/examples/go/with_proto/proto"
"google.golang.org/grpc"
Expand All @@ -12,7 +13,7 @@ func main() {
fmt.Printf("AddToTwo(%d) = %d", num, lib.AddToTwo(num))
}

// This function exists to check that:
// serv exists to check that:
// - Third party symbols (`grpc.Server`) resolve.
// - Code generated from protobuf resolves (symbols in the `proto` package).
// - We should be able to "go to definition" in `proto.FooServiceServer`.
Expand All @@ -22,3 +23,10 @@ func serv(grpcSrv *grpc.Server, protoSrv *proto.FooServiceServer) {
req := proto.HelloRequest{Name: &name}
req.GetName()
}

// CountExternalCats function exists to check that packages that are named `external` resolve:
//
// We should be able to "go to definition" in `external.ExternalCats()`.
func CountExternalCats() int {
return len(external.ExternalCats())
}
21 changes: 21 additions & 0 deletions examples/go/with_proto/go/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,24 @@ func TestEnvVars(t *testing.T) {
}
}
}

func TestCountExternalCats(t *testing.T) {
testCases := []struct {
name string
expectedCount int
}{
{
name: "Cats",
expectedCount: 2,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
count := CountExternalCats()
if count != tc.expectedCount {
t.Errorf("Expected to have %d cats, got %d cats", tc.expectedCount, count)
}
})
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ private static File toRealFile(@Nullable File maybeExternal) {
if (externalString.contains("/external/")
&& !externalString.contains("/bazel-out/")
&& !externalString.contains("/blaze-out/")) {
return new File(externalString.replaceAll("/execroot.*/external/", "/external/"));
return new File(externalString.replaceAll("/execroot.*?/external/", "/external/"));
}
return maybeExternal;
}
Expand Down

0 comments on commit cc2760e

Please sign in to comment.