Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
  • Loading branch information
surechen committed Mar 11, 2024
1 parent 9fb91aa commit 094e976
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 28 deletions.
42 changes: 40 additions & 2 deletions compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,23 @@ use crate::imports::ImportKind;
use crate::module_to_string;
use crate::Resolver;

use crate::NameBindingKind;
use crate::{LexicalScopeBinding, NameBindingKind};
use rustc_ast as ast;
use rustc_ast::visit::{self, Visitor};
use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
use rustc_data_structures::unord::UnordSet;
use rustc_errors::{pluralize, MultiSpan};
use rustc_hir::def::{DefKind, Res};
use rustc_session::lint::builtin::{MACRO_USE_EXTERN_CRATE, UNUSED_EXTERN_CRATES, UNUSED_IMPORTS};
use rustc_session::lint::builtin::{
MACRO_USE_EXTERN_CRATE, UNUSED_EXTERN_CRATES, UNUSED_IMPORTS, UNUSED_QUALIFICATIONS,
};
use rustc_session::lint::BuiltinLintDiag;
use rustc_span::symbol::sym;
use rustc_span::symbol::{kw, Ident};
use rustc_span::{Span, DUMMY_SP};
use thin_vec::ThinVec;

#[derive(Debug)]
struct UnusedImport {
use_tree: ast::UseTree,
use_tree_id: ast::NodeId,
Expand Down Expand Up @@ -494,6 +499,39 @@ impl Resolver<'_, '_> {
}

let unused_imports = visitor.unused_imports;

// The lint fixes for unused_import and unnecessary_qualification may conflict.
// Deleting both unused imports and unnecessary segments of an item may result
// in the item not being found.
for v in &self.potentially_unnecessary_qualification {
if let LexicalScopeBinding::Item(name_binding) = v.binding
&& let NameBindingKind::Import { binding: _, import } = name_binding.kind
&& let Some(unused_import) = unused_imports.get(&import.root_id)
&& let Some(id) = import.id()
&& unused_import.unused.contains(&id)
{
let def_id = self.local_def_id(id);
if let Some(attr) = tcx.get_attr(def_id, sym::allow) {
debug!("surechen attr:{:?}", attr);
for item in attr.meta_item_list().unwrap_or_else(ThinVec::new) {
debug!("surechen item:{:?}", item);
if !item.has_name(sym::unused_imports) {
debug!("surechen yes item:{:?}", item);
continue;
}
}
}
}

self.lint_buffer.buffer_lint_with_diagnostic(
UNUSED_QUALIFICATIONS,
v.finalize.node_id,
v.finalize.path_span,
"unnecessary qualification",
BuiltinLintDiag::UnusedQualifications { removal_span: v.span },
);
}

let mut check_redundant_imports = FxIndexSet::default();
for module in self.arenas.local_modules().iter() {
for (_key, resolution) in self.resolutions(*module).borrow().iter() {
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1352,6 +1352,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
target_bindings[ns].get(),
) {
Ok(other_binding) => {
debug!(
"surechen check_for_redundant_imports other_binding:{:?}",
other_binding
);
debug!("surechen check_for_redundant_imports binding:{:?}", binding);
is_redundant =
binding.res() == other_binding.res() && !other_binding.is_ambiguity();
if is_redundant {
Expand Down
25 changes: 14 additions & 11 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,14 @@ impl MaybeExported<'_> {
}
}

/// Used for recording UnnecessaryQualification.
#[derive(Debug)]
pub(crate) struct UnnecessaryQualification<'a> {
pub binding: LexicalScopeBinding<'a>,
pub finalize: Finalize,
pub span: Span,
}

#[derive(Default)]
struct DiagMetadata<'ast> {
/// The current trait's associated items' ident, used for diagnostic suggestions.
Expand Down Expand Up @@ -4653,20 +4661,15 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
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)
(res == binding.clone().res()).then_some((seg, binding))
});

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),
},
);
self.r.potentially_unnecessary_qualification.push(UnnecessaryQualification {
binding: unqualified.1,
finalize: finalize,
span: path[0].ident.span.until(unqualified.0.ident.span),
});
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ use std::fmt;

