Skip to content

Commit 5b6b81d

Browse files
shs96cshs96c
authored
feat(gazelle): print class names when split packages detected (#393)
When classes can come from multiple jars it's not always clear which one to use. By printing the class names, we make it easier for users to understand which dependency is actually required.
1 parent ddaeb04 commit 5b6b81d

File tree

6 files changed

+56
-28
lines changed

6 files changed

+56
-28
lines changed

‎java/gazelle/generate.go‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,13 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
153153
// Files and imports for code which isn't tests, and isn't used as helpers in tests.
154154
productionJavaFiles := sorted_set.NewSortedSet([]string{})
155155
productionJavaImports := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess)
156+
productionJavaImportedClasses := sorted_set.NewSortedSetFn([]types.ClassName{}, types.ClassNameLess)
156157
nonLocalJavaExports := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess)
157158

158159
// Files and imports for actual test classes.
159160
testJavaFiles := sorted_set.NewSortedSetFn([]javaFile{}, javaFileLess)
160161
testJavaImports := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess)
162+
testJavaImportedClasses := sorted_set.NewSortedSetFn([]types.ClassName{}, types.ClassNameLess)
161163

162164
// Java Test files which need to be generated separately from any others because they have explicit attribute overrides.
163165
separateTestJavaFiles := make(map[javaFile]separateJavaTestReasons)
@@ -178,14 +180,14 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
178180
allPackageNames.Add(mJavaPkg.Name)
179181

180182
if !mJavaPkg.TestPackage {
181-
addNonLocalImportsAndExports(productionJavaImports, nonLocalJavaExports, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, likelyLocalClassNames)
183+
addNonLocalImportsAndExports(productionJavaImports, productionJavaImportedClasses, nonLocalJavaExports, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, likelyLocalClassNames)
182184
for _, f := range mJavaPkg.Files.SortedSlice() {
183185
productionJavaFiles.Add(filepath.Join(mRel, f))
184186
}
185187
allMains.AddAll(mJavaPkg.Mains)
186188
} else {
187189
// Tests don't get to export things, as things shouldn't depend on them.
188-
addNonLocalImportsAndExports(testJavaImports, nil, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, likelyLocalClassNames)
190+
addNonLocalImportsAndExports(testJavaImports, testJavaImportedClasses, nil, mJavaPkg.ImportedClasses, mJavaPkg.ImportedPackagesWithoutSpecificClasses, mJavaPkg.ExportedClasses, mJavaPkg.Name, likelyLocalClassNames)
189191
for _, f := range mJavaPkg.Files.SortedSlice() {
190192
path := filepath.Join(mRel, f)
191193
file := javaFile{
@@ -203,9 +205,9 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
203205
allPackageNames.Add(javaPkg.Name)
204206
if javaPkg.TestPackage {
205207
// Tests don't get to export things, as things shouldn't depend on them.
206-
addNonLocalImportsAndExports(testJavaImports, nil, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, likelyLocalClassNames)
208+
addNonLocalImportsAndExports(testJavaImports, testJavaImportedClasses, nil, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, likelyLocalClassNames)
207209
} else {
208-
addNonLocalImportsAndExports(productionJavaImports, nonLocalJavaExports, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, likelyLocalClassNames)
210+
addNonLocalImportsAndExports(productionJavaImports, productionJavaImportedClasses, nonLocalJavaExports, javaPkg.ImportedClasses, javaPkg.ImportedPackagesWithoutSpecificClasses, javaPkg.ExportedClasses, javaPkg.Name, likelyLocalClassNames)
209211
}
210212
allMains.AddAll(javaPkg.Mains)
211213
for _, f := range srcFilenamesRelativeToPackage {
@@ -234,6 +236,14 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
234236
}
235237
return true
236238
})
239+
nonLocalProductionJavaImportedClasses := productionJavaImportedClasses.Filter(func(c types.ClassName) bool {
240+
for _, n := range allPackageNamesSlice {
241+
if c.PackageName().Name == n.Name {
242+
return false
243+
}
244+
}
245+
return true
246+
})
237247

238248
javaLibraryKind := "java_library"
239249
if hasKotlinFiles {
@@ -325,7 +335,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
325335
}
326336
}
327337

