Skip to content

Commit

Permalink
submodules: update clippy from 20da8f4 to 71be6f6
Browse files Browse the repository at this point in the history
Changes:
````
rustup rust-lang/rust#57428
Remove `to_string()`s from CompilerLintFunctions
Fix comment grammar
Fix .map(..).unwrap_or_else(..) bad suggestion
add suggestions for print/write with newline lint
````
  • Loading branch information
matthiaskrgr committed Jun 6, 2019
1 parent 73df096 commit f1391de
Show file tree
Hide file tree
Showing 10 changed files with 254 additions and 101 deletions.
14 changes: 14 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use syntax::symbol::LocalInternedString;

use crate::utils::paths;
use crate::utils::sugg;
use crate::utils::usage::mutated_variables;
use crate::utils::{
get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy,
is_ctor_function, is_expn_of, is_self, is_self_ty, iter_input_pats, last_path_segment, match_def_path, match_path,
Expand Down Expand Up @@ -1880,7 +1881,20 @@ fn lint_map_unwrap_or_else<'a, 'tcx>(
// lint if the caller of `map()` is an `Option`
let is_option = match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::OPTION);
let is_result = match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::RESULT);

if is_option || is_result {
// Don't make a suggestion that may fail to compile due to mutably borrowing
// the same variable twice.
let map_mutated_vars = mutated_variables(&map_args[0], cx);
let unwrap_mutated_vars = mutated_variables(&unwrap_args[1], cx);
if let (Some(map_mutated_vars), Some(unwrap_mutated_vars)) = (map_mutated_vars, unwrap_mutated_vars) {
if map_mutated_vars.intersection(&unwrap_mutated_vars).next().is_some() {
return;
}
} else {
return;
}

// lint message
let msg = if is_option {
"called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling \
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/utils/hir_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
&& over(&left.bindings, &right.bindings, |l, r| self.eq_type_binding(l, r))
} else if left.parenthesized && right.parenthesized {
over(left.inputs(), right.inputs(), |l, r| self.eq_ty(l, r))
&& both(&Some(&left.bindings[0].ty), &Some(&right.bindings[0].ty), |l, r| {
&& both(&Some(&left.bindings[0].ty()), &Some(&right.bindings[0].ty()), |l, r| {
self.eq_ty(l, r)
})
} else {
Expand Down Expand Up @@ -299,7 +299,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> {
}

fn eq_type_binding(&mut self, left: &TypeBinding, right: &TypeBinding) -> bool {
left.ident.name == right.ident.name && self.eq_ty(&left.ty, &right.ty)
left.ident.name == right.ident.name && self.eq_ty(&left.ty(), &right.ty())
}
}

Expand Down
16 changes: 8 additions & 8 deletions clippy_lints/src/utils/internal_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,17 +213,17 @@ impl<'a, 'tcx: 'a> Visitor<'tcx> for LintCollector<'a, 'tcx> {

#[derive(Clone, Default)]
pub struct CompilerLintFunctions {
map: FxHashMap<String, String>,
map: FxHashMap<&'static str, &'static str>,
}

impl CompilerLintFunctions {
pub fn new() -> Self {
let mut map = FxHashMap::default();
map.insert("span_lint".to_string(), "utils::span_lint".to_string());
map.insert("struct_span_lint".to_string(), "utils::span_lint".to_string());
map.insert("lint".to_string(), "utils::span_lint".to_string());
map.insert("span_lint_note".to_string(), "utils::span_note_and_lint".to_string());
map.insert("span_lint_help".to_string(), "utils::span_help_and_lint".to_string());
map.insert("span_lint", "utils::span_lint");
map.insert("struct_span_lint", "utils::span_lint");
map.insert("lint", "utils::span_lint");
map.insert("span_lint_note", "utils::span_note_and_lint");
map.insert("span_lint_help", "utils::span_help_and_lint");
Self { map }
}
}
Expand All @@ -234,8 +234,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CompilerLintFunctions {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if_chain! {
if let ExprKind::MethodCall(ref path, _, ref args) = expr.node;
let fn_name = path.ident.as_str().to_string();
if let Some(sugg) = self.map.get(&fn_name);
let fn_name = path.ident;
if let Some(sugg) = self.map.get(&*fn_name.as_str());
let ty = walk_ptrs_ty(cx.tables.expr_ty(&args[0]));
if match_type(cx, ty, &paths::EARLY_CONTEXT)
|| match_type(cx, ty, &paths::LATE_CONTEXT);
Expand Down
164 changes: 110 additions & 54 deletions clippy_lints/src/write.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg};
use crate::utils::{snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then};
use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
use rustc_errors::Applicability;
use std::borrow::Cow;
use syntax::ast::*;
use syntax::parse::{parser, token};
use syntax::tokenstream::{TokenStream, TokenTree};
use syntax_pos::symbol::Symbol;
use syntax::tokenstream::TokenStream;
use syntax_pos::{symbol::Symbol, BytePos, Span};

declare_clippy_lint! {
/// **What it does:** This lint warns when you use `println!("")` to
Expand Down Expand Up @@ -184,8 +184,8 @@ impl EarlyLintPass for Write {
fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &Mac) {
if mac.node.path == sym!(println) {
span_lint(cx, PRINT_STDOUT, mac.span, "use of `println!`");
if let Some(fmtstr) = check_tts(cx, &mac.node.tts, false).0 {
if fmtstr == "" {
if let (Some(fmt_str), _) = check_tts(cx, &mac.node.tts, false) {
if fmt_str.contents.is_empty() {
span_lint_and_sugg(
cx,
PRINTLN_EMPTY_STRING,
Expand All @@ -199,35 +199,52 @@ impl EarlyLintPass for Write {
}
} else if mac.node.path == sym!(print) {
span_lint(cx, PRINT_STDOUT, mac.span, "use of `print!`");
if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, false) {
if check_newlines(&fmtstr, is_raw) {
span_lint(
if let (Some(fmt_str), _) = check_tts(cx, &mac.node.tts, false) {
if check_newlines(&fmt_str) {
span_lint_and_then(
cx,
PRINT_WITH_NEWLINE,
mac.span,
"using `print!()` with a format string that ends in a \
single newline, consider using `println!()` instead",
"using `print!()` with a format string that ends in a single newline",
|err| {
err.multipart_suggestion(
"use `println!` instead",
vec![
(mac.node.path.span, String::from("println")),
(fmt_str.newline_span(), String::new()),
],
Applicability::MachineApplicable,
);
},
);
}
}
} else if mac.node.path == sym!(write) {
if let (Some(fmtstr), _, is_raw) = check_tts(cx, &mac.node.tts, true) {
if check_newlines(&fmtstr, is_raw) {
span_lint(
if let (Some(fmt_str), _) = check_tts(cx, &mac.node.tts, true) {
if check_newlines(&fmt_str) {
span_lint_and_then(
cx,
WRITE_WITH_NEWLINE,
mac.span,
"using `write!()` with a format string that ends in a \
single newline, consider using `writeln!()` instead",
);
"using `write!()` with a format string that ends in a single newline",
|err| {
err.multipart_suggestion(
"use `writeln!()` instead",
vec![
(mac.node.path.span, String::from("writeln")),
(fmt_str.newline_span(), String::new()),
],
Applicability::MachineApplicable,
);
},
)
}
}
} else if mac.node.path == sym!(writeln) {
let check_tts = check_tts(cx, &mac.node.tts, true);
if let Some(fmtstr) = check_tts.0 {
if fmtstr == "" {
if let (Some(fmt_str), expr) = check_tts(cx, &mac.node.tts, true) {
if fmt_str.contents.is_empty() {
let mut applicability = Applicability::MachineApplicable;
let suggestion = check_tts.1.map_or_else(
let suggestion = expr.map_or_else(
move || {
applicability = Applicability::HasPlaceholders;
Cow::Borrowed("v")
Expand All @@ -250,10 +267,44 @@ impl EarlyLintPass for Write {
}
}

/// The arguments of a `print[ln]!` or `write[ln]!` invocation.
struct FmtStr {
/// The contents of the format string (inside the quotes).
contents: String,
style: StrStyle,
/// The span of the format string, including quotes, the raw marker, and any raw hashes.
span: Span,
}

impl FmtStr {
/// Given a format string that ends in a newline and its span, calculates the span of the
/// newline.
fn newline_span(&self) -> Span {
let sp = self.span;

let newline_sp_hi = sp.hi()
- match self.style {
StrStyle::Cooked => BytePos(1),
StrStyle::Raw(hashes) => BytePos((1 + hashes).into()),
};

let newline_sp_len = if self.contents.ends_with('\n') {
BytePos(1)
} else if self.contents.ends_with(r"\n") {
BytePos(2)
} else {
panic!("expected format string to contain a newline");
};

sp.with_lo(newline_sp_hi - newline_sp_len).with_hi(newline_sp_hi)
}
}

/// Checks the arguments of `print[ln]!` and `write[ln]!` calls. It will return a tuple of two
/// options and a bool. The first part of the tuple is `format_str` of the macros. The second part
/// of the tuple is in the `write[ln]!` case the expression the `format_str` should be written to.
/// The final part is a boolean flag indicating if the string is a raw string.
/// `Option`s. The first `Option` of the tuple is the macro's format string. It includes
/// the contents of the string, whether it's a raw string, and the span of the literal in the
/// source. The second `Option` in the tuple is, in the `write[ln]!` case, the expression the
/// `format_str` should be written to.
///
/// Example:
///
Expand All @@ -266,49 +317,36 @@ impl EarlyLintPass for Write {
/// ```
/// will return
/// ```rust,ignore
/// (Some("string to write: {}"), Some(buf), false)
/// (Some("string to write: {}"), Some(buf))
/// ```
fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<String>, Option<Expr>, bool) {
fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<FmtStr>, Option<Expr>) {
use fmt_macros::*;
let tts = tts.clone();
let mut is_raw = false;
if let TokenStream(Some(tokens)) = &tts {
for token in tokens.iter() {
if let (TokenTree::Token(_, token::Token::Literal(lit)), _) = token {
match lit.kind {
token::Str => break,
token::StrRaw(_) => {
is_raw = true;
break;
},
_ => {},
}
}
}
}

let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false, None);
let mut expr: Option<Expr> = None;
if is_write {
expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
Ok(p) => Some(p.into_inner()),
Err(_) => return (None, None, is_raw),
Err(_) => return (None, None),
};
// might be `writeln!(foo)`
if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
return (None, expr, is_raw);
return (None, expr);
}
}

let fmtstr = match parser.parse_str().map_err(|mut err| err.cancel()) {
Ok(token) => token.0.to_string(),
Err(_) => return (None, expr, is_raw),
let (fmtstr, fmtstyle) = match parser.parse_str().map_err(|mut err| err.cancel()) {
Ok((fmtstr, fmtstyle)) => (fmtstr.to_string(), fmtstyle),
Err(_) => return (None, expr),
};
let fmtspan = parser.prev_span;
let tmp = fmtstr.clone();
let mut args = vec![];
let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
while let Some(piece) = fmt_parser.next() {
if !fmt_parser.errors.is_empty() {
return (None, expr, is_raw);
return (None, expr);
}
if let Piece::NextArgument(arg) = piece {
if arg.format.ty == "?" {
Expand All @@ -330,11 +368,26 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
ty: "",
};
if !parser.eat(&token::Comma) {
return (Some(fmtstr), expr, is_raw);
return (
Some(FmtStr {
contents: fmtstr,
style: fmtstyle,
span: fmtspan,
}),
expr,
);
}
let token_expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
Ok(expr) => expr,
Err(_) => return (Some(fmtstr), None, is_raw),
let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
expr
} else {
return (
Some(FmtStr {
contents: fmtstr,
style: fmtstyle,
span: fmtspan,
}),
None,
);
};
match &token_expr.node {
ExprKind::Lit(_) => {
Expand Down Expand Up @@ -383,12 +436,15 @@ fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (O
}
}

// Checks if `s` constains a single newline that terminates it
// Literal and escaped newlines are both checked (only literal for raw strings)
fn check_newlines(s: &str, is_raw: bool) -> bool {
/// Checks if the format string constains a single newline that terminates it.
///
/// Literal and escaped newlines are both checked (only literal for raw strings).
fn check_newlines(fmt_str: &FmtStr) -> bool {
let s = &fmt_str.contents;

if s.ends_with('\n') {
return true;
} else if is_raw {
} else if let StrStyle::Raw(_) = fmt_str.style {
return false;
}

Expand Down
15 changes: 15 additions & 0 deletions tests/ui/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,21 @@ fn option_methods() {
// Macro case.
// Should not lint.
let _ = opt_map!(opt, |x| x + 1).unwrap_or_else(|| 0);

// Issue #4144
{
let mut frequencies = HashMap::new();
let word = "foo";

frequencies
.get_mut(word)
.map(|count| {
*count += 1;
})
.unwrap_or_else(|| {
frequencies.insert(word.to_owned(), 1);
});
}
}

/// Checks implementation of `FILTER_NEXT` lint.
Expand Down
Loading

0 comments on commit f1391de

Please sign in to comment.