diff --git a/java/gazelle/constants.go b/java/gazelle/constants.go index 5a8f1791..a20f0b3a 100644 --- a/java/gazelle/constants.go +++ b/java/gazelle/constants.go @@ -4,3 +4,4 @@ package gazelle // rules. This attribute contains a list of package names (as type types.PackageName) it can be imported // for. Note that the Java plugin currently uses package names, not classes, as its importable unit. const packagesKey = "_java_packages" +const classesKey = "_java_classes" diff --git a/java/gazelle/generate.go b/java/gazelle/generate.go index 0a30ab17..4ea6b4a4 100644 --- a/java/gazelle/generate.go +++ b/java/gazelle/generate.go @@ -29,7 +29,13 @@ type javaFile struct { } func (jf *javaFile) ClassName() *types.ClassName { - className := types.NewClassName(jf.pkg, strings.TrimSuffix(filepath.Base(jf.pathRelativeToBazelWorkspaceRoot), ".java")) + name := filepath.Base(jf.pathRelativeToBazelWorkspaceRoot) + if strings.HasSuffix(name, ".java") { + name = strings.TrimSuffix(name, ".java") + } else if strings.HasSuffix(name, ".kt") { + name = strings.TrimSuffix(name, ".kt") + } + className := types.NewClassName(jf.pkg, name) return &className } @@ -155,6 +161,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes productionJavaImports := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) productionJavaImportedClasses := sorted_set.NewSortedSetFn([]types.ClassName{}, types.ClassNameLess) nonLocalJavaExports := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) + nonLocalJavaExportedClasses := sorted_set.NewSortedSetFn([]types.ClassName{}, types.ClassNameLess) // Files and imports for actual test classes. testJavaFiles := sorted_set.NewSortedSetFn([]javaFile{}, javaFileLess) @@ -180,14 +187,16 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes allPackageNames.Add(mJavaPkg.Name) if !mJavaPkg.TestPackage { - addNonLocalImportsAndExports(productionJavaImports, productionJavaImportedClasses, nonLocalJavaExports, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, likelyLocalClassNames) + addNonLocalImportsAndExports(productionJavaImports, productionJavaImportedClasses, nonLocalJavaExports, nil, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, likelyLocalClassNames) for _, f := range mJavaPkg.Files.SortedSlice() { productionJavaFiles.Add(filepath.Join(mRel, f)) + jf := javaFile{pathRelativeToBazelWorkspaceRoot: filepath.Join(mRel, f), pkg: mJavaPkg.Name} + nonLocalJavaExportedClasses.Add(*jf.ClassName()) } allMains.AddAll(mJavaPkg.Mains) } else { // Tests don't get to export things, as things shouldn't depend on them. - addNonLocalImportsAndExports(testJavaImports, testJavaImportedClasses, nil, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, likelyLocalClassNames) + addNonLocalImportsAndExports(testJavaImports, testJavaImportedClasses, nil, nil, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, likelyLocalClassNames) for _, f := range mJavaPkg.Files.SortedSlice() { path := filepath.Join(mRel, f) file := javaFile{ @@ -205,9 +214,9 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes allPackageNames.Add(javaPkg.Name) if javaPkg.TestPackage { // Tests don't get to export things, as things shouldn't depend on them. - addNonLocalImportsAndExports(testJavaImports, testJavaImportedClasses, nil, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, likelyLocalClassNames) + addNonLocalImportsAndExports(testJavaImports, testJavaImportedClasses, nil, nil, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, likelyLocalClassNames) } else { - addNonLocalImportsAndExports(productionJavaImports, productionJavaImportedClasses, nonLocalJavaExports, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, likelyLocalClassNames) + addNonLocalImportsAndExports(productionJavaImports, productionJavaImportedClasses, nonLocalJavaExports, nil, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, likelyLocalClassNames) } allMains.AddAll(javaPkg.Mains) for _, f := range srcFilenamesRelativeToPackage { @@ -220,6 +229,8 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes accumulateJavaFile(cfg, testJavaFiles, testHelperJavaFiles, separateTestJavaFiles, file, javaPkg.PerClassMetadata, log) } else { productionJavaFiles.Add(path) + jf := javaFile{pathRelativeToBazelWorkspaceRoot: path, pkg: javaPkg.Name} + nonLocalJavaExportedClasses.Add(*jf.ClassName()) } } for _, annotationClass := range javaPkg.AllAnnotations().SortedSlice() { @@ -335,7 +346,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes } } - l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), productionJavaFiles.SortedSlice(), resourcesDirectRef, resourcesRuntimeDep, allPackageNames, nonLocalProductionJavaImports, nonLocalProductionJavaImportedClasses, nonLocalJavaExports, annotationProcessorClasses, false, javaLibraryKind, &res, cfg, args.Config.RepoName) + l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), productionJavaFiles.SortedSlice(), resourcesDirectRef, resourcesRuntimeDep, allPackageNames, nonLocalProductionJavaImports, nonLocalProductionJavaImportedClasses, nonLocalJavaExports, nonLocalJavaExportedClasses, annotationProcessorClasses, false, javaLibraryKind, &res, cfg, args.Config.RepoName) } if cfg.GenerateBinary() { @@ -345,6 +356,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes // We add special packages to point to testonly libraries which - this accumulates them, // as well as the existing java imports of tests. testJavaImportsWithHelpers := testJavaImports.Clone() + testJavaImportedClassesWithHelpers := testJavaImportedClasses.Clone() if testHelperJavaFiles.Len() > 0 { // Suites generate their own helper library. @@ -355,10 +367,11 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes for _, tf := range testHelperJavaFiles.SortedSlice() { packages.Add(tf.pkg) testJavaImportsWithHelpers.Add(tf.pkg) + testJavaImportedClassesWithHelpers.Add(*tf.ClassName()) srcs = append(srcs, tf.pathRelativeToBazelWorkspaceRoot) } // Test helper libraries typically don't have resources - l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), srcs, "", "", packages, testJavaImports, testJavaImportedClasses, nonLocalJavaExports, annotationProcessorClasses, true, javaLibraryKind, &res, cfg, args.Config.RepoName) + l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), srcs, "", "", packages, testJavaImports, testJavaImportedClasses, nonLocalJavaExports, nonLocalJavaExportedClasses, annotationProcessorClasses, true, javaLibraryKind, &res, cfg, args.Config.RepoName) } } @@ -370,7 +383,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes case "file": for _, tf := range testJavaFiles.SortedSlice() { separateJavaTestReasons := separateTestJavaFiles[tf] - l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), tf, isModule, testJavaImportsWithHelpers, testJavaImportedClasses, annotationProcessorClasses, nil, separateJavaTestReasons.wrapper, separateJavaTestReasons.attributes, &res) + l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), tf, isModule, testJavaImportsWithHelpers, testJavaImportedClassesWithHelpers, annotationProcessorClasses, nil, separateJavaTestReasons.wrapper, separateJavaTestReasons.attributes, &res) } case "suite": @@ -390,6 +403,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes srcs = append(srcs, strings.TrimPrefix(filepath.ToSlash(src.pathRelativeToBazelWorkspaceRoot), args.Rel+"/")) } } + sort.Strings(srcs) if len(srcs) > 0 { l.generateJavaTestSuite( args.File, @@ -398,7 +412,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes packageNames, cfg.MavenRepositoryName(), testJavaImportsWithHelpers, - testJavaImportedClasses, + testJavaImportedClassesWithHelpers, annotationProcessorClasses, cfg.GetCustomJavaTestFileSuffixes(), testHelperJavaFiles.Len() > 0, @@ -416,7 +430,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes testHelperDep = ptr(testHelperLibname(suiteName)) } separateJavaTestReasons := separateTestJavaFiles[src] - l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), src, isModule, testJavaImportsWithHelpers, testJavaImportedClasses, annotationProcessorClasses, testHelperDep, separateJavaTestReasons.wrapper, separateJavaTestReasons.attributes, &res) + l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), src, isModule, testJavaImportsWithHelpers, testJavaImportedClassesWithHelpers, annotationProcessorClasses, testHelperDep, separateJavaTestReasons.wrapper, separateJavaTestReasons.attributes, &res) } } } @@ -521,11 +535,11 @@ func generateProtoLibraries(args language.GenerateArgs, log zerolog.Logger, res // We exclude intra-target imports because otherwise we'd get self-dependencies come resolve time. // 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], toImportedClasses *sorted_set.SortedSet[types.ClassName], 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]) { +func addNonLocalImportsAndExports(toImports *sorted_set.SortedSet[types.PackageName], toImportedClasses *sorted_set.SortedSet[types.ClassName], toExports *sorted_set.SortedSet[types.PackageName], toExportedClasses *sorted_set.SortedSet[types.ClassName], 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, toImportedClasses, fromImportedClasses, pkg, localClasses) if toExports != nil { - addFilteringOutOwnPackage(toExports, nil, fromExportedClasses, pkg, localClasses) + addFilteringOutOwnPackage(toExports, toExportedClasses, fromExportedClasses, pkg, localClasses) } } @@ -597,7 +611,7 @@ func accumulateJavaFile(cfg *javaconfig.Config, testJavaFiles, testHelperJavaFil } } -func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBazelWorkspace, name string, srcsRelativeToBazelWorkspace []string, resourcesDirectRef string, resourcesRuntimeDep string, packages, imports *sorted_set.SortedSet[types.PackageName], importedClasses *sorted_set.SortedSet[types.ClassName], exports *sorted_set.SortedSet[types.PackageName], annotationProcessorClasses *sorted_set.SortedSet[types.ClassName], testonly bool, javaLibraryRuleKind string, res *language.GenerateResult, cfg *javaconfig.Config, repoName string) { +func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBazelWorkspace, name string, srcsRelativeToBazelWorkspace []string, resourcesDirectRef string, resourcesRuntimeDep string, packages, imports *sorted_set.SortedSet[types.PackageName], importedClasses *sorted_set.SortedSet[types.ClassName], exports *sorted_set.SortedSet[types.PackageName], exportedClasses *sorted_set.SortedSet[types.ClassName], annotationProcessorClasses *sorted_set.SortedSet[types.ClassName], testonly bool, javaLibraryRuleKind string, res *language.GenerateResult, cfg *javaconfig.Config, repoName string) { r := rule.NewRule(javaLibraryRuleKind, name) srcs := make([]string, 0, len(srcsRelativeToBazelWorkspace)) @@ -641,6 +655,16 @@ func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBa resolvablePackages = append(resolvablePackages, *types.NewResolvableJavaPackage(pkg, testonly, false)) } r.SetPrivateAttr(packagesKey, resolvablePackages) + if exportedClasses != nil { + classes := exportedClasses.SortedSlice() + r.SetPrivateAttr(classesKey, classes) + // Cache the classes for class-level resolution during the resolve phase + ruleLabel := label.New("", pathToPackageRelativeToBazelWorkspace, name) + l.classExportCache[ruleLabel.String()] = classExportInfo{ + classes: classes, + testonly: testonly, + } + } res.Gen = append(res.Gen, r) resolveInput := types.ResolveInput{ @@ -648,6 +672,7 @@ func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBa ImportedPackageNames: imports, ImportedClasses: importedClasses, ExportedPackageNames: exports, + ExportedClassNames: exportedClasses, AnnotationProcessors: annotationProcessorClasses, } res.Imports = append(res.Imports, resolveInput) diff --git a/java/gazelle/generate_test.go b/java/gazelle/generate_test.go index b07ad0dc..78c1728e 100644 --- a/java/gazelle/generate_test.go +++ b/java/gazelle/generate_test.go @@ -309,7 +309,7 @@ func TestAddNonLocalImports(t *testing.T) { depsDst := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) exportsDst := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess) - addNonLocalImportsAndExports(depsDst, nil, 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"})) + addNonLocalImportsAndExports(depsDst, nil, exportsDst, nil, 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", diff --git a/java/gazelle/lang.go b/java/gazelle/lang.go index 5567b6fe..c4905287 100644 --- a/java/gazelle/lang.go +++ b/java/gazelle/lang.go @@ -11,6 +11,7 @@ import ( "github.com/bazel-contrib/rules_jvm/java/gazelle/private/logconfig" "github.com/bazel-contrib/rules_jvm/java/gazelle/private/maven" "github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_multiset" + "github.com/bazel-contrib/rules_jvm/java/gazelle/private/types" "github.com/bazelbuild/bazel-gazelle/config" "github.com/bazelbuild/bazel-gazelle/language" "github.com/bazelbuild/bazel-gazelle/resolve" @@ -36,6 +37,11 @@ type javaLang struct { // javaExportIndex holds information about java_export targets and which symbols they make available. javaExportIndex *java_export_index.JavaExportIndex + // classExportCache maps rule labels to their exported classes and testonly status. + // Used for class-level resolution when package resolution is ambiguous. + // Key is the stringified label (e.g., "//pkg:name"). + classExportCache map[string]classExportInfo + // hasHadErrors triggers the extension to fail at destroy time. // // this is used to return != 0 when some errors during the generation were @@ -43,6 +49,12 @@ type javaLang struct { hasHadErrors bool } +// classExportInfo holds the exported classes and testonly status for a rule. +type classExportInfo struct { + classes []types.ClassName + testonly bool +} + func NewLanguage() language.Language { goLevel, javaLevel := logconfig.LogLevel() @@ -70,6 +82,7 @@ func NewLanguage() language.Language { javaLogLevel: javaLevel, javaPackageCache: make(map[string]*java.Package), javaExportIndex: java_export_index.NewJavaExportIndex(languageName, logger), + classExportCache: make(map[string]classExportInfo), } l.logger = l.logger.Hook(shutdownServerOnFatalLogHook{ diff --git a/java/gazelle/private/maven/config.go b/java/gazelle/private/maven/config.go index 08937f5c..0d384fed 100644 --- a/java/gazelle/private/maven/config.go +++ b/java/gazelle/private/maven/config.go @@ -10,6 +10,7 @@ type lockFile interface { ListDependencies() []string GetDependencyCoordinates(name string) string ListDependencyPackages(name string) []string + ListDependencyClasses(name string) []string } type versionnedConfigFile struct { @@ -62,6 +63,10 @@ func (f *lockFileV1) ListDependencyPackages(name string) []string { panic(fmt.Sprintf("did not find package information for %s", name)) } +func (f *lockFileV1) ListDependencyClasses(name string) []string { + return nil +} + type lockFileV2 struct { AutogeneratedFileDoNotModifyThisFileManually string `json:"__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY"` InputArtifactsHash int `json:"__INPUT_ARTIFACTS_HASH"` @@ -89,6 +94,10 @@ func (f *lockFileV2) ListDependencyPackages(name string) []string { return f.Packages[name] } +func (f *lockFileV2) ListDependencyClasses(name string) []string { + return nil +} + type lockFileV2_Artifact struct { Shasums map[string]string `json:"shasums"` Version string `json:"version"` diff --git a/java/gazelle/private/maven/resolver.go b/java/gazelle/private/maven/resolver.go index 4f97e3c9..28fd10c9 100644 --- a/java/gazelle/private/maven/resolver.go +++ b/java/gazelle/private/maven/resolver.go @@ -32,19 +32,22 @@ func (e *MultipleExternalImportsError) Error() string { type Resolver interface { Resolve(pkg types.PackageName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) + ResolveClass(className types.ClassName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) } // resolver finds Maven provided packages by reading the maven_install.json // file from rules_jvm_external. type resolver struct { - data *multiset.StringMultiSet - logger zerolog.Logger + data *multiset.StringMultiSet + classIndex map[string]string + logger zerolog.Logger } func NewResolver(installFile string, logger zerolog.Logger) (Resolver, error) { r := resolver{ - data: multiset.NewStringMultiSet(), - logger: logger.With().Str("_c", "maven-resolver").Logger(), + data: multiset.NewStringMultiSet(), + classIndex: make(map[string]string), + logger: logger.With().Str("_c", "maven-resolver").Logger(), } c, err := loadConfiguration(installFile) @@ -65,6 +68,9 @@ func NewResolver(installFile string, logger zerolog.Logger) (Resolver, error) { for _, pkg := range c.ListDependencyPackages(depName) { r.data.Add(pkg, coords.ArtifactString()) } + for _, class := range c.ListDependencyClasses(depName) { + r.classIndex[class] = coords.ArtifactString() + } } return &r, nil @@ -105,6 +111,19 @@ func (r *resolver) Resolve(pkg types.PackageName, excludedArtifacts map[string]s } } +func (r *resolver) ResolveClass(className types.ClassName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) { + artifact, found := r.classIndex[className.FullyQualifiedClassName()] + if !found { + return label.NoLabel, nil + } + + if _, excluded := excludedArtifacts[LabelFromArtifact(mavenRepositoryName, artifact).String()]; excluded { + return label.NoLabel, nil + } + + return LabelFromArtifact(mavenRepositoryName, artifact), nil +} + func LabelFromArtifact(mavenRepositoryName string, artifact string) label.Label { return label.New(mavenRepositoryName, "", bazel.CleanupLabel(artifact)) } diff --git a/java/gazelle/private/types/types.go b/java/gazelle/private/types/types.go index 5b00ff31..6760e38a 100644 --- a/java/gazelle/private/types/types.go +++ b/java/gazelle/private/types/types.go @@ -112,6 +112,7 @@ type ResolveInput struct { ImportedPackageNames *sorted_set.SortedSet[PackageName] ImportedClasses *sorted_set.SortedSet[ClassName] ExportedPackageNames *sorted_set.SortedSet[PackageName] + ExportedClassNames *sorted_set.SortedSet[ClassName] AnnotationProcessors *sorted_set.SortedSet[ClassName] } diff --git a/java/gazelle/resolve.go b/java/gazelle/resolve.go index 0fece398..34d56468 100644 --- a/java/gazelle/resolve.go +++ b/java/gazelle/resolve.go @@ -30,6 +30,18 @@ const languageName = "java" type Resolver struct { lang *javaLang internalCache *lru.Cache + // classIndex is a lazy per-package index, built only for packages with ambiguous + // resolution (split packages). Maintains prod/test distinction. + classIndex map[types.PackageName]*packageClassIndex +} + +// packageClassIndex maps class names to their providing labels for a single package. +// Built lazily per-package only when that package has ambiguous resolution. +type packageClassIndex struct { + // prod maps bare outer class name -> providers (non-testonly rules) + prod map[string][]label.Label + // test maps bare outer class name -> providers (testonly rules) + test map[string][]label.Label } func NewResolver(lang *javaLang) *Resolver { @@ -41,6 +53,7 @@ func NewResolver(lang *javaLang) *Resolver { return &Resolver{ lang: lang, internalCache: internalCache, + classIndex: make(map[types.PackageName]*packageClassIndex), } } @@ -63,6 +76,10 @@ func (jr *Resolver) Imports(c *config.Config, r *rule.Rule, f *rule.File) []reso out = append(out, resolve.ImportSpec{Lang: languageName, Imp: pkg.String()}) } } + // NOTE: We intentionally do NOT register classes in Gazelle's global RuleIndex. + // Class-level resolution uses a lazy, per-package index built only when needed + // (when package-level resolution is ambiguous due to split packages). + // This keeps the global index small and fast. log.Debug().Str("out", fmt.Sprintf("%#v", out)).Str("label", lbl.String()).Msg("return") return out @@ -124,22 +141,61 @@ func (jr *Resolver) Resolve(c *config.Config, ix *resolve.RuleIndex, rc *repo.Re func (jr *Resolver) populateAttr(c *config.Config, pc *javaconfig.Config, r *rule.Rule, attrName string, requiredPackageNames *sorted_set.SortedSet[types.PackageName], importedClasses *sorted_set.SortedSet[types.ClassName], ix *resolve.RuleIndex, isTestRule bool, from label.Label, ownPackageNames *sorted_set.SortedSet[types.PackageName]) { labels := sorted_set.NewSortedSetFn[label.Label]([]label.Label{}, sorted_set.LabelLess) + // Build a map of package -> classes for efficient lookup during class-level resolution + classesByPackage := make(map[types.PackageName][]types.ClassName) + if importedClasses != nil { + for _, cls := range importedClasses.SortedSlice() { + pkg := cls.PackageName() + classesByPackage[pkg] = append(classesByPackage[pkg], cls) + } + } + for _, imp := range requiredPackageNames.SortedSlice() { var pkgClasses []string - if importedClasses != nil { - for _, cls := range importedClasses.SortedSlice() { - if cls.PackageName() == imp { - pkgClasses = append(pkgClasses, cls.BareOuterClassName()) - } - } + for _, cls := range classesByPackage[imp] { + pkgClasses = append(pkgClasses, cls.BareOuterClassName()) } - dep := jr.resolveSinglePackage(c, pc, imp, ix, from, isTestRule, ownPackageNames, pkgClasses) - if dep == label.NoLabel { + // Try package-level resolution first (fast path) + dep, ambiguous := jr.resolveSinglePackageWithAmbiguity(c, pc, imp, ix, from, isTestRule, ownPackageNames, pkgClasses) + if dep != label.NoLabel { + labels.Add(simplifyLabel(c.RepoName, dep, from)) continue } - labels.Add(simplifyLabel(c.RepoName, dep, from)) + // Only fall back to class-level resolution when package resolution is ambiguous + if ambiguous && len(classesByPackage[imp]) > 0 { + jr.lang.logger.Debug(). + Str("package", imp.Name). + Strs("classes", pkgClasses). + Stringer("from", from). + Msg("package has multiple providers, attempting class-level resolution") + + resolvedAny := false + for _, className := range classesByPackage[imp] { + l, err := jr.lang.mavenResolver.ResolveClass(className, pc.ExcludedArtifacts(), pc.MavenRepositoryName()) + if err != nil { + jr.lang.logger.Warn().Err(err).Str("class", className.FullyQualifiedClassName()).Msg("error resolving class") + continue + } + if l == label.NoLabel { + l = jr.resolveSingleClass(c, pc, className, ix, from, isTestRule) + } + if l != label.NoLabel { + labels.Add(simplifyLabel(c.RepoName, l, from)) + resolvedAny = true + } + } + + if !resolvedAny { + jr.lang.logger.Error(). + Str("package", imp.Name). + Strs("classes", pkgClasses). + Stringer("from", from). + Msg("package has multiple providers and class-level resolution failed for all classes") + jr.lang.hasHadErrors = true + } + } } setLabelAttrIncludingExistingValues(r, attrName, labels) @@ -215,11 +271,13 @@ func setLabelAttrIncludingExistingValues(r *rule.Rule, attrName string, labels * } } -func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config, imp types.PackageName, ix *resolve.RuleIndex, from label.Label, isTestRule bool, ownPackageNames *sorted_set.SortedSet[types.PackageName], pkgClasses []string) (out label.Label) { +// resolveSinglePackageWithAmbiguity resolves a package import and returns whether there was ambiguity. +// When ambiguous is true and out is NoLabel, the caller should attempt class-level resolution. +func (jr *Resolver) resolveSinglePackageWithAmbiguity(c *config.Config, pc *javaconfig.Config, imp types.PackageName, ix *resolve.RuleIndex, from label.Label, isTestRule bool, ownPackageNames *sorted_set.SortedSet[types.PackageName], pkgClasses []string) (out label.Label, ambiguous bool) { cacheKey := types.NewResolvableJavaPackage(imp, false, false) importSpec := resolve.ImportSpec{Lang: languageName, Imp: cacheKey.String()} if ol, found := resolve.FindRuleWithOverride(c, importSpec, languageName); found { - return ol + return ol, false } matches := ix.FindRulesByImportWithConfig(c, importSpec, languageName) @@ -237,24 +295,16 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config } if len(matches) == 1 { - return matches[0].Label + return matches[0].Label, false } if len(matches) > 1 { - labels := make([]string, 0, len(matches)) - for _, match := range matches { - labels = append(labels, match.Label.String()) - } - sort.Strings(labels) - - jr.lang.logger.Error(). - Str("pkg", imp.Name). - Strs("targets", labels). - Msg("resolveSinglePackage found MULTIPLE results in rule index") + // Multiple matches found - signal ambiguity so caller can try class-level resolution + return label.NoLabel, true } if v, ok := jr.internalCache.Get(cacheKey); ok { - return simplifyLabel(c.RepoName, v.(label.Label), from) + return simplifyLabel(c.RepoName, v.(label.Label), from), false } jr.lang.logger.Debug().Str("parsedImport", imp.Name).Stringer("from", from).Msg("not found yet") @@ -266,10 +316,10 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config }() if java.IsStdlib(imp) { - return label.NoLabel + return label.NoLabel, false } if kotlin.IsStdlib(imp) { - return label.NoLabel + return label.NoLabel, false } // As per https://github.com/bazelbuild/bazel/blob/347407a88fd480fc5e0fbd42cc8196e4356a690b/tools/java/runfiles/Runfiles.java#L41 @@ -278,9 +328,9 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config l, err := label.Parse(runfilesLabel) if err != nil { jr.lang.logger.Fatal().Str("label", runfilesLabel).Err(err).Msg("failed to parse known-good runfiles label") - return label.NoLabel + return label.NoLabel, false } - return l + return l, false } if l, err := jr.lang.mavenResolver.Resolve(imp, pc.ExcludedArtifacts(), pc.MavenRepositoryName()); err != nil { @@ -290,16 +340,18 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config if errors.As(err, &noExternal) { // do not fail, the package might be provided elsewhere } else if errors.As(err, &multipleExternal) { + // Maven has multiple options - show helpful error with resolution hints + // This is different from local split packages where we can try class-level resolution jr.lang.logger.Error().Strs("classes", pkgClasses).Msg("Append one of the following to BUILD.bazel:") for _, possible := range multipleExternal.PossiblePackages { jr.lang.logger.Error().Msgf("# gazelle:resolve java %s %s", imp.Name, possible) } - jr.lang.hasHadErrors = true + // Don't return here - let execution continue to produce the warning about unresolved package } else { jr.lang.logger.Fatal().Err(err).Msg("maven resolver error") } } else { - return l + return l, false } if isTestRule { @@ -309,10 +361,10 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config testonlyMatches := ix.FindRulesByImportWithConfig(c, testonlyImportSpec, languageName) if len(testonlyMatches) == 1 { cacheKey = testonlyCacheKey - return simplifyLabel(c.RepoName, testonlyMatches[0].Label, from) + return simplifyLabel(c.RepoName, testonlyMatches[0].Label, from), false } - // If there's exactly one testonly match, use it + // If there's exactly one testsuite match, use it testsuiteCacheKey := types.NewResolvableJavaPackage(imp, true, true) testsuiteImportSpec := resolve.ImportSpec{Lang: languageName, Imp: testsuiteCacheKey.String()} testsuiteMatches := ix.FindRulesByImportWithConfig(c, testsuiteImportSpec, languageName) @@ -321,14 +373,14 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config l := testsuiteMatches[0].Label if l != from { l.Name += "-test-lib" - return simplifyLabel(c.RepoName, l, from) + return simplifyLabel(c.RepoName, l, from), false } } } if isTestRule && ownPackageNames.Contains(imp) { // Tests may have unique packages which don't exist outside of those tests - don't treat this as an error. - return label.NoLabel + return label.NoLabel, false } jr.lang.logger.Warn(). @@ -338,6 +390,121 @@ func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config Msg("Unable to find package for import in any dependency") jr.lang.hasHadErrors = true + return label.NoLabel, false +} + +func (jr *Resolver) resolveSinglePackage(c *config.Config, pc *javaconfig.Config, imp types.PackageName, ix *resolve.RuleIndex, from label.Label, isTestRule bool, ownPackageNames *sorted_set.SortedSet[types.PackageName], pkgClasses []string) (out label.Label) { + out, _ = jr.resolveSinglePackageWithAmbiguity(c, pc, imp, ix, from, isTestRule, ownPackageNames, pkgClasses) + return out +} + +// buildPackageClassIndex lazily builds a class index for a specific package. +// Only called when package-level resolution is ambiguous (split packages). +func (jr *Resolver) buildPackageClassIndex(c *config.Config, pkg types.PackageName, ix *resolve.RuleIndex) *packageClassIndex { + if pci, ok := jr.classIndex[pkg]; ok { + return pci + } + + // Find all rules that provide this package + cacheKey := types.NewResolvableJavaPackage(pkg, false, false) + importSpec := resolve.ImportSpec{Lang: languageName, Imp: cacheKey.String()} + matches := ix.FindRulesByImportWithConfig(c, importSpec, languageName) + + // Also check for testonly providers + testCacheKey := types.NewResolvableJavaPackage(pkg, true, false) + testImportSpec := resolve.ImportSpec{Lang: languageName, Imp: testCacheKey.String()} + testMatches := ix.FindRulesByImportWithConfig(c, testImportSpec, languageName) + matches = append(matches, testMatches...) + + pci := &packageClassIndex{ + prod: make(map[string][]label.Label), + test: make(map[string][]label.Label), + } + + for _, m := range matches { + info, ok := jr.lang.classExportCache[m.Label.String()] + if !ok { + continue + } + for _, cls := range info.classes { + if cls.PackageName() != pkg { + continue + } + name := cls.BareOuterClassName() + if info.testonly { + pci.test[name] = append(pci.test[name], m.Label) + } else { + pci.prod[name] = append(pci.prod[name], m.Label) + } + } + } + + jr.classIndex[pkg] = pci + jr.lang.logger.Debug(). + Str("package", pkg.Name). + Int("prod_classes", len(pci.prod)). + Int("test_classes", len(pci.test)). + Msg("built class index for split package") + + return pci +} + +func (jr *Resolver) resolveSingleClass(c *config.Config, pc *javaconfig.Config, className types.ClassName, ix *resolve.RuleIndex, from label.Label, isTestRule bool) (out label.Label) { + imp := className.FullyQualifiedClassName() + // Check for manual override first + importSpec := resolve.ImportSpec{Lang: languageName, Imp: imp} + if ol, found := resolve.FindRuleWithOverride(c, importSpec, languageName); found { + return ol + } + + // Build/get the per-package class index + pkg := className.PackageName() + pci := jr.buildPackageClassIndex(c, pkg, ix) + bareClassName := className.BareOuterClassName() + + // Look up candidates - prefer prod classes, but test rules can also use test classes + var candidates []label.Label + if prodCandidates, ok := pci.prod[bareClassName]; ok { + candidates = prodCandidates + } + if isTestRule { + if testCandidates, ok := pci.test[bareClassName]; ok { + candidates = append(candidates, testCandidates...) + } + } + + if len(candidates) == 0 { + return label.NoLabel + } + + if len(candidates) == 1 { + return simplifyLabel(c.RepoName, candidates[0], from) + } + + // Multiple candidates - try java_export narrowing + if pc.ResolveToJavaExports() { + results := make([]resolve.FindResult, 0, len(candidates)) + for _, l := range candidates { + results = append(results, resolve.FindResult{Label: l}) + } + narrowed := jr.tryResolvingToJavaExport(results, from) + if len(narrowed) == 1 { + return simplifyLabel(c.RepoName, narrowed[0].Label, from) + } + } + + // Still ambiguous - log error + labels := make([]string, 0, len(candidates)) + for _, l := range candidates { + labels = append(labels, l.String()) + } + sort.Strings(labels) + + jr.lang.logger.Error(). + Str("class", imp). + Strs("targets", labels). + Msg("resolveSingleClass found MULTIPLE providers for class") + return label.NoLabel } diff --git a/java/gazelle/resolve_test.go b/java/gazelle/resolve_test.go index a8506491..36b7f7d5 100644 --- a/java/gazelle/resolve_test.go +++ b/java/gazelle/resolve_test.go @@ -381,6 +381,10 @@ func (*testResolver) Resolve(pkg types.PackageName, excludedArtifacts map[string return label.NoLabel, errors.New("not implemented") } +func (*testResolver) ResolveClass(className types.ClassName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) { + return label.NoLabel, errors.New("not implemented") +} + type mapResolver map[string]resolve.Resolver func (mr mapResolver) Resolver(r *rule.Rule, f string) resolve.Resolver { @@ -425,3 +429,7 @@ func (r *TestMavenResolver) Resolve(pkg types.PackageName, excludedArtifacts map } return l, nil } + +func (r *TestMavenResolver) ResolveClass(className types.ClassName, excludedArtifacts map[string]struct{}, mavenRepositoryName string) (label.Label, error) { + return label.NoLabel, nil +} diff --git a/java/gazelle/testdata/kt_split_package/BUILD.in b/java/gazelle/testdata/kt_split_package/BUILD.in new file mode 100644 index 00000000..feccf582 --- /dev/null +++ b/java/gazelle/testdata/kt_split_package/BUILD.in @@ -0,0 +1 @@ +# gazelle:jvm_kotlin_enabled true diff --git a/java/gazelle/testdata/kt_split_package/WORKSPACE b/java/gazelle/testdata/kt_split_package/WORKSPACE new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/kt_split_package/app/src/main/BUILD.in b/java/gazelle/testdata/kt_split_package/app/src/main/BUILD.in new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/kt_split_package/app/src/main/BUILD.out b/java/gazelle/testdata/kt_split_package/app/src/main/BUILD.out new file mode 100644 index 00000000..a60fc062 --- /dev/null +++ b/java/gazelle/testdata/kt_split_package/app/src/main/BUILD.out @@ -0,0 +1,11 @@ +load("@rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library") + +kt_jvm_library( + name = "main", + srcs = ["Main.kt"], + visibility = ["//:__subpackages__"], + deps = [ + "//one/src", + "//two/src", + ], +) diff --git a/java/gazelle/testdata/kt_split_package/app/src/main/Main.kt b/java/gazelle/testdata/kt_split_package/app/src/main/Main.kt new file mode 100644 index 00000000..fd9fceb2 --- /dev/null +++ b/java/gazelle/testdata/kt_split_package/app/src/main/Main.kt @@ -0,0 +1,9 @@ +package com.example.app + +import com.example.split.ClassA +import com.example.split.ClassB + +class Main { + val a = ClassA() + val b = ClassB() +} diff --git a/java/gazelle/testdata/kt_split_package/maven_install.json b/java/gazelle/testdata/kt_split_package/maven_install.json new file mode 100644 index 00000000..6c01c298 --- /dev/null +++ b/java/gazelle/testdata/kt_split_package/maven_install.json @@ -0,0 +1 @@ +{"version": "2"} diff --git a/java/gazelle/testdata/kt_split_package/one/src/BUILD.in b/java/gazelle/testdata/kt_split_package/one/src/BUILD.in new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/kt_split_package/one/src/BUILD.out b/java/gazelle/testdata/kt_split_package/one/src/BUILD.out new file mode 100644 index 00000000..299bf59c --- /dev/null +++ b/java/gazelle/testdata/kt_split_package/one/src/BUILD.out @@ -0,0 +1,7 @@ +load("@rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library") + +kt_jvm_library( + name = "src", + srcs = ["ClassA.kt"], + visibility = ["//:__subpackages__"], +) diff --git a/java/gazelle/testdata/kt_split_package/one/src/ClassA.kt b/java/gazelle/testdata/kt_split_package/one/src/ClassA.kt new file mode 100644 index 00000000..9dca3b1d --- /dev/null +++ b/java/gazelle/testdata/kt_split_package/one/src/ClassA.kt @@ -0,0 +1,3 @@ +package com.example.split + +class ClassA diff --git a/java/gazelle/testdata/kt_split_package/two/src/BUILD.in b/java/gazelle/testdata/kt_split_package/two/src/BUILD.in new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/kt_split_package/two/src/BUILD.out b/java/gazelle/testdata/kt_split_package/two/src/BUILD.out new file mode 100644 index 00000000..7148b475 --- /dev/null +++ b/java/gazelle/testdata/kt_split_package/two/src/BUILD.out @@ -0,0 +1,7 @@ +load("@rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library") + +kt_jvm_library( + name = "src", + srcs = ["ClassB.kt"], + visibility = ["//:__subpackages__"], +) diff --git a/java/gazelle/testdata/kt_split_package/two/src/ClassB.kt b/java/gazelle/testdata/kt_split_package/two/src/ClassB.kt new file mode 100644 index 00000000..b4c8520b --- /dev/null +++ b/java/gazelle/testdata/kt_split_package/two/src/ClassB.kt @@ -0,0 +1,3 @@ +package com.example.split + +class ClassB diff --git a/java/gazelle/testdata/split_package/BUILD.in b/java/gazelle/testdata/split_package/BUILD.in new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/split_package/WORKSPACE b/java/gazelle/testdata/split_package/WORKSPACE new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/split_package/app/src/java/BUILD.in b/java/gazelle/testdata/split_package/app/src/java/BUILD.in new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/split_package/app/src/java/BUILD.out b/java/gazelle/testdata/split_package/app/src/java/BUILD.out new file mode 100644 index 00000000..6c58a59d --- /dev/null +++ b/java/gazelle/testdata/split_package/app/src/java/BUILD.out @@ -0,0 +1,11 @@ +load("@rules_java//java:defs.bzl", "java_library") + +java_library( + name = "java", + srcs = ["Main.java"], + visibility = ["//:__subpackages__"], + deps = [ + "//one/src/java", + "//two/src/java", + ], +) diff --git a/java/gazelle/testdata/split_package/app/src/java/Main.java b/java/gazelle/testdata/split_package/app/src/java/Main.java new file mode 100644 index 00000000..f519e8e1 --- /dev/null +++ b/java/gazelle/testdata/split_package/app/src/java/Main.java @@ -0,0 +1,9 @@ +package com.example.app; + +import com.example.split.ClassA; +import com.example.split.ClassB; + +public class Main { + ClassA a; + ClassB b; +} diff --git a/java/gazelle/testdata/split_package/maven_install.json b/java/gazelle/testdata/split_package/maven_install.json new file mode 100644 index 00000000..6c01c298 --- /dev/null +++ b/java/gazelle/testdata/split_package/maven_install.json @@ -0,0 +1 @@ +{"version": "2"} diff --git a/java/gazelle/testdata/split_package/one/src/java/BUILD.in b/java/gazelle/testdata/split_package/one/src/java/BUILD.in new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/split_package/one/src/java/BUILD.out b/java/gazelle/testdata/split_package/one/src/java/BUILD.out new file mode 100644 index 00000000..99a3c436 --- /dev/null +++ b/java/gazelle/testdata/split_package/one/src/java/BUILD.out @@ -0,0 +1,7 @@ +load("@rules_java//java:defs.bzl", "java_library") + +java_library( + name = "java", + srcs = ["ClassA.java"], + visibility = ["//:__subpackages__"], +) diff --git a/java/gazelle/testdata/split_package/one/src/java/ClassA.java b/java/gazelle/testdata/split_package/one/src/java/ClassA.java new file mode 100644 index 00000000..1b7cf354 --- /dev/null +++ b/java/gazelle/testdata/split_package/one/src/java/ClassA.java @@ -0,0 +1,3 @@ +package com.example.split; + +public class ClassA {} diff --git a/java/gazelle/testdata/split_package/two/src/java/BUILD.in b/java/gazelle/testdata/split_package/two/src/java/BUILD.in new file mode 100644 index 00000000..e69de29b diff --git a/java/gazelle/testdata/split_package/two/src/java/BUILD.out b/java/gazelle/testdata/split_package/two/src/java/BUILD.out new file mode 100644 index 00000000..d697c663 --- /dev/null +++ b/java/gazelle/testdata/split_package/two/src/java/BUILD.out @@ -0,0 +1,7 @@ +load("@rules_java//java:defs.bzl", "java_library") + +java_library( + name = "java", + srcs = ["ClassB.java"], + visibility = ["//:__subpackages__"], +) diff --git a/java/gazelle/testdata/split_package/two/src/java/ClassB.java b/java/gazelle/testdata/split_package/two/src/java/ClassB.java new file mode 100644 index 00000000..781228a2 --- /dev/null +++ b/java/gazelle/testdata/split_package/two/src/java/ClassB.java @@ -0,0 +1,3 @@ +package com.example.split; + +public class ClassB {} 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 7b29c836..421f587d 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 @@ -195,6 +195,10 @@ public Void visitImport(ImportTree i, Void v) { @Override public Void visitClass(ClassTree t, Void v) { stack.addLast(t); + checkFullyQualifiedType(t.getExtendsClause()); + for (Tree implement : t.getImplementsClause()) { + checkFullyQualifiedType(implement); + } for (AnnotationTree annotation : t.getModifiers().getAnnotations()) { String annotationClassName = annotation.getAnnotationType().toString(); String importedFullyQualified = currentFileImports.get(annotationClassName); @@ -230,6 +234,10 @@ public Void visitMethod(com.sun.source.tree.MethodTree m, Void v) { } } + for (ExpressionTree thrown : m.getThrows()) { + checkFullyQualifiedType(thrown); + } + handleAnnotations(m.getModifiers().getAnnotations()); // Check to see if we have a main method @@ -337,6 +345,9 @@ public Void visitVariable(VariableTree node, Void unused) { @Nullable private Set checkFullyQualifiedType(Tree identifier) { + if (identifier == null) { + return null; + } Set types = new TreeSet<>(); if (identifier.getKind() == Tree.Kind.IDENTIFIER || identifier.getKind() == Tree.Kind.MEMBER_SELECT) {