328-
l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), productionJavaFiles.SortedSlice(), resourcesDirectRef, resourcesRuntimeDep, allPackageNames, nonLocalProductionJavaImports, nonLocalJavaExports, annotationProcessorClasses, false, javaLibraryKind, &res, cfg, args.Config.RepoName)
338+
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)
329339
}
330340

331341
if cfg.GenerateBinary() {
@@ -348,7 +358,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
348358
srcs = append(srcs, tf.pathRelativeToBazelWorkspaceRoot)
349359
}
350360
// Test helper libraries typically don't have resources
351-
l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), srcs, "", "", packages, testJavaImports, nonLocalJavaExports, annotationProcessorClasses, true, javaLibraryKind, &res, cfg, args.Config.RepoName)
361+
l.generateJavaLibrary(args.File, args.Rel, filepath.Base(args.Rel), srcs, "", "", packages, testJavaImports, testJavaImportedClasses, nonLocalJavaExports, annotationProcessorClasses, true, javaLibraryKind, &res, cfg, args.Config.RepoName)
352362
}
353363
}
354364

@@ -360,7 +370,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
360370
case "file":
361371
for _, tf := range testJavaFiles.SortedSlice() {
362372
separateJavaTestReasons := separateTestJavaFiles[tf]
363-
l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), tf, isModule, testJavaImportsWithHelpers, annotationProcessorClasses, nil, separateJavaTestReasons.wrapper, separateJavaTestReasons.attributes, &res)
373+
l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), tf, isModule, testJavaImportsWithHelpers, testJavaImportedClasses, annotationProcessorClasses, nil, separateJavaTestReasons.wrapper, separateJavaTestReasons.attributes, &res)
364374
}
365375

366376
case "suite":
@@ -388,6 +398,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
388398
packageNames,
389399
cfg.MavenRepositoryName(),
390400
testJavaImportsWithHelpers,
401+
testJavaImportedClasses,
391402
annotationProcessorClasses,
392403
cfg.GetCustomJavaTestFileSuffixes(),
393404
testHelperJavaFiles.Len() > 0,
@@ -405,7 +416,7 @@ func (l javaLang) GenerateRules(args language.GenerateArgs) language.GenerateRes
405416
testHelperDep = ptr(testHelperLibname(suiteName))
406417
}
407418
separateJavaTestReasons := separateTestJavaFiles[src]
408-
l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), src, isModule, testJavaImportsWithHelpers, annotationProcessorClasses, testHelperDep, separateJavaTestReasons.wrapper, separateJavaTestReasons.attributes, &res)
419+
l.generateJavaTest(args.File, args.Rel, cfg.MavenRepositoryName(), src, isModule, testJavaImportsWithHelpers, testJavaImportedClasses, annotationProcessorClasses, testHelperDep, separateJavaTestReasons.wrapper, separateJavaTestReasons.attributes, &res)
409420
}
410421
}
411422
}
@@ -510,15 +521,15 @@ func generateProtoLibraries(args language.GenerateArgs, log zerolog.Logger, res
510521

511522
// We exclude intra-target imports because otherwise we'd get self-dependencies come resolve time.
512523
// toExports is optional and may be nil. All other parameters are required and must be non-nil.
513-
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]) {
524+
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]) {
514525
toImports.AddAll(fromPackages)
515-
addFilteringOutOwnPackage(toImports, fromImportedClasses, pkg, localClasses)
526+
addFilteringOutOwnPackage(toImports, toImportedClasses, fromImportedClasses, pkg, localClasses)
516527
if toExports != nil {
517-
addFilteringOutOwnPackage(toExports, fromExportedClasses, pkg, localClasses)
528+
addFilteringOutOwnPackage(toExports, nil, fromExportedClasses, pkg, localClasses)
518529
}
519530
}
520531

