Skip to content

Commit

Permalink
Auto merge of rust-lang#9046 - xFrednet:rust-97660-expection-somethin…
Browse files Browse the repository at this point in the history
…g-something, r=Jarcho

Fix `#[expect]` for most clippy lints

This PR fixes most `#[expect]` - lint interactions listed in rust-lang#97660. [My comment in the issue](rust-lang#97660 (comment)) shows the current progress (Once this is merged). I plan to work on `duplicate_mod` and `multiple_inherent_impl` and leave the rest for later. I feel like stabilizing the feature is more important than fixing the last few nits, which currently also don't work with `#[allow]`.

---

changelog: none

r? `@Jarcho`

cc: rust-lang#97660
  • Loading branch information
bors committed Jun 28, 2022
2 parents a4130e1 + d11618e commit 4995b4e
Show file tree
Hide file tree
Showing 34 changed files with 464 additions and 231 deletions.
20 changes: 15 additions & 5 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,17 @@ declare_clippy_lint! {
/// Checks for `extern crate` and `use` items annotated with
/// lint attributes.
///
/// This lint permits `#[allow(unused_imports)]`, `#[allow(deprecated)]`,
/// `#[allow(unreachable_pub)]`, `#[allow(clippy::wildcard_imports)]` and
/// `#[allow(clippy::enum_glob_use)]` on `use` items and `#[allow(unused_imports)]` on
/// `extern crate` items with a `#[macro_use]` attribute.
/// This lint permits lint attributes for lints emitted on the items themself.
/// For `use` items these lints are:
/// * deprecated
/// * unreachable_pub
/// * unused_imports
/// * clippy::enum_glob_use
/// * clippy::macro_use_imports
/// * clippy::wildcard_imports
///
/// For `extern crate` items these lints are:
/// * `unused_imports` on items with `#[macro_use]`
///
/// ### Why is this bad?
/// Lint attributes have no effect on crate imports. Most
Expand Down Expand Up @@ -347,7 +354,10 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
|| extract_clippy_lint(lint).map_or(false, |s| {
matches!(
s.as_str(),
"wildcard_imports" | "enum_glob_use" | "redundant_pub_crate",
"wildcard_imports"
| "enum_glob_use"
| "redundant_pub_crate"
| "macro_use_imports",
)
})
{
Expand Down
8 changes: 5 additions & 3 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{eq_expr_value, get_trait_def_id, paths};
Expand Down Expand Up @@ -394,9 +394,10 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
continue 'simplified;
}
if stats.terminals[i] != 0 && simplified_stats.terminals[i] == 0 {
span_lint_and_then(
span_lint_hir_and_then(
self.cx,
LOGIC_BUG,
e.hir_id,
e.span,
"this boolean expression contains a logic bug",
|diag| {
Expand Down Expand Up @@ -429,9 +430,10 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
}
}
let nonminimal_bool_lint = |suggestions: Vec<_>| {
span_lint_and_then(
span_lint_hir_and_then(
self.cx,
NONMINIMAL_BOOL,
e.hir_id,
e.span,
"this boolean expression can be simplified",
|diag| {
Expand Down
5 changes: 3 additions & 2 deletions clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::diagnostics::span_lint_hir;
use clippy_utils::ty::contains_ty;
use rustc_hir::intravisit;
use rustc_hir::{self, AssocItemKind, Body, FnDecl, HirId, HirIdSet, Impl, ItemKind, Node};
Expand Down Expand Up @@ -118,9 +118,10 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal {
});

for node in v.set {
span_lint(
span_lint_hir(
cx,
BOXED_LOCAL,
node,
cx.tcx.hir().span(node),
"local variable doesn't need to be boxed here",
);
Expand Down
39 changes: 23 additions & 16 deletions clippy_lints/src/implicit_return.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::{
diagnostics::span_lint_and_sugg,
diagnostics::span_lint_hir_and_then,
get_async_fn_body, is_async_fn,
source::{snippet_with_applicability, snippet_with_context, walk_span_to_context},
visitors::expr_visitor_no_bodies,
Expand Down Expand Up @@ -43,31 +43,38 @@ declare_clippy_lint! {

declare_lint_pass!(ImplicitReturn => [IMPLICIT_RETURN]);

fn lint_return(cx: &LateContext<'_>, span: Span) {
fn lint_return(cx: &LateContext<'_>, emission_place: HirId, span: Span) {
let mut app = Applicability::MachineApplicable;
let snip = snippet_with_applicability(cx, span, "..", &mut app);
span_lint_and_sugg(
span_lint_hir_and_then(
cx,
IMPLICIT_RETURN,
emission_place,
span,
"missing `return` statement",
"add `return` as shown",
format!("return {}", snip),
app,
|diag| {
diag.span_suggestion(span, "add `return` as shown", format!("return {}", snip), app);
},
);
}

fn lint_break(cx: &LateContext<'_>, break_span: Span, expr_span: Span) {
fn lint_break(cx: &LateContext<'_>, emission_place: HirId, break_span: Span, expr_span: Span) {
let mut app = Applicability::MachineApplicable;
let snip = snippet_with_context(cx, expr_span, break_span.ctxt(), "..", &mut app).0;
span_lint_and_sugg(
span_lint_hir_and_then(
cx,
IMPLICIT_RETURN,
emission_place,
break_span,
"missing `return` statement",
"change `break` to `return` as shown",
format!("return {}", snip),
app,
|diag| {
diag.span_suggestion(
break_span,
"change `break` to `return` as shown",
format!("return {}", snip),
app,
);
},
);
}

Expand Down Expand Up @@ -152,7 +159,7 @@ fn lint_implicit_returns(
// At this point sub_expr can be `None` in async functions which either diverge, or return
// the unit type.
if let Some(sub_expr) = sub_expr {
lint_break(cx, e.span, sub_expr.span);
lint_break(cx, e.hir_id, e.span, sub_expr.span);
}
} else {
// the break expression is from a macro call, add a return to the loop
Expand All @@ -166,10 +173,10 @@ fn lint_implicit_returns(
if add_return {
#[expect(clippy::option_if_let_else)]
if let Some(span) = call_site_span {
lint_return(cx, span);
lint_return(cx, expr.hir_id, span);
LintLocation::Parent
} else {
lint_return(cx, expr.span);
lint_return(cx, expr.hir_id, expr.span);
LintLocation::Inner
}
} else {
Expand Down Expand Up @@ -198,10 +205,10 @@ fn lint_implicit_returns(
{
#[expect(clippy::option_if_let_else)]
if let Some(span) = call_site_span {
lint_return(cx, span);
lint_return(cx, expr.hir_id, span);
LintLocation::Parent
} else {
lint_return(cx, expr.span);
lint_return(cx, expr.hir_id, expr.span);
LintLocation::Inner
}
},
Expand Down
42 changes: 25 additions & 17 deletions clippy_lints/src/macro_use.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::source::snippet;
use hir::def::{DefKind, Res};
use if_chain::if_chain;
Expand Down Expand Up @@ -51,8 +51,9 @@ impl MacroRefData {
#[derive(Default)]
#[expect(clippy::module_name_repetitions)]
pub struct MacroUseImports {
/// the actual import path used and the span of the attribute above it.
imports: Vec<(String, Span)>,
/// the actual import path used and the span of the attribute above it. The value is
/// the location, where the lint should be emitted.
imports: Vec<(String, Span, hir::HirId)>,
/// the span of the macro reference, kept to ensure only one reference is used per macro call.
collected: FxHashSet<Span>,
mac_refs: Vec<MacroRefData>,
Expand Down Expand Up @@ -91,7 +92,8 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
if_chain! {
if cx.sess().opts.edition >= Edition::Edition2018;
if let hir::ItemKind::Use(path, _kind) = &item.kind;
let attrs = cx.tcx.hir().attrs(item.hir_id());
let hir_id = item.hir_id();
let attrs = cx.tcx.hir().attrs(hir_id);
if let Some(mac_attr) = attrs.iter().find(|attr| attr.has_name(sym::macro_use));
if let Res::Def(DefKind::Mod, id) = path.res;
if !id.is_local();
Expand All @@ -100,7 +102,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res {
let span = mac_attr.span;
let def_path = cx.tcx.def_path_str(mac_id);
self.imports.push((def_path, span));
self.imports.push((def_path, span, hir_id));
}
}
} else {
Expand Down Expand Up @@ -138,7 +140,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
fn check_crate_post(&mut self, cx: &LateContext<'_>) {
let mut used = FxHashMap::default();
let mut check_dup = vec![];
for (import, span) in &self.imports {
for (import, span, hir_id) in &self.imports {
let found_idx = self.mac_refs.iter().position(|mac| import.ends_with(&mac.name));

if let Some(idx) = found_idx {
Expand All @@ -151,7 +153,7 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
[] | [_] => return,
[root, item] => {
if !check_dup.contains(&(*item).to_string()) {
used.entry(((*root).to_string(), span))
used.entry(((*root).to_string(), span, hir_id))
.or_insert_with(Vec::new)
.push((*item).to_string());
check_dup.push((*item).to_string());
Expand All @@ -169,13 +171,13 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
}
})
.collect::<Vec<_>>();
used.entry(((*root).to_string(), span))
used.entry(((*root).to_string(), span, hir_id))
.or_insert_with(Vec::new)
.push(filtered.join("::"));
check_dup.extend(filtered);
} else {
let rest = rest.to_vec();
used.entry(((*root).to_string(), span))
used.entry(((*root).to_string(), span, hir_id))
.or_insert_with(Vec::new)
.push(rest.join("::"));
check_dup.extend(rest.iter().map(ToString::to_string));
Expand All @@ -186,27 +188,33 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports {
}

let mut suggestions = vec![];
for ((root, span), path) in used {
for ((root, span, hir_id), path) in used {
if path.len() == 1 {
suggestions.push((span, format!("{}::{}", root, path[0])));
suggestions.push((span, format!("{}::{}", root, path[0]), hir_id));
} else {
suggestions.push((span, format!("{}::{{{}}}", root, path.join(", "))));
suggestions.push((span, format!("{}::{{{}}}", root, path.join(", ")), hir_id));
}
}

// If mac_refs is not empty we have encountered an import we could not handle
// such as `std::prelude::v1::foo` or some other macro that expands to an import.
if self.mac_refs.is_empty() {
for (span, import) in suggestions {
for (span, import, hir_id) in suggestions {
let help = format!("use {};", import);
span_lint_and_sugg(
span_lint_hir_and_then(
cx,
MACRO_USE_IMPORTS,
*hir_id,
*span,
"`macro_use` attributes are no longer needed in the Rust 2018 edition",
"remove the attribute and import the macro directly, try",
help,
Applicability::MaybeIncorrect,
|diag| {
diag.span_suggestion(
*span,
"remove the attribute and import the macro directly, try",
help,
Applicability::MaybeIncorrect,
);
},
);
}
}
Expand Down
9 changes: 5 additions & 4 deletions clippy_lints/src/manual_non_exhaustive.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::snippet_opt;
use clippy_utils::{is_doc_hidden, is_lint_allowed, meets_msrv, msrvs};
use clippy_utils::{is_doc_hidden, meets_msrv, msrvs};
use rustc_ast::ast::{self, VisibilityKind};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -190,12 +190,13 @@ impl<'tcx> LateLintPass<'tcx> for ManualNonExhaustiveEnum {
!self
.constructed_enum_variants
.contains(&(enum_id.to_def_id(), variant_id.to_def_id()))
&& !is_lint_allowed(cx, MANUAL_NON_EXHAUSTIVE, cx.tcx.hir().local_def_id_to_hir_id(enum_id))
})
{
span_lint_and_then(
let hir_id = cx.tcx.hir().local_def_id_to_hir_id(enum_id);
span_lint_hir_and_then(
cx,
MANUAL_NON_EXHAUSTIVE,
hir_id,
enum_span,
"this seems like a manual implementation of the non-exhaustive pattern",
|diag| {
Expand Down
Loading

0 comments on commit 4995b4e

Please sign in to comment.