Merge pull request #642 from Manishearth/regex_macro

Regex macro

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 116 lints included in this crate:
+There are 117 lints included in this crate:
 
 name                                                                                                           | default | meaning
 ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -91,6 +91,7 @@ name
 [range_zip_with_len](https://github.com/Manishearth/rust-clippy/wiki#range_zip_with_len)                       | warn    | zipping iterator with a range when enumerate() would do
 [redundant_closure](https://github.com/Manishearth/rust-clippy/wiki#redundant_closure)                         | warn    | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)
 [redundant_pattern](https://github.com/Manishearth/rust-clippy/wiki#redundant_pattern)                         | warn    | using `name @ _` in a pattern
+[regex_macro](https://github.com/Manishearth/rust-clippy/wiki#regex_macro)                                     | warn    | finds use of `regex!(_)`, suggests `Regex::new(_)` instead
 [result_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#result_unwrap_used)                       | allow   | using `Result.unwrap()`, which might be better handled
 [reverse_range_loop](https://github.com/Manishearth/rust-clippy/wiki#reverse_range_loop)                       | warn    | Iterating over an empty range, such as `10..0` or `5..5`
 [search_is_some](https://github.com/Manishearth/rust-clippy/wiki#search_is_some)                               | warn    | using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()`

src/lib.rs

@@ -264,6 +264,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         ranges::RANGE_STEP_BY_ZERO,
         ranges::RANGE_ZIP_WITH_LEN,
         regex::INVALID_REGEX,
+        regex::REGEX_MACRO,
         regex::TRIVIAL_REGEX,
         returns::LET_AND_RETURN,
         returns::NEEDLESS_RETURN,

src/regex.rs

@@ -1,14 +1,16 @@
 use regex_syntax;
 use std::error::Error;
+use std::collections::HashSet;
 use syntax::ast::Lit_::LitStr;
 use syntax::codemap::{Span, BytePos};
 use syntax::parse::token::InternedString;
 use rustc_front::hir::*;
+use rustc_front::intravisit::{Visitor, walk_block};
 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, span_help_and_lint};
+use utils::{is_expn_of, match_path, match_type, REGEX_NEW_PATH, span_lint, span_help_and_lint};
 
 /// **What it does:** This lint checks `Regex::new(_)` invocations for correct regex syntax.
 ///
@@ -37,16 +39,35 @@ declare_lint! {
     "finds trivial regular expressions in `Regex::new(_)` invocations"
 }
 
+/// **What it does:** This lint checks for usage of `regex!(_)` which as of now is usually slower than `Regex::new(_)` unless called in a loop (which is a bad idea anyway).
+///
+/// **Why is this bad?** Performance, at least for now. The macro version is likely to catch up long-term, but for now the dynamic version is faster.
+///
+/// **Known problems:** None
+///
+/// **Example:** `regex!("foo|bar")`
+declare_lint! {
+    pub REGEX_MACRO,
+    Warn,
+    "finds use of `regex!(_)`, suggests `Regex::new(_)` instead"
+}
+
 #[derive(Copy,Clone)]
 pub struct RegexPass;
 
 impl LintPass for RegexPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(INVALID_REGEX, TRIVIAL_REGEX)
+        lint_array!(INVALID_REGEX, REGEX_MACRO, TRIVIAL_REGEX)
     }
 }
 
 impl LateLintPass for RegexPass {
+    fn check_crate(&mut self, cx: &LateContext, krate: &Crate) {
+        let mut visitor = RegexVisitor { cx: cx, spans: HashSet::new() };
+        krate.visit_all_items(&mut visitor);
+    }
+
+
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         if_let_chain!{[
             let ExprCall(ref fun, ref args) = expr.node,
@@ -139,3 +160,30 @@ fn is_trivial_regex(s: &regex_syntax::Expr) -> Option<&'static str> {
         _ => None,
     }
 }
+
+struct RegexVisitor<'v, 't: 'v> {
+    cx: &'v LateContext<'v, 't>,
+    spans: HashSet<Span>,
+}
+
+impl<'v, 't: 'v> Visitor<'v> for RegexVisitor<'v, 't> {
+    fn visit_block(&mut self, block: &'v Block) {
+        if_let_chain!{[
+            let Some(ref expr) = block.expr,
+            match_type(self.cx, self.cx.tcx.expr_ty(expr), &["regex", "re", "Regex"]),
+            let Some(span) = is_expn_of(self.cx, expr.span, "regex")
+        ], {
+                if self.spans.contains(&span) {
+                    return;
+                }
+                span_lint(self.cx, 
+                          REGEX_MACRO, 
+                          span,
+                          "`regex!(_)` found. \
+                          Please use `Regex::new(_)`, which is faster for now.");
+                self.spans.insert(span);
+                return;
+        }}
+        walk_block(self, block);
+    }
+}

tests/compile-fail/regex.rs

@@ -1,8 +1,8 @@
 #![feature(plugin)]
-#![plugin(clippy)]
+#![plugin(clippy, regex_macros)]
 
 #![allow(unused)]
-#![deny(invalid_regex, trivial_regex)]
+#![deny(invalid_regex, trivial_regex, regex_macro)]
 
 extern crate regex;
 
@@ -70,7 +70,14 @@ fn trivial_regex() {
     let non_trivial_ends_with = Regex::new("foo|bar");
 }
 
+fn regex_macro() {
+    let some_regex = regex!("for real!"); //~ERROR `regex!(_)`
+    let other_regex = regex!("[a-z]_[A-Z]"); //~ERROR `regex!(_)`
+}
+
+
 fn main() {
+    regex_macro();
     syntax_error();
     trivial_regex();
 }

tests/mut_mut_macro.rs

@@ -9,6 +9,7 @@ use std::collections::HashMap;
 
 #[test]
 #[deny(mut_mut)]
+#[allow(regex_macro)]
 fn test_regex() {
     let pattern = regex!(r"^(?P<level>[#]+)\s(?P<title>.+)$");
     assert!(pattern.is_match("# headline"));