Search code examples
rusttypesrust-clippy

Using a type definition to simplify a function signature


Clippy is giving the following warning:

warning: very complex type used. Consider factoring parts into `type` definitions
   --> src/regexp/mod.rs:845:56
    |
845 |     pub fn get_by_name<'b>(&'b self, name: &'b str) -> Vec<(&'b String, (usize, usize), (usize, usize))> {
    |                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
    = note: `#[warn(clippy::type_complexity)]` on by default

I agree with the warning. When I typed this code I didn't like it, but I can't seem to figure out where and how to put the type statement. I've looked in the book, followed the reference link clippy gave, and googled, and can't seem to find references to using type to simplify function signatures, or how to deal with the lifetime on the String parameter. Defining a new struct to hold the return value would work, but there isn't really any reason for such a structure besides simplifying the function definition.


Here's the code. You can see I fixed it by using the existing Report struct - it has a few extra fields that weren't in my design for the return, but the struct already exist, and it actually may be better to make the extra fields available. But thanks for the answer, it is something I should know whether or not it is used here.

pub struct Report {
    /// The string found matching the RE pattern
    pub found: String,
    /// The position in chars of the string. This cannot be used for slices, except for ASCII chars. To get a slice use **pos**
    pub pos: (usize, usize),
    /// The position in bytes of the string: that is, found[pos.0..pos.1] is a valid unicode substring containing the match
    pub bytes: (usize, usize),
    /// The name of the field: if None then the field should not be included in the Report tree, if Some("") it is included but
    /// unnamed, otherwise it is recorded with the given name
    pub name: Option<String>,
    /// Array of child Report structs, only non-empty for And and Or nodes. OrNodes will have only a single child node, AndNodes can have many.
    pub subreports: Vec<Report>,
}

impl Report {
    /// Constructor: creates a new report from a successful Path
    pub fn new(root: &crate::regexp::walk::Path, char_start: usize, byte_start: usize) -> Report {
        let (reports, _char_end)  = root.gather_reports(char_start, byte_start);
        reports[0].clone()
    }
    
    /// Pretty-prints a report with indentation to help make it easier to read
    pub fn display(&self, indent: isize) {
        let name_str = { if let Some(name) = &self.name { format!("<{}>", name) } else { "".to_string() }};
        println!("{}\"{}\" char position [{}, {}] byte position [{}, {}] {}",
                 pad(indent), self.found, self.pos.0, self.pos.1, self.bytes.0, self.bytes.1, name_str);
        self.subreports.iter().for_each(move |r| r.display(indent + 1));
    }

    /// Gets **Report** nodes representing matches for named Nodes. The return is a *Vec* because named matches can occur multiple
    /// times - for example, _\?\<name\>abc\)*_
    pub fn get_by_name<'b>(&'b self, name: &'b str) -> Vec<&Report> {
        let mut v = Vec::<&Report>::new();
        if let Some(n) = &self.name {
            if n == name { v.push(self); }
        }
        for r in &self.subreports {
            let mut x = r.get_by_name(name);
            v.append(&mut x);
        }
        v
    }

    /// Gets a hash of  **Report** nodes grouped by name. This just sets things up and calls **get_named_internal()** to do the work
    pub fn get_named(& self) -> HashMap<&str, Vec<&Report>> {
        let hash = HashMap::new();
        self.get_named_internal(hash)
    }

    /// internal function that does the work for **get_named()**
    fn get_named_internal<'b>(&'b self, mut hash: HashMap<&'b str, Vec<&'b Report>>) -> HashMap<&'b str, Vec<&Report>> {
        if let Some(name) = &self.name {
            if let Some(mut_v) = hash.get_mut(&name.as_str()) {
                mut_v.push(self);
            } else {
                hash.insert(name.as_str(), vec![self]);
            }
            for r in self.subreports.iter() {
                hash = r.get_named_internal(hash);
            }
        }
        hash
    }
}


Solution

  • There's a few ways you could simplify this. One would be to just alias the whole type (choosing a suitably-descriptive name instead of Foo):

    type Foo<'a> = Vec<(&'a String, (usize, usize), (usize, usize))>;
    

    Then your return type is Foo<'b>.

    If this tuple type shows up a lot, you could factor that out instead:

    type Foo<'a> = (&'a String, (usize, usize), (usize, usize));
    

    And then you would return Vec<Foo<'b>>.

    It's a bit hard to suggest alternatives without seeing the rest of your code.

    I can't seem to figure out where and how to put the type statement

    You can put type declarations anywhere you can put other type declarations, like struct, enum, etc.