Refine systemfeature codegen behavior

Tweak the semantics of codegen tool feature version args, such that a
`--feature=$FEATURE:$VERSION` arg has the following $VERSION semantics:
  * ""            == undefined (runtime API)
  * valid int     == enabled   (compile-time API)
  * "UNAVAILABLE" == disabled  (compile-time API)

This will simplify how feature args get collected from build flags, and
also aligns with terminology used elsewhere for explicitly disabling
features via the "<unavailable-feature />" tag.

Bug: 203143243
Test: atest systemfeatures-gen-tests systemfeatures-gen-golden-tests
Flag: EXEMPT refactor
Change-Id: I502d455effbe4b81794a5f529406b8b68ac2b372

5 files changed, 39 insertions, 28 deletions

diff --git a/tools/systemfeatures/Android.bp b/tools/systemfeatures/Android.bp
index a9e63289ee93..590f7190881a 100644
--- a/tools/systemfeatures/Android.bp
+++ b/tools/systemfeatures/Android.bp
@@ -30,8 +30,8 @@ genrule {
     name: "systemfeatures-gen-tests-srcs",
     cmd: "$(location systemfeatures-gen-tool) com.android.systemfeatures.RwNoFeatures --readonly=false > $(location RwNoFeatures.java) && " +
         "$(location systemfeatures-gen-tool) com.android.systemfeatures.RoNoFeatures --readonly=true --feature-apis=WATCH > $(location RoNoFeatures.java) && " +
-        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RwFeatures --readonly=false --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:-1 --feature=AUTO: > $(location RwFeatures.java) && " +
-        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RoFeatures --readonly=true --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:-1 --feature=AUTO: --feature-apis=WATCH,PC > $(location RoFeatures.java)",
+        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RwFeatures --readonly=false --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:UNAVAILABLE --feature=AUTO: > $(location RwFeatures.java) && " +
+        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RoFeatures --readonly=true --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:UNAVAILABLE --feature=AUTO: --feature-apis=WATCH,PC > $(location RoFeatures.java)",
     out: [
         "RwNoFeatures.java",
         "RoNoFeatures.java",
diff --git a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt
index 5df453deaf2a..dc0d8a39dbc1 100644
--- a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt
+++ b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt
@@ -31,7 +31,7 @@ import javax.lang.model.element.Modifier
  *
  * <pre>
  *   <cmd> com.foo.RoSystemFeatures --readonly=true \
- *           --feature=WATCH:0 --feature=AUTOMOTIVE: --feature=VULKAN:9348
+ *           --feature=WATCH:0 --feature=AUTOMOTIVE: --feature=VULKAN:9348 --feature=PC:UNAVAILABLE
  *           --feature-apis=WATCH,PC,LEANBACK
  * </pre>
  *
@@ -43,10 +43,10 @@ import javax.lang.model.element.Modifier
  *     @AssumeTrueForR8
  *     public static boolean hasFeatureWatch(Context context);
  *     @AssumeFalseForR8
- *     public static boolean hasFeatureAutomotive(Context context);
+ *     public static boolean hasFeaturePc(Context context);
  *     @AssumeTrueForR8
  *     public static boolean hasFeatureVulkan(Context context);
- *     public static boolean hasFeaturePc(Context context);
+ *     public static boolean hasFeatureAutomotive(Context context);
  *     public static boolean hasFeatureLeanback(Context context);
  *     public static Boolean maybeHasFeature(String feature, int version);
  * }
@@ -67,7 +67,10 @@ object SystemFeaturesGenerator {
         println("Usage: SystemFeaturesGenerator <outputClassName> [options]")
         println(" Options:")
         println("  --readonly=true|false    Whether to encode features as build-time constants")
-        println("  --feature=\$NAME:\$VER   A feature+version pair (blank version == disabled)")
+        println("  --feature=\$NAME:\$VER   A feature+version pair, where \$VER can be:")
+        println("                             * blank/empty == undefined (variable API)")
+        println("                             * valid int   == enabled   (constant API)")
+        println("                             * UNAVAILABLE == disabled  (constant API)")
         println("                           This will always generate associated query APIs,")
         println("                           adding to or replacing those from `--feature-apis=`.")
         println("  --feature-apis=\$NAME_1,\$NAME_2")
@@ -89,7 +92,7 @@ object SystemFeaturesGenerator {
 
         var readonly = false
         var outputClassName: ClassName? = null
-        val featureArgs = mutableListOf<FeatureArg>()
+        val featureArgs = mutableListOf<FeatureInfo>()
         // We could just as easily hardcode this list, as the static API surface should change
         // somewhat infrequently, but this decouples the codegen from the framework completely.
         val featureApiArgs = mutableSetOf<String>()
@@ -122,7 +125,7 @@ object SystemFeaturesGenerator {
         featureArgs.associateByTo(
             features,
             { it.name },
-            { FeatureInfo(it.name, it.version, readonly) },
+            { FeatureInfo(it.name, it.version, it.readonly && readonly) },
         )
 
         outputClassName
@@ -154,13 +157,17 @@ object SystemFeaturesGenerator {
      * Parses a feature argument of the form "--feature=$NAME:$VER", where "$VER" is optional.
      *   * "--feature=WATCH:0" -> Feature enabled w/ version 0 (default version when enabled)
      *   * "--feature=WATCH:7" -> Feature enabled w/ version 7
-     *   * "--feature=WATCH:"  -> Feature disabled
+     *   * "--feature=WATCH:"  -> Feature status undefined, runtime API generated
+     *   * "--feature=WATCH:UNAVAILABLE"  -> Feature disabled
      */
-    private fun parseFeatureArg(arg: String): FeatureArg {
+    private fun parseFeatureArg(arg: String): FeatureInfo {
         val featureArgs = arg.substring(FEATURE_ARG.length).split(":")
         val name = parseFeatureName(featureArgs[0])
-        val version = featureArgs.getOrNull(1)?.toIntOrNull()
-        return FeatureArg(name, version)
+        return when (featureArgs.getOrNull(1)) {
+            null, "" -> FeatureInfo(name, null, readonly = false)
+            "UNAVAILABLE" -> FeatureInfo(name, null, readonly = true)
+            else -> FeatureInfo(name, featureArgs[1].toIntOrNull(), readonly = true)
+        }
     }
 
     private fun parseFeatureName(name: String): String =
@@ -267,7 +274,5 @@ object SystemFeaturesGenerator {
         builder.addMethod(methodBuilder.build())
     }
 
-    private data class FeatureArg(val name: String, val version: Int?)
-
     private data class FeatureInfo(val name: String, val version: Int?, val readonly: Boolean)
 }
diff --git a/tools/systemfeatures/tests/golden/RoFeatures.java.gen b/tools/systemfeatures/tests/golden/RoFeatures.java.gen
index 724639b52d23..dfc2937dc41d 100644
--- a/tools/systemfeatures/tests/golden/RoFeatures.java.gen
+++ b/tools/systemfeatures/tests/golden/RoFeatures.java.gen
@@ -3,7 +3,7 @@
 //            --readonly=true \
 //            --feature=WATCH:1 \
 //            --feature=WIFI:0 \
-//            --feature=VULKAN:-1 \
+//            --feature=VULKAN:UNAVAILABLE \
 //            --feature=AUTO: \
 //            --feature-apis=WATCH,PC
 package com.android.systemfeatures;
@@ -62,9 +62,8 @@ public final class RoFeatures {
      *
      * @hide
      */
-    @AssumeFalseForR8
     public static boolean hasFeatureAuto(Context context) {
-        return false;
+        return hasFeatureFallback(context, PackageManager.FEATURE_AUTO);
     }
 
     private static boolean hasFeatureFallback(Context context, String featureName) {
@@ -79,8 +78,7 @@ public final class RoFeatures {
         switch (featureName) {
             case PackageManager.FEATURE_WATCH: return 1 >= version;
             case PackageManager.FEATURE_WIFI: return 0 >= version;
-            case PackageManager.FEATURE_VULKAN: return -1 >= version;
-            case PackageManager.FEATURE_AUTO: return false;
+            case PackageManager.FEATURE_VULKAN: return false;
             default: break;
         }
         return null;
diff --git a/tools/systemfeatures/tests/golden/RwFeatures.java.gen b/tools/systemfeatures/tests/golden/RwFeatures.java.gen
index 6f897591e48f..89097fbfd180 100644
--- a/tools/systemfeatures/tests/golden/RwFeatures.java.gen
+++ b/tools/systemfeatures/tests/golden/RwFeatures.java.gen
@@ -3,7 +3,7 @@
 //            --readonly=false \
 //            --feature=WATCH:1 \
 //            --feature=WIFI:0 \
-//            --feature=VULKAN:-1 \
+//            --feature=VULKAN:UNAVAILABLE \
 //            --feature=AUTO:
 package com.android.systemfeatures;
 
diff --git a/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java b/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java
index 6dfd244a807b..c3a23cbd8e48 100644
--- a/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java
+++ b/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java
@@ -110,10 +110,13 @@ public class SystemFeaturesGeneratorTest {
         assertThat(RoFeatures.hasFeatureWatch(mContext)).isTrue();
         assertThat(RoFeatures.hasFeatureWifi(mContext)).isTrue();
         assertThat(RoFeatures.hasFeatureVulkan(mContext)).isFalse();
-        assertThat(RoFeatures.hasFeatureAuto(mContext)).isFalse();
         verify(mPackageManager, never()).hasSystemFeature(anyString(), anyInt());
 
-        // For defined feature types, conditional queries should reflect the build-time versions.
+        // For defined feature types, conditional queries should reflect either:
+        //  * Enabled if the feature version is specified
+        //  * Disabled if UNAVAILABLE is specified
+        //  * Unknown if no version value is provided
+
         // VERSION=1
         assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_WATCH, -1)).isTrue();
         assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_WATCH, 0)).isTrue();
@@ -124,15 +127,19 @@ public class SystemFeaturesGeneratorTest {
         assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_WIFI, 0)).isTrue();
         assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_WIFI, 100)).isFalse();
 
-        // VERSION=-1
-        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, -1)).isTrue();
+        // VERSION=UNAVAILABLE
+        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, -1)).isFalse();
         assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isFalse();
         assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 100)).isFalse();
 
-        // DISABLED
-        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, -1)).isFalse();
-        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isFalse();
-        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 100)).isFalse();
+        // VERSION=
+        when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_AUTO, 0)).thenReturn(false);
+        assertThat(RoFeatures.hasFeatureAuto(mContext)).isFalse();
+        when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_AUTO, 0)).thenReturn(true);
+        assertThat(RoFeatures.hasFeatureAuto(mContext)).isTrue();
+        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, -1)).isNull();
+        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull();
+        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 100)).isNull();
 
         // For feature APIs without an associated feature definition, conditional queries should
         // report null, and explicit queries should report runtime-defined versions.
@@ -148,5 +155,6 @@ public class SystemFeaturesGeneratorTest {
         assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", -1)).isNull();
         assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull();
         assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", 100)).isNull();
+        assertThat(RoFeatures.maybeHasFeature("", 0)).isNull();
     }
 }