Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 206a382d authored by Mårten Kongstad's avatar Mårten Kongstad
Browse files

aconfig: improve duplicate flags error message: include paths 

Improve the error message returned when `aconfig dump` is fed multiple
declarations of the same flag: include the paths to the declaration
files.

In general all error messages from the protos::*::verify_* functions
should include paths to the offending files. This will be handled in a
follow-up CL.

Bug: 290300657
Test: atest aconfig.test
Test: manual: add duplicate flag and run `m all_aconfig_declarations`, inspect error message
Change-Id: I46dc23f7128dd5c68ced9f2e8518cfa89d81c2df
parent 82d233a1

tools/aconfig/src/protos.rs

+45 −3
Original line number Diff line number Diff line
@@ -200,6 +200,11 @@ pub mod parsed_flag { 


        Ok(())

    }



    pub fn path_to_declaration(pf: &ProtoParsedFlag) -> &str {

        debug_assert!(!pf.trace.is_empty());

        pf.trace[0].source()

    }

}



pub mod parsed_flags {
@@ -214,6 +219,8 @@ pub mod parsed_flags { 
    }



    pub fn verify_fields(pf: &ProtoParsedFlags) -> Result<()> {

        use crate::protos::parsed_flag::path_to_declaration;



        let mut previous: Option<&ProtoParsedFlag> = None;

        for parsed_flag in pf.parsed_flag.iter() {

            if let Some(prev) = previous {
@@ -221,7 +228,12 @@ pub mod parsed_flags { 
                let b = create_sorting_key(parsed_flag);

                match a.cmp(&b) {

                    Ordering::Less => {}

                    Ordering::Equal => bail!("bad parsed flags: duplicate flag {}", a),

                    Ordering::Equal => bail!(

                        "bad parsed flags: duplicate flag {} (defined in {} and {})",

                        a,

                        path_to_declaration(prev),

                        path_to_declaration(parsed_flag)

                    ),

                    Ordering::Greater => {

                        bail!("bad parsed flags: not sorted: {} comes before {}", a, b)

                    }
@@ -646,7 +658,37 @@ parsed_flag { 
}

"#;

        let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();

        assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.foo.bar");

        assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.foo.bar (defined in flags.declarations and flags.declarations)");

    }



    #[test]

    fn test_parsed_flag_path_to_declaration() {

        let text_proto = r#"

parsed_flag {

    package: "com.foo"

    name: "bar"

    namespace: "first_ns"

    description: "This is the description of the first flag."

    state: DISABLED

    permission: READ_ONLY

    trace {

        source: "flags.declarations"

        state: DISABLED

        permission: READ_ONLY

    }

    trace {

        source: "flags.values"

        state: ENABLED

        permission: READ_ONLY

    }

}

"#;

        let parsed_flags = try_from_binary_proto_from_text_proto(text_proto).unwrap();

        let parsed_flag = &parsed_flags.parsed_flag[0];

        assert_eq!(

            crate::protos::parsed_flag::path_to_declaration(parsed_flag),

            "flags.declarations"

        );

    }



    #[test]
@@ -717,7 +759,7 @@ parsed_flag { 


        // bad cases

        let error = parsed_flags::merge(vec![first.clone(), first.clone()]).unwrap_err();

        assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.first.first");

        assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.first.first (defined in flags.declarations and flags.declarations)");



        // valid cases

        assert!(parsed_flags::merge(vec![]).unwrap().parsed_flag.is_empty());