Skip to content

shadow_unrelated seems to get confused in nested maps #10780

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

Closed
mpalmer opened this issue May 15, 2023 · 1 comment · Fixed by #13677
Closed

shadow_unrelated seems to get confused in nested maps #10780

mpalmer opened this issue May 15, 2023 · 1 comment · Fixed by #13677
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

@mpalmer
Copy link

mpalmer commented May 15, 2023

Summary

When I use the same variable name inside a closure passed to Option.map as the variable is named, shadow_unrelated fires, which is odd because this is pretty much the definition of "uses the original value".

Lint Name

shadow_unrelated

Reproducer

I tried this code:

#![deny(clippy::shadow_unrelated)]                                                                                                                                                                                                             
                                                                                                                                                                                                                                               
fn main() {                                                                                                                                                                                                                                    
    let a: Vec<Option<u8>> = [100u8, 120, 140]                                                                                                                                                                                                 
        .iter()                                                                                                                                                                                                                                
        .map(|i| i.checked_mul(2))                                                                                                                                                                                                             
        .map(|i| i.map(|i| i - 10))                                                                                                                                                                                                            
        .collect();                                                                                                                                                                                                                            
    println!("{a:?}");                                                                                                                                                                                                                         
}                                                                                                                                                                                                                                              

I saw this happen:

    Checking shadow_test v0.1.0 (/tmp/shadow_test)
error: `i` shadows a previous, unrelated binding
 --> src/main.rs:7:25
  |
7 |         .map(|i| i.map(|i| i - 10))
  |                         ^
  |
note: previous binding is here
 --> src/main.rs:7:15
  |
7 |         .map(|i| i.map(|i| i - 10))
  |               ^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#shadow_unrelated
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![deny(clippy::shadow_unrelated)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^

error: could not compile `shadow_test` due to previous error

I expected to see this happen:

Clippy does not report a problem.

Version

rustc 1.69.0 (84c898d65 2023-04-16)
binary: rustc
commit-hash: 84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc
commit-date: 2023-04-16
host: x86_64-unknown-linux-gnu
release: 1.69.0
LLVM version: 15.0.7

(Same problem occurs with current nightly, `cargo 1.71.0-nightly (13413c64f 2023-05-10)`)

Additional Labels

No response

@mpalmer mpalmer 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 May 15, 2023
@kpreid
Copy link
Contributor

kpreid commented Nov 16, 2023

I was just trying out shadow_unrelated and got a lot of reports for other functions taking closures. Perhaps it would be useful to generally permit, for any function call, f(x, |x| {...}) — a shadowing binding in a closure parameter list that shadows a variable used directly in the enclosing function call arguments.

DJMcNab added a commit to DJMcNab/xilem that referenced this issue Nov 12, 2024
Note, this fix is only excluding the false positives as a result of:
rust-lang/rust-clippy#10780

When/if the fix for that trickles onto stable, we can remove the expects
DJMcNab added a commit to DJMcNab/xilem that referenced this issue Nov 12, 2024
Note, this fix is only excluding the false positives as a result of:
rust-lang/rust-clippy#10780

When/if the fix for that trickles onto stable, we can remove the expects
github-merge-queue bot pushed a commit to linebender/xilem that referenced this issue Nov 12, 2024
Note, this fix is only excluding the false positives as a result of:
rust-lang/rust-clippy#10780

When/if the fix for that
(rust-lang/rust-clippy#13677) trickles onto
stable, we can remove the expects.
github-merge-queue bot pushed a commit that referenced this issue Nov 30, 2024
Fixes #10780

We correctly no longer give a warning when a closure is passed to a
method, where one of the arguments to that method uses the variable
which would be shadowed by an argument to that closure.
Uses is defined loosely as any expression used in the calling expression
mentions the shadowee binding (except for the closure itself):

```rust
#![deny(clippy::shadow_unrelated)]
let x = Some(1);
let y = x.map(|x| x + 1);
```
will now succeed.

See linebender/xilem#745 - without this change,
all of the `expect(shadow_unrelated)` in the repository are met; with
it, none of them are.

changelog: [`shadow_unrelated`]: Don't treat closures arguments as
unrelated when the calling function uses them
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

Successfully merging a pull request may close this issue.

2 participants