dnrops
/
rust-clippy
mirror of https://github.com/rust-lang/rust-clippy.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1501 from scott-linder/types-borrow-box 

Types borrow box
This commit is contained in:
Oliver Schneider 2017-06-13 11:30:52 +02:00 committed by GitHub
parent 92fac4ac1a 5db8647c5e
commit 7056018335
8 changed files with 163 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -364,6 +364,7 @@ All notable changes to this project will be documented in this file.
[`block_in_if_condition_expr`]: https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr
[`block_in_if_condition_stmt`]: https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_stmt
[`bool_comparison`]: https://github.com/Manishearth/rust-clippy/wiki#bool_comparison
[`borrowed_box`]: https://github.com/Manishearth/rust-clippy/wiki#borrowed_box
[`box_vec`]: https://github.com/Manishearth/rust-clippy/wiki#box_vec
[`boxed_local`]: https://github.com/Manishearth/rust-clippy/wiki#boxed_local
[`builtin_type_shadow`]: https://github.com/Manishearth/rust-clippy/wiki#builtin_type_shadow

1
README.md
View File

@ -194,6 +194,7 @@ name
[block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr) | warn | braces that can be eliminated in conditions, e.g. `if { true } ...`
[block_in_if_condition_stmt](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_stmt) | warn | complex blocks in conditions, e.g. `if { let x = true; x } ...`
[bool_comparison](https://github.com/Manishearth/rust-clippy/wiki#bool_comparison) | warn | comparing a variable to a boolean, e.g. `if x == true`
[borrowed_box](https://github.com/Manishearth/rust-clippy/wiki#borrowed_box) | warn | a borrow of a boxed type
[box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec) | warn | usage of `Box<Vec<T>>`, vector elements are already on the heap
[boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local) | warn | using `Box<T>` where unnecessary
[builtin_type_shadow](https://github.com/Manishearth/rust-clippy/wiki#builtin_type_shadow) | warn | shadowing a builtin type

1
clippy_lints/src/lib.rs
View File

@ -506,6 +506,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
transmute::USELESS_TRANSMUTE,
transmute::WRONG_TRANSMUTE,
types::ABSURD_EXTREME_COMPARISONS,
types::BORROWED_BOX,
types::BOX_VEC,
types::CHAR_LIT_AS_U8,
types::LET_UNIT_VALUE,

97
clippy_lints/src/types.rs
View File

@ -8,7 +8,7 @@ use std::cmp::Ordering;
use syntax::ast::{IntTy, UintTy, FloatTy};
use syntax::attr::IntType;
use syntax::codemap::Span;
use utils::{comparisons, higher, in_external_macro, in_macro, match_def_path, snippet, span_help_and_lint, span_lint,
use utils::{comparisons, higher, in_external_macro, in_macro, match_def_path, snippet, span_help_and_lint, span_lint, span_lint_and_then,
opt_def_id, last_path_segment, type_size};
use utils::paths;
@ -65,9 +65,25 @@ declare_lint! {
structure like a VecDeque"
}
/// **What it does:** Checks for use of `&Box<T>` anywhere in the code.
///
/// **Why is this bad?** Any `&Box<T>` can also be a `&T`, which is more general.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn foo(bar: &Box<T>) { ... }
/// ```
declare_lint! {
pub BORROWED_BOX,
Warn,
"a borrow of a boxed type"
}
impl LintPass for TypePass {
fn get_lints(&self) -> LintArray {
lint_array!(BOX_VEC, LINKEDLIST)
lint_array!(BOX_VEC, LINKEDLIST, BORROWED_BOX)
}
}
@ -84,35 +100,46 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypePass {
}
fn check_struct_field(&mut self, cx: &LateContext, field: &StructField) {
check_ty(cx, &field.ty);
check_ty(cx, &field.ty, false);
}
fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) {
match item.node {
TraitItemKind::Const(ref ty, _) |
TraitItemKind::Type(_, Some(ref ty)) => check_ty(cx, ty),
TraitItemKind::Type(_, Some(ref ty)) => check_ty(cx, ty, false),
TraitItemKind::Method(ref sig, _) => check_fn_decl(cx, &sig.decl),
_ => (),
}
}
fn check_local(&mut self, cx: &LateContext, local: &Local) {
if let Some(ref ty) = local.ty {
check_ty(cx, ty, true);
}
}
}
fn check_fn_decl(cx: &LateContext, decl: &FnDecl) {
for input in &decl.inputs {
check_ty(cx, input);
check_ty(cx, input, false);
}
if let FunctionRetTy::Return(ref ty) = decl.output {
check_ty(cx, ty);
check_ty(cx, ty, false);
}
}
fn check_ty(cx: &LateContext, ast_ty: &hir::Ty) {
/// Recursively check for `TypePass` lints in the given type. Stop at the first
/// lint found.
///
/// 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) {
return;
}
match ast_ty.node {
TyPath(ref qpath) => {
TyPath(ref qpath) if !is_local => {
let def = cx.tables.qpath_def(qpath, ast_ty.id);
if let Some(def_id) = opt_def_id(def) {
if Some(def_id) == cx.tcx.lang_items.owned_box() {
@ -143,32 +170,70 @@ fn check_ty(cx: &LateContext, ast_ty: &hir::Ty) {
}
match *qpath {
QPath::Resolved(Some(ref ty), ref p) => {
check_ty(cx, ty);
check_ty(cx, ty, is_local);
for ty in p.segments.iter().flat_map(|seg| seg.parameters.types()) {
check_ty(cx, ty);
check_ty(cx, ty, is_local);
}
},
QPath::Resolved(None, ref p) => {
for ty in p.segments.iter().flat_map(|seg| seg.parameters.types()) {
check_ty(cx, ty);
check_ty(cx, ty, is_local);
}
},
QPath::TypeRelative(ref ty, ref seg) => {
check_ty(cx, ty);
check_ty(cx, ty, is_local);
for ty in seg.parameters.types() {
check_ty(cx, ty);
check_ty(cx, ty, is_local);
}
},
}
},
TyRptr(ref lt, MutTy { ref ty, ref mutbl }) => {
match ty.node {
TyPath(ref qpath) => {
let def = cx.tables.qpath_def(qpath, ast_ty.id);
if_let_chain! {[
let Some(def_id) = opt_def_id(def),
Some(def_id) == cx.tcx.lang_items.owned_box(),
let QPath::Resolved(None, ref path) = *qpath,
let [ref bx] = *path.segments,
let PathParameters::AngleBracketedParameters(ref ab_data) = bx.parameters,
let [ref inner] = *ab_data.types
], {
let ltopt = if lt.is_elided() {
"".to_owned()
} else {
format!("{} ", lt.name.as_str())
};
let mutopt = if *mutbl == Mutability::MutMutable {
"mut "
} else {
""
};
span_lint_and_then(cx,
BORROWED_BOX,
ast_ty.span,
"you seem to be trying to use `&Box<T>`. Consider using just `&T`",
|db| {
db.span_suggestion(ast_ty.span,
"try",
format!("&{}{}{}", ltopt, mutopt, &snippet(cx, inner.span, "..")));
}
);
return; // don't recurse into the type
}};
check_ty(cx, ty, is_local);
},
_ => check_ty(cx, ty, is_local),
}
},
// recurse
TySlice(ref ty) |
TyArray(ref ty, _) |
TyPtr(MutTy { ref ty, .. }) |
TyRptr(_, MutTy { ref ty, .. }) => check_ty(cx, ty),
TyPtr(MutTy { ref ty, .. }) => check_ty(cx, ty, is_local),
TyTup(ref tys) => {
for ty in tys {
check_ty(cx, ty);
check_ty(cx, ty, is_local);
}
},
_ => {},

34
clippy_tests/examples/borrow_box.rs Normal file
View File

@ -0,0 +1,34 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(borrowed_box)]
#![allow(blacklisted_name)]
#![allow(unused_variables)]
#![allow(dead_code)]
pub fn test1(foo: &mut Box<bool>) {
println!("{:?}", foo)
}
pub fn test2() {
let foo: &Box<bool>;
}
struct Test3<'a> {
foo: &'a Box<bool>
}
trait Test4 {
fn test4(a: &Box<bool>);
}
impl<'a> Test4 for Test3<'a> {
fn test4(a: &Box<bool>) {
unimplemented!();
}
}
fn main(){
test1(&mut Box::new(false));
test2();
}

35
clippy_tests/examples/borrow_box.stderr Normal file
View File

@ -0,0 +1,35 @@
error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
--> borrow_box.rs:9:19
|
9 | pub fn test1(foo: &mut Box<bool>) {
| ^^^^^^^^^^^^^^ help: try `&mut bool`
|
note: lint level defined here
--> borrow_box.rs:4:9
|
4 | #![deny(borrowed_box)]
| ^^^^^^^^^^^^
error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
--> borrow_box.rs:14:14
|
14 | let foo: &Box<bool>;
| ^^^^^^^^^^ help: try `&bool`
error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
--> borrow_box.rs:18:10
|
18 | foo: &'a Box<bool>
| ^^^^^^^^^^^^^ help: try `&'a bool`
error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
--> borrow_box.rs:22:17
|
22 | fn test4(a: &Box<bool>);
| ^^^^^^^^^^ help: try `&bool`
error: aborting due to previous error(s)
error: Could not compile `clippy_tests`.
To learn more, run the command again with --verbose.

5
clippy_tests/examples/box_vec.rs
View File

@ -22,8 +22,13 @@ pub fn test2(foo: Box<Fn(Vec<u32>)>) { // pass if #31 is fixed
foo(vec![1, 2, 3])
}
pub fn test_local_not_linted() {
let _: Box<Vec<bool>>;
}
fn main(){
test(Box::new(Vec::new()));
test2(Box::new(|v| println!("{:?}", v)));
test_macro();
test_local_not_linted();
}

5
clippy_tests/examples/dlist.rs
View File

@ -34,6 +34,11 @@ pub fn test_ret() -> Option<LinkedList<u8>> {
unimplemented!();
}
pub fn test_local_not_linted() {
let _: LinkedList<u8>;
}
fn main(){
test(LinkedList::new());
test_local_not_linted();
}