diff --git a/README.md b/README.md index 59a12e09b17b..08ad214e6903 100644 --- a/README.md +++ b/README.md @@ -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 112 lints included in this crate: +There are 113 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -104,6 +104,7 @@ name [string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String::to_string` which is inefficient [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg) | warn | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`. +[trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex) | warn | finds trivial regular expressions in `Regex::new(_)` invocations [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types; recommends factoring out parts into `type` definitions [unicode_not_nfc](https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc) | allow | using a unicode literal not in NFC normal form (see http://www.unicode.org/reports/tr15/ for further information) [unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp) | warn | comparing unit values (which is always `true` or `false`, respectively) diff --git a/src/cyclomatic_complexity.rs b/src/cyclomatic_complexity.rs index d4375abcd05c..84fb9b874ff1 100644 --- a/src/cyclomatic_complexity.rs +++ b/src/cyclomatic_complexity.rs @@ -62,8 +62,8 @@ impl CyclomaticComplexity { span_help_and_lint(cx, CYCLOMATIC_COMPLEXITY, span, - &format!("The function has a cyclomatic complexity of {}", rust_cc), - "You could split it up into multiple smaller functions"); + &format!("the function has a cyclomatic complexity of {}", rust_cc), + "you could split it up into multiple smaller functions"); } } } diff --git a/src/lib.rs b/src/lib.rs index 3f18bcb17ced..33de4d6fb792 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -257,6 +257,7 @@ pub fn plugin_registrar(reg: &mut Registry) { ranges::RANGE_STEP_BY_ZERO, ranges::RANGE_ZIP_WITH_LEN, regex::INVALID_REGEX, + regex::TRIVIAL_REGEX, returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, strings::STRING_LIT_AS_BYTES, diff --git a/src/open_options.rs b/src/open_options.rs index 541ed2444f48..e0b51cdfa2ad 100644 --- a/src/open_options.rs +++ b/src/open_options.rs @@ -120,7 +120,7 @@ fn check_open_options(cx: &LateContext, options: &[(OpenOption, Argument)], span span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, - "The method \"create\" is called more than once"); + "the method \"create\" is called more than once"); } else { create = true } @@ -131,7 +131,7 @@ fn check_open_options(cx: &LateContext, options: &[(OpenOption, Argument)], span span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, - "The method \"append\" is called more than once"); + "the method \"append\" is called more than once"); } else { append = true } @@ -142,7 +142,7 @@ fn check_open_options(cx: &LateContext, options: &[(OpenOption, Argument)], span span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, - "The method \"truncate\" is called more than once"); + "the method \"truncate\" is called more than once"); } else { truncate = true } @@ -153,7 +153,7 @@ fn check_open_options(cx: &LateContext, options: &[(OpenOption, Argument)], span span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, - "The method \"read\" is called more than once"); + "the method \"read\" is called more than once"); } else { read = true } @@ -164,7 +164,7 @@ fn check_open_options(cx: &LateContext, options: &[(OpenOption, Argument)], span span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, - "The method \"write\" is called more than once"); + "the method \"write\" is called more than once"); } else { write = true } @@ -174,12 +174,12 @@ fn check_open_options(cx: &LateContext, options: &[(OpenOption, Argument)], span } if read && truncate && read_arg && truncate_arg { - span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "File opened with \"truncate\" and \"read\""); + span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, "file opened with \"truncate\" and \"read\""); } if append && truncate && append_arg && truncate_arg { span_lint(cx, NONSENSICAL_OPEN_OPTIONS, span, - "File opened with \"append\" and \"truncate\""); + "file opened with \"append\" and \"truncate\""); } } diff --git a/src/regex.rs b/src/regex.rs index 259bbb1b64bd..0558b77acb03 100644 --- a/src/regex.rs +++ b/src/regex.rs @@ -8,7 +8,7 @@ use rustc::middle::const_eval::{eval_const_expr_partial, ConstVal}; use rustc::middle::const_eval::EvalHint::ExprTypeChecked; use rustc::lint::*; -use utils::{match_path, REGEX_NEW_PATH, span_lint}; +use utils::{match_path, REGEX_NEW_PATH, span_lint, span_help_and_lint}; /// **What it does:** This lint checks `Regex::new(_)` invocations for correct regex syntax. It is `deny` by default. /// @@ -23,12 +23,26 @@ declare_lint! { "finds invalid regular expressions in `Regex::new(_)` invocations" } +/// **What it does:** This lint checks for `Regex::new(_)` invocations with trivial regex. +/// +/// **Why is this bad?** This can likely be replaced by `==` or `str::starts_with`, +/// `str::ends_with` or `std::contains` or other `str` methods. +/// +/// **Known problems:** None. +/// +/// **Example:** `Regex::new("^foobar")` +declare_lint! { + pub TRIVIAL_REGEX, + Warn, + "finds trivial regular expressions in `Regex::new(_)` invocations" +} + #[derive(Copy,Clone)] pub struct RegexPass; impl LintPass for RegexPass { fn get_lints(&self) -> LintArray { - lint_array!(INVALID_REGEX) + lint_array!(INVALID_REGEX, TRIVIAL_REGEX) } } @@ -45,22 +59,29 @@ impl LateLintPass for RegexPass { span_lint(cx, INVALID_REGEX, str_span(args[0].span, &r, e.position()), - &format!("Regex syntax error: {}", + &format!("regex syntax error: {}", e.description())); } + else if let Some(repl) = is_trivial_regex(r) { + span_help_and_lint(cx, TRIVIAL_REGEX, args[0].span, + &"trivial regex", + &format!("consider using {}", repl)); + } } - } else { - if_let_chain!{[ - let Some(r) = const_str(cx, &*args[0]), - let Err(e) = regex_syntax::Expr::parse(&r) - ], { + } else if let Some(r) = const_str(cx, &*args[0]) { + if let Err(e) = regex_syntax::Expr::parse(&r) { span_lint(cx, INVALID_REGEX, args[0].span, - &format!("Regex syntax error on position {}: {}", + &format!("regex syntax error on position {}: {}", e.position(), e.description())); - }} + } + else if let Some(repl) = is_trivial_regex(&r) { + span_help_and_lint(cx, TRIVIAL_REGEX, args[0].span, + &"trivial regex", + &format!("{}", repl)); + } } }} } @@ -81,3 +102,26 @@ fn const_str(cx: &LateContext, e: &Expr) -> Option { _ => None } } + +fn is_trivial_regex(s: &str) -> Option<&'static str> { + // some unlikely but valid corner cases + match s { + "" | "^" | "$" => return Some("the regex is unlikely to be useful as it is"), + "^$" => return Some("consider using `str::is_empty`"), + _ => (), + } + + let (start, end, repl) = match (s.starts_with('^'), s.ends_with('$')) { + (true, true) => (1, s.len()-1, "consider using `==` on `str`s"), + (false, true) => (0, s.len()-1, "consider using `str::ends_with`"), + (true, false) => (1, s.len(), "consider using `str::starts_with`"), + (false, false) => (0, s.len(), "consider using `str::contains`"), + }; + + if !s.chars().take(end).skip(start).any(regex_syntax::is_punct) { + Some(repl) + } + else { + None + } +} diff --git a/tests/compile-fail/cyclomatic_complexity.rs b/tests/compile-fail/cyclomatic_complexity.rs index f79440af1210..5bdca7f36294 100644 --- a/tests/compile-fail/cyclomatic_complexity.rs +++ b/tests/compile-fail/cyclomatic_complexity.rs @@ -5,7 +5,7 @@ #![allow(unused)] -fn main() { //~ERROR The function has a cyclomatic complexity of 28 +fn main() { //~ERROR the function has a cyclomatic complexity of 28 if true { println!("a"); } @@ -90,7 +90,7 @@ fn main() { //~ERROR The function has a cyclomatic complexity of 28 } #[cyclomatic_complexity = "0"] -fn kaboom() { //~ ERROR: The function has a cyclomatic complexity of 8 +fn kaboom() { //~ ERROR: the function has a cyclomatic complexity of 8 let n = 0; 'a: for i in 0..20 { 'b: for j in i..20 { @@ -136,7 +136,7 @@ fn bloo() { } #[cyclomatic_complexity = "0"] -fn baa() { //~ ERROR: The function has a cyclomatic complexity of 2 +fn baa() { //~ ERROR: the function has a cyclomatic complexity of 2 let x = || match 99 { 0 => true, 1 => false, @@ -154,7 +154,7 @@ fn baa() { //~ ERROR: The function has a cyclomatic complexity of 2 } #[cyclomatic_complexity = "0"] -fn bar() { //~ ERROR: The function has a cyclomatic complexity of 2 +fn bar() { //~ ERROR: the function has a cyclomatic complexity of 2 match 99 { 0 => println!("hi"), _ => println!("bye"), @@ -162,7 +162,7 @@ fn bar() { //~ ERROR: The function has a cyclomatic complexity of 2 } #[cyclomatic_complexity = "0"] -fn barr() { //~ ERROR: The function has a cyclomatic complexity of 2 +fn barr() { //~ ERROR: the function has a cyclomatic complexity of 2 match 99 { 0 => println!("hi"), 1 => println!("bla"), @@ -172,7 +172,7 @@ fn barr() { //~ ERROR: The function has a cyclomatic complexity of 2 } #[cyclomatic_complexity = "0"] -fn barr2() { //~ ERROR: The function has a cyclomatic complexity of 3 +fn barr2() { //~ ERROR: the function has a cyclomatic complexity of 3 match 99 { 0 => println!("hi"), 1 => println!("bla"), @@ -188,7 +188,7 @@ fn barr2() { //~ ERROR: The function has a cyclomatic complexity of 3 } #[cyclomatic_complexity = "0"] -fn barrr() { //~ ERROR: The function has a cyclomatic complexity of 2 +fn barrr() { //~ ERROR: the function has a cyclomatic complexity of 2 match 99 { 0 => println!("hi"), 1 => panic!("bla"), @@ -198,7 +198,7 @@ fn barrr() { //~ ERROR: The function has a cyclomatic complexity of 2 } #[cyclomatic_complexity = "0"] -fn barrr2() { //~ ERROR: The function has a cyclomatic complexity of 3 +fn barrr2() { //~ ERROR: the function has a cyclomatic complexity of 3 match 99 { 0 => println!("hi"), 1 => panic!("bla"), @@ -214,7 +214,7 @@ fn barrr2() { //~ ERROR: The function has a cyclomatic complexity of 3 } #[cyclomatic_complexity = "0"] -fn barrrr() { //~ ERROR: The function has a cyclomatic complexity of 2 +fn barrrr() { //~ ERROR: the function has a cyclomatic complexity of 2 match 99 { 0 => println!("hi"), 1 => println!("bla"), @@ -224,7 +224,7 @@ fn barrrr() { //~ ERROR: The function has a cyclomatic complexity of 2 } #[cyclomatic_complexity = "0"] -fn barrrr2() { //~ ERROR: The function has a cyclomatic complexity of 3 +fn barrrr2() { //~ ERROR: the function has a cyclomatic complexity of 3 match 99 { 0 => println!("hi"), 1 => println!("bla"), @@ -240,7 +240,7 @@ fn barrrr2() { //~ ERROR: The function has a cyclomatic complexity of 3 } #[cyclomatic_complexity = "0"] -fn cake() { //~ ERROR: The function has a cyclomatic complexity of 2 +fn cake() { //~ ERROR: the function has a cyclomatic complexity of 2 if 4 == 5 { println!("yea"); } else { @@ -251,7 +251,7 @@ fn cake() { //~ ERROR: The function has a cyclomatic complexity of 2 #[cyclomatic_complexity = "0"] -pub fn read_file(input_path: &str) -> String { //~ ERROR: The function has a cyclomatic complexity of 4 +pub fn read_file(input_path: &str) -> String { //~ ERROR: the function has a cyclomatic complexity of 4 use std::fs::File; use std::io::{Read, Write}; use std::path::Path; @@ -282,7 +282,7 @@ pub fn read_file(input_path: &str) -> String { //~ ERROR: The function has a cyc enum Void {} #[cyclomatic_complexity = "0"] -fn void(void: Void) { //~ ERROR: The function has a cyclomatic complexity of 1 +fn void(void: Void) { //~ ERROR: the function has a cyclomatic complexity of 1 if true { match void { } diff --git a/tests/compile-fail/open_options.rs b/tests/compile-fail/open_options.rs index 35cc91c9d0fe..08024e37d4af 100644 --- a/tests/compile-fail/open_options.rs +++ b/tests/compile-fail/open_options.rs @@ -5,12 +5,12 @@ use std::fs::OpenOptions; #[allow(unused_must_use)] #[deny(nonsensical_open_options)] fn main() { - OpenOptions::new().read(true).truncate(true).open("foo.txt"); //~ERROR File opened with "truncate" and "read" - OpenOptions::new().append(true).truncate(true).open("foo.txt"); //~ERROR File opened with "append" and "truncate" - - OpenOptions::new().read(true).read(false).open("foo.txt"); //~ERROR The method "read" is called more than once - OpenOptions::new().create(true).create(false).open("foo.txt"); //~ERROR The method "create" is called more than once - OpenOptions::new().write(true).write(false).open("foo.txt"); //~ERROR The method "write" is called more than once - OpenOptions::new().append(true).append(false).open("foo.txt"); //~ERROR The method "append" is called more than once - OpenOptions::new().truncate(true).truncate(false).open("foo.txt"); //~ERROR The method "truncate" is called more than once + OpenOptions::new().read(true).truncate(true).open("foo.txt"); //~ERROR file opened with "truncate" and "read" + OpenOptions::new().append(true).truncate(true).open("foo.txt"); //~ERROR file opened with "append" and "truncate" + + OpenOptions::new().read(true).read(false).open("foo.txt"); //~ERROR the method "read" is called more than once + OpenOptions::new().create(true).create(false).open("foo.txt"); //~ERROR the method "create" is called more than once + OpenOptions::new().write(true).write(false).open("foo.txt"); //~ERROR the method "write" is called more than once + OpenOptions::new().append(true).append(false).open("foo.txt"); //~ERROR the method "append" is called more than once + OpenOptions::new().truncate(true).truncate(false).open("foo.txt"); //~ERROR the method "truncate" is called more than once } diff --git a/tests/compile-fail/regex.rs b/tests/compile-fail/regex.rs index 34dfc1ef25b3..cd10d47c1bbb 100644 --- a/tests/compile-fail/regex.rs +++ b/tests/compile-fail/regex.rs @@ -2,23 +2,66 @@ #![plugin(clippy)] #![allow(unused)] -#![deny(invalid_regex)] +#![deny(invalid_regex, trivial_regex)] extern crate regex; use regex::Regex; const OPENING_PAREN : &'static str = "("; +const NOT_A_REAL_REGEX : &'static str = "foobar"; -fn main() { +fn syntax_error() { let pipe_in_wrong_position = Regex::new("|"); - //~^ERROR: Regex syntax error: empty alternate - let wrong_char_ranice = Regex::new("[z-a]"); - //~^ERROR: Regex syntax error: invalid character class range - + //~^ERROR: regex syntax error: empty alternate + let wrong_char_ranice = Regex::new("[z-a]"); + //~^ERROR: regex syntax error: invalid character class range + let some_regex = Regex::new(OPENING_PAREN); - //~^ERROR: Regex syntax error on position 0: unclosed + //~^ERROR: regex syntax error on position 0: unclosed let closing_paren = ")"; let not_linted = Regex::new(closing_paren); } + +fn trivial_regex() { + let trivial_eq = Regex::new("^foobar$"); + //~^ERROR: trivial regex + //~|HELP consider using `==` on `str`s + + let trivial_starts_with = Regex::new("^foobar"); + //~^ERROR: trivial regex + //~|HELP consider using `str::starts_with` + + let trivial_ends_with = Regex::new("foobar$"); + //~^ERROR: trivial regex + //~|HELP consider using `str::ends_with` + + let trivial_contains = Regex::new("foobar"); + //~^ERROR: trivial regex + //~|HELP consider using `str::contains` + + let trivial_contains = Regex::new(NOT_A_REAL_REGEX); + //~^ERROR: trivial regex + //~|HELP consider using `str::contains` + + // unlikely corner cases + let trivial_empty = Regex::new(""); + //~^ERROR: trivial regex + //~|HELP the regex is unlikely to be useful + + let trivial_empty = Regex::new("^$"); + //~^ERROR: trivial regex + //~|HELP consider using `str::is_empty` + + // non-trivial regexes + let non_trivial_eq = Regex::new("^foo|bar$"); + let non_trivial_starts_with = Regex::new("^foo|bar"); + let non_trivial_ends_with = Regex::new("^foo|bar"); + let non_trivial_ends_with = Regex::new("foo|bar"); +} + +fn main() { + syntax_error(); + trivial_regex(); +}