521-
func addFilteringOutOwnPackage(to *sorted_set.SortedSet[types.PackageName], from *sorted_set.SortedSet[types.ClassName], ownPackage types.PackageName, localOuterClassNames *sorted_set.SortedSet[string]) {
532+
func addFilteringOutOwnPackage(to *sorted_set.SortedSet[types.PackageName], toClasses *sorted_set.SortedSet[types.ClassName], from *sorted_set.SortedSet[types.ClassName], ownPackage types.PackageName, localOuterClassNames *sorted_set.SortedSet[string]) {
522533
for _, fromPackage := range from.SortedSlice() {
523534
if ownPackage == fromPackage.PackageName() {
524535
if localOuterClassNames.Contains(fromPackage.BareOuterClassName()) {
@@ -531,6 +542,9 @@ func addFilteringOutOwnPackage(to *sorted_set.SortedSet[types.PackageName], from
531542
}
532543

533544
to.Add(fromPackage.PackageName())
545+
if toClasses != nil {
546+
toClasses.Add(fromPackage)
547+
}
534548
}
535549
}
536550

@@ -583,7 +597,7 @@ func accumulateJavaFile(cfg *javaconfig.Config, testJavaFiles, testHelperJavaFil
583597
}
584598
}
585599

586-
func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBazelWorkspace, name string, srcsRelativeToBazelWorkspace []string, resourcesDirectRef string, resourcesRuntimeDep string, packages, imports, exports *sorted_set.SortedSet[types.PackageName], annotationProcessorClasses *sorted_set.SortedSet[types.ClassName], testonly bool, javaLibraryRuleKind string, res *language.GenerateResult, cfg *javaconfig.Config, repoName string) {
600+
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) {
587601
r := rule.NewRule(javaLibraryRuleKind, name)
588602

589603
srcs := make([]string, 0, len(srcsRelativeToBazelWorkspace))
@@ -632,6 +646,7 @@ func (l javaLang) generateJavaLibrary(file *rule.File, pathToPackageRelativeToBa
632646
resolveInput := types.ResolveInput{
633647
PackageNames: packages,
634648
ImportedPackageNames: imports,
649+
ImportedClasses: importedClasses,
635650
ExportedPackageNames: exports,
636651
AnnotationProcessors: annotationProcessorClasses,
637652
}
@@ -682,7 +697,7 @@ func (l javaLang) generateJavaBinary(file *rule.File, m types.ClassName, libName
682697
})
683698
}
684699

685-
func (l javaLang) generateJavaTest(file *rule.File, pathToPackageRelativeToBazelWorkspace string, mavenRepositoryName string, f javaFile, includePackageInName bool, imports *sorted_set.SortedSet[types.PackageName], annotationProcessorClasses *sorted_set.SortedSet[types.ClassName], depOnTestHelpers *string, wrapper string, extraAttributes map[string]bzl.Expr, res *language.GenerateResult) {
700+
func (l javaLang) generateJavaTest(file *rule.File, pathToPackageRelativeToBazelWorkspace string, mavenRepositoryName string, f javaFile, includePackageInName bool, imports *sorted_set.SortedSet[types.PackageName], importedClasses *sorted_set.SortedSet[types.ClassName], annotationProcessorClasses *sorted_set.SortedSet[types.ClassName], depOnTestHelpers *string, wrapper string, extraAttributes map[string]bzl.Expr, res *language.GenerateResult) {
686701
className := f.ClassName()
687702
fullyQualifiedTestClass := className.FullyQualifiedClassName()
688703
var testName string
@@ -742,6 +757,7 @@ func (l javaLang) generateJavaTest(file *rule.File, pathToPackageRelativeToBazel
742757
resolveInput := types.ResolveInput{
743758
PackageNames: sorted_set.NewSortedSetFn([]types.PackageName{f.pkg}, types.PackageNameLess),
744759
ImportedPackageNames: testImports,
760+
ImportedClasses: importedClasses,
745761
AnnotationProcessors: annotationProcessorClasses,
746762
}
747763
res.Imports = append(res.Imports, resolveInput)
@@ -770,7 +786,7 @@ var junit5RuntimeDeps = []string{
770786
"org.junit.platform:junit-platform-reporting",
771787
}
772788

773-
func (l javaLang) generateJavaTestSuite(file *rule.File, name string, srcs []string, packageNames *sorted_set.SortedSet[types.PackageName], mavenRepositoryName string, imports *sorted_set.SortedSet[types.PackageName], annotationProcessorClasses *sorted_set.SortedSet[types.ClassName], customTestSuffixes *[]string, hasHelpers bool, res *language.GenerateResult) {
789+
func (l javaLang) generateJavaTestSuite(file *rule.File, name string, srcs []string, packageNames *sorted_set.SortedSet[types.PackageName], mavenRepositoryName string, imports *sorted_set.SortedSet[types.PackageName], importedClasses *sorted_set.SortedSet[types.ClassName], annotationProcessorClasses *sorted_set.SortedSet[types.ClassName], customTestSuffixes *[]string, hasHelpers bool, res *language.GenerateResult) {
774790
const ruleKind = "java_test_suite"
775791
r := rule.NewRule(ruleKind, name)
776792
r.SetAttr("srcs", srcs)
@@ -811,6 +827,7 @@ func (l javaLang) generateJavaTestSuite(file *rule.File, name string, srcs []str
811827
resolveInput := types.ResolveInput{
812828
PackageNames: packageNames,
813829
ImportedPackageNames: suiteImports,
830+
ImportedClasses: importedClasses,
814831
AnnotationProcessors: annotationProcessorClasses,
815832
}
816833
res.Imports = append(res.Imports, resolveInput)

‎java/gazelle/generate_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ func TestSingleJavaTestFile(t *testing.T) {
159159
var res language.GenerateResult
160160

161161
l := newTestJavaLang(t)
162-
l.generateJavaTest(nil, "", "maven", f, tc.includePackageInName, stringsToPackageNames(tc.importedPackages), nil, nil, tc.wrapper, nil, &res)
162+
l.generateJavaTest(nil, "", "maven", f, tc.includePackageInName, stringsToPackageNames(tc.importedPackages), nil, nil, nil, tc.wrapper, nil, &res)
163163

164164
require.Len(t, res.Gen, 1, "want 1 generated rule")
165165

@@ -252,7 +252,7 @@ func TestSuite(t *testing.T) {
252252
var res language.GenerateResult
253253

254254
l := newTestJavaLang(t)
255-
l.generateJavaTestSuite(nil, "blah", []string{src}, stringsToPackageNames([]string{pkg}), "maven", stringsToPackageNames(tc.importedPackages), nil, nil, false, &res)
255+
l.generateJavaTestSuite(nil, "blah", []string{src}, stringsToPackageNames([]string{pkg}), "maven", stringsToPackageNames(tc.importedPackages), nil, nil, nil, false, &res)
256256

257257
require.Len(t, res.Gen, 1, "want 1 generated rule")
258258

@@ -309,7 +309,7 @@ func TestAddNonLocalImports(t *testing.T) {
309309

310310
depsDst := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess)
311311
exportsDst := sorted_set.NewSortedSetFn([]types.PackageName{}, types.PackageNameLess)
312-
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"}))
312+
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"}))
313313

314314
want := stringsToPackageNames([]string{
315315
"com.another.a.b",

‎java/gazelle/private/types/types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ func ClassNameLess(l, r ClassName) bool {
110110
type ResolveInput struct {
111111
PackageNames *sorted_set.SortedSet[PackageName]
112112
ImportedPackageNames *sorted_set.SortedSet[PackageName]
113+
ImportedClasses *sorted_set.SortedSet[ClassName]
113114
ExportedPackageNames *sorted_set.SortedSet[PackageName]
114115
AnnotationProcessors *sorted_set.SortedSet[ClassName]
115116
}

0 commit comments

Comments
 (0)