Merge changes I74045709,I69b9a843 into main

* changes:
  aconfig: modify and filter flags before passing into java codegen
  aconfig: modify_parsed_flags_based_on_mode return Result

tools/aconfig/src/codegen/cpp.rs

@@ -1028,7 +1028,7 @@ bool com_android_aconfig_test_enabled_ro() {
         expected_src: &str,
     ) {
         let modified_parsed_flags =
-            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode);
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
         let generated =
             generate_cpp_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
                 .unwrap();

tools/aconfig/src/codegen/java.rs

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-use anyhow::{anyhow, Result};
+use anyhow::Result;
 use serde::Serialize;
 use std::collections::{BTreeMap, BTreeSet};
 use std::path::PathBuf;
@@ -35,8 +35,6 @@ where
 {
     let flag_elements: Vec<FlagElement> =
         parsed_flags_iter.map(|pf| create_flag_element(package, &pf)).collect();
-    let exported_flag_elements: Vec<FlagElement> =
-        flag_elements.iter().filter(|elem| elem.exported).cloned().collect();
     let namespace_flags = gen_flags_by_namespace(&flag_elements);
     let properties_set: BTreeSet<String> =
         flag_elements.iter().map(|fe| format_property_name(&fe.device_config_namespace)).collect();
@@ -45,13 +43,8 @@ where
     let runtime_lookup_required =
         flag_elements.iter().any(|elem| elem.is_read_write) || library_exported;
 
-    if library_exported && exported_flag_elements.is_empty() {
-        return Err(anyhow!("exported library contains no exported flags"));
-    }
-
     let context = Context {
         flag_elements,
-        exported_flag_elements,
         namespace_flags,
         is_test_mode,
         runtime_lookup_required,
@@ -110,7 +103,6 @@ fn gen_flags_by_namespace(flags: &[FlagElement]) -> Vec<NamespaceFlags> {
 #[derive(Serialize)]
 struct Context {
     pub flag_elements: Vec<FlagElement>,
-    pub exported_flag_elements: Vec<FlagElement>,
     pub namespace_flags: Vec<NamespaceFlags>,
     pub is_test_mode: bool,
     pub runtime_lookup_required: bool,
@@ -134,7 +126,6 @@ struct FlagElement {
     pub is_read_write: bool,
     pub method_name: String,
     pub properties: String,
-    pub exported: bool,
 }
 
 fn create_flag_element(package: &str, pf: &ProtoParsedFlag) -> FlagElement {
@@ -148,7 +139,6 @@ fn create_flag_element(package: &str, pf: &ProtoParsedFlag) -> FlagElement {
         is_read_write: pf.permission() == ProtoFlagPermission::READ_WRITE,
         method_name: format_java_method_name(pf.name()),
         properties: format_property_name(pf.namespace()),
-        exported: pf.is_exported.unwrap_or(false),
     }
 }
 
@@ -376,12 +366,12 @@ mod tests {
     #[test]
     fn test_generate_java_code_production() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
+        let mode = CodegenMode::Production;
-            crate::test::TEST_PACKAGE,
+        let modified_parsed_flags =
-            parsed_flags.parsed_flag.into_iter(),
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
-            CodegenMode::Production,
+        let generated_files =
-        )
+            generate_java_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
-        .unwrap();
+                .unwrap();
         let expect_flags_content = EXPECTED_FLAG_COMMON_CONTENT.to_string()
             + r#"
             private static FeatureFlags FEATURE_FLAGS = new FeatureFlagsImpl();
@@ -534,12 +524,12 @@ mod tests {
     #[test]
     fn test_generate_java_code_exported() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
+        let mode = CodegenMode::Exported;
-            crate::test::TEST_PACKAGE,
+        let modified_parsed_flags =
-            parsed_flags.parsed_flag.into_iter(),
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
-            CodegenMode::Exported,
+        let generated_files =
-        )
+            generate_java_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
-        .unwrap();
+                .unwrap();
 
         let expect_flags_content = r#"
         package com.android.aconfig.test;
@@ -594,7 +584,6 @@ mod tests {
         /** @hide */
         public final class FeatureFlagsImpl implements FeatureFlags {
             private static boolean aconfig_test_is_cached = false;
-            private static boolean other_namespace_is_cached = false;
             private static boolean disabledRwExported = false;
             private static boolean enabledFixedRoExported = false;
             private static boolean enabledRoExported = false;
@@ -622,22 +611,6 @@ mod tests {
                 aconfig_test_is_cached = true;
             }
 
-            private void load_overrides_other_namespace() {
-                try {
-                    Properties properties = DeviceConfig.getProperties("other_namespace");
-                } catch (NullPointerException e) {
-                    throw new RuntimeException(
-                        "Cannot read value from namespace other_namespace "
-                        + "from DeviceConfig. It could be that the code using flag "
-                        + "executed before SettingsProvider initialization. Please use "
-                        + "fixed read-only flag by adding is_fixed_read_only: true in "
-                        + "flag declaration.",
-                        e
-                    );
-                }
-                other_namespace_is_cached = true;
-            }
-
             @Override
             @UnsupportedAppUsage
             public boolean disabledRwExported() {
@@ -751,12 +724,12 @@ mod tests {
     #[test]
     fn test_generate_java_code_test() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
+        let mode = CodegenMode::Test;
-            crate::test::TEST_PACKAGE,
+        let modified_parsed_flags =
-            parsed_flags.parsed_flag.into_iter(),
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
-            CodegenMode::Test,
+        let generated_files =
-        )
+            generate_java_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
-        .unwrap();
+                .unwrap();
 
         let expect_flags_content = EXPECTED_FLAG_COMMON_CONTENT.to_string()
             + r#"

tools/aconfig/src/codegen/mod.rs

@@ -55,9 +55,19 @@ pub fn create_device_config_ident(package: &str, flag_name: &str) -> Result<Stri
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)]
 pub enum CodegenMode {
+    Exported,
     Production,
     Test,
-    Exported,
+}
+
+impl std::fmt::Display for CodegenMode {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match self {
+            CodegenMode::Exported => write!(f, "exported"),
+            CodegenMode::Production => write!(f, "production"),
+            CodegenMode::Test => write!(f, "test"),
+        }
+    }
 }
 
 #[cfg(test)]

tools/aconfig/src/codegen/rust.rs

@@ -557,7 +557,7 @@ pub fn enabled_ro_exported() -> bool {
     fn test_generate_rust_code(mode: CodegenMode) {
         let parsed_flags = crate::test::parse_test_flags();
         let modified_parsed_flags =
-            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode);
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
         let generated =
             generate_rust_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
                 .unwrap();

tools/aconfig/src/commands.rs

@@ -190,17 +190,17 @@ pub fn parse_flags(
 
 pub fn create_java_lib(mut input: Input, codegen_mode: CodegenMode) -> Result<Vec<OutputFile>> {
     let parsed_flags = input.try_parse_flags()?;
-    let filtered_parsed_flags = filter_parsed_flags(parsed_flags, codegen_mode);
+    let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode)?;
-    let Some(package) = find_unique_package(&filtered_parsed_flags) else {
+    let Some(package) = find_unique_package(&modified_parsed_flags) else {
         bail!("no parsed flags, or the parsed flags use different packages");
     };
     let package = package.to_string();
-    generate_java_code(&package, filtered_parsed_flags.into_iter(), codegen_mode)
+    generate_java_code(&package, modified_parsed_flags.into_iter(), codegen_mode)
 }
 
 pub fn create_cpp_lib(mut input: Input, codegen_mode: CodegenMode) -> Result<Vec<OutputFile>> {
     let parsed_flags = input.try_parse_flags()?;
-    let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode);
+    let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode)?;
     let Some(package) = find_unique_package(&modified_parsed_flags) else {
         bail!("no parsed flags, or the parsed flags use different packages");
     };
@@ -210,7 +210,7 @@ pub fn create_cpp_lib(mut input: Input, codegen_mode: CodegenMode) -> Result<Vec
 
 pub fn create_rust_lib(mut input: Input, codegen_mode: CodegenMode) -> Result<OutputFile> {
     let parsed_flags = input.try_parse_flags()?;
-    let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode);
+    let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode)?;
     let Some(package) = find_unique_package(&modified_parsed_flags) else {
         bail!("no parsed flags, or the parsed flags use different packages");
     };
@@ -316,22 +316,10 @@ fn find_unique_container(parsed_flags: &ProtoParsedFlags) -> Option<&str> {
     Some(container)
 }
 
-fn filter_parsed_flags(
-    parsed_flags: ProtoParsedFlags,
-    codegen_mode: CodegenMode,
-) -> Vec<ProtoParsedFlag> {
-    match codegen_mode {
-        CodegenMode::Exported => {
-            parsed_flags.parsed_flag.into_iter().filter(|pf| pf.is_exported()).collect()
-        }
-        _ => parsed_flags.parsed_flag,
-    }
-}
-
 pub fn modify_parsed_flags_based_on_mode(
     parsed_flags: ProtoParsedFlags,
     codegen_mode: CodegenMode,
-) -> Vec<ProtoParsedFlag> {
+) -> Result<Vec<ProtoParsedFlag>> {
     fn exported_mode_flag_modifier(mut parsed_flag: ProtoParsedFlag) -> ProtoParsedFlag {
         parsed_flag.set_state(ProtoFlagState::DISABLED);
         parsed_flag.set_permission(ProtoFlagPermission::READ_WRITE);
@@ -339,7 +327,7 @@ pub fn modify_parsed_flags_based_on_mode(
         parsed_flag
     }
 
-    match codegen_mode {
+    let modified_parsed_flags: Vec<_> = match codegen_mode {
         CodegenMode::Exported => parsed_flags
             .parsed_flag
             .into_iter()
@@ -349,7 +337,12 @@ pub fn modify_parsed_flags_based_on_mode(
         CodegenMode::Production | CodegenMode::Test => {
             parsed_flags.parsed_flag.into_iter().collect()
         }
+    };
+    if modified_parsed_flags.is_empty() {
+        bail!("{} library contains no exported flags.", codegen_mode);
     }
+
+    Ok(modified_parsed_flags)
 }
 
 #[cfg(test)]
@@ -623,23 +616,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn test_filter_parsed_flags() {
-        let mut input = parse_test_flags_as_input();
-        let parsed_flags = input.try_parse_flags().unwrap();
-
-        let filtered_parsed_flags =
-            filter_parsed_flags(parsed_flags.clone(), CodegenMode::Exported);
-        assert_eq!(3, filtered_parsed_flags.len());
-
-        let filtered_parsed_flags =
-            filter_parsed_flags(parsed_flags.clone(), CodegenMode::Production);
-        assert_eq!(9, filtered_parsed_flags.len());
-
-        let filtered_parsed_flags = filter_parsed_flags(parsed_flags.clone(), CodegenMode::Test);
-        assert_eq!(9, filtered_parsed_flags.len());
-    }
-
     fn parse_test_flags_as_input() -> Input {
         let parsed_flags = crate::test::parse_test_flags();
         let binary_proto = parsed_flags.write_to_bytes().unwrap();
@@ -652,7 +628,8 @@ mod tests {
     fn test_modify_parsed_flags_based_on_mode_prod() {
         let parsed_flags = crate::test::parse_test_flags();
         let p_parsed_flags =
-            modify_parsed_flags_based_on_mode(parsed_flags.clone(), CodegenMode::Production);
+            modify_parsed_flags_based_on_mode(parsed_flags.clone(), CodegenMode::Production)
+                .unwrap();
         assert_eq!(parsed_flags.parsed_flag.len(), p_parsed_flags.len());
         for (i, item) in p_parsed_flags.iter().enumerate() {
             assert!(parsed_flags.parsed_flag[i].eq(item));
@@ -662,7 +639,8 @@ mod tests {
     #[test]
     fn test_modify_parsed_flags_based_on_mode_exported() {
         let parsed_flags = crate::test::parse_test_flags();
-        let p_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, CodegenMode::Exported);
+        let p_parsed_flags =
+            modify_parsed_flags_based_on_mode(parsed_flags, CodegenMode::Exported).unwrap();
         assert_eq!(3, p_parsed_flags.len());
         for flag in p_parsed_flags.iter() {
             assert_eq!(ProtoFlagState::DISABLED, flag.state());
@@ -670,5 +648,14 @@ mod tests {
             assert!(!flag.is_fixed_read_only());
             assert!(flag.is_exported());
         }
+
+        let mut parsed_flags = crate::test::parse_test_flags();
+        parsed_flags.parsed_flag.retain_mut(|pf| !pf.is_exported());
+        let error =
+            modify_parsed_flags_based_on_mode(parsed_flags, CodegenMode::Exported).unwrap_err();
+        assert_eq!(
+            format!("{} library contains no exported flags.", CodegenMode::Exported),
+            format!("{:?}", error)
+        );
     }
 }

tools/aconfig/templates/FakeFeatureFlagsImpl.java.template

@@ -12,23 +12,11 @@ public class FakeFeatureFlagsImpl implements FeatureFlags \{
     }
 
 {{ for item in flag_elements}}
-{{ if library_exported }}
-
-{{ if item.exported }}
     @Override
     @UnsupportedAppUsage
     public boolean {item.method_name}() \{
         return getValue(Flags.FLAG_{item.flag_name_constant_suffix});
     }
-{{ endif }}
-
-{{ else }}
-    @Override
-    @UnsupportedAppUsage
-    public boolean {item.method_name}() \{
-        return getValue(Flags.FLAG_{item.flag_name_constant_suffix});
-    }
-{{ endif }}
 {{ endfor}}
     public void setFlag(String flagName, boolean value) \{
         if (!this.mFlagMap.containsKey(flagName)) \{
@@ -52,20 +40,11 @@ public class FakeFeatureFlagsImpl implements FeatureFlags \{
     }
 
     private Map<String, Boolean> mFlagMap = new HashMap<>(
-        {{ if library_exported }}
-        Map.ofEntries(
-            {{-for item in exported_flag_elements}}
-            Map.entry(Flags.FLAG_{item.flag_name_constant_suffix}, false)
-            {{ -if not @last }},{{ endif }}
-            {{ -endfor }}
-        )
-        {{ else }}
         Map.ofEntries(
             {{-for item in flag_elements}}
             Map.entry(Flags.FLAG_{item.flag_name_constant_suffix}, false)
             {{ -if not @last }},{{ endif }}
             {{ -endfor }}
         )
-        {{ endif }}
     );
 }

tools/aconfig/templates/FeatureFlags.java.template

@@ -6,14 +6,9 @@ import android.compat.annotation.UnsupportedAppUsage;
 public interface FeatureFlags \{
 {{ for item in flag_elements }}
 {{ if library_exported }}
-
-{{ if item.exported }}
     @UnsupportedAppUsage
     boolean {item.method_name}();
-{{ endif }}
-
 {{ else }}
-
 {{ -if not item.is_read_write }}
 {{ -if item.default_value }}
     @com.android.aconfig.annotations.AssumeTrueForR8
@@ -23,7 +18,6 @@ public interface FeatureFlags \{
 {{ endif }}
     @UnsupportedAppUsage
     boolean {item.method_name}();
-
 {{ endif }}
 {{ endfor }}
 }

tools/aconfig/templates/FeatureFlagsImpl.java.template

@@ -15,12 +15,8 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
 
 {{ for flag in flag_elements }}
 {{ if library_exported }}
-{{ if flag.exported }}
     private static boolean {flag.method_name} = false;
-{{ endif }}
-
 {{ else }}
-
 {{- if flag.is_read_write }}
     private static boolean {flag.method_name} = {flag.default_value};
 {{- endif- }}
@@ -34,19 +30,13 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
 
             {{- for flag in namespace_with_flags.flags }}
             {{ if library_exported }}
-
-            {{ if flag.exported }}
             {flag.method_name} =
                 properties.getBoolean("{flag.device_config_flag}", false);
-            {{ endif }}
-
             {{ else }}
-
             {{ if flag.is_read_write }}
             {flag.method_name} =
                 properties.getBoolean("{flag.device_config_flag}", {flag.default_value});
             {{ endif }}
-
             {{ endif }}
             {{ endfor }}
         } catch (NullPointerException e) \{
@@ -65,23 +55,15 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
 {{ endif- }}
 
 {{ for flag in flag_elements }}
-{{ if library_exported }}
-
-{{ if flag.exported }}
     @Override
     @UnsupportedAppUsage
     public boolean {flag.method_name}() \{
+    {{ -if library_exported }}
         if (!{flag.device_config_namespace}_is_cached) \{
             load_overrides_{flag.device_config_namespace}();
         }
         return {flag.method_name};
-    }
+    {{ else }}
-{{ endif }}
-
-{{ else }}
-    @Override
-    @UnsupportedAppUsage
-    public boolean {flag.method_name}() \{
     {{ -if flag.is_read_write }}
         if (!{flag.device_config_namespace}_is_cached) \{
             load_overrides_{flag.device_config_namespace}();
@@ -89,10 +71,9 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
         return {flag.method_name};
     {{ else }}
         return {flag.default_value};
-    {{ endif- }}
+    {{ -endif- }}
+    {{ -endif }}
     }
-{{ endif }}
-
 {{ endfor }}
 }
 {{ else }}

tools/aconfig/templates/Flags.java.template

@@ -6,28 +6,16 @@ import android.compat.annotation.UnsupportedAppUsage;
 /** @hide */
 public final class Flags \{
 {{- for item in flag_elements}}
-    {{ if library_exported }}
-    {{ if item.exported }}
     /** @hide */
     public static final String FLAG_{item.flag_name_constant_suffix} = "{item.device_config_flag}";
-    {{ endif }}
-    {{ else }}
-    /** @hide */
-    public static final String FLAG_{item.flag_name_constant_suffix} = "{item.device_config_flag}";
-    {{ endif }}
 {{- endfor }}
 {{ for item in flag_elements}}
-{{ if library_exported }}
+{{ -if library_exported }}
-
-{{ if item.exported }}
     @UnsupportedAppUsage
     public static boolean {item.method_name}() \{
         return FEATURE_FLAGS.{item.method_name}();
     }
-{{ endif }}
+{{ -else }}
-
-{{ else }}
-
 {{ -if not item.is_read_write }}
 {{ -if item.default_value }}
     @com.android.aconfig.annotations.AssumeTrueForR8