squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Merge branch 'master' into rustfmt_tests

Browse source
This commit is contained in:
Matthias Krüger 2018-12-11 01:41:59 +01:00
parent 625ca772b5 0fd7fe9a77
commit 743e9e3561
9 changed files with 151 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
CONTRIBUTING.md
View file

@ -23,6 +23,7 @@ All contributors are expected to follow the [Rust Code of Conduct](http://www.ru
* [How Clippy works](#how-clippy-works)
* [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
* [Issue and PR Triage](#issue-and-pr-triage)
* [Bors and Homu](#bors-and-homu)
* [Contributions](#contributions)
## Getting started
@ -156,7 +157,7 @@ to style guidelines. The code has to be formatted by `rustfmt` before a PR will
It can be installed via `rustup`:
```bash
rustup component add rustfmt-preview
rustup component add rustfmt
```
Use `cargo fmt --all` to format the whole codebase.
@ -220,7 +221,7 @@ That's why the `else_if_without_else` example uses the `register_early_lint_pass
### Fixing build failures caused by Rust
Clippy will sometimes break because it still depends on unstable internal Rust features. Most of the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures caused by Rust updates, can be a good way to learn about Rust internals.
Clippy will sometimes fail to build from source because building it depends on unstable internal Rust features. Most of the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures caused by Rust updates, can be a good way to learn about Rust internals.
In order to find out why Clippy does not work properly with a new Rust commit, you can use the [rust-toolstate commit history][toolstate_commit_history].
You will then have to look for the last commit that contains `test-pass -> build-fail` or `test-pass` -> `test-fail` for the `clippy-driver` component. [Here][toolstate_commit] is an example.
@ -257,6 +258,17 @@ Our highest priority is fixing [crashes][l-crash] and [bugs][l-bug]. We don't
want Clippy to crash on your code and we want it to be as reliable as the
suggestions from Rust compiler errors.
## Bors and Homu
We use a bot powered by [Homu][homu] to help automate testing and landing of pull
requests in Clippy. The bot's username is @bors.
You can find the Clippy bors queue [here][homu_queue].
If you have @bors permissions, you can find an overview of the available
commands [here][homu_instructions].
## Contributions
Contributions to Clippy should be made in the form of GitHub pull requests. Each pull request will
@ -288,3 +300,6 @@ or the [MIT](http://opensource.org/licenses/MIT) license.
[triage]: https://forge.rust-lang.org/triage-procedure.html
[l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash%20%3Aboom%3A
[l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug%20%3Abeetle%3A
[homu]: https://github.com/servo/homu
[homu_instructions]: https://buildbot2.rust-lang.org/homu/
[homu_queue]: https://buildbot2.rust-lang.org/homu/queue/clippy

2
ci/base-tests.sh
View file

@ -20,6 +20,8 @@ fi
# build clippy in debug mode and run tests
cargo build --features debugging
cargo test --features debugging
# for faster build, share target dir between subcrates
export CARGO_TARGET_DIR=`pwd`/target/
cd clippy_lints && cargo test && cd ..
cd rustc_tools_util && cargo test && cd ..
cd clippy_dev && cargo test && cd ..

2
clippy_lints/src/attrs.rs
View file

@ -332,7 +332,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
&format!("unknown clippy lint: clippy::{}", name),
|db| {
if name.as_str().chars().any(|c| c.is_uppercase()) {
let name_lower = name.as_str().to_lowercase().to_string();
let name_lower = name.as_str().to_lowercase();
match lint_store.check_lint_name(
&name_lower,
Some(tool_name.as_str())

5
clippy_lints/src/excessive_precision.rs
View file

@ -109,10 +109,9 @@ impl ExcessivePrecision {
}
}
#[allow(clippy::doc_markdown)]
/// Should we exclude the float because it has a `.0` or `.` suffix
/// Ex 1_000_000_000.0
/// Ex 1_000_000_000.
/// Ex `1_000_000_000.0`
/// Ex `1_000_000_000.`
fn dot_zero_exclusion(s: &str) -> bool {
if let Some(after_dec) = s.split('.').nth(1) {
let mut decpart = after_dec.chars().take_while(|c| *c != 'e' || *c != 'E');

10
clippy_lints/src/no_effect.rs
View file

@ -57,7 +57,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
}
match expr.node {
ExprKind::Lit(..) | ExprKind::Closure(.., _) => true,
ExprKind::Path(..) => !has_drop(cx, expr),
ExprKind::Path(..) => !has_drop(cx, cx.tables.expr_ty(expr)),
ExprKind::Index(ref a, ref b) | ExprKind::Binary(_, ref a, ref b) => {
has_no_effect(cx, a) && has_no_effect(cx, b)
},
@ -70,7 +70,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
| ExprKind::AddrOf(_, ref inner)
| ExprKind::Box(ref inner) => has_no_effect(cx, inner),
ExprKind::Struct(_, ref fields, ref base) => {
!has_drop(cx, expr)
!has_drop(cx, cx.tables.expr_ty(expr))
&& fields.iter().all(|field| has_no_effect(cx, &field.expr))
&& match *base {
Some(ref base) => has_no_effect(cx, base),
@ -82,7 +82,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
let def = cx.tables.qpath_def(qpath, callee.hir_id);
match def {
Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..) => {
!has_drop(cx, expr) && args.iter().all(|arg| has_no_effect(cx, arg))
!has_drop(cx, cx.tables.expr_ty(expr)) && args.iter().all(|arg| has_no_effect(cx, arg))
},
_ => false,
}
@ -161,7 +161,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<Vec
| ExprKind::AddrOf(_, ref inner)
| ExprKind::Box(ref inner) => reduce_expression(cx, inner).or_else(|| Some(vec![inner])),
ExprKind::Struct(_, ref fields, ref base) => {
if has_drop(cx, expr) {
if has_drop(cx, cx.tables.expr_ty(expr)) {
None
} else {
Some(fields.iter().map(|f| &f.expr).chain(base).map(Deref::deref).collect())
@ -172,7 +172,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<Vec
let def = cx.tables.qpath_def(qpath, callee.hir_id);
match def {
Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..)
if !has_drop(cx, expr) =>
if !has_drop(cx, cx.tables.expr_ty(expr)) =>
{
Some(args.iter().collect())
},

107
clippy_lints/src/redundant_clone.rs
View file

@ -12,7 +12,7 @@ use crate::rustc::hir::{def_id, Body, FnDecl};
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::mir::{
self, traversal,
visit::{MutatingUseContext, NonUseContext, PlaceContext, Visitor},
visit::{MutatingUseContext, PlaceContext, Visitor},
TerminatorKind,
};
use crate::rustc::ty;
@ -23,10 +23,11 @@ use crate::syntax::{
source_map::{BytePos, Span},
};
use crate::utils::{
in_macro, is_copy, match_def_path, match_type, paths, snippet_opt, span_lint_node, span_lint_node_and_then,
walk_ptrs_ty_depth,
has_drop, in_macro, is_copy, match_def_path, match_type, paths, snippet_opt, span_lint_node,
span_lint_node_and_then, walk_ptrs_ty_depth,
};
use if_chain::if_chain;
use matches::matches;
use std::convert::TryFrom;
macro_rules! unwrap_or_continue {
@ -126,7 +127,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
// _1 in MIR `{ _2 = &_1; clone(move _2); }` or `{ _2 = _1; to_path_buf(_2); } (from_deref)
// In case of `from_deref`, `arg` is already a reference since it is `deref`ed in the previous
// block.
let cloned = unwrap_or_continue!(find_stmt_assigns_to(arg, from_borrow, bbdata.statements.iter().rev()));
let (cloned, cannot_move_out) = unwrap_or_continue!(find_stmt_assigns_to(
cx,
mir,
arg,
from_borrow,
bbdata.statements.iter()
));
if from_borrow && cannot_move_out {
continue;
}
// _1 in MIR `{ _2 = &_1; _3 = deref(move _2); } -> { _4 = _3; to_path_buf(move _4); }`
let referent = if from_deref {
@ -150,7 +161,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
}
};
unwrap_or_continue!(find_stmt_assigns_to(pred_arg, true, mir[ps[0]].statements.iter().rev()))
let (local, cannot_move_out) = unwrap_or_continue!(find_stmt_assigns_to(
cx,
mir,
pred_arg,
true,
mir[ps[0]].statements.iter()
));
if cannot_move_out {
continue;
}
local
} else {
cloned
};
@ -227,27 +248,69 @@ fn is_call_with_ref_arg<'tcx>(
}
}
/// Finds the first `to = (&)from`, and returns `Some(from)`.
type CannotMoveOut = bool;
/// Finds the first `to = (&)from`, and returns
/// ``Some((from, [`true` if `from` cannot be moved out]))``.
fn find_stmt_assigns_to<'a, 'tcx: 'a>(
cx: &LateContext<'_, 'tcx>,
mir: &mir::Mir<'tcx>,
to: mir::Local,
by_ref: bool,
mut stmts: impl Iterator<Item = &'a mir::Statement<'tcx>>,
) -> Option<mir::Local> {
stmts.find_map(|stmt| {
if let mir::StatementKind::Assign(mir::Place::Local(local), v) = &stmt.kind {
if *local == to {
if by_ref {
if let mir::Rvalue::Ref(_, _, mir::Place::Local(r)) = **v {
return Some(r);
}
} else if let mir::Rvalue::Use(mir::Operand::Copy(mir::Place::Local(r))) = **v {
return Some(r);
stmts: impl DoubleEndedIterator<Item = &'a mir::Statement<'tcx>>,
) -> Option<(mir::Local, CannotMoveOut)> {
stmts
.rev()
.find_map(|stmt| {
if let mir::StatementKind::Assign(mir::Place::Local(local), v) = &stmt.kind {
if *local == to {
return Some(v);
}
}
}
None
})
None
})
.and_then(|v| {
if by_ref {
if let mir::Rvalue::Ref(_, _, ref place) = **v {
return base_local_and_movability(cx, mir, place);
}
} else if let mir::Rvalue::Use(mir::Operand::Copy(ref place)) = **v {
return base_local_and_movability(cx, mir, place);
}
None
})
}
/// Extracts and returns the undermost base `Local` of given `place`. Returns `place` itself
/// if it is already a `Local`.
///
/// Also reports whether given `place` cannot be moved out.
fn base_local_and_movability<'tcx>(
cx: &LateContext<'_, 'tcx>,
mir: &mir::Mir<'tcx>,
mut place: &mir::Place<'tcx>,
) -> Option<(mir::Local, CannotMoveOut)> {
use rustc::mir::Place::*;
// Dereference. You cannot move things out from a borrowed value.
let mut deref = false;
// Accessing a field of an ADT that has `Drop`. Moving the field out will cause E0509.
let mut field = false;
loop {
match place {
Local(local) => return Some((*local, deref || field)),
Projection(proj) => {
place = &proj.base;
deref = deref || matches!(proj.elem, mir::ProjectionElem::Deref);
if !field && matches!(proj.elem, mir::ProjectionElem::Field(..)) {
field = has_drop(cx, place.ty(&mir.local_decls, cx.tcx).to_ty(cx.tcx));
}
},
_ => return None,
}
}
}
struct LocalUseVisitor {
@ -279,9 +342,7 @@ impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor {
fn visit_local(&mut self, local: &mir::Local, ctx: PlaceContext<'tcx>, _: mir::Location) {
match ctx {
PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(NonUseContext::StorageDead) => {
return;
},
PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_) => return,
_ => {},
}

5
clippy_lints/src/utils/mod.rs
View file

@ -266,9 +266,8 @@ pub fn implements_trait<'a, 'tcx>(
}
/// Check whether this type implements Drop.
pub fn has_drop(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
let struct_ty = cx.tables.expr_ty(expr);
match struct_ty.ty_adt_def() {
pub fn has_drop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
match ty.ty_adt_def() {
Some(def) => def.has_dtor(cx.tcx),
_ => false,
}

23
tests/ui/redundant_clone.rs
View file

@ -35,14 +35,33 @@ fn main() {
// Check that lint level works
#[allow(clippy::redundant_clone)]
let _ = String::new().to_string();
let tup = (String::from("foo"),);
let _ = tup.0.clone();
let tup_ref = &(String::from("foo"),);
let _s = tup_ref.0.clone(); // this `.clone()` cannot be removed
}
#[derive(Clone)]
struct Alpha;
fn double(a: Alpha) -> (Alpha, Alpha) {
if true {
fn with_branch(a: Alpha, b: bool) -> (Alpha, Alpha) {
if b {
(a.clone(), a.clone())
} else {
(Alpha, a)
}
}
struct TypeWithDrop {
x: String,
}
impl Drop for TypeWithDrop {
fn drop(&mut self) {}
}
fn cannot_move_from_type_with_drop() -> String {
let s = TypeWithDrop { x: String::new() };
s.x.clone() // removing this `clone()` summons E0509
}

22
tests/ui/redundant_clone.stderr
View file

@ -96,16 +96,28 @@ note: this value is dropped without further use
| ^^^^^^^^^^^^^^^
error: redundant clone
--> $DIR/redundant_clone.rs:44:22
--> $DIR/redundant_clone.rs:40:18
|
44 | (a.clone(), a.clone())
40 | let _ = tup.0.clone();
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
--> $DIR/redundant_clone.rs:40:13
|
40 | let _ = tup.0.clone();
| ^^^^^
error: redundant clone
--> $DIR/redundant_clone.rs:50:22
|
50 | (a.clone(), a.clone())
| ^^^^^^^^ help: remove this
|
note: this value is dropped without further use
--> $DIR/redundant_clone.rs:44:21
--> $DIR/redundant_clone.rs:50:21
|
44 | (a.clone(), a.clone())
50 | (a.clone(), a.clone())
| ^
error: aborting due to 9 previous errors
error: aborting due to 10 previous errors