Fix edge case in inferring Java package from path to targets

When deriving the Java package from path to a target, actually derive it from
the path to the entire target, not just the target's BUILD file.

RELNOTES: none
PiperOrigin-RevId: 198426047

3 files changed, 37 insertions, 8 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index 56d4cb76d2..f9e7464e75 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -289,6 +289,14 @@ public class AndroidCommon {
     return ideInfoProviderBuilder.build();
   }
 
+  /**
+   * Gets the Java package for the current target.
+   *
+   * @deprecated If no custom_package is specified, this method will derive the Java package from
+   *     the package path, even if that path is not a valid Java path. Use {@link
+   *     AndroidManifest#getAndroidPackage(RuleContext)} instead.
+   */
+  @Deprecated
   public static String getJavaPackage(RuleContext ruleContext) {
     AttributeMap attributes = ruleContext.attributes();
     if (attributes.isAttributeValueExplicitlySpecified("custom_package")) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java
index 4db8e3d959..160ccb3980 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.android;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.packages.RuleErrorConsumer;
@@ -34,7 +35,6 @@ public class AndroidManifest {
   private final Artifact manifest;
   /** The Android package. Will be null if and only if this is an aar_import target. */
   @Nullable private final String pkg;
-
   private final boolean exported;
 
   public static StampedAndroidManifest forAarImport(Artifact manifest) {
@@ -121,7 +121,9 @@ public class AndroidManifest {
       boolean exportsManifest)
       throws InterruptedException {
     if (pkg == null) {
-      pkg = getDefaultPackage(dataContext.getActionConstructionContext(), errorConsumer);
+      pkg =
+          getDefaultPackage(
+              dataContext.getLabel(), dataContext.getActionConstructionContext(), errorConsumer);
     }
 
     if (rawManifest == null) {
@@ -224,13 +226,28 @@ public class AndroidManifest {
       return ruleContext.attributes().get(CUSTOM_PACKAGE_ATTR, Type.STRING);
     }
 
-    return getDefaultPackage(ruleContext, ruleContext);
+    return getDefaultPackage(ruleContext.getLabel(), ruleContext, ruleContext);
   }
 
-  /** Gets the default Java package */
+  /**
+   * Gets the default Java package for this target, based on the path to it.
+   *
+   * <p>For example, target "//some/path/java/com/foo/bar:baz" will have the default Java package of
+   * "com.foo.bar".
+   *
+   * <p>A rule error will be registered if this path does not contain a "java" or "javatests"
+   * segment indicating where the package begins.
+   *
+   * <p>This method should not be called if the target specifies a custom package; in that case,
+   * that package should be used instead.
+   */
   public static String getDefaultPackage(
-      ActionConstructionContext context, RuleErrorConsumer errorConsumer) {
-    PathFragment dummyJar = context.getPackageDirectory().getChild("Dummy.jar");
+      Label label, ActionConstructionContext context, RuleErrorConsumer errorConsumer) {
+    PathFragment dummyJar =
+        // For backwards compatibility, also include the target's name in case it contains multiple
+        // directories - for example, target "//foo/bar:java/baz/quux" is a legal one and results in
+        // Java path of "baz/quux"
+        context.getPackageDirectory().getRelative(label.getName() + "Dummy.jar");
     return getJavaPackageFromPath(context, errorConsumer, dummyJar);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
index 55fbbe574c..e61ed5ae60 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
@@ -144,7 +144,9 @@ public abstract class AndroidSkylarkData {
         SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
       String pkg = fromNoneable(customPackage, String.class);
       if (pkg == null) {
-        pkg = AndroidManifest.getDefaultPackage(ctx.getActionConstructionContext(), errorReporter);
+        pkg =
+            AndroidManifest.getDefaultPackage(
+                env.getCallerLabel(), ctx.getActionConstructionContext(), errorReporter);
       }
       return ResourceApk.processFromTransitiveLibraryData(
               ctx,
@@ -206,7 +208,9 @@ public abstract class AndroidSkylarkData {
     try (SkylarkErrorReporter errorReporter =
         SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
       if (pkg == null) {
-        pkg = AndroidManifest.getDefaultPackage(ctx.getActionConstructionContext(), errorReporter);
+        pkg =
+            AndroidManifest.getDefaultPackage(
+                env.getCallerLabel(), ctx.getActionConstructionContext(), errorReporter);
       }
     }