Skip to content

Commit

Permalink
Auto merge of rust-lang#10566 - ebobrow:iss10549, r=giraffate
Browse files Browse the repository at this point in the history
fix `single_component_path_imports` FP on `self::<import>::..`

fixes rust-lang#10549

I noticed that a couple functions in the file I was working on took `cx` as a parameter but didn't use them, so I removed that. Can revert if desired because it isn't related to my changes.

changelog: [`single_component_path_imports`] don't suggest removing import when it is used as `self::<import>::..`
  • Loading branch information
bors committed Apr 7, 2023
2 parents de5c6d6 + a216553 commit 9408d01
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 15 deletions.
59 changes: 46 additions & 13 deletions clippy_lints/src/single_component_path_imports.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
use rustc_ast::node_id::{NodeId, NodeMap};
use rustc_ast::{ptr::P, Crate, Item, ItemKind, MacroDef, ModKind, UseTreeKind};
use rustc_ast::visit::{walk_expr, Visitor};
use rustc_ast::{ptr::P, Crate, Expr, ExprKind, Item, ItemKind, MacroDef, ModKind, Ty, TyKind, UseTreeKind};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_session::{declare_tool_lint, impl_lint_pass};
Expand Down Expand Up @@ -55,7 +56,7 @@ impl EarlyLintPass for SingleComponentPathImports {
return;
}

self.check_mod(cx, &krate.items);
self.check_mod(&krate.items);
}

fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
Expand Down Expand Up @@ -84,8 +85,43 @@ impl EarlyLintPass for SingleComponentPathImports {
}
}

#[derive(Default)]
struct ImportUsageVisitor {
// keep track of imports reused with `self` keyword, such as `self::std` in the example below.
// Removing the `use std;` would make this a compile error (#10549)
// ```
// use std;
//
// fn main() {
// let _ = self::std::io::stdout();
// }
// ```
imports_referenced_with_self: Vec<Symbol>,
}

impl<'tcx> Visitor<'tcx> for ImportUsageVisitor {
fn visit_expr(&mut self, expr: &Expr) {
if let ExprKind::Path(_, path) = &expr.kind
&& path.segments.len() > 1
&& path.segments[0].ident.name == kw::SelfLower
{
self.imports_referenced_with_self.push(path.segments[1].ident.name);
}
walk_expr(self, expr);
}

fn visit_ty(&mut self, ty: &Ty) {
if let TyKind::Path(_, path) = &ty.kind
&& path.segments.len() > 1
&& path.segments[0].ident.name == kw::SelfLower
{
self.imports_referenced_with_self.push(path.segments[1].ident.name);
}
}
}

impl SingleComponentPathImports {
fn check_mod(&mut self, cx: &EarlyContext<'_>, items: &[P<Item>]) {
fn check_mod(&mut self, items: &[P<Item>]) {
// keep track of imports reused with `self` keyword, such as `self::crypto_hash` in the example
// below. Removing the `use crypto_hash;` would make this a compile error
// ```
Expand All @@ -108,26 +144,23 @@ impl SingleComponentPathImports {
// ```
let mut macros = Vec::new();

let mut import_usage_visitor = ImportUsageVisitor::default();
for item in items {
self.track_uses(
cx,
item,
&mut imports_reused_with_self,
&mut single_use_usages,
&mut macros,
);
self.track_uses(item, &mut imports_reused_with_self, &mut single_use_usages, &mut macros);
import_usage_visitor.visit_item(item);
}

for usage in single_use_usages {
if !imports_reused_with_self.contains(&usage.name) {
if !imports_reused_with_self.contains(&usage.name)
&& !import_usage_visitor.imports_referenced_with_self.contains(&usage.name)
{
self.found.entry(usage.item_id).or_default().push(usage);
}
}
}

fn track_uses(
&mut self,
cx: &EarlyContext<'_>,
item: &Item,
imports_reused_with_self: &mut Vec<Symbol>,
single_use_usages: &mut Vec<SingleUse>,
Expand All @@ -139,7 +172,7 @@ impl SingleComponentPathImports {

match &item.kind {
ItemKind::Mod(_, ModKind::Loaded(ref items, ..)) => {
self.check_mod(cx, items);
self.check_mod(items);
},
ItemKind::MacroDef(MacroDef { macro_rules: true, .. }) => {
macros.push(item.ident.name);
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/single_component_path_imports.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
#![warn(clippy::single_component_path_imports)]
#![allow(unused_imports)]

use core;

use serde as edres;
pub use serde;
use std;

macro_rules! m {
() => {
Expand All @@ -17,6 +19,10 @@ fn main() {

// False positive #5154, shouldn't trigger lint.
m!();

// False positive #10549
let _ = self::std::io::stdout();
let _ = 0 as self::core::ffi::c_uint;
}

mod hello_mod {
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/single_component_path_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
#![warn(clippy::single_component_path_imports)]
#![allow(unused_imports)]

use core;
use regex;
use serde as edres;
pub use serde;
use std;

macro_rules! m {
() => {
Expand All @@ -17,6 +19,10 @@ fn main() {

// False positive #5154, shouldn't trigger lint.
m!();

// False positive #10549
let _ = self::std::io::stdout();
let _ = 0 as self::core::ffi::c_uint;
}

mod hello_mod {
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/single_component_path_imports.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
error: this import is redundant
--> $DIR/single_component_path_imports.rs:5:1
--> $DIR/single_component_path_imports.rs:6:1
|
LL | use regex;
| ^^^^^^^^^^ help: remove it entirely
|
= note: `-D clippy::single-component-path-imports` implied by `-D warnings`

error: this import is redundant
--> $DIR/single_component_path_imports.rs:23:5
--> $DIR/single_component_path_imports.rs:29:5
|
LL | use regex;
| ^^^^^^^^^^ help: remove it entirely
Expand Down

0 comments on commit 9408d01

Please sign in to comment.