Skip to content

Commit

Permalink
Consider middle segments of paths in unused_qualifications
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexendoo committed Mar 3, 2024
1 parent b6a23b8 commit 4ea9f72
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 29 deletions.
46 changes: 24 additions & 22 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
)
);
}
}

Expand Down
30 changes: 27 additions & 3 deletions tests/ui/lint/lint-qualification.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-rustfix
#![deny(unused_qualifications)]
#![allow(deprecated)]
#![allow(deprecated, dead_code)]

mod foo {
pub fn bar() {}
Expand All @@ -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<String> = Vec::<String>::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);
}
}
30 changes: 27 additions & 3 deletions tests/ui/lint/lint-qualification.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ run-rustfix
#![deny(unused_qualifications)]
#![allow(deprecated)]
#![allow(deprecated, dead_code)]

mod foo {
pub fn bar() {}
Expand All @@ -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<String> = std::vec::Vec::<String>::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);
}
}
74 changes: 73 additions & 1 deletion tests/ui/lint/lint-qualification.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> = std::vec::Vec::<String>::new();
| ^^^^^^^^^^^^^^^^^^^^^
|
help: remove the unnecessary path segments
|
LL - let _: std::vec::Vec<String> = std::vec::Vec::<String>::new();
LL + let _: Vec<String> = std::vec::Vec::<String>::new();
|

error: unnecessary qualification
--> $DIR/lint-qualification.rs:20:36
|
LL | let _: std::vec::Vec<String> = std::vec::Vec::<String>::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: remove the unnecessary path segments
|
LL - let _: std::vec::Vec<String> = std::vec::Vec::<String>::new();
LL + let _: std::vec::Vec<String> = Vec::<String>::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

0 comments on commit 4ea9f72

Please sign in to comment.