mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-23 21:23:56 +00:00
Add a lint to warn about call to .*or(foo(..))
This commit is contained in:
parent
604be945d2
commit
c6604bb281
5 changed files with 90 additions and 4 deletions
|
@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
|
|||
[Jump to usage instructions](#usage)
|
||||
|
||||
##Lints
|
||||
There are 92 lints included in this crate:
|
||||
There are 93 lints included in this crate:
|
||||
|
||||
name | default | meaning
|
||||
---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|
||||
|
@ -63,6 +63,7 @@ name
|
|||
[option_map_unwrap_or](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or) | warn | using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`
|
||||
[option_map_unwrap_or_else](https://github.com/Manishearth/rust-clippy/wiki#option_map_unwrap_or_else) | warn | using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)`
|
||||
[option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used) | allow | using `Option.unwrap()`, which should at least get a better message using `expect()`
|
||||
[or_fun_call](https://github.com/Manishearth/rust-clippy/wiki#or_fun_call) | warn | using any `*or` method when the `*or_else` would do
|
||||
[out_of_bounds_indexing](https://github.com/Manishearth/rust-clippy/wiki#out_of_bounds_indexing) | deny | out of bound constant indexing
|
||||
[panic_params](https://github.com/Manishearth/rust-clippy/wiki#panic_params) | warn | missing parameters in `panic!`
|
||||
[precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence) | warn | catches operations where precedence may be unclear. See the wiki for a list of cases caught
|
||||
|
|
|
@ -194,6 +194,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
|
|||
methods::OK_EXPECT,
|
||||
methods::OPTION_MAP_UNWRAP_OR,
|
||||
methods::OPTION_MAP_UNWRAP_OR_ELSE,
|
||||
methods::OR_FUN_CALL,
|
||||
methods::SEARCH_IS_SOME,
|
||||
methods::SHOULD_IMPLEMENT_TRAIT,
|
||||
methods::STR_TO_STRING,
|
||||
|
|
|
@ -4,6 +4,7 @@ use rustc::middle::ty;
|
|||
use rustc::middle::subst::{Subst, TypeSpace};
|
||||
use std::iter;
|
||||
use std::borrow::Cow;
|
||||
use syntax::ptr::P;
|
||||
|
||||
use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args, match_trait_method,
|
||||
walk_ptrs_ty_depth, walk_ptrs_ty};
|
||||
|
@ -170,6 +171,25 @@ declare_lint!(pub SEARCH_IS_SOME, Warn,
|
|||
"using an iterator search followed by `is_some()`, which is more succinctly \
|
||||
expressed as a call to `any()`");
|
||||
|
||||
/// **What it does:** This lint checks for calls to `.or(foo(..))`, `.unwrap_or(foo(..))`, etc., and
|
||||
/// suggests to use `or_else`, `unwrap_or_else`, etc., instead.
|
||||
///
|
||||
/// **Why is this bad?** The function will always be called and potentially allocate an object
|
||||
/// in expressions such as:
|
||||
/// ```rust
|
||||
/// foo.unwrap_or(String::new())
|
||||
/// ```
|
||||
/// this can instead be written:
|
||||
/// ```rust
|
||||
/// foo.unwrap_or_else(String::new)
|
||||
/// ```
|
||||
///
|
||||
/// **Known problems:** If the function as side-effects, not calling it will change the semantic of
|
||||
/// the program, but you shouldn't rely on that anyway. The will won't catch
|
||||
/// `foo.unwrap_or(vec![])`.
|
||||
declare_lint!(pub OR_FUN_CALL, Warn,
|
||||
"using any `*or` method when the `*or_else` would do");
|
||||
|
||||
impl LintPass for MethodsPass {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array!(OPTION_UNWRAP_USED,
|
||||
|
@ -181,13 +201,15 @@ impl LintPass for MethodsPass {
|
|||
WRONG_PUB_SELF_CONVENTION,
|
||||
OK_EXPECT,
|
||||
OPTION_MAP_UNWRAP_OR,
|
||||
OPTION_MAP_UNWRAP_OR_ELSE)
|
||||
OPTION_MAP_UNWRAP_OR_ELSE,
|
||||
OR_FUN_CALL)
|
||||
}
|
||||
}
|
||||
|
||||
impl LateLintPass for MethodsPass {
|
||||
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
|
||||
if let ExprMethodCall(_, _, _) = expr.node {
|
||||
if let ExprMethodCall(name, _, ref args) = expr.node {
|
||||
// Chain calls
|
||||
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
|
||||
lint_unwrap(cx, expr, arglists[0]);
|
||||
} else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
|
||||
|
@ -207,6 +229,8 @@ impl LateLintPass for MethodsPass {
|
|||
} else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
|
||||
lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
|
||||
}
|
||||
|
||||
lint_or_fun_call(cx, expr, &name.node.as_str(), &args);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -258,6 +282,39 @@ impl LateLintPass for MethodsPass {
|
|||
}
|
||||
}
|
||||
|
||||
/// Checks for the `OR_FUN_CALL` lint.
|
||||
fn lint_or_fun_call(cx: &LateContext, expr: &Expr, name: &str, args: &[P<Expr>]) {
|
||||
if args.len() == 2 && ["map_or", "ok_or", "or", "unwrap_or"].contains(&name) {
|
||||
let self_ty = cx.tcx.expr_ty(&args[0]);
|
||||
|
||||
let is_result = if match_type(cx, self_ty, &RESULT_PATH) {
|
||||
true
|
||||
}
|
||||
else if match_type(cx, self_ty, &OPTION_PATH) {
|
||||
false
|
||||
}
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
||||
if let ExprCall(ref fun, ref or_args) = args[1].node {
|
||||
let sugg = match (is_result, or_args.is_empty()) {
|
||||
(true, _) => format!("|_| {}", snippet(cx, args[1].span, "..")),
|
||||
(false, false) => format!("|| {}", snippet(cx, args[1].span, "..")),
|
||||
(false, true) => format!("{}", snippet(cx, fun.span, "..")),
|
||||
};
|
||||
|
||||
span_lint(cx, OR_FUN_CALL, expr.span,
|
||||
&format!("use of `{}` followed by a function call", name))
|
||||
.span_suggestion(expr.span, "try this",
|
||||
format!("{}.{}_else({})",
|
||||
snippet(cx, args[0].span, "_"),
|
||||
name,
|
||||
sugg));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(ptr_arg)]
|
||||
// Type of MethodArgs is potentially a Vec
|
||||
/// lint use of `unwrap()` for `Option`s and `Result`s
|
||||
|
|
|
@ -244,7 +244,7 @@ pub fn is_from_for_desugar(decl: &Decl) -> bool {
|
|||
/// snippet(cx, expr.span, "..")
|
||||
/// ```
|
||||
pub fn snippet<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow<'a, str> {
|
||||
cx.sess().codemap().span_to_snippet(span).map(From::from).unwrap_or(Cow::Borrowed(default))
|
||||
cx.sess().codemap().span_to_snippet(span).map(From::from).unwrap_or_else(|_| Cow::Borrowed(default))
|
||||
}
|
||||
|
||||
/// Convert a span to a code snippet. Returns `None` if not available.
|
||||
|
|
|
@ -175,6 +175,33 @@ fn search_is_some() {
|
|||
let _ = foo.rposition().is_some();
|
||||
}
|
||||
|
||||
/// Checks implementation of the OR_FUN_CALL lint
|
||||
fn or_fun_call() {
|
||||
let foo = Some(vec![1]);
|
||||
foo.unwrap_or(Vec::new());
|
||||
//~^ERROR use of `unwrap_or`
|
||||
//~|HELP try this
|
||||
//~|SUGGESTION foo.unwrap_or_else(Vec::new);
|
||||
|
||||
let bar = Some(vec![1]);
|
||||
bar.unwrap_or(Vec::with_capacity(12));
|
||||
//~^ERROR use of `unwrap_or`
|
||||
//~|HELP try this
|
||||
//~|SUGGESTION bar.unwrap_or_else(|| Vec::with_capacity(12));
|
||||
|
||||
let baz : Result<_, ()> = Ok(vec![1]);
|
||||
baz.unwrap_or(Vec::new());
|
||||
//~^ERROR use of `unwrap_or`
|
||||
//~|HELP try this
|
||||
//~|SUGGESTION baz.unwrap_or_else(|_| Vec::new());
|
||||
|
||||
let qux : Result<_, ()> = Ok(vec![1]);
|
||||
qux.unwrap_or(Vec::with_capacity(12));
|
||||
//~^ERROR use of `unwrap_or`
|
||||
//~|HELP try this
|
||||
//~|SUGGESTION qux.unwrap_or_else(|_| Vec::with_capacity(12));
|
||||
}
|
||||
|
||||
fn main() {
|
||||
use std::io;
|
||||
|
||||
|
|
Loading…
Reference in a new issue