From 4ea9f72c72cb2fbe210f1141aea4a85aea3ef6d1 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Fri, 23 Feb 2024 17:00:50 +0000 Subject: [PATCH] Consider middle segments of paths in `unused_qualifications` --- compiler/rustc_resolve/src/late.rs | 46 +++++++-------- tests/ui/lint/lint-qualification.fixed | 30 +++++++++- tests/ui/lint/lint-qualification.rs | 30 +++++++++- tests/ui/lint/lint-qualification.stderr | 74 ++++++++++++++++++++++++- 4 files changed, 151 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 2f4da29133f1b..87aa3bed2ca53 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -4150,34 +4150,36 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { PathResult::Indeterminate => bug!("indeterminate path result in resolve_qpath"), }; - if path.len() > 1 - && let Some(res) = result.full_res() - && let Some((&last_segment, prev_segs)) = path.split_last() - && prev_segs.iter().all(|seg| !seg.has_generic_args) - && res != Res::Err - && path[0].ident.name != kw::PathRoot - && path[0].ident.name != kw::DollarCrate - { - let unqualified_result = { - match self.resolve_path(&[last_segment], Some(ns), None) { - PathResult::NonModule(path_res) => path_res.expect_full_res(), - PathResult::Module(ModuleOrUniformRoot::Module(module)) => { - module.res().unwrap() - } - _ => return Ok(Some(result)), - } - }; - if res == unqualified_result { - let lint = lint::builtin::UNUSED_QUALIFICATIONS; + if path.iter().all(|seg| !seg.ident.span.from_expansion()) { + let end_pos = + path.iter().position(|seg| seg.has_generic_args).map_or(path.len(), |pos| pos + 1); + let unqualified = + path[..end_pos].iter().enumerate().skip(1).rev().find_map(|(i, seg)| { + // Preserve the current namespace for the final path segment, but use the type + // namespace for all preceding segments + // + // e.g. for `std::env::args` check the `ValueNS` for `args` but the `TypeNS` for + // `std` and `env` + // + // If the final path segment is beyond `end_pos` all the segments to check will + // use the type namespace + let ns = if i + 1 == path.len() { ns } else { TypeNS }; + let res = self.r.partial_res_map.get(&seg.id?)?.full_res()?; + let binding = self.resolve_ident_in_lexical_scope(seg.ident, ns, None, None)?; + + (res == binding.res()).then_some(seg) + }); + + if let Some(unqualified) = unqualified { self.r.lint_buffer.buffer_lint_with_diagnostic( - lint, + lint::builtin::UNUSED_QUALIFICATIONS, finalize.node_id, finalize.path_span, "unnecessary qualification", lint::BuiltinLintDiagnostics::UnusedQualifications { - removal_span: finalize.path_span.until(last_segment.ident.span), + removal_span: finalize.path_span.until(unqualified.ident.span), }, - ) + ); } } diff --git a/tests/ui/lint/lint-qualification.fixed b/tests/ui/lint/lint-qualification.fixed index 18d69ef1b53a2..ab450f8b7347d 100644 --- a/tests/ui/lint/lint-qualification.fixed +++ b/tests/ui/lint/lint-qualification.fixed @@ -1,6 +1,6 @@ //@ run-rustfix #![deny(unused_qualifications)] -#![allow(deprecated)] +#![allow(deprecated, dead_code)] mod foo { pub fn bar() {} @@ -9,13 +9,37 @@ mod foo { fn main() { use foo::bar; bar(); //~ ERROR: unnecessary qualification + bar(); //~ ERROR: unnecessary qualification bar(); let _ = || -> Result<(), ()> { try!(Ok(())); Ok(()) }; // issue #37345 - macro_rules! m { () => { + let _ = String::new(); //~ ERROR: unnecessary qualification + let _ = std::env::current_dir(); //~ ERROR: unnecessary qualification + + let _: Vec = Vec::::new(); + //~^ ERROR: unnecessary qualification + //~| ERROR: unnecessary qualification + + use std::fmt; + let _: fmt::Result = Ok(()); //~ ERROR: unnecessary qualification + + macro_rules! m { ($a:ident, $b:ident) => { $crate::foo::bar(); // issue #37357 ::foo::bar(); // issue #38682 + foo::bar(); + foo::$b(); // issue #96698 + $a::bar(); } } - m!(); + m!(foo, bar); +} + +mod conflicting_names { + mod std {} + mod cell {} + + fn f() { + let _ = ::std::env::current_dir(); + let _ = core::cell::Cell::new(1); + } } diff --git a/tests/ui/lint/lint-qualification.rs b/tests/ui/lint/lint-qualification.rs index 8cf3425db2f2b..84a36f509eb9e 100644 --- a/tests/ui/lint/lint-qualification.rs +++ b/tests/ui/lint/lint-qualification.rs @@ -1,6 +1,6 @@ //@ run-rustfix #![deny(unused_qualifications)] -#![allow(deprecated)] +#![allow(deprecated, dead_code)] mod foo { pub fn bar() {} @@ -9,13 +9,37 @@ mod foo { fn main() { use foo::bar; foo::bar(); //~ ERROR: unnecessary qualification + crate::foo::bar(); //~ ERROR: unnecessary qualification bar(); let _ = || -> Result<(), ()> { try!(Ok(())); Ok(()) }; // issue #37345 - macro_rules! m { () => { + let _ = std::string::String::new(); //~ ERROR: unnecessary qualification + let _ = ::std::env::current_dir(); //~ ERROR: unnecessary qualification + + let _: std::vec::Vec = std::vec::Vec::::new(); + //~^ ERROR: unnecessary qualification + //~| ERROR: unnecessary qualification + + use std::fmt; + let _: std::fmt::Result = Ok(()); //~ ERROR: unnecessary qualification + + macro_rules! m { ($a:ident, $b:ident) => { $crate::foo::bar(); // issue #37357 ::foo::bar(); // issue #38682 + foo::bar(); + foo::$b(); // issue #96698 + $a::bar(); } } - m!(); + m!(foo, bar); +} + +mod conflicting_names { + mod std {} + mod cell {} + + fn f() { + let _ = ::std::env::current_dir(); + let _ = core::cell::Cell::new(1); + } } diff --git a/tests/ui/lint/lint-qualification.stderr b/tests/ui/lint/lint-qualification.stderr index 2448a64f11d90..45e1525f4bf8c 100644 --- a/tests/ui/lint/lint-qualification.stderr +++ b/tests/ui/lint/lint-qualification.stderr @@ -15,5 +15,77 @@ LL - foo::bar(); LL + bar(); | -error: aborting due to 1 previous error +error: unnecessary qualification + --> $DIR/lint-qualification.rs:12:5 + | +LL | crate::foo::bar(); + | ^^^^^^^^^^^^^^^ + | +help: remove the unnecessary path segments + | +LL - crate::foo::bar(); +LL + bar(); + | + +error: unnecessary qualification + --> $DIR/lint-qualification.rs:17:13 + | +LL | let _ = std::string::String::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the unnecessary path segments + | +LL - let _ = std::string::String::new(); +LL + let _ = String::new(); + | + +error: unnecessary qualification + --> $DIR/lint-qualification.rs:18:13 + | +LL | let _ = ::std::env::current_dir(); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the unnecessary path segments + | +LL - let _ = ::std::env::current_dir(); +LL + let _ = std::env::current_dir(); + | + +error: unnecessary qualification + --> $DIR/lint-qualification.rs:20:12 + | +LL | let _: std::vec::Vec = std::vec::Vec::::new(); + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the unnecessary path segments + | +LL - let _: std::vec::Vec = std::vec::Vec::::new(); +LL + let _: Vec = std::vec::Vec::::new(); + | + +error: unnecessary qualification + --> $DIR/lint-qualification.rs:20:36 + | +LL | let _: std::vec::Vec = std::vec::Vec::::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the unnecessary path segments + | +LL - let _: std::vec::Vec = std::vec::Vec::::new(); +LL + let _: std::vec::Vec = Vec::::new(); + | + +error: unnecessary qualification + --> $DIR/lint-qualification.rs:25:12 + | +LL | let _: std::fmt::Result = Ok(()); + | ^^^^^^^^^^^^^^^^ + | +help: remove the unnecessary path segments + | +LL - let _: std::fmt::Result = Ok(()); +LL + let _: fmt::Result = Ok(()); + | + +error: aborting due to 7 previous errors