From 59a4a2b8d210e74e6625794f954bd11bb6157002 Mon Sep 17 00:00:00 2001
From: Spandan Das <spandandas@google.com>
Date: Tue, 9 Jan 2024 21:35:56 +0000
Subject: [PATCH] Replace panic with ModuleErrorf

This is a followup cleanup for aosp/2876754 and replaces panic with
ctx.ModuleErrorf. The latter creates a more expressive build error.

Implementation details
- export moduleErrorf interface from build/soong/android. This minimal
  interface will be used as a parameter for `DexJarBuildPath`
- Add ModuleErrorf to the function signature of DexJarBuildPath. This
  parameter only gets used for Import and SdkLibraryImport structs.
  These two can have duplicate deapexer definitions, and ModuleErrorf
  will be used to report that error
- Create a minimal implementation of `ModuleErrorf` in tests of java and
  apex

Test: m nothing --no-skip-soong-tests
Change-Id: I0febec651f40c3f04deb957e64133c94b80fbd78
---
 android/paths.go                    |  8 ++++----
 apex/apex.go                        |  4 ++--
 apex/apex_test.go                   | 15 ++++++++++++---
 apex/bootclasspath_fragment_test.go |  2 +-
 java/aar.go                         |  2 +-
 java/app.go                         |  2 +-
 java/base.go                        |  2 +-
 java/device_host_converter.go       |  2 +-
 java/hiddenapi_modular.go           |  6 ++++--
 java/hiddenapi_singleton_test.go    |  3 ++-
 java/java.go                        | 12 ++++++------
 java/java_test.go                   | 14 ++++++++++++--
 java/sdk_library.go                 |  8 ++++----
 13 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/android/paths.go b/android/paths.go