use diagnostics::{ImportSuggestion, LabelSuggestion, Suggestion};
use imports::{Import, ImportData, ImportKind, NameResolution};
use late::{HasGenericParams, PathSource, PatternSource};
use late::{HasGenericParams, PathSource, PatternSource, UnnecessaryQualification};
use macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};

use crate::effective_visibilities::EffectiveVisibilitiesVisitor;
Expand Down Expand Up @@ -372,7 +372,7 @@ impl<'a> From<&'a ast::PathSegment> for Segment {
/// This refers to the thing referred by a name. The difference between `Res` and `Item` is that
/// items are visible in their whole block, while `Res`es only from the place they are defined
/// forward.
#[derive(Debug)]
#[derive(Debug, Clone)]
enum LexicalScopeBinding<'a> {
Item(NameBinding<'a>),
Res(Res),
Expand Down Expand Up @@ -1105,6 +1105,8 @@ pub struct Resolver<'a, 'tcx> {

potentially_unused_imports: Vec<Import<'a>>,

potentially_unnecessary_qualification: Vec<UnnecessaryQualification<'a>>,

/// Table for mapping struct IDs into struct constructor IDs,
/// it's not used during normal resolution, only for better error reporting.
/// Also includes of list of each fields visibility
Expand Down Expand Up @@ -1464,6 +1466,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
local_macro_def_scopes: FxHashMap::default(),
name_already_seen: FxHashMap::default(),
potentially_unused_imports: Vec::new(),
potentially_unnecessary_qualification: Default::default(),
struct_constructors: Default::default(),
unused_macros: Default::default(),
unused_macro_rules: Default::default(),
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/lint/lint-qualification.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ fn main() {
//~^ ERROR: unnecessary qualification
//~| ERROR: unnecessary qualification

use std::fmt;
let _: fmt::Result = Ok(()); //~ ERROR: unnecessary qualification
//~ ERROR: unused import: `std::fmt`
let _: std::fmt::Result = Ok(()); // issue #121999

let _ = <bool as Default>::default(); // issue #121999
//~^ ERROR: unnecessary qualification
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/lint/lint-qualification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ fn main() {
//~^ ERROR: unnecessary qualification
//~| ERROR: unnecessary qualification

use std::fmt;
let _: std::fmt::Result = Ok(()); //~ ERROR: unnecessary qualification
#[deny(unused_imports)]
use std::fmt; //~ ERROR: unused import: `std::fmt`
let _: std::fmt::Result = Ok(()); // issue #121331

let _ = <bool as ::std::default::Default>::default(); // issue #121999
//~^ ERROR: unnecessary qualification
Expand Down
18 changes: 9 additions & 9 deletions tests/ui/lint/lint-qualification.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,20 @@ 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(());
| ^^^^^^^^^^^^^^^^
error: unused import: `std::fmt`
--> $DIR/lint-qualification.rs:25:9
|
help: remove the unnecessary path segments
LL | use std::fmt;
| ^^^^^^^^
|
LL - let _: std::fmt::Result = Ok(());
LL + let _: fmt::Result = Ok(());
note: the lint level is defined here
--> $DIR/lint-qualification.rs:24:12
|
LL | #[deny(unused_imports)]
| ^^^^^^^^^^^^^^

error: unnecessary qualification
--> $DIR/lint-qualification.rs:27:13
--> $DIR/lint-qualification.rs:28:13
|
LL | let _ = <bool as ::std::default::Default>::default(); // issue #121999
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/lint/unused/lint-qualification-issue-121331.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//@ run-rustfix
//@ edition:2021
#![deny(unused_qualifications)]
#![feature(coroutines, coroutine_trait)]

use std::ops::{
Coroutine,
CoroutineState::{self, *},
};
use std::pin::Pin;

fn finish<T>(mut amt: usize, mut t: T) -> T::Return
where T: Coroutine<(), Yield = ()> + Unpin,
{
loop {
match Pin::new(&mut t).resume(()) {
CoroutineState::Yielded(()) => amt = amt.checked_sub(1).unwrap(),
CoroutineState::Complete(ret) => {
assert_eq!(amt, 0);
return ret
}
}
}

}

pub fn main() {}

0 comments on commit 094e976

Please sign in to comment.