Skip to content

Commit

Permalink
Rollup merge of rust-lang#122038 - Alexendoo:unused-qualifications, r…
Browse files Browse the repository at this point in the history
…=petrochenkov

Fix linting paths with qself in `unused_qualifications`

Fixes rust-lang#121999

`resolve_qpath` ends up being called again with `qself` set to `None` to check trait items from fully qualified paths. To avoid this the lint is moved to a place that accounts for this already

https://github.com/rust-lang/rust/blob/96561a8fd134e8f2b205769a4fca03b392d9f484/compiler/rustc_resolve/src/late.rs#L4074-L4088

r? ```@petrochenkov```
  • Loading branch information
matthiaskrgr authored Mar 6, 2024
2 parents 5fbfb5b + 6120de9 commit 7ce4f1e
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 34 deletions.
70 changes: 37 additions & 33 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3954,6 +3954,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
// Avoid recording definition of `A::B` in `<T as A>::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
Expand Down Expand Up @@ -4165,39 +4166,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))
}

Expand Down Expand Up @@ -4676,6 +4644,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
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/lint/lint-qualification.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ fn main() {
use std::fmt;
let _: fmt::Result = Ok(()); //~ ERROR: unnecessary qualification

let _ = <bool as Default>::default(); // issue #121999
//~^ ERROR: unnecessary qualification

macro_rules! m { ($a:ident, $b:ident) => {
$crate::foo::bar(); // issue #37357
::foo::bar(); // issue #38682
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/lint/lint-qualification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ fn main() {
use std::fmt;
let _: std::fmt::Result = Ok(()); //~ ERROR: unnecessary qualification

let _ = <bool as ::std::default::Default>::default(); // issue #121999
//~^ ERROR: unnecessary qualification

macro_rules! m { ($a:ident, $b:ident) => {
$crate::foo::bar(); // issue #37357
::foo::bar(); // issue #38682
Expand Down
14 changes: 13 additions & 1 deletion tests/ui/lint/lint-qualification.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 _ = <bool as ::std::default::Default>::default(); // issue #121999
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: remove the unnecessary path segments
|
LL - let _ = <bool as ::std::default::Default>::default(); // issue #121999
LL + let _ = <bool as Default>::default(); // issue #121999
|

error: aborting due to 8 previous errors

0 comments on commit 7ce4f1e

Please sign in to comment.