Clippy issue with manual implementation of `Option::map`

help
1

Hello, I have an issue, but I'm not sure if it is caused by clippy or if my code is just poorly written.

I have the following code:

// Let's assume there is an object variable with a state field.

struct State{
   substate: SomeOtherType,
}
struct MyObject{
   state: Option<Rc<RefCell<State>>>,
}

fn example(){
  // Let's assume there is an object variable with a state field.
  let object = get_object();
  let state: &Option<Rc<RefCell<State>>>  = &object.state;
  // I want to try to get a mutable reference to state
  let substate: Option<&mut SubState> = match state {
      Some(state) => Some(&mut state.borrow_mut().substate),
      None => None,
  };

  // Using substate here ...
}

For context, stateneeds to be shared between different parts of my code with complex lifetime requirements, so I need to have an Rc<RefCell<...>> here.

This code works, but I'm getting the clippy warning "manual implementation of Option::map" which is true, I'm using a match over an option.

However, if I decide to apply the linting suggestion, I get the following:

fn example(){
    let object = get_object();
    let state: &Option<Rc<RefCell<State>>>  = &object.state;
    let substate = state.as_ref().map(|o| &mut o.borrow_mut().substate); // Error!
    // ...
}

This is an error because "cannot return value referencing temporary value", which is fair.

How can I get code that works without any linter warnings? What is considered the best practice? I prefer not to silence clippy, he is "right" that I am reimplementing map, but I don't want to create a compiler error.

2

I think it's a false positive of clippy.

the match statement works because of temporary lifetime extension, but you cannot extend the lifetime across Option::map().

however, there is an way to use Option::map(), if you prefer that: you can return a RefMut<'_, SubState> instead of a regular &mut SubState, using the RefMut::map() api, but I think it's even harder to read and understand than a simple match, so I wouldn't recommend it:

    let substate: Option<RefMut<'_, SubState>> = state
        .as_ref()
        .map(|s| RefMut::map(s.borrow_mut(), |s| &mut s.substate));
2 Likes
3

I would just disable the lint for that piece of code, over doing gymnastics to appease it. There's a few false positive issues about similar code already.

1 Like