Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clippy::question_mark suggests code that is semantically different to the original #7924

Open
louismrose opened this issue Nov 3, 2021 · 4 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@louismrose
Copy link

Lint name:
clippy::question_mark

I tried this code:

fn multiply(
    first_number_str: &str,
    second_number_str: &str,
) -> std::result::Result<i32, ParseIntError> {
    let first_number = first_number_str.parse::<i32>()?;
    let second_number = second_number_str.parse::<i32>()?;

    Ok(first_number * second_number)
}

if multiply("4", "5").is_err() {
  return Ok(());
}

I expected to see this happen: no recommendation

Instead, this happened:

65 | /             if multiply("4", "5").is_err() {
66 | |                 return Ok(());
67 | |             }
   | |_____________^ help: replace it with: `multiply("4", "5")?;`

The suggested replacement is not semantically equivalent to the original.

Meta

Rust version (rustc -Vv):

rustc 1.58.0-nightly (18bc4bee9 2021-11-02)
binary: rustc
commit-hash: 18bc4bee9710b181b440a472635150f0d6257713
commit-date: 2021-11-02
host: x86_64-apple-darwin
release: 1.58.0-nightly
LLVM version: 13.0.0
@louismrose louismrose added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Nov 3, 2021
@klensy
Copy link
Contributor

klensy commented Nov 3, 2021

Can you provide isolated, self working example? Code that you posted won't compile without additional work (that can affect clippy work).

@xFrednet
Copy link
Member

xFrednet commented Nov 3, 2021

Here is a self-contained example:

fn multiply(
    first_number: i32,
    second_number: i32,
) -> std::result::Result<i32, ()> {
    Ok(first_number * second_number)
}

fn test() -> std::result::Result<(), ()> {
    if multiply(4, 5).is_err() {
        return Ok(());
    }
    
    println!("Some other code");
    
    Err(())
}

fn main() {
    let _test = test();
}

Playground. If you want to work on this, you can comment @rustbot claim 🙃

@louismrose
Copy link
Author

This version does not exhibit the issue:

rustc 1.58.0-nightly (547a6ffee 2021-10-21)
binary: rustc
commit-hash: 547a6ffee0cf4da9929a9e3d49546dc87d607735
commit-date: 2021-10-21
host: x86_64-apple-darwin
release: 1.58.0-nightly
LLVM version: 13.0.0

This version does exhibit the issue:

rustc 1.58.0-nightly (514b38779 2021-10-22)
binary: rustc
commit-hash: 514b3877956dc594823106b66c164f8cdbc8b3da
commit-date: 2021-10-22
host: x86_64-apple-darwin
release: 1.58.0-nightly
LLVM version: 13.0.0

@dswij
Copy link
Member

dswij commented Jan 21, 2022

This is fixed by either #7860 or #8080, not an issue anymore on nightly: (playground)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

4 participants