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

Commit 4394d275 authored by Martin Geisler's avatar Martin Geisler Committed by Gerrit Code Review
Browse files

Merge changes from topic "pdl-prettyplease"

* changes:
  pdl: Remove rustfmt from no_alloc test
  pdl: Format generated code using prettyplease
  pdl: Generate canonical tests with prettyplease
  pdl: Generate snapshots using prettyplease
parents c8964ff7 418cd123
Loading
Loading
Loading
Loading
+10 −38
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ rust_defaults {
        "libcodespan_reporting",
        "libheck",
        "libpest",
        "libprettyplease",
        "libproc_macro2",
        "libquote",
        "libserde",
@@ -121,15 +122,7 @@ rust_test_host {
        "libpaste",
    ],
    test_suites: ["general-tests"],
    enabled: false, // rustfmt is only available on x86.
    arch: {
        x86_64: {
            enabled: true,
        },
    },
    data: [
        ":rustfmt",
        ":rustfmt.toml",
        ":pdl_generated_files",
    ],
}
@@ -153,13 +146,8 @@ rust_test_host {

genrule_defaults {
    name: "pdl_rust_generator_defaults",
    cmd: "set -o pipefail;" +
        " $(location :pdl) --output-format rust $(in) |" +
        " $(location :rustfmt) > $(out)",
    tools: [
        ":pdl",
        ":rustfmt",
    ],
    cmd: "$(location :pdl) --output-format rust $(in) > $(out)",
    tools: [":pdl"],
    defaults_visibility: [
        "//external/uwb/src",
        "//packages/modules/Bluetooth:__subpackages__",
@@ -252,6 +240,7 @@ rust_binary_host {
    name: "pdl_generate_tests",
    srcs: ["src/bin/generate-canonical-tests.rs"],
    rustlibs: [
        "libprettyplease",
        "libproc_macro2",
        "libquote",
        "libserde",
@@ -261,32 +250,20 @@ rust_binary_host {
    ],
}

genrule_defaults {
    name: "pdl_rust_generator_src_defaults",
    tools: [
        ":pdl_generate_tests",
        ":rustfmt",
    ],
}

genrule {
    name: "pdl_rust_generator_tests_le_src",
    cmd: "set -o pipefail;" +
        " $(location :pdl_generate_tests) $(in) pdl_le_backend |" +
        " $(location :rustfmt) > $(out)",
    cmd: "$(location :pdl_generate_tests) $(in) pdl_le_backend > $(out)",
    srcs: ["tests/canonical/le_test_vectors.json"],
    out: ["le_canonical.rs"],
    defaults: ["pdl_rust_generator_src_defaults"],
    tools: [":pdl_generate_tests"],
}

genrule {
    name: "pdl_rust_generator_tests_be_src",
    cmd: "set -o pipefail;" +
        " $(location :pdl_generate_tests) $(in) pdl_be_backend |" +
        " $(location :rustfmt) > $(out)",
    cmd: "$(location :pdl_generate_tests) $(in) pdl_be_backend > $(out)",
    srcs: ["tests/canonical/be_test_vectors.json"],
    out: ["be_canonical.rs"],
    defaults: ["pdl_rust_generator_src_defaults"],
    tools: [":pdl_generate_tests"],
}

rust_test_host {
@@ -404,13 +381,8 @@ python_test_host {
// Defaults for the rust_noalloc backend
genrule_defaults {
    name: "pdl_rust_noalloc_generator_defaults",
    cmd: "set -o pipefail;" +
        " $(location :pdl) --output-format rust_no_alloc $(in) |" +
        " $(location :rustfmt) > $(out)",
    tools: [
        ":pdl",
        ":rustfmt",
    ],
    cmd: "$(location :pdl) --output-format rust_no_alloc $(in) > $(out)",
    tools: [":pdl"],
}

// Generate the rust_noalloc backend srcs against the little-endian test vectors
+1 −0
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ quote = "1.0.21"
serde_json = "1.0.86"
argh = "0.1.7"
syn = "1.0.102"
prettyplease = "0.1.25"

[dependencies.serde]
version = "1.0.145"
+15 −19
Original line number Diff line number Diff line
@@ -964,24 +964,20 @@ fn generate_decl(
    scope: &lint::Scope<'_>,
    file: &analyzer_ast::File,
    decl: &analyzer_ast::Decl,
) -> String {
) -> proc_macro2::TokenStream {
    match &decl.desc {
        ast::DeclDesc::Packet { id, .. } => {
            generate_packet_decl(scope, file.endianness.value, id).to_string()
        }
        ast::DeclDesc::Packet { id, .. } => generate_packet_decl(scope, file.endianness.value, id),
        ast::DeclDesc::Struct { id, parent_id: None, .. } => {
            // TODO(mgeisler): handle structs with parents. We could
            // generate code for them, but the code is not useful
            // since it would require the caller to unpack everything
            // manually. We either need to change the API, or
            // implement the recursive (de)serialization.
            generate_struct_decl(scope, file.endianness.value, id).to_string()
        }
        ast::DeclDesc::Enum { id, tags, width } => {
            generate_enum_decl(id, tags, *width, false).to_string()
            generate_struct_decl(scope, file.endianness.value, id)
        }
        ast::DeclDesc::Enum { id, tags, width } => generate_enum_decl(id, tags, *width, false),
        ast::DeclDesc::CustomField { id, width: Some(width), .. } => {
            generate_custom_field_decl(id, *width).to_string()
            generate_custom_field_decl(id, *width)
        }
        _ => todo!("unsupported Decl::{:?}", decl),
    }
@@ -992,18 +988,18 @@ fn generate_decl(
/// The code is not formatted, pipe it through `rustfmt` to get
/// readable source code.
pub fn generate(sources: &ast::SourceDatabase, file: &analyzer_ast::File) -> String {
    let mut code = String::new();

    let source = sources.get(file.file).expect("could not read source");
    code.push_str(&preamble::generate(Path::new(source.name())));
    let preamble = preamble::generate(Path::new(source.name()));

    let scope = lint::Scope::new(file);
    for decl in &file.declarations {
        code.push_str(&generate_decl(&scope, file, decl));
        code.push_str("\n\n");
    }
    let decls = file.declarations.iter().map(|decl| generate_decl(&scope, file, decl));
    let code = quote! {
        #preamble

    code
        #(#decls)*
    };
    let syntax_tree = syn::parse2(code).expect("Could not parse code");
    prettyplease::unparse(&syntax_tree)
}

#[cfg(test)]
@@ -1012,7 +1008,7 @@ mod tests {
    use crate::analyzer;
    use crate::ast;
    use crate::parser::parse_inline;
    use crate::test_utils::{assert_snapshot_eq, rustfmt};
    use crate::test_utils::{assert_snapshot_eq, format_rust};
    use paste::paste;

    /// Parse a string fragment as a PDL file.
@@ -1095,7 +1091,7 @@ mod tests {
                    let actual_code = generate(&db, &file);
                    assert_snapshot_eq(
                        &format!("tests/generated/{name}_{endianness}.rs"),
                        &rustfmt(&actual_code),
                        &format_rust(&actual_code),
                    );
                }
            }
+25 −21
Original line number Diff line number Diff line
@@ -12,14 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use quote::quote;
use std::path::Path;

use crate::quote_block;

/// Generate the file preamble.
pub fn generate(path: &Path) -> String {
    let mut code = String::new();
    let filename = path.file_name().unwrap().to_str().expect("non UTF-8 filename");
pub fn generate(path: &Path) -> proc_macro2::TokenStream {
    // TODO(mgeisler): Make the  generated code free from warnings.
    //
    // The code either needs
@@ -34,22 +31,35 @@ pub fn generate(path: &Path) -> String {
    // to the generated code. We cannot add the module-level attribute
    // here because of how the generated code is used with include! in
    // lmp/src/packets.rs.
    code.push_str(&format!("// @generated rust packets from {filename}\n\n"));
    let filename = path.file_name().unwrap().to_str().expect("non UTF-8 filename");
    let module_doc_string = format!(" @generated rust packets from {filename}.");
    // TODO(mgeisler): the doc comment below should be an outer
    // comment (#![doc = ...]). However, people include the generated
    // code in the middle of another module via include_str!:
    //
    // fn before() {}
    // include_str!("generated.rs")
    // fn after() {}
    //
    // It is illegal to have a //! comment in the middle of a file. We
    // should refactor such usages to instead look like this:
    //
    // fn before() {}
    // mod foo { include_str!("generated.rs") }
    // use foo::*;
    // fn after() {}
    quote! {
        #[doc = #module_doc_string]

    code.push_str(&quote_block! {
        use bytes::{Buf, BufMut, Bytes, BytesMut};
        use std::convert::{TryFrom, TryInto};
        use std::cell::Cell;
        use std::fmt;
        use std::sync::Arc;
        use thiserror::Error;
    });

    code.push_str(&quote_block! {
        type Result<T> = std::result::Result<T, Error>;
    });

    code.push_str(&quote_block! {
        /// Private prevents users from creating arbitrary scalar values
        /// in situations where the value needs to be validated.
        /// Users can freely deref the value, but only the backend
@@ -63,9 +73,7 @@ pub fn generate(path: &Path) -> String {
                &self.0
            }
        }
    });

    code.push_str(&quote_block! {
        #[derive(Debug, Error)]
        pub enum Error {
            #[error("Packet parsing failed")]
@@ -85,26 +93,22 @@ pub fn generate(path: &Path) -> String {
            #[error("expected child {expected}, got {actual}")]
            InvalidChildError { expected: &'static str, actual: String },
        }
    });

    code.push_str(&quote_block! {
        pub trait Packet {
            fn to_bytes(self) -> Bytes;
            fn to_vec(self) -> Vec<u8>;
        }
    });

    code
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::test_utils::{assert_snapshot_eq, rustfmt};
    use crate::test_utils::{assert_snapshot_eq, format_rust};

    #[test]
    fn test_generate_preamble() {
        let actual_code = generate(Path::new("some/path/foo.pdl"));
        assert_snapshot_eq("tests/generated/preamble.rs", &rustfmt(&actual_code));
        let actual_code = generate(Path::new("some/path/foo.pdl")).to_string();
        assert_snapshot_eq("tests/generated/preamble.rs", &format_rust(&actual_code));
    }
}
+10 −11
Original line number Diff line number Diff line
@@ -128,17 +128,16 @@ fn generate_unit_tests(input: &str, packet_names: &[&str], module_name: &str) {
    }

    // TODO(mgeisler): make the generated code clean from warnings.
    println!("#![allow(warnings, missing_docs)]");
    println!();
    println!(
        "{}",
        &quote! {
    let code = quote! {
        #![allow(warnings, missing_docs)]

        use #module::Packet;
        use serde_json::json;

        #(#tests)*
        }
    );
    };
    let syntax_tree = syn::parse2::<syn::File>(code).expect("Could not parse {code:#?}");
    println!("{}", prettyplease::unparse(&syntax_tree));
}

fn main() {
Loading