mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-27 17:24:06 +00:00
extract is_interior_mutable_type
from [mut_key
] to clippy_utils::ty
;
fix configuration of [`ifs_same_cond`]; add some style improvement for [`ifs_same_cond`];
This commit is contained in:
parent
f0ae2b71ca
commit
f4ccb06d69
@ -1,6 +1,6 @@
|
|||||||
use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then};
|
use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then};
|
||||||
use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt};
|
use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt};
|
||||||
use clippy_utils::ty::needs_ordered_drop;
|
use clippy_utils::ty::{is_interior_mut_ty, needs_ordered_drop};
|
||||||
use clippy_utils::visitors::for_each_expr;
|
use clippy_utils::visitors::for_each_expr;
|
||||||
use clippy_utils::{
|
use clippy_utils::{
|
||||||
capture_local_usage, def_path_def_ids, eq_expr_value, find_binding_init, get_enclosing_block, hash_expr, hash_stmt,
|
capture_local_usage, def_path_def_ids, eq_expr_value, find_binding_init, get_enclosing_block, hash_expr, hash_stmt,
|
||||||
@ -8,9 +8,8 @@ use clippy_utils::{
|
|||||||
};
|
};
|
||||||
use core::iter;
|
use core::iter;
|
||||||
use core::ops::ControlFlow;
|
use core::ops::ControlFlow;
|
||||||
use rustc_data_structures::fx::FxHashSet;
|
|
||||||
use rustc_errors::Applicability;
|
use rustc_errors::Applicability;
|
||||||
use rustc_hir::def_id::DefId;
|
use rustc_hir::def_id::DefIdSet;
|
||||||
use rustc_hir::intravisit;
|
use rustc_hir::intravisit;
|
||||||
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, HirIdSet, Stmt, StmtKind};
|
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, HirIdSet, Stmt, StmtKind};
|
||||||
use rustc_lint::{LateContext, LateLintPass};
|
use rustc_lint::{LateContext, LateLintPass};
|
||||||
@ -164,12 +163,14 @@ declare_clippy_lint! {
|
|||||||
|
|
||||||
pub struct CopyAndPaste {
|
pub struct CopyAndPaste {
|
||||||
ignore_interior_mutability: Vec<String>,
|
ignore_interior_mutability: Vec<String>,
|
||||||
|
ignored_ty_ids: DefIdSet,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl CopyAndPaste {
|
impl CopyAndPaste {
|
||||||
pub fn new(ignore_interior_mutability: Vec<String>) -> Self {
|
pub fn new(ignore_interior_mutability: Vec<String>) -> Self {
|
||||||
Self {
|
Self {
|
||||||
ignore_interior_mutability,
|
ignore_interior_mutability,
|
||||||
|
ignored_ty_ids: DefIdSet::new(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -182,17 +183,18 @@ impl_lint_pass!(CopyAndPaste => [
|
|||||||
]);
|
]);
|
||||||
|
|
||||||
impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
|
impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
|
||||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
fn check_crate(&mut self, cx: &LateContext<'tcx>) {
|
||||||
if !expr.span.from_expansion() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) {
|
|
||||||
let (conds, blocks) = if_sequence(expr);
|
|
||||||
let mut ignored_ty_ids = FxHashSet::default();
|
|
||||||
for ignored_ty in &self.ignore_interior_mutability {
|
for ignored_ty in &self.ignore_interior_mutability {
|
||||||
let path: Vec<&str> = ignored_ty.split("::").collect();
|
let path: Vec<&str> = ignored_ty.split("::").collect();
|
||||||
for id in def_path_def_ids(cx, path.as_slice()) {
|
for id in def_path_def_ids(cx, path.as_slice()) {
|
||||||
ignored_ty_ids.insert(id);
|
self.ignored_ty_ids.insert(id);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
lint_same_cond(cx, &conds, &ignored_ty_ids);
|
}
|
||||||
|
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||||
|
if !expr.span.from_expansion() && matches!(expr.kind, ExprKind::If(..)) && !is_else_clause(cx.tcx, expr) {
|
||||||
|
let (conds, blocks) = if_sequence(expr);
|
||||||
|
lint_same_cond(cx, &conds, &self.ignored_ty_ids);
|
||||||
lint_same_fns_in_if_cond(cx, &conds);
|
lint_same_fns_in_if_cond(cx, &conds);
|
||||||
let all_same =
|
let all_same =
|
||||||
!is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && lint_if_same_then_else(cx, &conds, &blocks);
|
!is_lint_allowed(cx, IF_SAME_THEN_ELSE, expr.hir_id) && lint_if_same_then_else(cx, &conds, &blocks);
|
||||||
@ -569,38 +571,30 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
fn method_caller_is_ignored_or_mutable(
|
fn method_caller_is_mutable(cx: &LateContext<'_>, caller_expr: &Expr<'_>, ignored_ty_ids: &DefIdSet) -> bool {
|
||||||
cx: &LateContext<'_>,
|
|
||||||
caller_expr: &Expr<'_>,
|
|
||||||
ignored_ty_ids: &FxHashSet<DefId>,
|
|
||||||
) -> bool {
|
|
||||||
let caller_ty = cx.typeck_results().expr_ty(caller_expr);
|
let caller_ty = cx.typeck_results().expr_ty(caller_expr);
|
||||||
let is_ignored_ty = if let Some(adt_id) = caller_ty.ty_adt_id() && ignored_ty_ids.contains(&adt_id) {
|
// Check if given type has inner mutability and was not set to ignored by the configuration
|
||||||
true
|
let is_inner_mut_ty = is_interior_mut_ty(cx, caller_ty)
|
||||||
} else {
|
&& !matches!(caller_ty.ty_adt_id(), Some(adt_id) if ignored_ty_ids.contains(&adt_id));
|
||||||
false
|
|
||||||
};
|
|
||||||
|
|
||||||
if is_ignored_ty
|
is_inner_mut_ty
|
||||||
|| caller_ty.is_mutable_ptr()
|
|| caller_ty.is_mutable_ptr()
|
||||||
|
// `find_binding_init` will return the binding iff its not mutable
|
||||||
|| path_to_local(caller_expr)
|
|| path_to_local(caller_expr)
|
||||||
.and_then(|hid| find_binding_init(cx, hid))
|
.and_then(|hid| find_binding_init(cx, hid))
|
||||||
.is_none()
|
.is_none()
|
||||||
{
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
false
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Implementation of `IFS_SAME_COND`.
|
/// Implementation of `IFS_SAME_COND`.
|
||||||
fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>], ignored_ty_ids: &FxHashSet<DefId>) {
|
fn lint_same_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>], ignored_ty_ids: &DefIdSet) {
|
||||||
for (i, j) in search_same(
|
for (i, j) in search_same(
|
||||||
conds,
|
conds,
|
||||||
|e| hash_expr(cx, e),
|
|e| hash_expr(cx, e),
|
||||||
|lhs, rhs| {
|
|lhs, rhs| {
|
||||||
|
// Ignore eq_expr side effects iff one of the expressin kind is a method call
|
||||||
|
// and the caller is not a mutable, including inner mutable type.
|
||||||
if let ExprKind::MethodCall(_, caller, _, _) = lhs.kind {
|
if let ExprKind::MethodCall(_, caller, _, _) = lhs.kind {
|
||||||
if method_caller_is_ignored_or_mutable(cx, caller, ignored_ty_ids) {
|
if method_caller_is_mutable(cx, caller, ignored_ty_ids) {
|
||||||
false
|
false
|
||||||
} else {
|
} else {
|
||||||
SpanlessEq::new(cx).eq_expr(lhs, rhs)
|
SpanlessEq::new(cx).eq_expr(lhs, rhs)
|
||||||
|
@ -1,10 +1,11 @@
|
|||||||
use clippy_utils::diagnostics::span_lint;
|
use clippy_utils::diagnostics::span_lint;
|
||||||
|
use clippy_utils::ty::is_interior_mut_ty;
|
||||||
use clippy_utils::{def_path_def_ids, trait_ref_of_method};
|
use clippy_utils::{def_path_def_ids, trait_ref_of_method};
|
||||||
use rustc_data_structures::fx::FxHashSet;
|
use rustc_data_structures::fx::FxHashSet;
|
||||||
use rustc_hir as hir;
|
use rustc_hir as hir;
|
||||||
use rustc_lint::{LateContext, LateLintPass};
|
use rustc_lint::{LateContext, LateLintPass};
|
||||||
use rustc_middle::ty::TypeVisitableExt;
|
use rustc_middle::query::Key;
|
||||||
use rustc_middle::ty::{Adt, Array, Ref, Slice, Tuple, Ty};
|
use rustc_middle::ty::{Adt, Ty};
|
||||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||||
use rustc_span::def_id::LocalDefId;
|
use rustc_span::def_id::LocalDefId;
|
||||||
use rustc_span::source_map::Span;
|
use rustc_span::source_map::Span;
|
||||||
@ -153,53 +154,18 @@ impl MutableKeyType {
|
|||||||
let is_keyed_type = [sym::HashMap, sym::BTreeMap, sym::HashSet, sym::BTreeSet]
|
let is_keyed_type = [sym::HashMap, sym::BTreeMap, sym::HashSet, sym::BTreeSet]
|
||||||
.iter()
|
.iter()
|
||||||
.any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did()));
|
.any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did()));
|
||||||
if is_keyed_type && self.is_interior_mutable_type(cx, substs.type_at(0)) {
|
if !is_keyed_type {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let subst_ty = substs.type_at(0);
|
||||||
|
// Determines if a type contains interior mutability which would affect its implementation of
|
||||||
|
// [`Hash`] or [`Ord`].
|
||||||
|
if is_interior_mut_ty(cx, subst_ty)
|
||||||
|
&& !matches!(subst_ty.ty_adt_id(), Some(adt_id) if self.ignore_mut_def_ids.contains(&adt_id))
|
||||||
|
{
|
||||||
span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
|
span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Determines if a type contains interior mutability which would affect its implementation of
|
|
||||||
/// [`Hash`] or [`Ord`].
|
|
||||||
fn is_interior_mutable_type<'tcx>(&self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
|
|
||||||
match *ty.kind() {
|
|
||||||
Ref(_, inner_ty, mutbl) => mutbl == hir::Mutability::Mut || self.is_interior_mutable_type(cx, inner_ty),
|
|
||||||
Slice(inner_ty) => self.is_interior_mutable_type(cx, inner_ty),
|
|
||||||
Array(inner_ty, size) => {
|
|
||||||
size.try_eval_target_usize(cx.tcx, cx.param_env)
|
|
||||||
.map_or(true, |u| u != 0)
|
|
||||||
&& self.is_interior_mutable_type(cx, inner_ty)
|
|
||||||
},
|
|
||||||
Tuple(fields) => fields.iter().any(|ty| self.is_interior_mutable_type(cx, ty)),
|
|
||||||
Adt(def, substs) => {
|
|
||||||
// Special case for collections in `std` who's impl of `Hash` or `Ord` delegates to
|
|
||||||
// that of their type parameters. Note: we don't include `HashSet` and `HashMap`
|
|
||||||
// because they have no impl for `Hash` or `Ord`.
|
|
||||||
let def_id = def.did();
|
|
||||||
let is_std_collection = [
|
|
||||||
sym::Option,
|
|
||||||
sym::Result,
|
|
||||||
sym::LinkedList,
|
|
||||||
sym::Vec,
|
|
||||||
sym::VecDeque,
|
|
||||||
sym::BTreeMap,
|
|
||||||
sym::BTreeSet,
|
|
||||||
sym::Rc,
|
|
||||||
sym::Arc,
|
|
||||||
]
|
|
||||||
.iter()
|
|
||||||
.any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def_id));
|
|
||||||
let is_box = Some(def_id) == cx.tcx.lang_items().owned_box();
|
|
||||||
if is_std_collection || is_box || self.ignore_mut_def_ids.contains(&def_id) {
|
|
||||||
// The type is mutable if any of its type parameters are
|
|
||||||
substs.types().any(|ty| self.is_interior_mutable_type(cx, ty))
|
|
||||||
} else {
|
|
||||||
!ty.has_escaping_bound_vars()
|
|
||||||
&& cx.tcx.layout_of(cx.param_env.and(ty)).is_ok()
|
|
||||||
&& !ty.is_freeze(cx.tcx, cx.param_env)
|
|
||||||
}
|
|
||||||
},
|
|
||||||
_ => false,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
@ -1121,3 +1121,47 @@ pub fn make_normalized_projection<'tcx>(
|
|||||||
}
|
}
|
||||||
helper(tcx, param_env, make_projection(tcx, container_id, assoc_ty, substs)?)
|
helper(tcx, param_env, make_projection(tcx, container_id, assoc_ty, substs)?)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Check if given type has inner mutability such as [`std::cell::Cell`] or [`std::cell::RefCell`]
|
||||||
|
/// etc.
|
||||||
|
pub fn is_interior_mut_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
|
||||||
|
match *ty.kind() {
|
||||||
|
ty::Ref(_, inner_ty, mutbl) => mutbl == Mutability::Mut || is_interior_mut_ty(cx, inner_ty),
|
||||||
|
ty::Slice(inner_ty) => is_interior_mut_ty(cx, inner_ty),
|
||||||
|
ty::Array(inner_ty, size) => {
|
||||||
|
size.try_eval_target_usize(cx.tcx, cx.param_env)
|
||||||
|
.map_or(true, |u| u != 0)
|
||||||
|
&& is_interior_mut_ty(cx, inner_ty)
|
||||||
|
},
|
||||||
|
ty::Tuple(fields) => fields.iter().any(|ty| is_interior_mut_ty(cx, ty)),
|
||||||
|
ty::Adt(def, substs) => {
|
||||||
|
// Special case for collections in `std` who's impl of `Hash` or `Ord` delegates to
|
||||||
|
// that of their type parameters. Note: we don't include `HashSet` and `HashMap`
|
||||||
|
// because they have no impl for `Hash` or `Ord`.
|
||||||
|
let def_id = def.did();
|
||||||
|
let is_std_collection = [
|
||||||
|
sym::Option,
|
||||||
|
sym::Result,
|
||||||
|
sym::LinkedList,
|
||||||
|
sym::Vec,
|
||||||
|
sym::VecDeque,
|
||||||
|
sym::BTreeMap,
|
||||||
|
sym::BTreeSet,
|
||||||
|
sym::Rc,
|
||||||
|
sym::Arc,
|
||||||
|
]
|
||||||
|
.iter()
|
||||||
|
.any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def_id));
|
||||||
|
let is_box = Some(def_id) == cx.tcx.lang_items().owned_box();
|
||||||
|
if is_std_collection || is_box {
|
||||||
|
// The type is mutable if any of its type parameters are
|
||||||
|
substs.types().any(|ty| is_interior_mut_ty(cx, ty))
|
||||||
|
} else {
|
||||||
|
!ty.has_escaping_bound_vars()
|
||||||
|
&& cx.tcx.layout_of(cx.param_env.and(ty)).is_ok()
|
||||||
|
&& !ty.is_freeze(cx.tcx, cx.param_env)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@ -6,19 +6,13 @@ fn main() {}
|
|||||||
fn issue10272() {
|
fn issue10272() {
|
||||||
use std::cell::Cell;
|
use std::cell::Cell;
|
||||||
|
|
||||||
|
// Because the `ignore-interior-mutability` configuration
|
||||||
|
// is set to ignore for `std::cell::Cell`, the following `get()` calls
|
||||||
|
// should trigger warning
|
||||||
let x = Cell::new(true);
|
let x = Cell::new(true);
|
||||||
if x.get() {
|
if x.get() {
|
||||||
} else if !x.take() {
|
} else if !x.take() {
|
||||||
} else if x.get() {
|
} else if x.get() {
|
||||||
// ok, x is interior mutable type
|
|
||||||
} else {
|
|
||||||
}
|
|
||||||
|
|
||||||
let a = [Cell::new(true)];
|
|
||||||
if a[0].get() {
|
|
||||||
} else if a[0].take() {
|
|
||||||
} else if a[0].get() {
|
|
||||||
// ok, a contains interior mutable type
|
|
||||||
} else {
|
} else {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
15
tests/ui-toml/ifs_same_cond/ifs_same_cond.stderr
Normal file
15
tests/ui-toml/ifs_same_cond/ifs_same_cond.stderr
Normal file
@ -0,0 +1,15 @@
|
|||||||
|
error: this `if` has the same condition as a previous `if`
|
||||||
|
--> $DIR/ifs_same_cond.rs:15:15
|
||||||
|
|
|
||||||
|
LL | } else if x.get() {
|
||||||
|
| ^^^^^^^
|
||||||
|
|
|
||||||
|
note: same as this
|
||||||
|
--> $DIR/ifs_same_cond.rs:13:8
|
||||||
|
|
|
||||||
|
LL | if x.get() {
|
||||||
|
| ^^^^^^^
|
||||||
|
= note: `-D clippy::ifs-same-cond` implied by `-D warnings`
|
||||||
|
|
||||||
|
error: aborting due to previous error
|
||||||
|
|
@ -59,6 +59,14 @@ fn issue10272() {
|
|||||||
// ok, p is mutable pointer
|
// ok, p is mutable pointer
|
||||||
} else {
|
} else {
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let x = std::cell::Cell::new(true);
|
||||||
|
if x.get() {
|
||||||
|
} else if !x.take() {
|
||||||
|
} else if x.get() {
|
||||||
|
// ok, x is interior mutable type
|
||||||
|
} else {
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn main() {}
|
fn main() {}
|
||||||
|
Loading…
Reference in New Issue
Block a user