Skip to content

Commit

Permalink
Auto merge of rust-lang#8305 - camsteffen:util-cleanup, r=flip1995
Browse files Browse the repository at this point in the history
Factor out several utils, add `path_def_id`

changelog: none

This is generally an effort to reduce the total number of utils. `path_def_id` is added which I believe is more "cross-cutting" and also complements `path_to_local`. Best reviewed one commit at a time.

Added:
* `path_def_id`
* `path_res`

Removed:
 * `is_qpath_def_path`
 * `match_any_diagnostic_items`
 * `expr_path_res`
 * `single_segment_path`
 * `differing_macro_contexts`
 * `is_ty_param_lang_item`
 * `is_ty_param_diagnostic_item`
 * `get_qpath_generics`

Renamed:
* `path_to_res` to `def_path_res`
* `get_qpath_generic_tys` to `qpath_generic_tys`

CC `@Jarcho` since this relates to some of your work and you may have input.
  • Loading branch information
bors committed Feb 7, 2022
2 parents 8dc719c + bd583d9 commit 3d43826
Show file tree
Hide file tree
Showing 29 changed files with 249 additions and 309 deletions.
6 changes: 3 additions & 3 deletions clippy_lints/src/blocks_in_if_conditions.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
use clippy_utils::get_parent_expr;
use clippy_utils::higher;
use clippy_utils::source::snippet_block_with_applicability;
use clippy_utils::ty::implements_trait;
use clippy_utils::{differing_macro_contexts, get_parent_expr};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, Visitor};
Expand Down Expand Up @@ -97,7 +97,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
if let Some(ex) = &block.expr {
// don't dig into the expression here, just suggest that they remove
// the block
if expr.span.from_expansion() || differing_macro_contexts(expr.span, ex.span) {
if expr.span.from_expansion() || ex.span.from_expansion() {
return;
}
let mut applicability = Applicability::MachineApplicable;
Expand All @@ -122,7 +122,7 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
}
} else {
let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
if span.from_expansion() || differing_macro_contexts(expr.span, span) {
if span.from_expansion() || expr.span.from_expansion() {
return;
}
// move block higher
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/disallowed_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedMethods {
fn check_crate(&mut self, cx: &LateContext<'_>) {
for (index, conf) in self.conf_disallowed.iter().enumerate() {
let segs: Vec<_> = conf.path().split("::").collect();
if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs) {
if let Res::Def(_, id) = clippy_utils::def_path_res(cx, &segs) {
self.disallowed.insert(id, index);
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/disallowed_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedTypes {
),
};
let segs: Vec<_> = path.split("::").collect();
match clippy_utils::path_to_res(cx, &segs) {
match clippy_utils::def_path_res(cx, &segs) {
Res::Def(_, id) => {
self.def_ids.insert(id, reason);
},
Expand Down
14 changes: 6 additions & 8 deletions clippy_lints/src/formatting.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note};
use clippy_utils::differing_macro_contexts;
use clippy_utils::source::snippet_opt;
use if_chain::if_chain;
use rustc_ast::ast::{BinOpKind, Block, Expr, ExprKind, StmtKind, UnOp};
Expand Down Expand Up @@ -135,7 +134,7 @@ impl EarlyLintPass for Formatting {
/// Implementation of the `SUSPICIOUS_ASSIGNMENT_FORMATTING` lint.
fn check_assign(cx: &EarlyContext<'_>, expr: &Expr) {
if let ExprKind::Assign(ref lhs, ref rhs, _) = expr.kind {
if !differing_macro_contexts(lhs.span, rhs.span) && !lhs.span.from_expansion() {
if !lhs.span.from_expansion() && !rhs.span.from_expansion() {
let eq_span = lhs.span.between(rhs.span);
if let ExprKind::Unary(op, ref sub_rhs) = rhs.kind {
if let Some(eq_snippet) = snippet_opt(cx, eq_span) {
Expand Down Expand Up @@ -165,7 +164,7 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &Expr) {
fn check_unop(cx: &EarlyContext<'_>, expr: &Expr) {
if_chain! {
if let ExprKind::Binary(ref binop, ref lhs, ref rhs) = expr.kind;
if !differing_macro_contexts(lhs.span, rhs.span) && !lhs.span.from_expansion();
if !lhs.span.from_expansion() && !rhs.span.from_expansion();
// span between BinOp LHS and RHS
let binop_span = lhs.span.between(rhs.span);
// if RHS is an UnOp
Expand Down Expand Up @@ -206,8 +205,8 @@ fn check_else(cx: &EarlyContext<'_>, expr: &Expr) {
if_chain! {
if let ExprKind::If(_, then, Some(else_)) = &expr.kind;
if is_block(else_) || is_if(else_);
if !differing_macro_contexts(then.span, else_.span);
if !then.span.from_expansion() && !in_external_macro(cx.sess(), expr.span);
if !then.span.from_expansion() && !else_.span.from_expansion();
if !in_external_macro(cx.sess(), expr.span);

// workaround for rust-lang/rust#43081
if expr.span.lo().0 != 0 && expr.span.hi().0 != 0;
Expand Down Expand Up @@ -268,7 +267,7 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {
for element in array {
if_chain! {
if let ExprKind::Binary(ref op, ref lhs, _) = element.kind;
if has_unary_equivalent(op.node) && !differing_macro_contexts(lhs.span, op.span);
if has_unary_equivalent(op.node) && lhs.span.ctxt() == op.span.ctxt();
let space_span = lhs.span.between(op.span);
if let Some(space_snippet) = snippet_opt(cx, space_span);
let lint_span = lhs.span.with_lo(lhs.span.hi());
Expand All @@ -291,8 +290,7 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {

fn check_missing_else(cx: &EarlyContext<'_>, first: &Expr, second: &Expr) {
if_chain! {
if !differing_macro_contexts(first.span, second.span);
if !first.span.from_expansion();
if !first.span.from_expansion() && !second.span.from_expansion();
if let ExprKind::If(cond_expr, ..) = &first.kind;
if is_block(second) || is_if(second);

Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/implicit_hasher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use rustc_typeck::hir_ty_to_ty;
use if_chain::if_chain;

use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
use clippy_utils::differing_macro_contexts;
use clippy_utils::source::{snippet, snippet_opt};
use clippy_utils::ty::is_type_diagnostic_item;

Expand Down Expand Up @@ -123,7 +122,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher {
vis.visit_ty(impl_.self_ty);

for target in &vis.found {
if differing_macro_contexts(item.span, target.span()) {
if item.span.ctxt() != target.span().ctxt() {
return;
}

Expand Down
12 changes: 4 additions & 8 deletions clippy_lints/src/infinite_iter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{get_trait_def_id, higher, is_qpath_def_path, paths};
use clippy_utils::{get_trait_def_id, higher, match_def_path, path_def_id, paths};
use rustc_hir::{BorrowKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -167,13 +167,9 @@ fn is_infinite(cx: &LateContext<'_>, expr: &Expr<'_>) -> Finiteness {
},
ExprKind::Block(block, _) => block.expr.as_ref().map_or(Finite, |e| is_infinite(cx, e)),
ExprKind::Box(e) | ExprKind::AddrOf(BorrowKind::Ref, _, e) => is_infinite(cx, e),
ExprKind::Call(path, _) => {
if let ExprKind::Path(ref qpath) = path.kind {
is_qpath_def_path(cx, qpath, path.hir_id, &paths::ITER_REPEAT).into()
} else {
Finite
}
},
ExprKind::Call(path, _) => path_def_id(cx, path)
.map_or(false, |id| match_def_path(cx, id, &paths::ITER_REPEAT))
.into(),
ExprKind::Struct(..) => higher::Range::hir(expr).map_or(false, |r| r.end.is_none()).into(),
_ => Finite,
}
Expand Down
24 changes: 10 additions & 14 deletions clippy_lints/src/loops/single_element_loop.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use super::SINGLE_ELEMENT_LOOP;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::single_segment_path;
use clippy_utils::source::{indent_of, snippet};
use clippy_utils::source::{indent_of, snippet_with_applicability};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, Pat, PatKind};
use rustc_hir::{BorrowKind, Expr, ExprKind, Pat};
use rustc_lint::LateContext;

pub(super) fn check<'tcx>(
Expand All @@ -16,33 +15,30 @@ pub(super) fn check<'tcx>(
) {
let arg_expr = match arg.kind {
ExprKind::AddrOf(BorrowKind::Ref, _, ref_arg) => ref_arg,
ExprKind::MethodCall(method, args, _) if args.len() == 1 && method.ident.name == rustc_span::sym::iter => {
&args[0]
},
ExprKind::MethodCall(method, [arg], _) if method.ident.name == rustc_span::sym::iter => arg,
_ => return,
};
if_chain! {
if let PatKind::Binding(.., target, _) = pat.kind;
if let ExprKind::Array([arg_expression]) = arg_expr.kind;
if let ExprKind::Path(ref list_item) = arg_expression.kind;
if let Some(list_item_name) = single_segment_path(list_item).map(|ps| ps.ident.name);
if let ExprKind::Block(block, _) = body.kind;
if !block.stmts.is_empty();

then {
let mut block_str = snippet(cx, block.span, "..").into_owned();
let mut applicability = Applicability::MachineApplicable;
let pat_snip = snippet_with_applicability(cx, pat.span, "..", &mut applicability);
let arg_snip = snippet_with_applicability(cx, arg_expression.span, "..", &mut applicability);
let mut block_str = snippet_with_applicability(cx, block.span, "..", &mut applicability).into_owned();
block_str.remove(0);
block_str.pop();

let indent = " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0));

span_lint_and_sugg(
cx,
SINGLE_ELEMENT_LOOP,
expr.span,
"for loop over a single element",
"try",
format!("{{\n{}let {} = &{};{}}}", " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0)), target.name, list_item_name, block_str),
Applicability::MachineApplicable
format!("{{\n{}let {} = &{};{}}}", indent, pat_snip, arg_snip, block_str),
applicability,
)
}
}
Expand Down
62 changes: 35 additions & 27 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1860,22 +1860,22 @@ where
mod redundant_pattern_match {
use super::REDUNDANT_PATTERN_MATCHING;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher;
use clippy_utils::source::snippet;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, is_type_lang_item, match_type};
use clippy_utils::{is_lang_ctor, is_qpath_def_path, is_trait_method, paths};
use clippy_utils::{higher, match_def_path};
use clippy_utils::{is_lang_ctor, is_trait_method, paths};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk};
use rustc_hir::LangItem::{OptionNone, PollPending};
use rustc_hir::{
intravisit::{walk_expr, Visitor},
Arm, Block, Expr, ExprKind, LangItem, MatchSource, Node, Pat, PatKind, QPath, UnOp,
};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
use rustc_middle::ty::{self, subst::GenericArgKind, DefIdTree, Ty};
use rustc_span::sym;

pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
Expand Down Expand Up @@ -2051,28 +2051,31 @@ mod redundant_pattern_match {
has_else: bool,
) {
// also look inside refs
let mut kind = &let_pat.kind;
// if we have &None for example, peel it so we can detect "if let None = x"
if let PatKind::Ref(inner, _mutability) = kind {
kind = &inner.kind;
}
let check_pat = match let_pat.kind {
PatKind::Ref(inner, _mutability) => inner,
_ => let_pat,
};
let op_ty = cx.typeck_results().expr_ty(let_expr);
// Determine which function should be used, and the type contained by the corresponding
// variant.
let (good_method, inner_ty) = match kind {
PatKind::TupleStruct(ref path, [sub_pat], _) => {
let (good_method, inner_ty) = match check_pat.kind {
PatKind::TupleStruct(ref qpath, [sub_pat], _) => {
if let PatKind::Wild = sub_pat.kind {
if is_lang_ctor(cx, path, ResultOk) {
let res = cx.typeck_results().qpath_res(qpath, check_pat.hir_id);
let Some(id) = res.opt_def_id().and_then(|ctor_id| cx.tcx.parent(ctor_id)) else { return };
let lang_items = cx.tcx.lang_items();
if Some(id) == lang_items.result_ok_variant() {
("is_ok()", try_get_generic_ty(op_ty, 0).unwrap_or(op_ty))
} else if is_lang_ctor(cx, path, ResultErr) {
} else if Some(id) == lang_items.result_err_variant() {
("is_err()", try_get_generic_ty(op_ty, 1).unwrap_or(op_ty))
} else if is_lang_ctor(cx, path, OptionSome) {
} else if Some(id) == lang_items.option_some_variant() {
("is_some()", op_ty)
} else if is_lang_ctor(cx, path, PollReady) {
} else if Some(id) == lang_items.poll_ready_variant() {
("is_ready()", op_ty)
} else if is_qpath_def_path(cx, path, sub_pat.hir_id, &paths::IPADDR_V4) {
} else if match_def_path(cx, id, &paths::IPADDR_V4) {
("is_ipv4()", op_ty)
} else if is_qpath_def_path(cx, path, sub_pat.hir_id, &paths::IPADDR_V6) {
} else if match_def_path(cx, id, &paths::IPADDR_V6) {
("is_ipv6()", op_ty)
} else {
return;
Expand Down Expand Up @@ -2272,17 +2275,22 @@ mod redundant_pattern_match {
should_be_left: &'a str,
should_be_right: &'a str,
) -> Option<&'a str> {
let body_node_pair = if is_qpath_def_path(cx, path_left, arms[0].pat.hir_id, expected_left)
&& is_qpath_def_path(cx, path_right, arms[1].pat.hir_id, expected_right)
{
(&(*arms[0].body).kind, &(*arms[1].body).kind)
} else if is_qpath_def_path(cx, path_right, arms[1].pat.hir_id, expected_left)
&& is_qpath_def_path(cx, path_left, arms[0].pat.hir_id, expected_right)
{
(&(*arms[1].body).kind, &(*arms[0].body).kind)
} else {
return None;
};
let left_id = cx
.typeck_results()
.qpath_res(path_left, arms[0].pat.hir_id)
.opt_def_id()?;
let right_id = cx
.typeck_results()
.qpath_res(path_right, arms[1].pat.hir_id)
.opt_def_id()?;
let body_node_pair =
if match_def_path(cx, left_id, expected_left) && match_def_path(cx, right_id, expected_right) {
(&(*arms[0].body).kind, &(*arms[1].body).kind)
} else if match_def_path(cx, right_id, expected_left) && match_def_path(cx, right_id, expected_right) {
(&(*arms[1].body).kind, &(*arms[0].body).kind)
} else {
return None;
};

match body_node_pair {
(ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => match (&lit_left.node, &lit_right.node) {
Expand Down
15 changes: 6 additions & 9 deletions clippy_lints/src/methods/chars_cmp.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{method_chain_args, single_segment_path};
use clippy_utils::{method_chain_args, path_def_id};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_lint::Lint;
use rustc_middle::ty;
use rustc_span::sym;
use rustc_middle::ty::{self, DefIdTree};

/// Wrapper fn for `CHARS_NEXT_CMP` and `CHARS_LAST_CMP` lints.
pub(super) fn check(
Expand All @@ -19,11 +18,9 @@ pub(super) fn check(
) -> bool {
if_chain! {
if let Some(args) = method_chain_args(info.chain, chain_methods);
if let hir::ExprKind::Call(fun, arg_char) = info.other.kind;
if arg_char.len() == 1;
if let hir::ExprKind::Path(ref qpath) = fun.kind;
if let Some(segment) = single_segment_path(qpath);
if segment.ident.name == sym::Some;
if let hir::ExprKind::Call(fun, [arg_char]) = info.other.kind;
if let Some(id) = path_def_id(cx, fun).and_then(|ctor_id| cx.tcx.parent(ctor_id));
if Some(id) == cx.tcx.lang_items().option_some_variant();
then {
let mut applicability = Applicability::MachineApplicable;
let self_ty = cx.typeck_results().expr_ty_adjusted(&args[0][0]).peel_refs();
Expand All @@ -42,7 +39,7 @@ pub(super) fn check(
if info.eq { "" } else { "!" },
snippet_with_applicability(cx, args[0][0].span, "..", &mut applicability),
suggest,
snippet_with_applicability(cx, arg_char[0].span, "..", &mut applicability)),
snippet_with_applicability(cx, arg_char.span, "..", &mut applicability)),
applicability,
);

Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/methods/manual_saturating_arithmetic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_qpath_def_path;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{match_def_path, path_def_id};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -93,12 +93,12 @@ fn is_min_or_max<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>) -> Option<M
let ty_str = ty.to_string();

// `std::T::MAX` `std::T::MIN` constants
if let hir::ExprKind::Path(path) = &expr.kind {
if is_qpath_def_path(cx, path, expr.hir_id, &["core", &ty_str, "MAX"][..]) {
if let Some(id) = path_def_id(cx, expr) {
if match_def_path(cx, id, &["core", &ty_str, "MAX"]) {
return Some(MinMax::Max);
}

if is_qpath_def_path(cx, path, expr.hir_id, &["core", &ty_str, "MIN"][..]) {
if match_def_path(cx, id, &["core", &ty_str, "MIN"]) {
return Some(MinMax::Min);
}
}
Expand Down
13 changes: 6 additions & 7 deletions clippy_lints/src/methods/option_map_or_none.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_lang_ctor, single_segment_path};
use clippy_utils::{is_lang_ctor, path_def_id};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_lint::LateContext;
use rustc_middle::ty::DefIdTree;
use rustc_span::symbol::sym;

use super::OPTION_MAP_OR_NONE;
Expand Down Expand Up @@ -76,13 +77,11 @@ pub(super) fn check<'tcx>(
if let hir::ExprKind::Closure(_, _, id, span, _) = map_arg.kind;
let arg_snippet = snippet(cx, span, "..");
let body = cx.tcx.hir().body(id);
if let Some((func, arg_char)) = reduce_unit_expression(cx, &body.value);
if arg_char.len() == 1;
if let hir::ExprKind::Path(ref qpath) = func.kind;
if let Some(segment) = single_segment_path(qpath);
if segment.ident.name == sym::Some;
if let Some((func, [arg_char])) = reduce_unit_expression(cx, &body.value);
if let Some(id) = path_def_id(cx, func).and_then(|ctor_id| cx.tcx.parent(ctor_id));
if Some(id) == cx.tcx.lang_items().option_some_variant();
then {
let func_snippet = snippet(cx, arg_char[0].span, "..");
let func_snippet = snippet(cx, arg_char.span, "..");
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
`map(..)` instead";
return span_lint_and_sugg(
Expand Down
Loading

0 comments on commit 3d43826

Please sign in to comment.