diff --git a/BUILD.bazel b/BUILD.bazel index 81bd04b6..2b812651 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -15,6 +15,7 @@ buildifier( # gazelle:resolve java com.beust.jcommander.converters @contrib_rules_jvm_deps//:com_beust_jcommander # gazelle:resolve java com.gazelle.java.javaparser.v0 //java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0:gazelle_java_build_v0_java_library # gazelle:resolve java com.google.common.annotations @contrib_rules_jvm_deps//:com_google_guava_guava +# gazelle:resolve java com.google.common.base @contrib_rules_jvm_deps//:com_google_guava_guava # gazelle:resolve java com.google.common.collect @contrib_rules_jvm_deps//:com_google_guava_guava # gazelle:resolve java com.google.errorprone.annotations.concurrent @contrib_rules_jvm_deps//:com_google_errorprone_error_prone_annotations # gazelle:resolve java com.google.gson @contrib_rules_jvm_deps//:com_google_code_gson_gson diff --git a/java/gazelle/README.md b/java/gazelle/README.md index 07efb5e3..84236fde 100644 --- a/java/gazelle/README.md +++ b/java/gazelle/README.md @@ -81,6 +81,11 @@ Currently, the gazelle plugin makes the following assumptions about the code it' 1. Non-test code doesn't depend on test code. 1. Non-test code used by one package of tests either lives in the same directory as those tests, or lives in a non-test-code directory. We also detect non-test code used from another test package, if that other package doesn't have a corresponding non-test code directory, but require you to manually set the visibility on the depended-on target, because this is an unexpected set-up. 1. Package names and class/interface names follow standard java conventions; that is: package names are all lower-case, and class and interface names start with Upper Case letters. +1. Code doesn't use types which it doesn't name _only_ through unnamed means, across multiple calls. For example, if some code calls `x.foo().bar()` where the return type of `foo` is defined in another target, and the calling code explicitly uses a type from that target somewhere else. In the case of `x.foo()`, we add exports so that the caller will have access to the return type of `foo()`, but do not track dependencies on the return types across _multiple_ calls. + + This limitation could be lifted, but would require us to export all _transitively_ used symbols from every function. This would serve to add direct dependencies between lots of targets, which can slow down compilation and reduce cache hits. + + In our experience, this kind of code is rare in Java - most code tends to either introduce intermediate variables (at which point the type gets used and we detect that a dependency needs to be added), or tends to already use the targets containing the intermediate types somewhere else (at which point the dependency will already exist), but we're open to discussion about this heuristic if it poses problems for a real-world codebase. If these assumptions are violated, the rest of the generation should still function properly, but the specific files which violate the assumptions (or depend on files which violate the assumptions) will not get complete results. We strive to emit warnings when this happens. diff --git a/java/gazelle/generate.go b/java/gazelle/generate.go index 0fc3b5f5..d56482b0 100644 --- a/java/gazelle/generate.go +++ b/java/gazelle/generate.go @@ -155,6 +155,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes // Files and imports for code which isn't tests, and isn't used as helpers in tests. productionJavaFiles := sorted_set.NewSortedSet([]string{}) productionJavaImports := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) + nonLocalJavaExports := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) // Files and imports for actual test classes. testJavaFiles := sorted_set.NewSortedSetFn([]javaFile{}, javaFileLess) @@ -177,13 +178,14 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes allPackageNames.Add(mJavaPkg.Name) if !mJavaPkg.TestPackage { - addNonLocalImports(productionJavaImports, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.Name, javaClassNamesFromFileNames) + addNonLocalImportsAndExports(productionJavaImports, nonLocalJavaExports, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, javaClassNamesFromFileNames) for _, f := range mJavaPkg.Files.SortedSlice() { productionJavaFiles.Add(filepath.Join(mRel, f)) } allMains.AddAll(mJavaPkg.Mains) } else { - addNonLocalImports(testJavaImports, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.Name, javaClassNamesFromFileNames) + // Tests don't get to export things, as things shouldn't depend on them. + addNonLocalImportsAndExports(testJavaImports, nil, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, javaClassNamesFromFileNames) for _, f := range mJavaPkg.Files.SortedSlice() { path := filepath.Join(mRel, f) file := javaFile{ @@ -197,9 +199,10 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes } else { allPackageNames.Add(javaPkg.Name) if javaPkg.TestPackage { - addNonLocalImports(testJavaImports, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.Name, javaClassNamesFromFileNames) + // Tests don't get to export things, as things shouldn't depend on them. + addNonLocalImportsAndExports(testJavaImports, nil, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, javaClassNamesFromFileNames) } else { - addNonLocalImports(productionJavaImports, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.Name, javaClassNamesFromFileNames) + addNonLocalImportsAndExports(productionJavaImports, nonLocalJavaExports, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, javaClassNamesFromFileNames) } allMains.AddAll(javaPkg.Mains) for _, f := range javaFilenamesRelativeToPackage { @@ -227,7 +230,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes }) if productionJavaFiles.Len() > 0 { - l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), productionJavaFiles.SortedSlice(), allPackageNames, nonLocalProductionJavaImports, false, &res) + l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), productionJavaFiles.SortedSlice(), allPackageNames, nonLocalProductionJavaImports, nonLocalJavaExports, false, &res) } for _, m := range allMains.SortedSlice() { @@ -249,7 +252,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes testJavaImportsWithHelpers.Add(tf.pkg) srcs = append(srcs, tf.pathRelativeToBazelWorkspaceRoot) } - l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), srcs, packages, testJavaImports, true, &res) + l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), srcs, packages, testJavaImports, nonLocalJavaExports, true, &res) } } @@ -349,21 +352,29 @@ func (l javaLang) collectRuntimeDeps(kind, name string, file *rule.File) *sorted } // We exclude intra-target imports because otherwise we'd get self-dependencies come resolve time. -func addNonLocalImports(to *sorted_set.SortedSet[types.PackageName], fromClasses *sorted_set.SortedSet[types.ClassName], fromPackages *sorted_set.SortedSet[types.PackageName], pkg types.PackageName, localClasses *sorted_set.SortedSet[string]) { - for _, imp := range fromClasses.SortedSlice() { - if pkg == imp.PackageName() { - if localClasses.Contains(imp.BareOuterClassName()) { +// toExports is optional and may be nil. All other parameters are required and must be non-nil. +func addNonLocalImportsAndExports(toImports *sorted_set.SortedSet[types.PackageName], toExports *sorted_set.SortedSet[types.PackageName], fromImportedClasses *sorted_set.SortedSet[types.ClassName], fromPackages *sorted_set.SortedSet[types.PackageName], fromExportedClasses *sorted_set.SortedSet[types.ClassName], pkg types.PackageName, localClasses *sorted_set.SortedSet[string]) { + toImports.AddAll(fromPackages) + addFilteringOutOwnPackage(toImports, fromImportedClasses, pkg, localClasses) + if toExports != nil { + addFilteringOutOwnPackage(toExports, fromExportedClasses, pkg, localClasses) + } +} + +func addFilteringOutOwnPackage(to *sorted_set.SortedSet[types.PackageName], from *sorted_set.SortedSet[types.ClassName], ownPackage types.PackageName, localOuterClassNames *sorted_set.SortedSet[string]) { + for _, fromPackage := range from.SortedSlice() { + if ownPackage == fromPackage.PackageName() { + if localOuterClassNames.Contains(fromPackage.BareOuterClassName()) { continue } } - if imp.PackageName().Name == "" { + if fromPackage.PackageName().Name == "" { continue } - to.Add(imp.PackageName()) + to.Add(fromPackage.PackageName()) } - to.AddAll(fromPackages) } func accumulateJavaFile(cfg *javaconfig.Config, testJavaFiles, testHelperJavaFiles *sorted_set.SortedSet[javaFile], separateTestJavaFiles map[javaFile]map[string]bzl.Expr, file javaFile, perClassMetadata map[string]java.PerClassMetadata, log zerolog.Logger) { @@ -389,7 +400,7 @@ func accumulateJavaFile(cfg *javaconfig.Config, testJavaFiles, testHelperJavaFil } } -func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBazelWorkspace string, name string, srcsRelativeToBazelWorkspace []string, packages, imports *sorted_set.SortedSet[types.PackageName], testonly bool, res *language.GenerateResult) { +func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBazelWorkspace string, name string, srcsRelativeToBazelWorkspace []string, packages, imports *sorted_set.SortedSet[types.PackageName], exports *sorted_set.SortedSet[types.PackageName], testonly bool, res *language.GenerateResult) { const ruleKind = "java_library" r := rule.NewRule(ruleKind, name) @@ -421,6 +432,7 @@ func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBa resolveInput := types.ResolveInput{ PackageNames: packages, ImportedPackageNames: imports, + ExportedPackageNames: exports, } res.Imports = append(res.Imports, resolveInput) } diff --git a/java/gazelle/generate_test.go b/java/gazelle/generate_test.go index 463ea3ae..3809298a 100644 --- a/java/gazelle/generate_test.go +++ b/java/gazelle/generate_test.go @@ -282,8 +282,9 @@ func TestAddNonLocalImports(t *testing.T) { src.Add(*name) } - dst := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) - addNonLocalImports(dst, src, sorted_set.NewSortedSetFn[types.PackageName]([]types.PackageName{}, types.PackageNameLess), types.NewPackageName("com.example.a.b"), sorted_set.NewSortedSet([]string{"Foo", "Bar"})) + depsDst := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) + exportsDst := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) + addNonLocalImportsAndExports(depsDst, exportsDst, src, sorted_set.NewSortedSetFn[types.PackageName]([]types.PackageName{}, types.PackageNameLess), sorted_set.NewSortedSetFn([]types.ClassName{}, types.ClassNameLess), types.NewPackageName("com.example.a.b"), sorted_set.NewSortedSet([]string{"Foo", "Bar"})) want := stringsToPackageNames([]string{ "com.another.a.b", @@ -292,7 +293,7 @@ func TestAddNonLocalImports(t *testing.T) { "com.example.a.b.c", }).SortedSlice() - if diff := cmp.Diff(want, dst.SortedSlice()); diff != "" { + if diff := cmp.Diff(want, depsDst.SortedSlice()); diff != "" { t.Errorf("filterImports() mismatch (-want +got):\n%s", diff) } } diff --git a/java/gazelle/lang.go b/java/gazelle/lang.go index 532ac2e1..b852d286 100644 --- a/java/gazelle/lang.go +++ b/java/gazelle/lang.go @@ -74,11 +74,25 @@ var kindWithoutRuntimeDeps = rule.KindInfo{ }, } +var javaLibraryKind = rule.KindInfo{ + NonEmptyAttrs: map[string]bool{ + "deps": true, + "exports": true, + "srcs": true, + }, + MergeableAttrs: map[string]bool{"srcs": true}, + ResolveAttrs: map[string]bool{ + "deps": true, + "exports": true, + "runtime_deps": true, + }, +} + func (l javaLang) Kinds() map[string]rule.KindInfo { return map[string]rule.KindInfo{ "java_binary": kindWithRuntimeDeps, "java_junit5_test": kindWithRuntimeDeps, - "java_library": kindWithRuntimeDeps, + "java_library": javaLibraryKind, "java_test": kindWithRuntimeDeps, "java_test_suite": kindWithRuntimeDeps, "java_proto_library": kindWithoutRuntimeDeps, diff --git a/java/gazelle/private/java/package.go b/java/gazelle/private/java/package.go index e0ea4d5b..25ff6bfc 100644 --- a/java/gazelle/private/java/package.go +++ b/java/gazelle/private/java/package.go @@ -9,6 +9,7 @@ type Package struct { Name types.PackageName ImportedClasses *sorted_set.SortedSet[types.ClassName] + ExportedClasses *sorted_set.SortedSet[types.ClassName] ImportedPackagesWithoutSpecificClasses *sorted_set.SortedSet[types.PackageName] Mains *sorted_set.SortedSet[types.ClassName] diff --git a/java/gazelle/private/javaparser/javaparser.go b/java/gazelle/private/javaparser/javaparser.go index 4f620e3f..ccc76894 100644 --- a/java/gazelle/private/javaparser/javaparser.go +++ b/java/gazelle/private/javaparser/javaparser.go @@ -73,6 +73,14 @@ func (r Runner) ParsePackage(ctx context.Context, in *ParsePackageRequest) (*jav } importedClasses.Add(*className) } + exportedClasses := sorted_set.NewSortedSetFn([]types.ClassName{}, types.ClassNameLess) + for _, export := range resp.GetExportedClasses() { + className, err := types.ParseClassName(export) + if err != nil { + return nil, fmt.Errorf("failed to parse exports: %w", err) + } + exportedClasses.Add(*className) + } importedPackages := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) for _, pkg := range resp.GetImportedPackagesWithoutSpecificClasses() { importedPackages.Add(types.NewPackageName(pkg)) @@ -85,6 +93,7 @@ func (r Runner) ParsePackage(ctx context.Context, in *ParsePackageRequest) (*jav return &java.Package{ Name: packageName, ImportedClasses: importedClasses, + ExportedClasses: exportedClasses, ImportedPackagesWithoutSpecificClasses: importedPackages, Mains: mains, Files: sorted_set.NewSortedSet(in.Files), diff --git a/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto b/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto index 000261e4..772a446d 100644 --- a/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto +++ b/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto @@ -45,6 +45,9 @@ message Package { repeated string mains = 3; map per_class_metadata = 4; + + // The list of classes which dependers may need to have exported to them because they are part of the public interface of the request. + repeated string exported_classes = 6; } message PerClassMetadata { diff --git a/java/gazelle/private/types/types.go b/java/gazelle/private/types/types.go index f5fcfe0a..ea2d05bf 100644 --- a/java/gazelle/private/types/types.go +++ b/java/gazelle/private/types/types.go @@ -96,6 +96,7 @@ func ClassNameLess(l, r ClassName) bool { type ResolveInput struct { PackageNames *sorted_set.SortedSet[PackageName] ImportedPackageNames *sorted_set.SortedSet[PackageName] + ExportedPackageNames *sorted_set.SortedSet[PackageName] } type ResolvableJavaPackage struct { diff --git a/java/gazelle/resolve.go b/java/gazelle/resolve.go index 888e055b..05a3780f 100644 --- a/java/gazelle/resolve.go +++ b/java/gazelle/resolve.go @@ -80,25 +80,26 @@ func (Resolver) Embeds(r *rule.Rule, from label.Label) []label.Label { func (jr Resolver) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.RemoteCache, r *rule.Rule, imports interface{}, from label.Label) { resolveInput := imports.(types.ResolveInput) - javaImports := resolveInput.ImportedPackageNames - if javaImports.Len() == 0 { - return - } - deps := sorted_set.NewSortedSetFn[label.Label]([]label.Label{}, labelLess) + isTestRule := isTestRule(r.Kind()) + + jr.populateAttr(c, r, "deps", resolveInput.ImportedPackageNames, ix, isTestRule, from, resolveInput.PackageNames) + jr.populateAttr(c, r, "exports", resolveInput.ExportedPackageNames, ix, isTestRule, from, resolveInput.PackageNames) +} + +func (jr Resolver) populateAttr(c *config.Config, r *rule.Rule, attrName string, requiredPackageNames *sorted_set.SortedSet[types.PackageName], ix *resolve.RuleIndex, isTestRule bool, from label.Label, ownPackageNames *sorted_set.SortedSet[types.PackageName]) { + labels := sorted_set.NewSortedSetFn[label.Label]([]label.Label{}, labelLess) - for _, implicitDep := range r.AttrStrings("deps") { + for _, implicitDep := range r.AttrStrings(attrName) { l, err := label.Parse(implicitDep) if err != nil { - panic(fmt.Sprintf("error converting implicit dep %q to label: %v", implicitDep, err)) + panic(fmt.Sprintf("error converting implicit %s %q to label: %v", attrName, implicitDep, err)) } - deps.Add(l) + labels.Add(l) } - isTestRule := isTestRule(r.Kind()) - - for _, imp := range javaImports.SortedSlice() { - dep, err := jr.convertImport(c, imp, ix, from, isTestRule, resolveInput.PackageNames) + for _, imp := range requiredPackageNames.SortedSlice() { + dep, err := jr.resolveSinglePackage(c, imp, ix, from, isTestRule, ownPackageNames) if err != nil { jr.lang.logger.Error().Str("import", dep.String()).Err(err).Msg("error converting import") panic(fmt.Sprintf("error converting import: %s", err)) @@ -107,18 +108,20 @@ func (jr Resolver) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.Rem continue } - deps.Add(simplifyLabel(c.RepoName, dep, from)) + labels.Add(simplifyLabel(c.RepoName, dep, from)) } - if deps.Len() > 0 { - var exprs []build.Expr - for _, l := range deps.SortedSlice() { + var exprs []build.Expr + if labels.Len() > 0 { + for _, l := range labels.SortedSlice() { if l.Relative && l.Name == from.Name { continue } exprs = append(exprs, &build.StringExpr{Value: l.String()}) } - r.SetAttr("deps", exprs) + } + if len(exprs) > 0 { + r.SetAttr(attrName, exprs) } } @@ -148,7 +151,7 @@ func simplifyLabel(repoName string, l label.Label, from label.Label) label.Label return l } -func (jr *Resolver) convertImport(c *config.Config, imp types.PackageName, ix *resolve.RuleIndex, from label.Label, isTestRule bool, ownPackageNames *sorted_set.SortedSet[types.PackageName]) (out label.Label, err error) { +func (jr *Resolver) resolveSinglePackage(c *config.Config, imp types.PackageName, ix *resolve.RuleIndex, from label.Label, isTestRule bool, ownPackageNames *sorted_set.SortedSet[types.PackageName]) (out label.Label, err error) { cacheKey := types.NewResolvableJavaPackage(imp, false, false) importSpec := resolve.ImportSpec{Lang: languageName, Imp: cacheKey.String()} if ol, found := resolve.FindRuleWithOverride(c, importSpec, languageName); found { @@ -170,7 +173,7 @@ func (jr *Resolver) convertImport(c *config.Config, imp types.PackageName, ix *r jr.lang.logger.Error(). Str("pkg", imp.Name). Strs("targets", labels). - Msg("convertImport found MULTIPLE results in rule index") + Msg("resolveSinglePackage found MULTIPLE results in rule index") } if v, ok := jr.internalCache.Get(cacheKey); ok { diff --git a/java/gazelle/testdata/interface_exports/.bazelrc b/java/gazelle/testdata/interface_exports/.bazelrc new file mode 100644 index 00000000..5992d3f3 --- /dev/null +++ b/java/gazelle/testdata/interface_exports/.bazelrc @@ -0,0 +1,4 @@ +build --java_runtime_version=remotejdk_11 +build --java_language_version=11 +build --tool_java_runtime_version=remotejdk_11 +build --tool_java_language_version=11 diff --git a/java/gazelle/testdata/interface_exports/WORKSPACE b/java/gazelle/testdata/interface_exports/WORKSPACE new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/interface_exports/expectedStderr.txt b/java/gazelle/testdata/interface_exports/expectedStderr.txt new file mode 100644 index 00000000..ca3e793a --- /dev/null +++ b/java/gazelle/testdata/interface_exports/expectedStderr.txt @@ -0,0 +1 @@ +12:00AM WRN java/gazelle/private/maven/resolver.go:XXX > not loading maven dependencies error="open %WORKSPACEPATH%/maven_install.json: no such file or directory" _c=maven-resolver diff --git a/java/gazelle/testdata/interface_exports/src/main/java/com/example/consumer/BUILD.out b/java/gazelle/testdata/interface_exports/src/main/java/com/example/consumer/BUILD.out new file mode 100644 index 00000000..ec004ab7 --- /dev/null +++ b/java/gazelle/testdata/interface_exports/src/main/java/com/example/consumer/BUILD.out @@ -0,0 +1,8 @@ +load("@rules_java//java:defs.bzl", "java_library") + +java_library( + name = "consumer", + srcs = ["Consumer.java"], + visibility = ["//:__subpackages__"], + deps = ["//src/main/java/com/example/iface"], +) diff --git a/java/gazelle/testdata/interface_exports/src/main/java/com/example/consumer/Consumer.java b/java/gazelle/testdata/interface_exports/src/main/java/com/example/consumer/Consumer.java new file mode 100644 index 00000000..c50bc11e --- /dev/null +++ b/java/gazelle/testdata/interface_exports/src/main/java/com/example/consumer/Consumer.java @@ -0,0 +1,10 @@ +package com.example.consumer; + +import com.example.iface.Interface; + +public class Consumer { + public void useInterface(Interface i) { + var t = i.get(); + System.out.println(t.stringify() + "hello"); + } +} diff --git a/java/gazelle/testdata/interface_exports/src/main/java/com/example/iface/BUILD.out b/java/gazelle/testdata/interface_exports/src/main/java/com/example/iface/BUILD.out new file mode 100644 index 00000000..cc4194b1 --- /dev/null +++ b/java/gazelle/testdata/interface_exports/src/main/java/com/example/iface/BUILD.out @@ -0,0 +1,9 @@ +load("@rules_java//java:defs.bzl", "java_library") + +java_library( + name = "iface", + srcs = ["Interface.java"], + visibility = ["//:__subpackages__"], + exports = ["//src/main/java/com/example/types"], + deps = ["//src/main/java/com/example/types"], +) diff --git a/java/gazelle/testdata/interface_exports/src/main/java/com/example/iface/Interface.java b/java/gazelle/testdata/interface_exports/src/main/java/com/example/iface/Interface.java new file mode 100644 index 00000000..27ad474c --- /dev/null +++ b/java/gazelle/testdata/interface_exports/src/main/java/com/example/iface/Interface.java @@ -0,0 +1,7 @@ +package com.example.iface; + +import com.example.types.Type; + +public interface Interface { + public Type get(); +} diff --git a/java/gazelle/testdata/interface_exports/src/main/java/com/example/types/BUILD.out b/java/gazelle/testdata/interface_exports/src/main/java/com/example/types/BUILD.out new file mode 100644 index 00000000..b1ff310a --- /dev/null +++ b/java/gazelle/testdata/interface_exports/src/main/java/com/example/types/BUILD.out @@ -0,0 +1,7 @@ +load("@rules_java//java:defs.bzl", "java_library") + +java_library( + name = "types", + srcs = ["Type.java"], + visibility = ["//:__subpackages__"], +) diff --git a/java/gazelle/testdata/interface_exports/src/main/java/com/example/types/Type.java b/java/gazelle/testdata/interface_exports/src/main/java/com/example/types/Type.java new file mode 100644 index 00000000..2b60bee0 --- /dev/null +++ b/java/gazelle/testdata/interface_exports/src/main/java/com/example/types/Type.java @@ -0,0 +1,7 @@ +package com.example.types; + +public class Type { + public String stringify() { + return "I am Type"; + } +} diff --git a/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java b/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java index 429fa9bf..77f25c85 100644 --- a/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java +++ b/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java @@ -1,8 +1,10 @@ package com.github.bazel_contrib.contrib_rules_jvm.javaparser.generators; +import static javax.lang.model.element.Modifier.PRIVATE; import static javax.lang.model.element.Modifier.PUBLIC; import static javax.lang.model.element.Modifier.STATIC; +import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; @@ -48,6 +50,8 @@ public class ClasspathParser { private final Set usedTypes = new TreeSet<>(); private final Set usedPackagesWithoutSpecificTypes = new TreeSet<>(); + + private final Set exportedTypes = new TreeSet<>(); private final Set packages = new TreeSet<>(); private final Set mainClasses = new TreeSet<>(); @@ -71,6 +75,10 @@ public ImmutableSet getUsedPackagesWithoutSpecificTypes() { return ImmutableSet.copyOf(usedPackagesWithoutSpecificTypes); } + public ImmutableSet getExportedTypes() { + return ImmutableSet.copyOf(exportedTypes); + } + public ImmutableSet getPackages() { return ImmutableSet.copyOf(packages); } @@ -216,7 +224,10 @@ public Void visitMethod(com.sun.source.tree.MethodTree m, Void v) { && ((PrimitiveTypeTree) m.getReturnType()).getPrimitiveTypeKind() == TypeKind.VOID) { isVoidReturn = true; } else if (m.getReturnType() != null) { - checkFullyQualifiedType(m.getReturnType()); + Set types = checkFullyQualifiedType(m.getReturnType()); + if (!m.getModifiers().getFlags().contains(PRIVATE)) { + exportedTypes.addAll(types); + } } handleAnnotations(m.getModifiers().getAnnotations()); @@ -290,23 +301,33 @@ public Void visitVariable(VariableTree node, Void unused) { return super.visitVariable(node, unused); } - private void checkFullyQualifiedType(Tree identifier) { + @Nullable + private Set checkFullyQualifiedType(Tree identifier) { + Set types = new TreeSet<>(); if (identifier.getKind() == Tree.Kind.IDENTIFIER || identifier.getKind() == Tree.Kind.MEMBER_SELECT) { String typeName = identifier.toString(); - if (typeName.contains(".")) { + List components = Splitter.on(".").splitToList(typeName); + if (currentFileImports.containsKey(components.get(0))) { + String importedType = currentFileImports.get(components.get(0)); + usedTypes.add(importedType); + types.add(importedType); + } else if (components.size() > 1) { usedTypes.add(typeName); + types.add(typeName); } } else if (identifier.getKind() == Tree.Kind.PARAMETERIZED_TYPE) { Tree baseType = ((ParameterizedTypeTree) identifier).getType(); checkFullyQualifiedType(baseType); ((ParameterizedTypeTree) identifier) - .getTypeArguments() - .forEach(this::checkFullyQualifiedType); + .getTypeArguments().stream() + .flatMap(t -> checkFullyQualifiedType(t).stream()) + .forEach(types::add); } else if (identifier.getKind() == Tree.Kind.ARRAY_TYPE) { Tree baseType = ((ArrayTypeTree) identifier).getType(); - checkFullyQualifiedType(baseType); + types.addAll(checkFullyQualifiedType(baseType)); } + return types; } } diff --git a/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java b/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java index a20e9e4e..9975dffa 100644 --- a/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java +++ b/java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/GrpcServer.java @@ -152,6 +152,7 @@ private Package getImports(ParsePackageRequest request) { Package.newBuilder() .setName(Iterables.getOnlyElement(packages)) .addAllImportedClasses(parser.getUsedTypes()) + .addAllExportedClasses(parser.getExportedTypes()) .addAllImportedPackagesWithoutSpecificClasses( parser.getUsedPackagesWithoutSpecificTypes()) .addAllMains(parser.getMainClasses()); diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java index 2f215d7b..3cb2b0f6 100644 --- a/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java @@ -261,6 +261,49 @@ public void testAnonymousInnerClass() throws IOException { assertEquals(expected, parser.getUsedTypes()); } + @Test + public void testMethodWithImportedType() throws IOException { + List files = + List.of( + testFiles.get( + "/workspace/com/gazelle/java/javaparser/generators/MethodWithImportedType.java")); + parser.parseClasses(files); + + Set expected = + Set.of( + "com.example.OuterReturnType", + "com.example.OtherOuterReturnType", + "com.example.Outer.InnerReturnType"); + assertEquals(expected, parser.getUsedTypes()); + } + + @Test + public void testInterfaceExports() throws IOException { + List files = + List.of( + testFiles.get( + "/workspace/com/gazelle/java/javaparser/generators/ExportingInterface.java")); + parser.parseClasses(files); + + Set expected = Set.of("example.external.NeedsExporting", "example.external.Outer"); + assertEquals(expected, parser.getExportedTypes()); + } + + @Test + public void testClassExports() throws IOException { + List files = + List.of( + testFiles.get("/workspace/com/gazelle/java/javaparser/generators/ExportingClass.java")); + parser.parseClasses(files); + + Set expected = + Set.of( + "example.external.PackageReturn", + "example.external.ProtectedReturn", + "example.external.PublicReturn"); + assertEquals(expected, parser.getExportedTypes()); + } + private TreeSet treeSet(T... values) { TreeSet set = new TreeSet<>(); for (T value : values) { diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/ExportingClass.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/ExportingClass.java new file mode 100644 index 00000000..eba5f3de --- /dev/null +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/ExportingClass.java @@ -0,0 +1,24 @@ +package com.github.bazel_contrib.contrib_rules_jvm.javaparser.generators.workspace.com.gazelle.java.javaparser.generators; + +import example.external.PackageReturn; +import example.external.PrivateReturn; +import example.external.ProtectedReturn; +import example.external.PublicReturn; + +public class ExportingClass { + PackageReturn getPackage() { + return null; + } + + private PrivateReturn getPrivate() { + return null; + } + + protected ProtectedReturn getProtected() { + return null; + } + + public PublicReturn getPublic() { + return null; + } +} diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/ExportingInterface.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/ExportingInterface.java new file mode 100644 index 00000000..4a27225b --- /dev/null +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/ExportingInterface.java @@ -0,0 +1,10 @@ +package workspace.com.gazelle.java.javaparser.generators; + +import example.external.NeedsExporting; +import example.external.Outer; + +public interface ExportingInterface { + NeedsExporting get(); + + Outer.Inner getInner(); +} diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/MethodWithImportedType.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/MethodWithImportedType.java new file mode 100644 index 00000000..0b789d5c --- /dev/null +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/MethodWithImportedType.java @@ -0,0 +1,13 @@ +package workspace.com.gazelle.java.javaparser.generators; + +import com.example.OtherOuterReturnType; +import com.example.Outer.InnerReturnType; +import com.example.OuterReturnType; + +public interface MethodWithImportedType { + public OuterReturnType getOne(); + + public OtherOuterReturnType.Inner getTwo(); + + public InnerReturnType getThree(); +}