Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider middle segments of paths in unused_qualifications #121528

Merged
merged 1 commit into from Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 24 additions & 22 deletions compiler/rustc_resolve/src/late.rs
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)| {
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
// 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
@@ -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
@@ -1,6 +1,6 @@
//@ run-rustfix
#![deny(unused_qualifications)]
#![allow(deprecated)]
#![allow(deprecated, dead_code)]
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved

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
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();
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
|

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