Cargo Clippy throws error 'Question mark operator is useless here' - are suggestions okay to implement?

Question

I've taken over a deploy process and part of this runs Cargo Clippy which was working fine up until late last week, when I started getting this error:

error: Question mark operator is useless here

I've read the suggested link https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark and understand that the ? is no longer required, and have had a look at the suggested changes it offers. I just want to know whether it is okay for me to make the changes as suggested, as some seem to remove some code so I'm not sure whether the result will be the same.

Some of the suggested changes seem to be simple and okay: From this

 Ok(dsl::orgs
     .filter(dsl::salesforce_id.eq(salesforce_id))
     .get_result(conn)?)

To this:

dsl::orgs
     .filter(dsl::salesforce_id.eq(salesforce_id))
     .get_result(conn)

So I'm guessing that the above type of change is safe to accept?

Then I have these

1. Here the 'optional' has disappeared in the suggested fix. From:

   Ok(dsl::orgs
       .filter(
           dsl::customer_id
               .eq(new_org.customer_id)
               .and(dsl::name.eq(&new_org.name)),
       )
       .get_result(conn)
       .optional()?)
   

   To

   dsl::orgs
       .filter(
           dsl::customer_id
               .eq(new_org.customer_id)
               .and(dsl::name.eq(&new_org.name)),
   )
   

2. And this one, where the inner join has disappeared in the suggested fix:

   Ok(orgs::dsl::orgs
    .filter(orgs::dsl::customer_id.eq(customer_id))
    .filter(orgs::dsl::kind.eq(org_kind))
    .filter(
        orgs::dsl::kind
            .eq(OrgKind::Production)
            .or(orgs::dsl::name.eq(&org_name)),
    )
    .inner_join(schema::customers::dsl::customers)
    .get_result(conn)?)
   

   to this:

   orgs::dsl::orgs
       .filter(orgs::dsl::customer_id.eq(customer_id))
       .filter(orgs::dsl::kind.eq(org_kind))
       .filter(
           orgs::dsl::kind
               .eq(OrgKind::Production)
   

Are these 2 suggested fixes okay to implement? Could someone please provide some help?

Answer 1

I've taken over a deploy process and part of this runs Cargo Clippy which was working fine up until late last week,

The lint appeared last week because Rust 1.51 and a new accompanying Clippy version was released last Thursday, and it added needless_question_mark.

when I started getting this error:

Your CI should probably not consider all Clippy lints to be errors. Clippy is designed to give warnings about many things that are simply code style improvements, or possible problems; counting them as errors will lead to unnecessary breakage in the future.

So I'm guessing that the above type of change is safe to accept?

Ok(some_expression?) is almost the same as some_expression. The only difference is that the version with ? may implicitly convert the error type of the expression (the E in Result<T, E>) to the error type expected by the function. If the error types are the same and no conversion is required, then you can remove the Ok and ? and get the same results; if the error types are different, the simplified version will not compile.

Here the 'optional' has disappeared in the suggested fix.

This seems like a bug in Clippy or whatever tool is applying the fix, as the resulting code is semantically different and would not even compile. I'd remove the Ok and ? manually in that case, and report the bug.