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

Commit c6eda2c0 authored by Martin Geisler's avatar Martin Geisler
Browse files

pdl: Generate snapshots using prettyplease 

The prettyplease library has been explicitly designed to be used for
programmatic formatting of Rust code. It is faster and simpler to use
than calling ‘rustfmt’.

Because ‘prettyplease’ sometimes uses a slightly different formatting
compared to ‘rustfmt’, I’m adding a ‘#![rustfmt::skip]’ like to the
top of every auto-generated file.

Tag: #refactor
Bug: 274187738
Test: atest pdl_tests pdl_rust_generator_tests_{le,be}
Change-Id: Ifd74db6c6bd06b020960d586ca803a26a4603c2d
parent fec3d3ff

tools/pdl/Android.bp

+1 −8
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",

    ],

}

tools/pdl/Cargo.toml

+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"

tools/pdl/src/backends/rust.rs

+2 −2
Original line number Diff line number Diff line
@@ -1012,7 +1012,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 +1095,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),

                    );

                }

            }

tools/pdl/src/backends/rust/preamble.rs

+21 −9
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ 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");

    // TODO(mgeisler): Make the  generated code free from warnings.

    //

    // The code either needs
@@ -34,22 +33,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() {}

    code.push_str(&quote_block! {

        #[doc = #module_doc_string]



        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
@@ -100,11 +112,11 @@ pub fn generate(path: &Path) -> String { 
#[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));

        assert_snapshot_eq("tests/generated/preamble.rs", &format_rust(&actual_code));

    }

}

tools/pdl/src/test_utils.rs

+6 −46
Original line number Diff line number Diff line
@@ -22,54 +22,14 @@ 
use std::fs;

use std::io::Write;

use std::path::Path;

use std::process::{Command, Stdio};

use std::process::Command;

use tempfile::NamedTempFile;



/// Search for a binary in `$PATH` or as a sibling to the current

/// executable (typically the test binary).

pub fn find_binary(name: &str) -> Result<std::path::PathBuf, String> {

    let mut current_exe = std::env::current_exe().unwrap();

    current_exe.pop();

    let paths = std::env::var_os("PATH").unwrap();

    for mut path in std::iter::once(current_exe.clone()).chain(std::env::split_paths(&paths)) {

        path.push(name);

        if path.exists() {

            return Ok(path);

        }

    }



    Err(format!(

        "could not find '{}' in the directory of the binary ({}) or in $PATH ({})",

        name,

        current_exe.to_string_lossy(),

        paths.to_string_lossy(),

    ))

}



/// Run `input` through `rustfmt`.

///

/// # Panics

///

/// Panics if `rustfmt` cannot be found in the same directory as the

/// test executable or if it returns a non-zero exit code.

pub fn rustfmt(input: &str) -> String {

    let rustfmt_path = find_binary("rustfmt").expect("cannot find rustfmt");

    let mut rustfmt = Command::new(&rustfmt_path)

        .stdin(Stdio::piped())

        .stdout(Stdio::piped())

        .spawn()

        .unwrap_or_else(|_| panic!("failed to start {:?}", &rustfmt_path));



    let mut stdin = rustfmt.stdin.take().unwrap();

    // Owned copy which we can move into the writing thread.

    let input = String::from(input);

    std::thread::spawn(move || {

        stdin.write_all(input.as_bytes()).expect("could not write to stdin");

    });



    let output = rustfmt.wait_with_output().expect("error executing rustfmt");

    assert!(output.status.success(), "rustfmt failed: {}", output.status);

    String::from_utf8(output.stdout).expect("rustfmt output was not UTF-8")

/// Format Rust code in `input`.

pub fn format_rust(input: &str) -> String {

    let syntax_tree = syn::parse_file(input).expect("Could not parse {input:#?} as Rust code");

    let formatted = prettyplease::unparse(&syntax_tree);

    format!("#![rustfmt::skip]\n{formatted}")

}



/// Find the unified diff between two strings using `diff`.