Merge pull request #767 from mcarton/fix-ice

Fix #765 & #763 & #768

Author: Manishearth

src/array_indexing.rs

@@ -83,7 +83,7 @@ impl LateLintPass for ArrayIndexing {
                         eval_const_expr_partial(cx.tcx, end, ExprTypeChecked, None)).map(|v| v.ok());
 
                     if let Some((start, end)) = to_const_range(start, end, range.limits, size) {
-                        if start >= size || end >= size {
+                        if start > size || end > size {
                             utils::span_lint(cx,
                                              OUT_OF_BOUNDS_INDEXING,
                                              e.span,
@@ -109,14 +109,11 @@ impl LateLintPass for ArrayIndexing {
     }
 }
 
-/// Returns an option containing a tuple with the start and end (exclusive) of the range
-///
-/// Note: we assume the start and the end of the range are unsigned, since array slicing
-/// works only on usize
+/// Returns an option containing a tuple with the start and end (exclusive) of the range.
 fn to_const_range(start: Option<Option<ConstVal>>,
-                    end: Option<Option<ConstVal>>,
-                    limits: RangeLimits,
-                    array_size: ConstInt)
+                  end: Option<Option<ConstVal>>,
+                  limits: RangeLimits,
+                  array_size: ConstInt)
                     -> Option<(ConstInt, ConstInt)> {
     let start = match start {
         Some(Some(ConstVal::Integral(x))) => x,
@@ -127,13 +124,13 @@ fn to_const_range(start: Option<Option<ConstVal>>,
     let end = match end {
         Some(Some(ConstVal::Integral(x))) => {
             if limits == RangeLimits::Closed {
-                x
+                (x + ConstInt::Infer(1)).expect("such a big array is not realistic")
             } else {
-                (x - ConstInt::Infer(1)).expect("x > 0")
+                x
             }
         }
         Some(_) => return None,
-        None => (array_size - ConstInt::Infer(1)).expect("array_size > 0"),
+        None => array_size
     };
 
     Some((start, end))

src/panic.rs

@@ -1,7 +1,7 @@
 use rustc::lint::*;
 use rustc_front::hir::*;
 use syntax::ast::LitKind;
-use utils::{span_lint, in_external_macro, match_path, BEGIN_UNWIND};
+use utils::{span_lint, is_direct_expn_of, match_path, BEGIN_UNWIND};
 
 /// **What it does:** This lint checks for missing parameters in `panic!`.
 ///
@@ -28,24 +28,20 @@ impl LintPass for PanicPass {
 impl LateLintPass for PanicPass {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         if_let_chain! {[
-            in_external_macro(cx, expr.span),
             let ExprBlock(ref block) = expr.node,
             let Some(ref ex) = block.expr,
             let ExprCall(ref fun, ref params) = ex.node,
             params.len() == 2,
             let ExprPath(None, ref path) = fun.node,
             match_path(path, &BEGIN_UNWIND),
             let ExprLit(ref lit) = params[0].node,
+            is_direct_expn_of(cx, params[0].span, "panic").is_some(),
             let LitKind::Str(ref string, _) = lit.node,
             let Some(par) = string.find('{'),
-            string[par..].contains('}'),
-            let Some(sp) = cx.sess().codemap()
-                             .with_expn_info(expr.span.expn_id,
-                                             |info| info.map(|i| i.call_site))
+            string[par..].contains('}')
         ], {
-
-            span_lint(cx, PANIC_PARAMS, sp,
-                      "You probably are missing some parameter in your `panic!` call");
+            span_lint(cx, PANIC_PARAMS, params[0].span,
+                      "you probably are missing some parameter in your format string");
         }}
     }
 }

src/strings.rs

@@ -8,7 +8,7 @@ use rustc_front::hir::*;
 use syntax::codemap::Spanned;
 use utils::STRING_PATH;
 use utils::SpanlessEq;
-use utils::{match_type, span_lint, walk_ptrs_ty, get_parent_expr};
+use utils::{match_type, span_lint, span_lint_and_then, walk_ptrs_ty, get_parent_expr};
 
 /// **What it does:** This lint matches code of the form `x = x + y` (without `let`!).
 ///
@@ -140,12 +140,19 @@ impl LateLintPass for StringLitAsBytes {
             if name.node.as_str() == "as_bytes" {
                 if let ExprLit(ref lit) = args[0].node {
                     if let LitKind::Str(ref lit_content, _) = lit.node {
-                        if lit_content.chars().all(|c| c.is_ascii()) && !in_macro(cx, e.span) {
-                            let msg = format!("calling `as_bytes()` on a string literal. \
-                                               Consider using a byte string literal instead: \
-                                               `b{}`",
-                                              snippet(cx, args[0].span, r#""foo""#));
-                            span_lint(cx, STRING_LIT_AS_BYTES, e.span, &msg);
+                        if lit_content.chars().all(|c| c.is_ascii()) && !in_macro(cx, args[0].span) {
+                            span_lint_and_then(cx,
+                                               STRING_LIT_AS_BYTES,
+                                               e.span,
+                                               "calling `as_bytes()` on a string literal",
+                                               |db| {
+                                                   let sugg = format!("b{}",
+                                                                      snippet(cx, args[0].span, r#""foo""#));
+                                                   db.span_suggestion(e.span,
+                                                                      "consider using a byte string literal instead",
+                                                                      sugg);
+                                               });
+
                         }
                     }
                 }

src/utils/mod.rs

@@ -604,6 +604,7 @@ fn parse_attrs<F: FnMut(u64)>(sess: &Session, attrs: &[ast::Attribute], name: &'
 }
 
 /// Return the pre-expansion span if is this comes from an expansion of the macro `name`.
+/// See also `is_direct_expn_of`.
 pub fn is_expn_of(cx: &LateContext, mut span: Span, name: &str) -> Option<Span> {
     loop {
         let span_name_span = cx.tcx
@@ -619,6 +620,25 @@ pub fn is_expn_of(cx: &LateContext, mut span: Span, name: &str) -> Option<Span>
     }
 }
 
+/// Return the pre-expansion span if is this directly comes from an expansion of the macro `name`.
+/// The difference with `is_expn_of` is that in
+/// ```rust,ignore
+/// foo!(bar!(42));
+/// ```
+/// `42` is considered expanded from `foo!` and `bar!` by `is_expn_of` but only `bar!` by
+/// `is_direct_expn_of`.
+pub fn is_direct_expn_of(cx: &LateContext, span: Span, name: &str) -> Option<Span> {
+    let span_name_span = cx.tcx
+                           .sess
+                           .codemap()
+                           .with_expn_info(span.expn_id, |expn| expn.map(|ei| (ei.callee.name(), ei.call_site)));
+
+    match span_name_span {
+        Some((mac_name, new_span)) if mac_name.as_str() == name => Some(new_span),
+        _ => None,
+    }
+}
+
 /// Returns index of character after first CamelCase component of `s`
 pub fn camel_case_until(s: &str) -> usize {
     let mut iter = s.char_indices();

tests/compile-fail/array_indexing.rs

@@ -16,6 +16,8 @@ fn main() {
     &x[0...4]; //~ERROR: range is out of bounds
     &x[..];
     &x[1..];
+    &x[4..];
+    &x[5..]; //~ERROR: range is out of bounds
     &x[..4];
     &x[..5]; //~ERROR: range is out of bounds
 
@@ -24,4 +26,16 @@ fn main() {
     &y[1..2]; //~ERROR: slicing may panic
     &y[..];
     &y[0...4]; //~ERROR: slicing may panic
+
+    let empty: [i8; 0] = [];
+    empty[0]; //~ERROR: const index is out of bounds
+    &empty[1..5]; //~ERROR: range is out of bounds
+    &empty[0...4]; //~ERROR: range is out of bounds
+    &empty[..];
+    &empty[0..];
+    &empty[0..0];
+    &empty[0...0]; //~ERROR: range is out of bounds
+    &empty[..0];
+    &empty[1..]; //~ERROR: range is out of bounds
+    &empty[..4]; //~ERROR: range is out of bounds
 }

tests/compile-fail/panic.rs

@@ -1,26 +1,32 @@
 #![feature(plugin)]
 #![plugin(clippy)]
 
-#[deny(panic_params)]
+#![deny(panic_params)]
 
 fn missing() {
     if true {
-        panic!("{}"); //~ERROR: You probably are missing some parameter
+        panic!("{}"); //~ERROR: you probably are missing some parameter
+    } else if false {
+        panic!("{:?}"); //~ERROR: you probably are missing some parameter
     } else {
-        panic!("{:?}"); //~ERROR: You probably are missing some parameter
+        assert!(true, "here be missing values: {}"); //~ERROR you probably are missing some parameter
     }
 }
 
 fn ok_single() {
     panic!("foo bar");
 }
 
+fn ok_inner() {
+    // Test for #768
+    assert!("foo bar".contains(&format!("foo {}", "bar")));
+}
+
 fn ok_multiple() {
     panic!("{}", "This is {ok}");
 }
 
 fn ok_bracket() {
-    // the match is just here because of #759, it serves no other purpose for the lint
     match 42 {
         1337 => panic!("{so is this"),
         666 => panic!("so is this}"),
@@ -33,4 +39,5 @@ fn main() {
     ok_single();
     ok_multiple();
     ok_bracket();
+    ok_inner();
 }

tests/compile-fail/strings.rs

@@ -47,9 +47,15 @@ fn both() {
 #[allow(dead_code, unused_variables)]
 #[deny(string_lit_as_bytes)]
 fn str_lit_as_bytes() {
-    let bs = "hello there".as_bytes(); //~ERROR calling `as_bytes()`
+    let bs = "hello there".as_bytes();
+    //~^ERROR calling `as_bytes()`
+    //~|HELP byte string literal
+    //~|SUGGESTION b"hello there"
+
     // no warning, because this cannot be written as a byte string literal:
     let ubs = "☃".as_bytes();
+
+    let strify = stringify!(foobar).as_bytes();
 }
 
 fn main() {

util/update_wiki.py

@@ -23,7 +23,7 @@ def parse_path(p="src"):
 
 def parse_conf(p):
     c = {}
-    with open(p + '/conf.rs') as f:
+    with open(p + '/utils/conf.rs') as f:
         f = f.read()
 
         m = re.search(conf_re, f)