-// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
-// file at the top-level directory of this distribution.
-//
-// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
-// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
-// option. This file may not be copied, modified, or distributed
-// except according to those terms.
-
-
#![allow(clippy::default_hash_types)]
use crate::consts::{constant, Constant};
use crate::reexport::*;
-use crate::rustc::hir;
-use crate::rustc::hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor};
-use crate::rustc::hir::*;
-use crate::rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
-use crate::rustc::ty::layout::LayoutOf;
-use crate::rustc::ty::{self, Ty, TyCtxt, TypeckTables};
-use crate::rustc::{declare_tool_lint, lint_array};
-use crate::rustc_errors::Applicability;
-use crate::rustc_target::spec::abi::Abi;
-use crate::rustc_typeck::hir_ty_to_ty;
-use crate::syntax::ast::{FloatTy, IntTy, UintTy};
-use crate::syntax::errors::DiagnosticBuilder;
-use crate::syntax::source_map::Span;
use crate::utils::paths;
use crate::utils::{
clip, comparisons, differing_macro_contexts, higher, in_constant, in_macro, int_bits, last_path_segment,
match_def_path, match_path, multispan_sugg, opt_def_id, same_tys, sext, snippet, snippet_opt,
snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext,
- AbsolutePathBuffer
+ AbsolutePathBuffer,
};
use if_chain::if_chain;
+use rustc::hir;
+use rustc::hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor};
+use rustc::hir::*;
+use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
+use rustc::ty::layout::LayoutOf;
+use rustc::ty::{self, Ty, TyCtxt, TypeckTables};
+use rustc::{declare_tool_lint, lint_array};
+use rustc_errors::Applicability;
+use rustc_target::spec::abi::Abi;
+use rustc_typeck::hir_ty_to_ty;
use std::borrow::Cow;
use std::cmp::Ordering;
use std::collections::BTreeMap;
+use syntax::ast::{FloatTy, IntTy, UintTy};
+use syntax::errors::DiagnosticBuilder;
+use syntax::source_map::Span;
/// Handles all the linting of funky types
pub struct TypePass;
"usage of `Box<Vec<T>>`, vector elements are already on the heap"
}
+/// **What it does:** Checks for use of `Vec<Box<T>>` where T: Sized anywhere in the code.
+///
+/// **Why is this bad?** `Vec` already keeps its contents in a separate area on
+/// the heap. So if you `Box` its contents, you just add another level of indirection.
+///
+/// **Known problems:** Vec<Box<T: Sized>> makes sense if T is a large type (see #3530,
+/// 1st comment).
+///
+/// **Example:**
+/// ```rust
+/// struct X {
+/// values: Vec<Box<i32>>,
+/// }
+/// ```
+///
+/// Better:
+///
+/// ```rust
+/// struct X {
+/// values: Vec<i32>,
+/// }
+/// ```
+declare_clippy_lint! {
+ pub VEC_BOX,
+ complexity,
+ "usage of `Vec<Box<T>>` where T: Sized, vector elements are already on the heap"
+}
+
/// **What it does:** Checks for use of `Option<Option<_>>` in function signatures and type
/// definitions
///
impl LintPass for TypePass {
fn get_lints(&self) -> LintArray {
- lint_array!(BOX_VEC, OPTION_OPTION, LINKEDLIST, BORROWED_BOX)
+ lint_array!(BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX)
+ }
+
+ fn name(&self) -> &'static str {
+ "Types"
}
}
check_fn_decl(cx, decl);
}
- fn check_struct_field(&mut self, cx: &LateContext<'_, '_>, field: &StructField) {
+ fn check_struct_field(&mut self, cx: &LateContext<'_, '_>, field: &hir::StructField) {
check_ty(cx, &field.ty, false);
}
///
/// The parameter `is_local` distinguishes the context of the type; types from
/// local bindings should only be checked for the `BORROWED_BOX` lint.
-fn check_ty(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool) {
- if in_macro(ast_ty.span) {
+fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) {
+ if in_macro(hir_ty.span) {
return;
}
- match ast_ty.node {
+ match hir_ty.node {
TyKind::Path(ref qpath) if !is_local => {
- let hir_id = cx.tcx.hir().node_to_hir_id(ast_ty.id);
+ let hir_id = cx.tcx.hir().node_to_hir_id(hir_ty.id);
let def = cx.tables.qpath_def(qpath, hir_id);
if let Some(def_id) = opt_def_id(def) {
if Some(def_id) == cx.tcx.lang_items().owned_box() {
span_help_and_lint(
cx,
BOX_VEC,
- ast_ty.span,
+ hir_ty.span,
"you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
"`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.",
);
return; // don't recurse into the type
}
+ } else if match_def_path(cx.tcx, def_id, &paths::VEC) {
+ if_chain! {
+ // Get the _ part of Vec<_>
+ if let Some(ref last) = last_path_segment(qpath).args;
+ if let Some(ty) = last.args.iter().find_map(|arg| match arg {
+ GenericArg::Type(ty) => Some(ty),
+ GenericArg::Lifetime(_) => None,
+ });
+ // ty is now _ at this point
+ if let TyKind::Path(ref ty_qpath) = ty.node;
+ let def = cx.tables.qpath_def(ty_qpath, ty.hir_id);
+ if let Some(def_id) = opt_def_id(def);
+ if Some(def_id) == cx.tcx.lang_items().owned_box();
+ // At this point, we know ty is Box<T>, now get T
+ if let Some(ref last) = last_path_segment(ty_qpath).args;
+ if let Some(boxed_ty) = last.args.iter().find_map(|arg| match arg {
+ GenericArg::Type(ty) => Some(ty),
+ GenericArg::Lifetime(_) => None,
+ });
+ then {
+ let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty);
+ if ty_ty.is_sized(cx.tcx.at(ty.span), cx.param_env) {
+ span_lint_and_sugg(
+ cx,
+ VEC_BOX,
+ hir_ty.span,
+ "`Vec<T>` is already on the heap, the boxing is unnecessary.",
+ "try",
+ format!("Vec<{}>", ty_ty),
+ Applicability::MachineApplicable,
+ );
+ return; // don't recurse into the type
+ }
+ }
+ }
} else if match_def_path(cx.tcx, def_id, &paths::OPTION) {
if match_type_parameter(cx, qpath, &paths::OPTION) {
span_lint(
cx,
OPTION_OPTION,
- ast_ty.span,
+ hir_ty.span,
"consider using `Option<T>` instead of `Option<Option<T>>` or a custom \
enum if you need to distinguish all 3 cases",
);
span_help_and_lint(
cx,
LINKEDLIST,
- ast_ty.span,
+ hir_ty.span,
"I see you're using a LinkedList! Perhaps you meant some other data structure?",
"a VecDeque might work",
);
},
}
},
- TyKind::Rptr(ref lt, ref mut_ty) => check_ty_rptr(cx, ast_ty, is_local, lt, mut_ty),
+ TyKind::Rptr(ref lt, ref mut_ty) => check_ty_rptr(cx, hir_ty, is_local, lt, mut_ty),
// recurse
TyKind::Slice(ref ty) | TyKind::Array(ref ty, _) | TyKind::Ptr(MutTy { ref ty, .. }) => {
check_ty(cx, ty, is_local)
}
}
-fn check_ty_rptr(cx: &LateContext<'_, '_>, ast_ty: &hir::Ty, is_local: bool, lt: &Lifetime, mut_ty: &MutTy) {
+fn check_ty_rptr(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool, lt: &Lifetime, mut_ty: &MutTy) {
match mut_ty.ty.node {
TyKind::Path(ref qpath) => {
let hir_id = cx.tcx.hir().node_to_hir_id(mut_ty.ty.id);
span_lint_and_sugg(
cx,
BORROWED_BOX,
- ast_ty.span,
+ hir_ty.span,
"you seem to be trying to use `&Box<T>`. Consider using just `&T`",
"try",
format!(
"creating a let binding to a value of unit type, which usually can't be used afterwards"
}
-fn check_let_unit(cx: &LateContext<'_, '_>, decl: &Decl) {
- if let DeclKind::Local(ref local) = decl.node {
- if is_unit(cx.tables.pat_ty(&local.pat)) {
- if in_external_macro(cx.sess(), decl.span) || in_macro(local.pat.span) {
- return;
- }
- if higher::is_from_for_desugar(decl) {
- return;
- }
- span_lint(
- cx,
- LET_UNIT_VALUE,
- decl.span,
- &format!(
- "this let-binding has unit value. Consider omitting `let {} =`",
- snippet(cx, local.pat.span, "..")
- ),
- );
- }
- }
-}
-
impl LintPass for LetPass {
fn get_lints(&self) -> LintArray {
lint_array!(LET_UNIT_VALUE)
}
+
+ fn name(&self) -> &'static str {
+ "LetUnitValue"
+ }
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetPass {
- fn check_decl(&mut self, cx: &LateContext<'a, 'tcx>, decl: &'tcx Decl) {
- check_let_unit(cx, decl)
+ fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) {
+ if let StmtKind::Local(ref local) = stmt.node {
+ if is_unit(cx.tables.pat_ty(&local.pat)) {
+ if in_external_macro(cx.sess(), stmt.span) || in_macro(local.pat.span) {
+ return;
+ }
+ if higher::is_from_for_desugar(local) {
+ return;
+ }
+ span_lint(
+ cx,
+ LET_UNIT_VALUE,
+ stmt.span,
+ &format!(
+ "this let-binding has unit value. Consider omitting `let {} =`",
+ snippet(cx, local.pat.span, "..")
+ ),
+ );
+ }
+ }
}
}
fn get_lints(&self) -> LintArray {
lint_array!(UNIT_CMP)
}
+
+ fn name(&self) -> &'static str {
+ "UnicCmp"
+ }
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitCmp {
fn get_lints(&self) -> LintArray {
lint_array!(UNIT_ARG)
}
+
+ fn name(&self) -> &'static str {
+ "UnitArg"
+ }
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnitArg {
if in_macro(expr.span) {
return;
}
+
+ // apparently stuff in the desugaring of `?` can trigger this
+ // so check for that here
+ // only the calls to `Try::from_error` is marked as desugared,
+ // so we need to check both the current Expr and its parent.
+ if is_questionmark_desugar_marked_call(expr) {
+ return;
+ }
+ if_chain! {
+ let map = &cx.tcx.hir();
+ let opt_parent_node = map.find(map.get_parent_node(expr.id));
+ if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node;
+ if is_questionmark_desugar_marked_call(parent_expr);
+ then {
+ return;
+ }
+ }
+
match expr.node {
ExprKind::Call(_, ref args) | ExprKind::MethodCall(_, _, ref args) => {
for arg in args {
if is_unit(cx.tables.expr_ty(arg)) && !is_unit_literal(arg) {
- let map = &cx.tcx.hir();
- // apparently stuff in the desugaring of `?` can trigger this
- // so check for that here
- // only the calls to `Try::from_error` is marked as desugared,
- // so we need to check both the current Expr and its parent.
- if !is_questionmark_desugar_marked_call(expr) {
- if_chain! {
- let opt_parent_node = map.find(map.get_parent_node(expr.id));
- if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node;
- if is_questionmark_desugar_marked_call(parent_expr);
- then {}
- else {
- // `expr` and `parent_expr` where _both_ not from
- // desugaring `?`, so lint
- span_lint_and_sugg(
- cx,
- UNIT_ARG,
- arg.span,
- "passing a unit value to a function",
- "if you intended to pass a unit value, use a unit literal instead",
- "()".to_string(),
- Applicability::MachineApplicable,
- );
- }
+ if let ExprKind::Match(.., match_source) = &arg.node {
+ if *match_source == MatchSource::TryDesugar {
+ continue;
}
}
+
+ span_lint_and_sugg(
+ cx,
+ UNIT_ARG,
+ arg.span,
+ "passing a unit value to a function",
+ "if you intended to pass a unit value, use a unit literal instead",
+ "()".to_string(),
+ Applicability::MachineApplicable,
+ );
}
}
},
}
fn is_questionmark_desugar_marked_call(expr: &Expr) -> bool {
- use crate::syntax_pos::hygiene::CompilerDesugaringKind;
+ use syntax_pos::hygiene::CompilerDesugaringKind;
if let ExprKind::Call(ref callee, _) = expr.node {
callee.span.is_compiler_desugaring(CompilerDesugaringKind::QuestionMark)
} else {
FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
)
}
+
+ fn name(&self) -> &'static str {
+ "Casts"
+ }
}
// Check if the given type is either `core::ffi::c_void` or
let mut apb = AbsolutePathBuffer { names: vec![] };
tcx.push_item_path(&mut apb, adt.did, false);
- if apb.names.is_empty() { return false }
+ if apb.names.is_empty() {
+ return false;
+ }
if apb.names[0] == "libc" || apb.names[0] == "core" && *apb.names.last().unwrap() == "c_void" {
- return true
+ return true;
}
}
false
let (cast_from, cast_to) = (cx.tables.expr_ty(ex), cx.tables.expr_ty(expr));
lint_fn_to_numeric_cast(cx, expr, ex, cast_from, cast_to);
if let ExprKind::Lit(ref lit) = ex.node {
- use crate::syntax::ast::{LitIntType, LitKind};
+ use syntax::ast::{LitIntType, LitKind};
match lit.node {
LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::FloatUnsuffixed(_) => {},
_ => {
fn get_lints(&self) -> LintArray {
lint_array!(TYPE_COMPLEXITY)
}
+
+ fn name(&self) -> &'static str {
+ "TypeComplexityPass"
+ }
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypeComplexityPass {
self.check_fndecl(cx, decl);
}
- fn check_struct_field(&mut self, cx: &LateContext<'a, 'tcx>, field: &'tcx StructField) {
+ fn check_struct_field(&mut self, cx: &LateContext<'a, 'tcx>, field: &'tcx hir::StructField) {
// enum variants are also struct fields now
self.check_type(cx, &field.ty);
}
fn get_lints(&self) -> LintArray {
lint_array!(CHAR_LIT_AS_U8)
}
+
+ fn name(&self) -> &'static str {
+ "CharLiteralAsU8"
+ }
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CharLitAsU8 {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
- use crate::syntax::ast::{LitKind, UintTy};
+ use syntax::ast::{LitKind, UintTy};
if let ExprKind::Cast(ref e, _) = expr.node {
if let ExprKind::Lit(ref l) = e.node {
fn get_lints(&self) -> LintArray {
lint_array!(ABSURD_EXTREME_COMPARISONS)
}
+
+ fn name(&self) -> &'static str {
+ "AbsurdExtremeComparisons"
+ }
}
enum ExtremeType {
fn get_lints(&self) -> LintArray {
lint_array!(INVALID_UPCAST_COMPARISONS)
}
+
+ fn name(&self) -> &'static str {
+ "InvalidUpcastComparisons"
+ }
}
#[derive(Copy, Clone, Debug, Eq)]
}
fn numeric_cast_precast_bounds<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<(FullInt, FullInt)> {
- use crate::syntax::ast::{IntTy, UintTy};
use std::*;
+ use syntax::ast::{IntTy, UintTy};
if let ExprKind::Cast(ref cast_exp, _) = expr.node {
let pre_cast_ty = cx.tables.expr_ty(cast_exp);
fn get_lints(&self) -> LintArray {
lint_array!(IMPLICIT_HASHER)
}
+
+ fn name(&self) -> &'static str {
+ "ImplicitHasher"
+ }
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher {
#[allow(clippy::cast_possible_truncation)]
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
- use crate::syntax_pos::BytePos;
+ use syntax_pos::BytePos;
fn suggestion<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir())
}
}
+
+/// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code.
+///
+/// **Why is this bad?** It’s basically guaranteed to be undefined behaviour.
+/// `UnsafeCell` is the only way to obtain aliasable data that is considered
+/// mutable.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// fn x(r: &i32) {
+/// unsafe {
+/// *(r as *const _ as *mut _) += 1;
+/// }
+/// }
+/// ```
+///
+/// Instead consider using interior mutability types.
+///
+/// ```rust
+/// fn x(r: &UnsafeCell<i32>) {
+/// unsafe {
+/// *r.get() += 1;
+/// }
+/// }
+/// ```
+declare_clippy_lint! {
+ pub CAST_REF_TO_MUT,
+ correctness,
+ "a cast of reference to a mutable pointer"
+}
+
+pub struct RefToMut;
+
+impl LintPass for RefToMut {
+ fn get_lints(&self) -> LintArray {
+ lint_array!(CAST_REF_TO_MUT)
+ }
+
+ fn name(&self) -> &'static str {
+ "RefToMut"
+ }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RefToMut {
+ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+ if_chain! {
+ if let ExprKind::Unary(UnOp::UnDeref, e) = &expr.node;
+ if let ExprKind::Cast(e, t) = &e.node;
+ if let TyKind::Ptr(MutTy { mutbl: Mutability::MutMutable, .. }) = t.node;
+ if let ExprKind::Cast(e, t) = &e.node;
+ if let TyKind::Ptr(MutTy { mutbl: Mutability::MutImmutable, .. }) = t.node;
+ if let ty::Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty;
+ then {
+ span_lint(
+ cx,
+ CAST_REF_TO_MUT,
+ expr.span,
+ "casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell",
+ );
+ }
+ }
+ }
+}
projects / rust.git / blobdiff