Merge pull request #622 from mcarton/regex

Lint about trivial regexes
This commit is contained in:
Manish Goregaokar 2016-02-06 04:27:35 +05:30
commit b9bb298a4b
8 changed files with 137 additions and 48 deletions

View File

@ -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)

View File

@ -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");
}
}
}

View File

@ -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,

View File

@ -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\"");
}
}

View File

@ -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<InternedString> {
_ => 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
}
}

View File

@ -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 {
}

View File

@ -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
}

View File

@ -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();
}