Enable clippy checks and fix them (#132) * Enable clippy checks and fix them *… - emurgo/cddl-codegen@43650488

View on GitHub

File Changes

m

+1/-6

- package-ecosystem: cargo

  directory: "/"

  schedule:

-

    interval: monthly

-

    timezone: PST8PDT

+

    interval: weekly

  open-pull-requests-limit: 99

-

  ignore:

-

  - dependency-name: cddl

-

    versions:

-

    - 0.8.5

  rebase-strategy: disabled

m

.github/workflows/build.yml

+18/-20

          override: true

          components: rustfmt, clippy

+

      - name: "Format check"

+

        uses: actions-rs/[email protected]

+

        with:

+

          command: fmt

+

          args: --all -- --check

+

      - name: "Linter checks"

+

        uses: actions-rs/[email protected]

+

        with:

+

          command: clippy

+

          args: --locked --workspace --all-features --all-targets -- --deny "clippy::all"

+

      - name: "Run cargo build"

+

        uses: actions-rs/[email protected]

+

        with:

+

          command: build

+

          args: --locked --workspace --all-features --all-targets

+

      - name: "Run cargo test"

        uses: actions-rs/[email protected]

        with:

          command: test

          args: --all-features --all-targets

-

# Uncomment when supported:

-

#      - name: "Format check"

-

#        uses: actions-rs/[email protected]

-

#        with:

-

#          command: fmt

-

#          args: -- --check

-

#      - name: "Linter checks"

-

#        uses: actions-rs/[email protected]

-

#        with:

-

#          command: clippy

-

#          args: --all-features --all-targets -- --deny "clippy::all"

-

#      - name: "Check"

-

#        uses: actions-rs/[email protected]

-

#        with:

-

#          command: check

-

#          args: --all-features --all-targets

-

m

src/cli.rs

+1/-1

    pub package_json: bool,

-

pub static CLI_ARGS: Lazy<Cli> = Lazy::new(|| Cli::parse());

+

pub static CLI_ARGS: Lazy<Cli> = Lazy::new(Cli::parse);

m

src/dep_graph.rs

+4/-7

use cddl::ast::*;

-

pub fn topological_rule_order<'a>(rules: &'a Vec<&'a Rule<'a>>) -> Vec<&'a Rule<'a>> {

+

pub fn topological_rule_order<'a>(rules: &'a [&'a Rule<'a>]) -> Vec<&'a Rule<'a>> {

    let mut adj_list = BTreeMap::new();

    for cddl_rule in rules.iter() {

        let (ident, refs) = find_references(cddl_rule);

        adj_list.insert(ident.ident, (*cddl_rule, refs));

-

    let mut unvisited = adj_list

-

        .iter()

-

        .map(|(k, _v)| *k)

-

        .collect::<BTreeSet<&str>>();

+

    let mut unvisited = adj_list.keys().copied().collect::<BTreeSet<&str>>();

    let mut topo_order = Vec::new();

    let mut processing: BTreeSet<&'a str> = BTreeSet::new();

-

    while let Some(u) = unvisited.iter().next().map(|u| *u) {

+

    while let Some(u) = unvisited.iter().next().copied() {

        dfs_visit(

            &mut topo_order,

            &mut unvisited,

    let (rule, neighbors) = adj_list.get(u).unwrap();

    for v in neighbors.iter() {

        if processing.contains(v.ident) {

-

            eprintln!("Recursive type: '{}' / '{}' - code will possibly need to be edited by hand to use Box/etc", u, v);

+

            eprintln!("Recursive type: '{u}' / '{v}' - code will possibly need to be edited by hand to use Box/etc");

            continue;

        if unvisited.contains(v.ident) {

m

src/generation.rs

+380/-456

m

src/intermediate.rs

+71/-71

+

    #[allow(unused)]

    pub fn has_ident(&self, ident: &RustIdent) -> bool {

-

        let foo: Vec<RustIdent> = self.type_aliases.keys().fold(vec![], |mut acc, alias| {

+

        let idents: Vec<RustIdent> = self.type_aliases.keys().fold(vec![], |mut acc, alias| {

            match alias {

                AliasIdent::Reserved(_) => {}

                AliasIdent::Rust(ident) => acc.push(ident.clone()),

            "{:?}",

            self.plain_groups

                .keys()

-

                .chain(foo.iter())

+

                .chain(idents.iter())

                .chain(self.rust_structs.keys())

                .chain(self.generic_defs.keys())

                .chain(self.generic_instances.keys())

        let mut aliases = BTreeMap::<AliasIdent, (RustType, bool, bool)>::new();

        let mut insert_alias = |name: &str, rust_type: RustType| {

            let ident = AliasIdent::new(CDDLIdent::new(name));

-

            aliases.insert(ident.clone(), (rust_type, false, false));

+

            aliases.insert(ident, (rust_type, false, false));

};

        insert_alias("uint", ConceptualRustType::Primitive(Primitive::U64).into());

        insert_alias("nint", ConceptualRustType::Primitive(Primitive::N64).into());

                        None

                    } else {

                        // we auto-include only the parts of the cddl prelude necessary (and supported)

-

                        cddl_prelude(reserved).expect(&format!(

-

                            "Reserved ident {} not a part of cddl_prelude?",

-

                            reserved

-

));

+

                        cddl_prelude(reserved).unwrap_or_else(|| {

+

                            panic!(

+

                                "{}",

+

                                "Reserved ident {reserved} not a part of cddl_prelude?"

+

});

                        self.emit_prelude(reserved.clone());

                        Some((

                            ConceptualRustType::Rust(RustIdent::new(CDDLIdent::new(format!(

-

                                "prelude_{}",

-

                                reserved

+

                                "prelude_{reserved}"

                            ))))

                            .into(),

                            true,

                used_as_key.insert(ident.clone());

-

        fn check_used_as_key<'a>(

+

        fn check_used_as_key(

            ty: &ConceptualRustType,

-

            types: &IntermediateTypes<'a>,

+

            types: &IntermediateTypes<'_>,

            used_as_key: &mut BTreeSet<RustIdent>,

) {

            if let ConceptualRustType::Map(k, _v) = ty {

    pub fn is_referenced(&self, ident: &RustIdent) -> bool {

        let mut found = false;

-

        self.visit_types(&mut |ty| match ty {

-

            ConceptualRustType::Rust(id) => {

+

        self.visit_types(&mut |ty| {

+

            if let ConceptualRustType::Rust(id) = ty {

                if id == ident {

                    found = true

-

            _ => (),

});

        found

) {

        if let Some(plain_group) = self.plain_groups.get(ident) {

            // the clone is to get around the borrow checker

-

            if let Some(group) = plain_group.as_ref().map(|g| g.clone()) {

+

            if let Some(group) = plain_group.as_ref().cloned() {

                // we are defined via .cddl and thus need to register a concrete

                // representation of the plain group

                if let Some(rust_struct) = self.rust_structs.get(ident) {

        if !self.type_aliases.is_empty() {

            println!("\n\nAliases:");

            for (alias_name, alias_type) in self.type_aliases.iter() {

-

                println!("{:?} -> {:?}", alias_name, alias_type);

+

                println!("{alias_name:?} -> {alias_type:?}");

        if !self.generic_defs.is_empty() {

            println!("\n\nGeneric Definitions:");

            for (ident, def) in self.generic_defs.iter() {

-

                println!("{} -> {:?}", ident, def);

+

                println!("{ident} -> {def:?}");

        if !self.generic_instances.is_empty() {

            println!("\n\nGeneric Instances:");

            for (ident, def) in self.generic_instances.iter() {

-

                println!("{} -> {:?}", ident, def);

+

                println!("{ident} -> {def:?}");

        if !self.rust_structs.is_empty() {

            println!("\n\nRustStructs:");

            for (ident, rust_struct) in self.rust_structs.iter() {

-

                println!("{} -> {:?}\n", ident, rust_struct);

+

                println!("{ident} -> {rust_struct:?}\n");

                true => "True",

                false => "False",

}),

-

            FixedValue::Nint(i) => VariantIdent::new_custom(format!("U{}", i)),

-

            FixedValue::Uint(u) => VariantIdent::new_custom(format!("I{}", u)),

-

            FixedValue::Float(f) => VariantIdent::new_custom(format!("F{}", f)),

+

            FixedValue::Nint(i) => VariantIdent::new_custom(format!("U{i}")),

+

            FixedValue::Uint(u) => VariantIdent::new_custom(format!("I{u}")),

+

            FixedValue::Float(f) => VariantIdent::new_custom(format!("F{f}")),

            FixedValue::Text(s) => {

-

                VariantIdent::new_custom(convert_to_alphanumeric(&convert_to_camel_case(&s)))

+

                VariantIdent::new_custom(convert_to_alphanumeric(&convert_to_camel_case(s)))

            FixedValue::Nint(i) => i.to_string(),

            FixedValue::Uint(u) => u.to_string(),

            FixedValue::Float(f) => f.to_string(),

-

            FixedValue::Text(s) => format!("\"{}\".to_owned()", s),

+

            FixedValue::Text(s) => format!("\"{s}\".to_owned()"),

    /// Converts a literal to a valid rust comparison valid for comparisons

    /// e.g. Text can be &str to avoid creating a String

    pub fn to_primitive_str_compare(&self) -> String {

        match self {

-

            FixedValue::Text(s) => format!("\"{}\"", s),

+

            FixedValue::Text(s) => format!("\"{s}\""),

            _ => self.to_primitive_str_assign(),

    Bytes,

-

// TODO: impl display or fmt or whatever rust uses

-

impl Primitive {

-

    pub fn to_string(&self) -> String {

+

impl ToString for Primitive {

+

    fn to_string(&self) -> String {

        String::from(match self {

            Primitive::Bool => "bool",

            Primitive::F32 => "f32",

            Primitive::Bytes => "Vec<u8>",

})

-

+

// TODO: impl display or fmt or whatever rust uses

+

impl Primitive {

    pub fn to_variant(&self) -> VariantIdent {

        VariantIdent::new_custom(match self {

            Primitive::Bool => "Bool",

mod idents {

-

    use crate::{

-

        cli::CLI_ARGS,

-

        rust_reserved::STD_TYPES,

-

        utils::{is_identifier_in_our_prelude, is_identifier_reserved},

-

};

+

    use crate::{rust_reserved::STD_TYPES, utils::is_identifier_reserved};

    // to resolve ambiguities between raw (from CDDL) and already-formatted

    // for things like type aliases, etc, we use these wrapper structs

    impl std::fmt::Display for VariantIdent {

        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {

            match self {

-

                VariantIdent::Custom(name) => write!(f, "{}", name),

+

                VariantIdent::Custom(name) => write!(f, "{name}"),

                VariantIdent::RustStruct(ident) => ident.fmt(f),

...

m

src/main.rs

+3/-3

        .iter()

        .filter(|cddl_rule| {

            // We inserted string constants with specific prefixes earlier to mark scope

-

            if let Some(new_scope) = rule_is_scope_marker(*cddl_rule) {

-

                println!("Switching from scope '{}' to '{}'", scope, new_scope);

+

            if let Some(new_scope) = rule_is_scope_marker(cddl_rule) {

+

                println!("Switching from scope '{scope}' to '{new_scope}'");

                scope = new_scope;

                false

            } else {

    // Creating intermediate form from the CDDL

    for cddl_rule in dep_graph::topological_rule_order(&cddl_rules) {

-

        println!("\n\n------------------------------------------\n- Handling rule: {}\n------------------------------------", scope);

+

        println!("\n\n------------------------------------------\n- Handling rule: {scope}\n------------------------------------");

        parse_rule(&mut types, &pv, cddl_rule);

    types.finalize(&pv);

m

src/parsing.rs

+30/-40

};

#[derive(Clone, Debug)]

+

#[allow(clippy::upper_case_acronyms)]

enum ControlOperator {

    Range((Option<i128>, Option<i128>)),

    CBOR(RustType),

    Default(FixedValue),

-

pub const SCOPE_MARKER: &'static str = "_CDDL_CODEGEN_SCOPE_MARKER_";

-

pub const EXTERN_MARKER: &'static str = "_CDDL_CODEGEN_EXTERN_TYPE_";

+

pub const SCOPE_MARKER: &str = "_CDDL_CODEGEN_SCOPE_MARKER_";

+

pub const EXTERN_MARKER: &str = "_CDDL_CODEGEN_EXTERN_TYPE_";

/// Some means it is a scope marker, containing the scope

pub fn rule_is_scope_marker(cddl_rule: &cddl::ast::Rule) -> Option<String> {

                                        RustStruct::new_wrapper(

                                            type_name.clone(),

                                            outer_tag,

-

                                            field_type().clone().into(),

+

                                            field_type().into(),

                                            Some(min_max),

),

),

);

        Type2::TaggedData { tag, t, .. } => {

-

            if let Some(_) = outer_tag {

+

            if outer_tag.is_some() {

                panic!("doubly nested tags are not supported");

            let tag_unwrap = tag.expect("not sure what empty tag here would mean - unsupported");

pub fn create_variants_from_type_choices(

    types: &mut IntermediateTypes,

    parent_visitor: &ParentVisitor,

-

    type_choices: &Vec<TypeChoice>,

+

    type_choices: &[TypeChoice],

) -> Vec<EnumVariant> {

    let mut variant_names_used = BTreeMap::<String, u32>::new();

    type_choices

                        Some(MemberKey::Type1 { t1, .. }) => {

                            // TODO: Do we need to handle cuts for what we're doing?

                            // Does the range control operator matter?

-

                            return Some((&t1, &ge.entry_type));

+

                            return Some((t1, &ge.entry_type));

                        // has a fixed value - this is just a 1-element struct

                        Some(MemberKey::Value { .. }) => return None,

                            GroupEntry::ValueMemberKey { ge, .. } => {

                                Some(format!("{}s", type_to_field_name(&ge.entry_type)?))

-

                            GroupEntry::TypeGroupname { ge, .. } => {

-

                                Some(format!("{}s", ge.name.to_string()))

-

+

                            GroupEntry::TypeGroupname { ge, .. } => Some(format!("{}s", ge.name)),

                            GroupEntry::InlineGroup { .. } => None,

-

fn _field_name_from_comments<'a>(comments: &Option<Comments<'a>>) -> Option<String> {

+

fn _field_name_from_comments(comments: &Option<Comments<'_>>) -> Option<String> {

    comments

        .as_ref()?

.0

                        RuleMetadata {

                            name: Some(name), ..

                        } => name,

-

                        _ => format!("key_{}", value),

+

                        _ => format!("key_{value}"),

                MemberKey::Bareword { ident, .. } => ident.to_string(),

                MemberKey::Type1 { t1, .. } => match t1.type2 {

-

                    Type2::UintValue { value, .. } => format!("key_{}", value),

+

                    Type2::UintValue { value, .. } => format!("key_{value}"),

                    _ => panic!(

                        "Encountered Type1 member key in multi-field map - not supported: {:?}",

                        entry

                    RuleMetadata {

                        name: Some(name), ..

                    } => name,

-

                    _ => format!("index_{}", index),

+

                    _ => format!("index_{index}"),

}),

},

                    RuleMetadata {

                        name: Some(name), ..

                    } => name,

-

                    _ => format!("index_{}", index),

+

                    _ => format!("index_{index}"),

            false => name.to_string(),

            group, group

),

});

-

    append_number_if_duplicate(already_generated, field_name.clone())

+

    append_number_if_duplicate(already_generated, field_name)

// Only returns Some(String) if there was an explicit field name provided, otherwise None.

                        .map(|t| t.for_variant().to_string())

                        .collect::<Vec<String>>()

                        .join("_");

-

                    let instance_cddl_ident =

-

                        CDDLIdent::new(format!("{}_{}", cddl_ident, args_name));

+

                    let instance_cddl_ident = CDDLIdent::new(format!("{cddl_ident}_{args_name}"));

                    let instance_ident = RustIdent::new(instance_cddl_ident.clone());

                    let generic_ident = RustIdent::new(cddl_ident);

                    types.register_generic_instance(GenericInstance::new(

                                rust_type(types, parent_visitor, &ge.entry_type)

                            GroupEntry::TypeGroupname { ge, .. } => {

-

                                types.new_type(&CDDLIdent::new(&ge.name.to_string()))

+

                                types.new_type(&CDDLIdent::new(ge.name.to_string()))

                            _ => panic!("UNSUPPORTED_ARRAY_ELEMENT<{:?}>", entry),

};

    occur

        .as_ref()

-

        .map(|o| match o.occur {

-

            Occur::Optional { .. } => true,

-

            _ => false,

-

})

+

        .map(|o| matches!(o.occur, Occur::Optional { .. }))

        .unwrap_or(false)

    parent_visitor: &ParentVisitor,

    entry: &GroupEntry,

) -> RustType {

-

    let ret = match entry {

+

    match entry {

        GroupEntry::ValueMemberKey { ge, .. } => rust_type(types, parent_visitor, &ge.entry_type),

        GroupEntry::TypeGroupname { ge, .. } => {

            if ge.generic_args.is_some() {

            types.new_type(&cddl_ident)

        GroupEntry::InlineGroup { .. } => panic!("inline group entries are not implemented"),

-

};

-

    //println!("group_entry_to_typename({:?}) = {:?}\n", entry, ret);

-

ret

+

fn group_entry_to_key(entry: &GroupEntry) -> Option<FixedValue> {

    RustRecord { rep, fields }

-

fn parse_group_choice<'a>(

+

fn parse_group_choice(

    types: &mut IntermediateTypes,

    parent_visitor: &ParentVisitor,

-

    group_choice: &'a GroupChoice,

+

    group_choice: &GroupChoice,

    name: &RustIdent,

    rep: Representation,

    tag: Option<usize>,

                } else {

                    let rule_metadata =

                        RuleMetadata::from(group_choice.comments_before_grpchoice.as_ref());

-

                    let ident_name = rule_metadata

-

                        .name

-

                        .unwrap_or_else(|| format!("{}{}", name, i));

+

                    let ident_name = rule_metadata.name.unwrap_or_else(|| format!("{name}{i}"));

                    // General case, GroupN type identifiers and generate group choice since it's inlined here

                    let variant_name = RustIdent::new(CDDLIdent::new(ident_name));

                    types.mark_plain_group(variant_name.clone(), None);

        CDDLType::Value(_) => None,

        CDDLType::ValueMemberKeyEntry(_) => None,

        CDDLType::TypeGroupnameEntry(_) => None,

-

        CDDLType::MemberKey(t) => match t {

-

            MemberKey::NonMemberKey {

-

                comments_after_type_or_group,

-

..

-

            } => comments_after_type_or_group.clone(),

...

m

src/test.rs

+11/-11

) {

    use std::str::FromStr;

    let export_path = match export_suffix {

-

        Some(suffix) => format!("export_{}", suffix),

+

        Some(suffix) => format!("export_{suffix}"),

        None => "export".to_owned(),

};

    let test_path = std::path::PathBuf::from_str("tests").unwrap().join(dir);

-

    println!("--------- running test: {} ---------", dir);

+

    println!("--------- running test: {dir} ---------");

    // build and run to generate code

    let mut cargo_run = std::process::Command::new("cargo");

    cargo_run

    let mut lib_rs = std::fs::OpenOptions::new()

        .write(true)

        .append(true)

-

        .open(test_path.join(format!("{}/core/src/lib.rs", export_path)))

+

        .open(test_path.join(format!("{export_path}/core/src/lib.rs")))

        .unwrap();

    // copy external file in too (if needed) too

    if let Some(external_core_file_path) = external_core_file_path {

        let extern_rs = std::fs::read_to_string(external_core_file_path).unwrap();

-

        lib_rs.write("\n\n".as_bytes()).unwrap();

+

        lib_rs.write_all("\n\n".as_bytes()).unwrap();

        lib_rs.write_all(extern_rs.as_bytes()).unwrap();

    let deser_test_rs = std::fs::read_to_string(

            .join("deser_test"),

    .unwrap();

-

    lib_rs.write("\n\n".as_bytes()).unwrap();

+

    lib_rs.write_all("\n\n".as_bytes()).unwrap();

    lib_rs.write_all(deser_test_rs.as_bytes()).unwrap();

    let test_rs = std::fs::read_to_string(test_path.join("tests.rs")).unwrap();

-

    lib_rs.write("\n\n".as_bytes()).unwrap();

+

    lib_rs.write_all("\n\n".as_bytes()).unwrap();

    lib_rs.write_all(test_rs.as_bytes()).unwrap();

    std::mem::drop(lib_rs);

    // run tests in generated code

    println!("   ------ testing ------");

    let cargo_test = std::process::Command::new("cargo")

        .arg("test")

-

        .current_dir(test_path.join(format!("{}/core", export_path)))

+

        .current_dir(test_path.join(format!("{export_path}/core")))

        .output()

        .unwrap();

    if !cargo_test.status.success() {

    assert!(cargo_test.status.success());

    // wasm

-

    let wasm_export_dir = test_path.join(format!("{}/wasm", export_path));

+

    let wasm_export_dir = test_path.join(format!("{export_path}/wasm"));

    let wasm_test_dir = test_path.join("tests_wasm.rs");

    // copy external wasm defs if they exist

    if let Some(external_wasm_file_path) = external_wasm_file_path {

-

        println!("trying to open: {:?}", external_wasm_file_path);

+

        println!("trying to open: {external_wasm_file_path:?}");

        let mut wasm_lib_rs = std::fs::OpenOptions::new()

            .write(true)

            .append(true)

-

            .open(test_path.join(format!("{}/wasm/src/lib.rs", export_path)))

+

            .open(test_path.join(format!("{export_path}/wasm/src/lib.rs")))

            .unwrap();

        let extern_rs = std::fs::read_to_string(external_wasm_file_path).unwrap();

-

        wasm_lib_rs.write("\n\n".as_bytes()).unwrap();

+

        wasm_lib_rs.write_all("\n\n".as_bytes()).unwrap();

        wasm_lib_rs.write_all(extern_rs.as_bytes()).unwrap();

    if wasm_test_dir.exists() {

m

src/utils.rs

+4/-6

use cbor_event::Type as CBORType;

use std::collections::BTreeMap;

-

use crate::cli::CLI_ARGS;

-

pub fn _cbor_type_code_str(cbor_type: CBORType) -> &'static str {

    match cbor_type {

        CBORType::UnsignedInteger => "CBORType::UnsignedInteger",

// as we also support our own identifiers for selecting integer precision, we need this too

#[rustfmt::skip]

+

#[allow(unused)]

pub fn is_identifier_in_our_prelude(name: &str) -> bool {

-

    match name {

+

    matches!(name,

        "u8" |

        "i8" |

        "u16" |

        "f32" |

        "u64" |

        "i64" |

-

        "f64" => true,

-

        _ => false,

-

+

        "f64"

+

pub fn is_identifier_user_defined(name: &str) -> bool {

3,441 commits this week Mar 18, 2023 - Mar 25, 2023

Showing 25 of 3441 recent commits.

Yesterday's Report

New Repo Discovered

New Releases