rust-lang / rust-lang/rust-clippy

`std_instead_of_core` creates an invalid fix on stacked imports

Open
#12,468 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

C-bug I-suggestion-causes-error
Dominant language
Rust
Stars
13.5k
Forks
2.2k
Avg merge
2d 10h
Merged PRs (30d)
32

Description

Summary

Found while fixing this one https://github.com/rust-lang/rust-clippy/pull/12406.

It has existed probably since this lint was created, but became problematic once the clippy-fix was added.

The problem is that check_path will get each path separately, i.e. for:

use std::{fmt::Result, io::Write};

There will be two passes, and to the code, the paths will look like:

use std::fmt::Result;
// And separately
use std::io::Write;

But, the code will deduce that they are from the same import because the std-span is the same on those two passes, and it keeps the last span saved.

// Not problematic, will skip everything after the first io::Write which can't be taken from core
use std::{io::Write, fmt::Result};
// Is problematic, since it will recognize that fmt::Result can be taken from core
use std::{fmt::Result, io::Write};

There's some nuance here, in the case that the path is a part of a stacked import, each path in that import needs to be known to ensure that the fix will be valid, and at least with check_path that's hard to know without looking ahead.

Reproducer

I tried this code:

use std::{fmt::Result, io::Write};

I expected to see this happen:
Warn that std::fmt::Result can be imported from core and either:

  1. Don't fix, since core::io::Write does not exist.
  2. Fix by splitting up the import.

Instead, this happened:
std is replaced by core which doesn't compile.

Version
rustc 1.78.0-nightly (7d3702e47 2024-03-06)
binary: rustc
commit-hash: 7d3702e472b99be0f5de6608dd87af1df8f99428
commit-date: 2024-03-06
host: x86_64-unknown-linux-gnu
release: 1.78.0-nightly
LLVM version: 18.1.0
Additional Labels

No response

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the std_instead_of_core lint's check_path handling and reproduce the stacked import example from the issue. Add or update a regression test showing that the fix does not produce an invalid core::io::Write import; done means the lint either avoids the fix or splits the import safely.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
devtools
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.