Skip to content

Fix #765 & #763 & #768 #767

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 5 commits into from
Mar 15, 2016
Merged
Show file tree
Hide file tree
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
19 changes: 8 additions & 11 deletions src/array_indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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))
Expand Down
14 changes: 5 additions & 9 deletions src/panic.rs
Original file line number Diff line number Diff line change
@@ -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!`.
///
Expand All @@ -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");
}}
}
}
21 changes: 14 additions & 7 deletions src/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`!).
///
Expand Down Expand Up @@ -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);
});

}
}
}
Expand Down
20 changes: 20 additions & 0 deletions src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down
14 changes: 14 additions & 0 deletions tests/compile-fail/array_indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
}
15 changes: 11 additions & 4 deletions tests/compile-fail/panic.rs
Original file line number Diff line number Diff line change
@@ -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}"),
Expand All @@ -33,4 +39,5 @@ fn main() {
ok_single();
ok_multiple();
ok_bracket();
ok_inner();
}
8 changes: 7 additions & 1 deletion tests/compile-fail/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion util/update_wiki.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down