skip(..).next() lint incorrectly suggest using nth() in some cases

[tslater2006]

Summary

Clippy incorrectly reports that I should switch to using nth(..) instead of the skip(..).next() but does not account for the fact that my variable is not mutable, and thus cannot use nth(..). I found this while attempting a cargo clippy --fix and it applied this fix and caused compile error and prevented any further clippy fixes from being applied.

Reproducer

I tried this code:

fn main() {
    let test_string = "1|1 2";
    let sp = test_string.split('|').map(|s| s.trim());
    let out: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect();

    println!("{:?}", out);
}

I expected to see this happen:

Either for clippy to realize I cannot use nth() as suggested because the variable is not mutable, or for it to change that variable to mutable as part of the fix (I could see how this could be complicated/bad).

I suppose changing the help to mention that nth() would be available if the variable was mutable and that the the user should consider changing it.

Instead, this happened:

Clippy reports that I should be using nth() instead of skip(1).next() but without other changes in my code this is not an allowable fix.

Version

rustc 1.56.1 (59eed8a2a 2021-11-01)
binary: rustc
commit-hash: 59eed8a2aac0230a8b53e89d4e99d55912ba6b35
commit-date: 2021-11-01
host: x86_64-pc-windows-msvc
release: 1.56.1
LLVM version: 13.0.0

Additional Labels

• l-suggestion-causes-error

[xFrednet]

Thank you for the report. I think a good solution would be to add a note that the iterator has to be set to mutable and use Unspecified as the Applicability if the iterator is immutable.

[surechen]

Hi. I'm interested in this issue.
@rustbot claim

[surechen]

Thank you for the report. I think a good solution would be to add a note that the iterator has to be set to mutable and use Unspecified as the Applicability if the iterator is immutable.

Hi, I have a problem, could you help me?

for example:

fn main() {
    let test_string = "1|1 2";
    let sp = test_string.split('|').map(|s| s.trim());  // 1
    let out: Vec<&str> = sp.skip(1).next().unwrap().split('').collect(); // 2
}

The way I can think of is to find stmt 1, then according to stmt -> local -> pat->binding to see if it is mut.
But I don't know how to find stmt 1 according to the hir_id of sp in 2. Is it feasible to find the block of main through parent_iter?
@xFrednet

[xFrednet]

Hey @surechen, you can get the parent block by using parent_iter. clippy_utils also has a function for this. However, I think it would be easier to call path_to_local on the self argument of skip. Then you get the hir_id of the pat which you can then use to retrieve the pattern. With that, you should be able to check the binding type 🙃

[surechen]

Hey @surechen, you can get the parent block by using parent_iter. clippy_utils also has a function for this. However, I think it would be easier to call path_to_local on the self argument of skip. Then you get the hir_id of the pat which you can then use to retrieve the pattern. With that, you should be able to check the binding type 🙃

Thank you very much.