From 6120de99f7be901f94e55c2cdf09b6fb98f93409 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Tue, 5 Mar 2024 18:31:32 +0000 Subject: [PATCH] Fix linting paths with qself in `unused_qualifications` --- compiler/rustc_resolve/src/late.rs | 70 +++++++++++++------------ tests/ui/lint/lint-qualification.fixed | 3 ++ tests/ui/lint/lint-qualification.rs | 3 ++ tests/ui/lint/lint-qualification.stderr | 14 ++++- 4 files changed, 56 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index de6e121c79bbb..36279de83d34b 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -3953,6 +3953,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { // Avoid recording definition of `A::B` in `::B::C`. self.r.record_partial_res(node_id, partial_res); self.resolve_elided_lifetimes_in_path(partial_res, path, source, path_span); + self.lint_unused_qualifications(path, ns, finalize); } partial_res @@ -4145,39 +4146,6 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { PathResult::Indeterminate => bug!("indeterminate path result in resolve_qpath"), }; - 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::builtin::UNUSED_QUALIFICATIONS, - finalize.node_id, - finalize.path_span, - "unnecessary qualification", - lint::BuiltinLintDiag::UnusedQualifications { - removal_span: finalize.path_span.until(unqualified.ident.span), - }, - ); - } - } - Ok(Some(result)) } @@ -4656,6 +4624,42 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { self.r.doc_link_traits_in_scope = doc_link_traits_in_scope; } } + + fn lint_unused_qualifications(&mut self, path: &[Segment], ns: Namespace, finalize: Finalize) { + if path.iter().any(|seg| seg.ident.span.from_expansion()) { + return; + } + + 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::builtin::UNUSED_QUALIFICATIONS, + finalize.node_id, + finalize.path_span, + "unnecessary qualification", + lint::BuiltinLintDiag::UnusedQualifications { + removal_span: path[0].ident.span.until(unqualified.ident.span), + }, + ); + } + } } /// Walks the whole crate in DFS order, visiting each item, counting the declared number of diff --git a/tests/ui/lint/lint-qualification.fixed b/tests/ui/lint/lint-qualification.fixed index ab450f8b7347d..2b1f8b591752a 100644 --- a/tests/ui/lint/lint-qualification.fixed +++ b/tests/ui/lint/lint-qualification.fixed @@ -24,6 +24,9 @@ fn main() { use std::fmt; let _: fmt::Result = Ok(()); //~ ERROR: unnecessary qualification + let _ = ::default(); // issue #121999 + //~^ ERROR: unnecessary qualification + macro_rules! m { ($a:ident, $b:ident) => { $crate::foo::bar(); // issue #37357 ::foo::bar(); // issue #38682 diff --git a/tests/ui/lint/lint-qualification.rs b/tests/ui/lint/lint-qualification.rs index 84a36f509eb9e..002fdbf7724a7 100644 --- a/tests/ui/lint/lint-qualification.rs +++ b/tests/ui/lint/lint-qualification.rs @@ -24,6 +24,9 @@ fn main() { use std::fmt; let _: std::fmt::Result = Ok(()); //~ ERROR: unnecessary qualification + let _ = ::default(); // issue #121999 + //~^ ERROR: unnecessary qualification + macro_rules! m { ($a:ident, $b:ident) => { $crate::foo::bar(); // issue #37357 ::foo::bar(); // issue #38682 diff --git a/tests/ui/lint/lint-qualification.stderr b/tests/ui/lint/lint-qualification.stderr index 45e1525f4bf8c..8dddcf23f7575 100644 --- a/tests/ui/lint/lint-qualification.stderr +++ b/tests/ui/lint/lint-qualification.stderr @@ -87,5 +87,17 @@ LL - let _: std::fmt::Result = Ok(()); LL + let _: fmt::Result = Ok(()); | -error: aborting due to 7 previous errors +error: unnecessary qualification + --> $DIR/lint-qualification.rs:27:13 + | +LL | let _ = ::default(); // issue #121999 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the unnecessary path segments + | +LL - let _ = ::default(); // issue #121999 +LL + let _ = ::default(); // issue #121999 + | + +error: aborting due to 8 previous errors