Merge pull request #619 from Manishearth/regex_syntax

new regex syntax lint

Author: Manishearth

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"

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()`

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,

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, &REGEX_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<InternedString> {
+    match eval_const_expr_partial(cx.tcx, e, ExprTypeChecked, None) {
+        Ok(ConstVal::Str(r)) => Some(r),
+        _ => None
+    }
+}

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"];

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);
+}

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)