diff --git a/Cargo.toml b/Cargo.toml index b74db8f95..c0d600e41 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ plugin = true [dependencies] unicode-normalization = "0.1" semver = "0.2.1" +regex-syntax = "0.2.2" [dev-dependencies] compiletest_rs = "0.0.11" diff --git a/README.md b/README.md index d6daa49c5..f7908ffdc 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 110 lints included in this crate: +There are 111 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -47,6 +47,7 @@ name [identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1` [ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask) | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2` [inline_always](https://github.com/Manishearth/rust-clippy/wiki#inline_always) | warn | `#[inline(always)]` is a bad idea in most cases +[invalid_regex](https://github.com/Manishearth/rust-clippy/wiki#invalid_regex) | deny | finds invalid regular expressions in `Regex::new(_)` invocations [items_after_statements](https://github.com/Manishearth/rust-clippy/wiki#items_after_statements) | warn | finds blocks where an item comes after a statement [iter_next_loop](https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop) | warn | for-looping over `_.next()` which is probably not intended [len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty) | warn | traits and impls that have `.len()` but not `.is_empty()` diff --git a/src/lib.rs b/src/lib.rs index 482472b66..56a2a0724 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,9 @@ extern crate unicode_normalization; // for semver check in attrs.rs extern crate semver; +// for regex checking +extern crate regex_syntax; + extern crate rustc_plugin; use rustc_plugin::Registry; @@ -82,6 +85,7 @@ pub mod derive; pub mod print; pub mod vec; pub mod drop_ref; +pub mod regex; mod reexport { pub use syntax::ast::{Name, NodeId}; @@ -150,7 +154,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box vec::UselessVec); reg.register_late_lint_pass(box drop_ref::DropRefPass); reg.register_late_lint_pass(box types::AbsurdUnsignedComparisons); - + reg.register_late_lint_pass(box regex::RegexPass); reg.register_lint_group("clippy_pedantic", vec![ matches::SINGLE_MATCH_ELSE, @@ -251,6 +255,7 @@ pub fn plugin_registrar(reg: &mut Registry) { ptr_arg::PTR_ARG, ranges::RANGE_STEP_BY_ZERO, ranges::RANGE_ZIP_WITH_LEN, + regex::INVALID_REGEX, returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, strings::STRING_LIT_AS_BYTES, diff --git a/src/regex.rs b/src/regex.rs new file mode 100644 index 000000000..259bbb1b6 --- /dev/null +++ b/src/regex.rs @@ -0,0 +1,83 @@ +use regex_syntax; +use std::error::Error; +use syntax::ast::Lit_::LitStr; +use syntax::codemap::{Span, BytePos}; +use syntax::parse::token::InternedString; +use rustc_front::hir::*; +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}; + +/// **What it does:** This lint checks `Regex::new(_)` invocations for correct regex syntax. It is `deny` by default. +/// +/// **Why is this bad?** This will lead to a runtime panic. +/// +/// **Known problems:** None. +/// +/// **Example:** `Regex::new("|")` +declare_lint! { + pub INVALID_REGEX, + Deny, + "finds invalid 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) + } +} + +impl LateLintPass for RegexPass { + fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { + if_let_chain!{[ + let ExprCall(ref fun, ref args) = expr.node, + let ExprPath(_, ref path) = fun.node, + match_path(path, ®EX_NEW_PATH) && args.len() == 1 + ], { + if let ExprLit(ref lit) = args[0].node { + if let LitStr(ref r, _) = lit.node { + if let Err(e) = regex_syntax::Expr::parse(r) { + span_lint(cx, + INVALID_REGEX, + str_span(args[0].span, &r, e.position()), + &format!("Regex syntax error: {}", + e.description())); + } + } + } else { + if_let_chain!{[ + let Some(r) = const_str(cx, &*args[0]), + let Err(e) = regex_syntax::Expr::parse(&r) + ], { + span_lint(cx, + INVALID_REGEX, + args[0].span, + &format!("Regex syntax error on position {}: {}", + e.position(), + e.description())); + }} + } + }} + } +} + +#[allow(cast_possible_truncation)] +fn str_span(base: Span, s: &str, c: usize) -> Span { + let lo = match s.char_indices().nth(c) { + Some((b, _)) => base.lo + BytePos(b as u32), + _ => base.hi + }; + Span{ lo: lo, hi: lo, ..base } +} + +fn const_str(cx: &LateContext, e: &Expr) -> Option { + match eval_const_expr_partial(cx.tcx, e, ExprTypeChecked, None) { + Ok(ConstVal::Str(r)) => Some(r), + _ => None + } +} diff --git a/src/utils.rs b/src/utils.rs index 78fb45cd6..8f542431d 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -36,6 +36,7 @@ pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedLis pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"]; pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"]; +pub const REGEX_NEW_PATH: [&'static str; 3] = ["regex", "Regex", "new"]; pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"]; pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"]; pub const VEC_FROM_ELEM_PATH: [&'static str; 3] = ["std", "vec", "from_elem"]; diff --git a/tests/compile-fail/regex.rs b/tests/compile-fail/regex.rs new file mode 100644 index 000000000..34dfc1ef2 --- /dev/null +++ b/tests/compile-fail/regex.rs @@ -0,0 +1,24 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![allow(unused)] +#![deny(invalid_regex)] + +extern crate regex; + +use regex::Regex; + +const OPENING_PAREN : &'static str = "("; + +fn main() { + 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 + + let some_regex = Regex::new(OPENING_PAREN); + //~^ERROR: Regex syntax error on position 0: unclosed + + let closing_paren = ")"; + let not_linted = Regex::new(closing_paren); +} diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 602937a40..92d2671ea 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -7,7 +7,7 @@ fn run_mode(mode: &'static str) { let mut config = compiletest::default_config(); let cfg_mode = mode.parse().ok().expect("Invalid mode"); - config.target_rustcflags = Some("-L target/debug/".to_owned()); + config.target_rustcflags = Some("-L target/debug/ -L target/debug/deps".to_owned()); if let Ok(name) = var::<&str>("TESTNAME") { let s : String = name.to_owned(); config.filter = Some(s)