index 6aabe4f9e..95f53ea4c 100644
--- a/android/paths.go
+++ b/android/paths.go
@@ -181,13 +181,13 @@ type errorfContext interface {
 
 var _ errorfContext = blueprint.SingletonContext(nil)
 
-// moduleErrorf is the interface containing the ModuleErrorf method matching
+// ModuleErrorfContext is the interface containing the ModuleErrorf method matching
 // the ModuleErrorf method in blueprint.ModuleContext.
-type moduleErrorf interface {
+type ModuleErrorfContext interface {
 	ModuleErrorf(format string, args ...interface{})
 }
 
-var _ moduleErrorf = blueprint.ModuleContext(nil)
+var _ ModuleErrorfContext = blueprint.ModuleContext(nil)
 
 // reportPathError will register an error with the attached context. It
 // attempts ctx.ModuleErrorf for a better error message first, then falls
@@ -200,7 +200,7 @@ func reportPathError(ctx PathContext, err error) {
 // attempts ctx.ModuleErrorf for a better error message first, then falls
 // back to ctx.Errorf.
 func ReportPathErrorf(ctx PathContext, format string, args ...interface{}) {
-	if mctx, ok := ctx.(moduleErrorf); ok {
+	if mctx, ok := ctx.(ModuleErrorfContext); ok {
 		mctx.ModuleErrorf(format, args...)
 	} else if ectx, ok := ctx.(errorfContext); ok {
 		ectx.Errorf(format, args...)
diff --git a/apex/apex.go b/apex/apex.go
index 42a7d73e7..4dd77281e 100644
--- a/apex/apex.go
+++ b/apex/apex.go
@@ -1628,7 +1628,7 @@ func apexFileForCompatConfig(ctx android.BaseModuleContext, config java.Platform
 type javaModule interface {
 	android.Module
 	BaseModuleName() string
-	DexJarBuildPath() java.OptionalDexJarPath
+	DexJarBuildPath(ctx android.ModuleErrorfContext) java.OptionalDexJarPath
 	JacocoReportClassesFile() android.Path
 	LintDepSets() java.LintDepSets
 	Stem() string
@@ -1642,7 +1642,7 @@ var _ javaModule = (*java.SdkLibraryImport)(nil)
 
 // apexFileForJavaModule creates an apexFile for a java module's dex implementation jar.
 func apexFileForJavaModule(ctx android.BaseModuleContext, module javaModule) apexFile {
-	return apexFileForJavaModuleWithFile(ctx, module, module.DexJarBuildPath().PathOrNil())
+	return apexFileForJavaModuleWithFile(ctx, module, module.DexJarBuildPath(ctx).PathOrNil())
 }
 
 // apexFileForJavaModuleWithFile creates an apexFile for a java module with the supplied file.
diff --git a/apex/apex_test.go b/apex/apex_test.go
index 1b9fa19f0..a943e4e02 100644
--- a/apex/apex_test.go
+++ b/apex/apex_test.go
@@ -5355,6 +5355,13 @@ func TestPrebuiltApexNameWithPlatformBootclasspath(t *testing.T) {
 	).RunTest(t)
 }
 
+// A minimal context object for use with DexJarBuildPath
+type moduleErrorfTestCtx struct {
+}
+
+func (ctx moduleErrorfTestCtx) ModuleErrorf(format string, args ...interface{}) {
+}
+
 // These tests verify that the prebuilt_apex/deapexer to java_import wiring allows for the
 // propagation of paths to dex implementation jars from the former to the latter.
 func TestPrebuiltExportDexImplementationJars(t *testing.T) {
@@ -5364,7 +5371,7 @@ func TestPrebuiltExportDexImplementationJars(t *testing.T) {
 		t.Helper()
 		// Make sure the import has been given the correct path to the dex jar.
 		p := ctx.ModuleForTests(name, "android_common_myapex").Module().(java.UsesLibraryDependency)
-		dexJarBuildPath := p.DexJarBuildPath().PathOrNil()
+		dexJarBuildPath := p.DexJarBuildPath(moduleErrorfTestCtx{}).PathOrNil()
 		stem := android.RemoveOptionalPrebuiltPrefix(name)
 		android.AssertStringEquals(t, "DexJarBuildPath should be apex-related path.",
 			".intermediates/myapex.deapexer/android_common/deapexer/javalib/"+stem+".jar",
@@ -8491,6 +8498,8 @@ func TestDuplicateButEquivalentDeapexersFromPrebuiltApexes(t *testing.T) {
 		PrepareForTestWithApexBuildComponents,
 	)
 
+	errCtx := moduleErrorfTestCtx{}
+
 	bpBase := `
 		apex_set {
 			name: "com.android.myapex",
@@ -8540,7 +8549,7 @@ func TestDuplicateButEquivalentDeapexersFromPrebuiltApexes(t *testing.T) {
 		usesLibraryDep := module.(java.UsesLibraryDependency)
 		android.AssertPathRelativeToTopEquals(t, "dex jar path",
 			"out/soong/.intermediates/com.android.myapex.deapexer/android_common/deapexer/javalib/libfoo.jar",
-			usesLibraryDep.DexJarBuildPath().Path())
+			usesLibraryDep.DexJarBuildPath(errCtx).Path())
 	})
 
 	t.Run("java_sdk_library_import", func(t *testing.T) {
@@ -8563,7 +8572,7 @@ func TestDuplicateButEquivalentDeapexersFromPrebuiltApexes(t *testing.T) {
 		usesLibraryDep := module.(java.UsesLibraryDependency)
 		android.AssertPathRelativeToTopEquals(t, "dex jar path",
 			"out/soong/.intermediates/com.android.myapex.deapexer/android_common/deapexer/javalib/libfoo.jar",
-			usesLibraryDep.DexJarBuildPath().Path())
+			usesLibraryDep.DexJarBuildPath(errCtx).Path())
 	})
 
 	t.Run("prebuilt_bootclasspath_fragment", func(t *testing.T) {
diff --git a/apex/bootclasspath_fragment_test.go b/apex/bootclasspath_fragment_test.go
index 159e9e1c4..2600169a5 100644
--- a/apex/bootclasspath_fragment_test.go
+++ b/apex/bootclasspath_fragment_test.go
@@ -677,7 +677,7 @@ func TestBootclasspathFragmentContentsNoName(t *testing.T) {
 
 func getDexJarPath(result *android.TestResult, name string) string {
 	module := result.Module(name, "android_common")
-	return module.(java.UsesLibraryDependency).DexJarBuildPath().Path().RelativeToTop().String()
+	return module.(java.UsesLibraryDependency).DexJarBuildPath(moduleErrorfTestCtx{}).Path().RelativeToTop().String()
 }
 
 // TestBootclasspathFragment_HiddenAPIList checks to make sure that the correct parameters are
diff --git a/java/aar.go b/java/aar.go
index eb07e0fb2..15a542e1d 100644
--- a/java/aar.go
+++ b/java/aar.go
@@ -1284,7 +1284,7 @@ func (a *AARImport) ImplementationAndResourcesJars() android.Paths {
 	return android.Paths{a.classpathFile}
 }
 
-func (a *AARImport) DexJarBuildPath() android.Path {
+func (a *AARImport) DexJarBuildPath(ctx android.ModuleErrorfContext) android.Path {
 	return nil
 }
 
diff --git a/java/app.go b/java/app.go
index 8b28dac78..cb05807b8 100755
--- a/java/app.go
+++ b/java/app.go
@@ -1639,7 +1639,7 @@ func (u *usesLibrary) classLoaderContextForUsesLibDeps(ctx android.ModuleContext
 				replaceInList(u.usesLibraryProperties.Optional_uses_libs, dep, libName)
 			}
 			clcMap.AddContext(ctx, tag.sdkVersion, libName, tag.optional,
-				lib.DexJarBuildPath().PathOrNil(), lib.DexJarInstallPath(),
+				lib.DexJarBuildPath(ctx).PathOrNil(), lib.DexJarInstallPath(),
 				lib.ClassLoaderContexts())
 		} else if ctx.Config().AllowMissingDependencies() {
 			ctx.AddMissingDependencies([]string{dep})
diff --git a/java/base.go b/java/base.go
index 51471ea3a..1ac3d30a6 100644
--- a/java/base.go
+++ b/java/base.go
@@ -1929,7 +1929,7 @@ func (j *Module) ImplementationJars() android.Paths {
 	return android.Paths{j.implementationJarFile}
 }
 
-func (j *Module) DexJarBuildPath() OptionalDexJarPath {
+func (j *Module) DexJarBuildPath(ctx android.ModuleErrorfContext) OptionalDexJarPath {
 	return j.dexJarFile
 }
 
diff --git a/java/device_host_converter.go b/java/device_host_converter.go
index 834651f54..efd13b8d3 100644
--- a/java/device_host_converter.go
+++ b/java/device_host_converter.go
@@ -151,7 +151,7 @@ func (d *DeviceHostConverter) ImplementationAndResourcesJars() android.Paths {
 	return d.implementationAndResourceJars
 }
 
-func (d *DeviceHostConverter) DexJarBuildPath() android.Path {
+func (d *DeviceHostConverter) DexJarBuildPath(ctx android.ModuleErrorfContext) android.Path {
 	return nil
 }
 
diff --git a/java/hiddenapi_modular.go b/java/hiddenapi_modular.go
index c3caa084f..a51286064 100644
--- a/java/hiddenapi_modular.go
+++ b/java/hiddenapi_modular.go
@@ -291,7 +291,7 @@ func hiddenAPIRetrieveDexJarBuildPath(ctx android.ModuleContext, module android.
 	if sdkLibrary, ok := module.(SdkLibraryDependency); ok {
 		dexJar = sdkLibrary.SdkApiStubDexJar(ctx, kind)
 	} else if j, ok := module.(UsesLibraryDependency); ok {
-		dexJar = j.DexJarBuildPath()
+		dexJar = j.DexJarBuildPath(ctx)
 	} else {
 		ctx.ModuleErrorf("dependency %s of module type %s does not support providing a dex jar", module, ctx.OtherModuleType(module))
 		return nil
@@ -1457,7 +1457,9 @@ func handleMissingDexBootFile(ctx android.ModuleContext, module android.Module,
 // However, under certain conditions, e.g. errors, or special build configurations it will return
 // a path to a fake file.
 func retrieveEncodedBootDexJarFromModule(ctx android.ModuleContext, module android.Module) android.Path {
-	bootDexJar := module.(interface{ DexJarBuildPath() OptionalDexJarPath }).DexJarBuildPath()
+	bootDexJar := module.(interface {
+		DexJarBuildPath(ctx android.ModuleErrorfContext) OptionalDexJarPath
+	}).DexJarBuildPath(ctx)
 	if !bootDexJar.Valid() {
 		fake := android.PathForModuleOut(ctx, fmt.Sprintf("fake/encoded-dex/%s.jar", module.Name()))
 		handleMissingDexBootFile(ctx, module, fake, bootDexJar.InvalidReason())
diff --git a/java/hiddenapi_singleton_test.go b/java/hiddenapi_singleton_test.go
index ef792f970..37afe297c 100644
--- a/java/hiddenapi_singleton_test.go
+++ b/java/hiddenapi_singleton_test.go
@@ -309,7 +309,8 @@ func TestHiddenAPIEncoding_JavaSdkLibrary(t *testing.T) {
 		android.AssertStringEquals(t, "encode embedded java_library", unencodedDexJar, actualUnencodedDexJar.String())
 
 		// Make sure that the encoded dex jar is the exported one.
-		exportedDexJar := moduleForTests.Module().(UsesLibraryDependency).DexJarBuildPath().Path()
+		errCtx := moduleErrorfTestCtx{}
+		exportedDexJar := moduleForTests.Module().(UsesLibraryDependency).DexJarBuildPath(errCtx).Path()
 		android.AssertPathRelativeToTopEquals(t, "encode embedded java_library", encodedDexJar, exportedDexJar)
 	}
 
diff --git a/java/java.go b/java/java.go
index 2a4fafa8b..1ac3730df 100644
--- a/java/java.go
+++ b/java/java.go
@@ -315,7 +315,7 @@ type ApexDependency interface {
 
 // Provides build path and install path to DEX jars.
 type UsesLibraryDependency interface {
-	DexJarBuildPath() OptionalDexJarPath
+	DexJarBuildPath(ctx android.ModuleErrorfContext) OptionalDexJarPath
 	DexJarInstallPath() android.Path
 	ClassLoaderContexts() dexpreopt.ClassLoaderContextMap
 }
@@ -2011,7 +2011,7 @@ func (al *ApiLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	})
 }
 
-func (al *ApiLibrary) DexJarBuildPath() OptionalDexJarPath {
+func (al *ApiLibrary) DexJarBuildPath(ctx android.ModuleErrorfContext) OptionalDexJarPath {
 	return al.dexJarFile
 }
 
@@ -2378,9 +2378,9 @@ func (j *Import) ImplementationAndResourcesJars() android.Paths {
 	return android.Paths{j.combinedClasspathFile}
 }
 
-func (j *Import) DexJarBuildPath() OptionalDexJarPath {
+func (j *Import) DexJarBuildPath(ctx android.ModuleErrorfContext) OptionalDexJarPath {
 	if j.dexJarFileErr != nil {
-		panic(j.dexJarFileErr.Error())
+		ctx.ModuleErrorf(j.dexJarFileErr.Error())
 	}
 	return j.dexJarFile
 }
@@ -2631,7 +2631,7 @@ func (j *DexImport) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	}
 }
 
-func (j *DexImport) DexJarBuildPath() OptionalDexJarPath {
+func (j *DexImport) DexJarBuildPath(ctx android.ModuleErrorfContext) OptionalDexJarPath {
 	return j.dexJarFile
 }
 
@@ -2799,7 +2799,7 @@ func addCLCFromDep(ctx android.ModuleContext, depModule android.Module,
 	// from its CLC should be added to the current CLC.
 	if sdkLib != nil {
 		clcMap.AddContext(ctx, dexpreopt.AnySdkVersion, *sdkLib, false,
-			dep.DexJarBuildPath().PathOrNil(), dep.DexJarInstallPath(), dep.ClassLoaderContexts())
+			dep.DexJarBuildPath(ctx).PathOrNil(), dep.DexJarInstallPath(), dep.ClassLoaderContexts())
 	} else {
 		clcMap.AddContextMap(dep.ClassLoaderContexts(), depName)
 	}
diff --git a/java/java_test.go b/java/java_test.go
index b9dc453b3..59a64beaf 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -528,6 +528,15 @@ func TestHostBinaryNoJavaDebugInfoOverride(t *testing.T) {
 	}
 }
 
+// A minimal context object for use with DexJarBuildPath
+type moduleErrorfTestCtx struct {
+}
+
+func (ctx moduleErrorfTestCtx) ModuleErrorf(format string, args ...interface{}) {
+}
+
+var _ android.ModuleErrorfContext = (*moduleErrorfTestCtx)(nil)
+
 func TestPrebuilts(t *testing.T) {
 	ctx, _ := testJava(t, `
 		java_library {
@@ -595,7 +604,8 @@ func TestPrebuilts(t *testing.T) {
 		t.Errorf("foo classpath %v does not contain %q", javac.Args["classpath"], barJar.String())
 	}
 
-	barDexJar := barModule.Module().(*Import).DexJarBuildPath()
+	errCtx := moduleErrorfTestCtx{}
+	barDexJar := barModule.Module().(*Import).DexJarBuildPath(errCtx)
 	if barDexJar.IsSet() {
 		t.Errorf("bar dex jar build path expected to be set, got %s", barDexJar)
 	}
@@ -608,7 +618,7 @@ func TestPrebuilts(t *testing.T) {
 		t.Errorf("foo combineJar inputs %v does not contain %q", combineJar.Inputs, bazJar.String())
 	}
 
-	bazDexJar := bazModule.Module().(*Import).DexJarBuildPath().Path()
+	bazDexJar := bazModule.Module().(*Import).DexJarBuildPath(errCtx).Path()
 	expectedDexJar := "out/soong/.intermediates/baz/android_common/dex/baz.jar"
 	android.AssertPathRelativeToTopEquals(t, "baz dex jar build path", expectedDexJar, bazDexJar)
 
diff --git a/java/sdk_library.go b/java/sdk_library.go
index ef34fb6cd..e696251cb 100644
--- a/java/sdk_library.go
+++ b/java/sdk_library.go
@@ -678,7 +678,7 @@ func (paths *scopePaths) extractStubsLibraryInfoFromDependency(ctx android.Modul
 		paths.stubsImplPath = lib.ImplementationJars
 
 		libDep := dep.(UsesLibraryDependency)
-		paths.stubsDexJarPath = libDep.DexJarBuildPath()
+		paths.stubsDexJarPath = libDep.DexJarBuildPath(ctx)
 		return nil
 	} else {
 		return fmt.Errorf("expected module that has JavaInfoProvider, e.g. java_library")
@@ -2752,11 +2752,11 @@ func (module *SdkLibraryImport) SdkImplementationJars(ctx android.BaseModuleCont
 }
 
 // to satisfy UsesLibraryDependency interface
-func (module *SdkLibraryImport) DexJarBuildPath() OptionalDexJarPath {
+func (module *SdkLibraryImport) DexJarBuildPath(ctx android.ModuleErrorfContext) OptionalDexJarPath {
 	// The dex implementation jar extracted from the .apex file should be used in preference to the
 	// source.
 	if module.dexJarFileErr != nil {
-		panic(module.dexJarFileErr.Error())
+		ctx.ModuleErrorf(module.dexJarFileErr.Error())
 	}
 	if module.dexJarFile.IsSet() {
 		return module.dexJarFile
@@ -2764,7 +2764,7 @@ func (module *SdkLibraryImport) DexJarBuildPath() OptionalDexJarPath {
 	if module.implLibraryModule == nil {
 		return makeUnsetDexJarPath()
 	} else {
-		return module.implLibraryModule.DexJarBuildPath()
+		return module.implLibraryModule.DexJarBuildPath(ctx)
 	}
 }