Skip to content

Lint about trivial regexes #622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 5, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 2 additions & 2 deletions src/cyclomatic_complexity.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -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,
14 changes: 7 additions & 7 deletions src/open_options.rs
Original file line number Diff line number Diff line change
@@ -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\"");
}
}
64 changes: 54 additions & 10 deletions src/regex.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
26 changes: 13 additions & 13 deletions tests/compile-fail/cyclomatic_complexity.rs
Original file line number Diff line number Diff line change
@@ -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,15 +154,15 @@ 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"),
}
}

#[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 {
}
16 changes: 8 additions & 8 deletions tests/compile-fail/open_options.rs
Original file line number Diff line number Diff line change
@@ -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
}
57 changes: 50 additions & 7 deletions tests/compile-fail/regex.rs
Original file line number Diff line number Diff line change
@@ -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();
}