mirror of
https://github.com/rust-lang/rust.git
synced 2024-12-03 12:13:43 +00:00
New lints for write! / writeln! macros.
This commit is contained in:
parent
22df45f1ac
commit
d712991917
@ -10,7 +10,6 @@
|
||||
#![feature(macro_vis_matcher)]
|
||||
#![allow(unknown_lints, indexing_slicing, shadow_reuse, missing_docs_in_private_items)]
|
||||
#![recursion_limit = "256"]
|
||||
|
||||
// FIXME(mark-i-m) remove after i128 stablization merges
|
||||
#![allow(stable_features)]
|
||||
#![feature(i128, i128_type)]
|
||||
@ -172,7 +171,6 @@ pub mod overflow_check_conditional;
|
||||
pub mod panic;
|
||||
pub mod partialeq_ne_impl;
|
||||
pub mod precedence;
|
||||
pub mod print;
|
||||
pub mod ptr;
|
||||
pub mod question_mark;
|
||||
pub mod ranges;
|
||||
@ -195,6 +193,7 @@ pub mod unused_io_amount;
|
||||
pub mod unused_label;
|
||||
pub mod use_self;
|
||||
pub mod vec;
|
||||
pub mod write;
|
||||
pub mod zero_div_zero;
|
||||
// end lints modules, do not remove this comment, it’s used in `update_lints`
|
||||
|
||||
@ -343,7 +342,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
|
||||
reg.register_late_lint_pass(box strings::StringLitAsBytes);
|
||||
reg.register_late_lint_pass(box derive::Derive);
|
||||
reg.register_late_lint_pass(box types::CharLitAsU8);
|
||||
reg.register_late_lint_pass(box print::Pass);
|
||||
reg.register_late_lint_pass(box write::Pass);
|
||||
reg.register_late_lint_pass(box vec::Pass);
|
||||
reg.register_early_lint_pass(box non_expressive_names::NonExpressiveNames {
|
||||
single_char_binding_names_threshold: conf.single_char_binding_names_threshold,
|
||||
@ -418,8 +417,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
|
||||
methods::WRONG_PUB_SELF_CONVENTION,
|
||||
misc::FLOAT_CMP_CONST,
|
||||
missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
|
||||
print::PRINT_STDOUT,
|
||||
print::USE_DEBUG,
|
||||
write::PRINT_STDOUT,
|
||||
write::USE_DEBUG,
|
||||
shadow::SHADOW_REUSE,
|
||||
shadow::SHADOW_SAME,
|
||||
shadow::SHADOW_UNRELATED,
|
||||
@ -610,9 +609,9 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
|
||||
panic::PANIC_PARAMS,
|
||||
partialeq_ne_impl::PARTIALEQ_NE_IMPL,
|
||||
precedence::PRECEDENCE,
|
||||
print::PRINT_LITERAL,
|
||||
print::PRINT_WITH_NEWLINE,
|
||||
print::PRINTLN_EMPTY_STRING,
|
||||
write::PRINT_LITERAL,
|
||||
write::PRINT_WITH_NEWLINE,
|
||||
write::PRINTLN_EMPTY_STRING,
|
||||
ptr::CMP_NULL,
|
||||
ptr::MUT_FROM_REF,
|
||||
ptr::PTR_ARG,
|
||||
@ -724,9 +723,9 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
|
||||
non_expressive_names::MANY_SINGLE_CHAR_NAMES,
|
||||
ok_if_let::IF_LET_SOME_RESULT,
|
||||
panic::PANIC_PARAMS,
|
||||
print::PRINT_LITERAL,
|
||||
print::PRINT_WITH_NEWLINE,
|
||||
print::PRINTLN_EMPTY_STRING,
|
||||
write::PRINT_LITERAL,
|
||||
write::PRINT_WITH_NEWLINE,
|
||||
write::PRINTLN_EMPTY_STRING,
|
||||
ptr::CMP_NULL,
|
||||
ptr::PTR_ARG,
|
||||
question_mark::QUESTION_MARK,
|
||||
|
@ -1,292 +0,0 @@
|
||||
use std::ops::Deref;
|
||||
use rustc::hir::*;
|
||||
use rustc::hir::map::Node::{NodeImplItem, NodeItem};
|
||||
use rustc::lint::*;
|
||||
use syntax::ast::LitKind;
|
||||
use syntax::symbol::InternedString;
|
||||
use syntax_pos::Span;
|
||||
use utils::{is_expn_of, match_def_path, match_path, resolve_node, span_lint, span_lint_and_sugg};
|
||||
use utils::{opt_def_id, paths};
|
||||
|
||||
/// **What it does:** This lint warns when you use `println!("")` to
|
||||
/// print a newline.
|
||||
///
|
||||
/// **Why is this bad?** You should use `println!()`, which is simpler.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// println!("");
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub PRINTLN_EMPTY_STRING,
|
||||
style,
|
||||
"using `println!(\"\")` with an empty string"
|
||||
}
|
||||
|
||||
/// **What it does:** This lint warns when you use `print!()` with a format
|
||||
/// string that
|
||||
/// ends in a newline.
|
||||
///
|
||||
/// **Why is this bad?** You should use `println!()` instead, which appends the
|
||||
/// newline.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// print!("Hello {}!\n", name);
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub PRINT_WITH_NEWLINE,
|
||||
style,
|
||||
"using `print!()` with a format string that ends in a newline"
|
||||
}
|
||||
|
||||
/// **What it does:** Checks for printing on *stdout*. The purpose of this lint
|
||||
/// is to catch debugging remnants.
|
||||
///
|
||||
/// **Why is this bad?** People often print on *stdout* while debugging an
|
||||
/// application and might forget to remove those prints afterward.
|
||||
///
|
||||
/// **Known problems:** Only catches `print!` and `println!` calls.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// println!("Hello world!");
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub PRINT_STDOUT,
|
||||
restriction,
|
||||
"printing on stdout"
|
||||
}
|
||||
|
||||
/// **What it does:** Checks for use of `Debug` formatting. The purpose of this
|
||||
/// lint is to catch debugging remnants.
|
||||
///
|
||||
/// **Why is this bad?** The purpose of the `Debug` trait is to facilitate
|
||||
/// debugging Rust code. It should not be used in in user-facing output.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// println!("{:?}", foo);
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub USE_DEBUG,
|
||||
restriction,
|
||||
"use of `Debug`-based formatting"
|
||||
}
|
||||
|
||||
/// **What it does:** This lint warns about the use of literals as `print!`/`println!` args.
|
||||
///
|
||||
/// **Why is this bad?** Using literals as `println!` args is inefficient
|
||||
/// (c.f., https://github.com/matthiaskrgr/rust-str-bench) and unnecessary
|
||||
/// (i.e., just put the literal in the format string)
|
||||
///
|
||||
/// **Known problems:** Will also warn with macro calls as arguments that expand to literals
|
||||
/// -- e.g., `println!("{}", env!("FOO"))`.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// println!("{}", "foo");
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub PRINT_LITERAL,
|
||||
style,
|
||||
"printing a literal with a format string"
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone, Debug)]
|
||||
pub struct Pass;
|
||||
|
||||
impl LintPass for Pass {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array!(PRINT_WITH_NEWLINE, PRINTLN_EMPTY_STRING, PRINT_STDOUT, USE_DEBUG, PRINT_LITERAL)
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
|
||||
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
|
||||
if_chain! {
|
||||
if let ExprCall(ref fun, ref args) = expr.node;
|
||||
if let ExprPath(ref qpath) = fun.node;
|
||||
if let Some(fun_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id));
|
||||
then {
|
||||
|
||||
// Search for `std::io::_print(..)` which is unique in a
|
||||
// `print!` expansion.
|
||||
if match_def_path(cx.tcx, fun_id, &paths::IO_PRINT) {
|
||||
if let Some(span) = is_expn_of(expr.span, "print") {
|
||||
// `println!` uses `print!`.
|
||||
let (span, name) = match is_expn_of(span, "println") {
|
||||
Some(span) => (span, "println"),
|
||||
None => (span, "print"),
|
||||
};
|
||||
|
||||
span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name));
|
||||
|
||||
// Check for literals in the print!/println! args
|
||||
// Also, ensure the format string is `{}` with no special options, like `{:X}`
|
||||
check_print_args_for_literal(cx, args);
|
||||
|
||||
if_chain! {
|
||||
// ensure we're calling Arguments::new_v1
|
||||
if args.len() == 1;
|
||||
if let ExprCall(ref args_fun, ref args_args) = args[0].node;
|
||||
if let ExprPath(ref qpath) = args_fun.node;
|
||||
if let Some(const_def_id) = opt_def_id(resolve_node(cx, qpath, args_fun.hir_id));
|
||||
if match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1);
|
||||
if args_args.len() == 2;
|
||||
if let ExprAddrOf(_, ref match_expr) = args_args[1].node;
|
||||
if let ExprMatch(ref args, _, _) = match_expr.node;
|
||||
if let ExprTup(ref args) = args.node;
|
||||
if let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]);
|
||||
then {
|
||||
match name {
|
||||
"print" => check_print(cx, span, args, fmtstr, fmtlen),
|
||||
"println" => check_println(cx, span, fmtstr, fmtlen),
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Search for something like
|
||||
// `::std::fmt::ArgumentV1::new(__arg0, ::std::fmt::Debug::fmt)`
|
||||
else if args.len() == 2 && match_def_path(cx.tcx, fun_id, &paths::FMT_ARGUMENTV1_NEW) {
|
||||
if let ExprPath(ref qpath) = args[1].node {
|
||||
if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, args[1].hir_id)) {
|
||||
if match_def_path(cx.tcx, def_id, &paths::DEBUG_FMT_METHOD)
|
||||
&& !is_in_debug_impl(cx, expr) && is_expn_of(expr.span, "panic").is_none() {
|
||||
span_lint(cx, USE_DEBUG, args[0].span, "use of `Debug`-based formatting");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check for literals in print!/println! args
|
||||
// ensuring the format string for the literal is `DISPLAY_FMT_METHOD`
|
||||
// e.g., `println!("... {} ...", "foo")`
|
||||
// ^ literal in `println!`
|
||||
fn check_print_args_for_literal<'a, 'tcx>(
|
||||
cx: &LateContext<'a, 'tcx>,
|
||||
args: &HirVec<Expr>
|
||||
) {
|
||||
if_chain! {
|
||||
if args.len() == 1;
|
||||
if let ExprCall(_, ref args_args) = args[0].node;
|
||||
if args_args.len() > 1;
|
||||
if let ExprAddrOf(_, ref match_expr) = args_args[1].node;
|
||||
if let ExprMatch(ref matchee, ref arms, _) = match_expr.node;
|
||||
if let ExprTup(ref tup) = matchee.node;
|
||||
if arms.len() == 1;
|
||||
if let ExprArray(ref arm_body_exprs) = arms[0].body.node;
|
||||
then {
|
||||
// it doesn't matter how many args there are in the `print!`/`println!`,
|
||||
// if there's one literal, we should warn the user
|
||||
for (idx, tup_arg) in tup.iter().enumerate() {
|
||||
if_chain! {
|
||||
// first, make sure we're dealing with a literal (i.e., an ExprLit)
|
||||
if let ExprAddrOf(_, ref tup_val) = tup_arg.node;
|
||||
if let ExprLit(_) = tup_val.node;
|
||||
|
||||
// next, check the corresponding match arm body to ensure
|
||||
// this is "{}", or DISPLAY_FMT_METHOD
|
||||
if let ExprCall(_, ref body_args) = arm_body_exprs[idx].node;
|
||||
if body_args.len() == 2;
|
||||
if let ExprPath(ref body_qpath) = body_args[1].node;
|
||||
if let Some(fun_def_id) = opt_def_id(resolve_node(cx, body_qpath, body_args[1].hir_id));
|
||||
if match_def_path(cx.tcx, fun_def_id, &paths::DISPLAY_FMT_METHOD) ||
|
||||
match_def_path(cx.tcx, fun_def_id, &paths::DEBUG_FMT_METHOD);
|
||||
then {
|
||||
span_lint(cx, PRINT_LITERAL, tup_val.span, "printing a literal with an empty format string");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check for print!("... \n", ...).
|
||||
fn check_print<'a, 'tcx>(
|
||||
cx: &LateContext<'a, 'tcx>,
|
||||
span: Span,
|
||||
args: &HirVec<Expr>,
|
||||
fmtstr: InternedString,
|
||||
fmtlen: usize,
|
||||
) {
|
||||
if_chain! {
|
||||
// check the final format string part
|
||||
if let Some('\n') = fmtstr.chars().last();
|
||||
|
||||
// "foo{}bar" is made into two strings + one argument,
|
||||
// if the format string starts with `{}` (eg. "{}foo"),
|
||||
// the string array is prepended an empty string "".
|
||||
// We only want to check the last string after any `{}`:
|
||||
if args.len() < fmtlen;
|
||||
then {
|
||||
span_lint(cx, PRINT_WITH_NEWLINE, span,
|
||||
"using `print!()` with a format string that ends in a \
|
||||
newline, consider using `println!()` instead");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Check for println!("")
|
||||
fn check_println<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, fmtstr: InternedString, fmtlen: usize) {
|
||||
if_chain! {
|
||||
// check that the string is empty
|
||||
if fmtlen == 1;
|
||||
if fmtstr.deref() == "\n";
|
||||
|
||||
// check the presence of that string
|
||||
if let Ok(snippet) = cx.sess().codemap().span_to_snippet(span);
|
||||
if snippet.contains("\"\"");
|
||||
then {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
PRINT_WITH_NEWLINE,
|
||||
span,
|
||||
"using `println!(\"\")`",
|
||||
"replace it with",
|
||||
"println!()".to_string(),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_in_debug_impl(cx: &LateContext, expr: &Expr) -> bool {
|
||||
let map = &cx.tcx.hir;
|
||||
|
||||
// `fmt` method
|
||||
if let Some(NodeImplItem(item)) = map.find(map.get_parent(expr.id)) {
|
||||
// `Debug` impl
|
||||
if let Some(NodeItem(item)) = map.find(map.get_parent(item.id)) {
|
||||
if let ItemImpl(_, _, _, _, Some(ref tr), _, _) = item.node {
|
||||
return match_path(&tr.path, &["Debug"]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
/// Returns the slice of format string parts in an `Arguments::new_v1` call.
|
||||
fn get_argument_fmtstr_parts(expr: &Expr) -> Option<(InternedString, usize)> {
|
||||
if_chain! {
|
||||
if let ExprAddrOf(_, ref expr) = expr.node; // &["…", "…", …]
|
||||
if let ExprArray(ref exprs) = expr.node;
|
||||
if let Some(expr) = exprs.last();
|
||||
if let ExprLit(ref lit) = expr.node;
|
||||
if let LitKind::Str(ref lit, _) = lit.node;
|
||||
then {
|
||||
return Some((lit.as_str(), exprs.len()));
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
@ -26,6 +26,7 @@ pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"];
|
||||
pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];
|
||||
pub const DROP: [&str; 3] = ["core", "mem", "drop"];
|
||||
pub const FMT_ARGUMENTS_NEWV1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"];
|
||||
pub const FMT_ARGUMENTS_NEWV1FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"];
|
||||
pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"];
|
||||
pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"];
|
||||
pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"];
|
||||
|
437
clippy_lints/src/write.rs
Normal file
437
clippy_lints/src/write.rs
Normal file
@ -0,0 +1,437 @@
|
||||
use rustc::hir::map::Node::{NodeImplItem, NodeItem};
|
||||
use rustc::hir::*;
|
||||
use rustc::lint::*;
|
||||
use std::ops::Deref;
|
||||
use syntax::ast::LitKind;
|
||||
use syntax::ptr;
|
||||
use syntax::symbol::InternedString;
|
||||
use syntax_pos::Span;
|
||||
use utils::{is_expn_of, match_def_path, match_path, resolve_node, span_lint, span_lint_and_sugg};
|
||||
use utils::{opt_def_id, paths};
|
||||
|
||||
/// **What it does:** This lint warns when you use `println!("")` to
|
||||
/// print a newline.
|
||||
///
|
||||
/// **Why is this bad?** You should use `println!()`, which is simpler.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// println!("");
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub PRINTLN_EMPTY_STRING,
|
||||
style,
|
||||
"using `println!(\"\")` with an empty string"
|
||||
}
|
||||
|
||||
/// **What it does:** This lint warns when you use `print!()` with a format
|
||||
/// string that
|
||||
/// ends in a newline.
|
||||
///
|
||||
/// **Why is this bad?** You should use `println!()` instead, which appends the
|
||||
/// newline.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// print!("Hello {}!\n", name);
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub PRINT_WITH_NEWLINE,
|
||||
style,
|
||||
"using `print!()` with a format string that ends in a newline"
|
||||
}
|
||||
|
||||
/// **What it does:** Checks for printing on *stdout*. The purpose of this lint
|
||||
/// is to catch debugging remnants.
|
||||
///
|
||||
/// **Why is this bad?** People often print on *stdout* while debugging an
|
||||
/// application and might forget to remove those prints afterward.
|
||||
///
|
||||
/// **Known problems:** Only catches `print!` and `println!` calls.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// println!("Hello world!");
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub PRINT_STDOUT,
|
||||
restriction,
|
||||
"printing on stdout"
|
||||
}
|
||||
|
||||
/// **What it does:** Checks for use of `Debug` formatting. The purpose of this
|
||||
/// lint is to catch debugging remnants.
|
||||
///
|
||||
/// **Why is this bad?** The purpose of the `Debug` trait is to facilitate
|
||||
/// debugging Rust code. It should not be used in in user-facing output.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// println!("{:?}", foo);
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub USE_DEBUG,
|
||||
restriction,
|
||||
"use of `Debug`-based formatting"
|
||||
}
|
||||
|
||||
/// **What it does:** This lint warns about the use of literals as `print!`/`println!` args.
|
||||
///
|
||||
/// **Why is this bad?** Using literals as `println!` args is inefficient
|
||||
/// (c.f., https://github.com/matthiaskrgr/rust-str-bench) and unnecessary
|
||||
/// (i.e., just put the literal in the format string)
|
||||
///
|
||||
/// **Known problems:** Will also warn with macro calls as arguments that expand to literals
|
||||
/// -- e.g., `println!("{}", env!("FOO"))`.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// println!("{}", "foo");
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub PRINT_LITERAL,
|
||||
style,
|
||||
"printing a literal with a format string"
|
||||
}
|
||||
|
||||
/// **What it does:** This lint warns when you use `writeln!(buf, "")` to
|
||||
/// print a newline.
|
||||
///
|
||||
/// **Why is this bad?** You should use `writeln!(buf)`, which is simpler.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// writeln!("");
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub WRITELN_EMPTY_STRING,
|
||||
style,
|
||||
"using `writeln!(\"\")` with an empty string"
|
||||
}
|
||||
|
||||
/// **What it does:** This lint warns when you use `write!()` with a format
|
||||
/// string that
|
||||
/// ends in a newline.
|
||||
///
|
||||
/// **Why is this bad?** You should use `writeln!()` instead, which appends the
|
||||
/// newline.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// write!(buf, "Hello {}!\n", name);
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub WRITE_WITH_NEWLINE,
|
||||
style,
|
||||
"using `write!()` with a format string that ends in a newline"
|
||||
}
|
||||
|
||||
/// **What it does:** This lint warns about the use of literals as `write!`/`writeln!` args.
|
||||
///
|
||||
/// **Why is this bad?** Using literals as `writeln!` args is inefficient
|
||||
/// (c.f., https://github.com/matthiaskrgr/rust-str-bench) and unnecessary
|
||||
/// (i.e., just put the literal in the format string)
|
||||
///
|
||||
/// **Known problems:** Will also warn with macro calls as arguments that expand to literals
|
||||
/// -- e.g., `writeln!(buf, "{}", env!("FOO"))`.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// writeln!(buf, "{}", "foo");
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub WRITE_LITERAL,
|
||||
style,
|
||||
"writing a literal with a format string"
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone, Debug)]
|
||||
pub struct Pass;
|
||||
|
||||
impl LintPass for Pass {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array!(
|
||||
PRINT_WITH_NEWLINE,
|
||||
PRINTLN_EMPTY_STRING,
|
||||
PRINT_STDOUT,
|
||||
USE_DEBUG,
|
||||
PRINT_LITERAL,
|
||||
WRITE_WITH_NEWLINE,
|
||||
WRITELN_EMPTY_STRING,
|
||||
WRITE_LITERAL
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
|
||||
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
|
||||
match expr.node {
|
||||
// print!()
|
||||
ExprCall(ref fun, ref args) => {
|
||||
if_chain! {
|
||||
if let ExprPath(ref qpath) = fun.node;
|
||||
if let Some(fun_id) = opt_def_id(resolve_node(cx, qpath, fun.hir_id));
|
||||
then {
|
||||
check_print_variants(cx, expr, fun_id, args);
|
||||
}
|
||||
}
|
||||
},
|
||||
// write!()
|
||||
ExprMethodCall(ref fun, _, ref args) => {
|
||||
if fun.name == "write_fmt" {
|
||||
check_write_variants(cx, expr, args);
|
||||
}
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_write_variants<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, write_args: &ptr::P<[Expr]>) {
|
||||
// `writeln!` uses `write!`.
|
||||
if let Some(span) = is_expn_of(expr.span, "write") {
|
||||
let (span, name) = match is_expn_of(span, "writeln") {
|
||||
Some(span) => (span, "writeln"),
|
||||
None => (span, "write"),
|
||||
};
|
||||
|
||||
if_chain! {
|
||||
// ensure we're calling Arguments::new_v1 or Arguments::new_v1_formatted
|
||||
if write_args.len() == 2;
|
||||
if let ExprCall(ref args_fun, ref args_args) = write_args[1].node;
|
||||
if let ExprPath(ref qpath) = args_fun.node;
|
||||
if let Some(const_def_id) = opt_def_id(resolve_node(cx, qpath, args_fun.hir_id));
|
||||
if match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1) ||
|
||||
match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1FORMATTED);
|
||||
then {
|
||||
// Check for literals in the write!/writeln! args
|
||||
check_fmt_args_for_literal(cx, args_args, |span| {
|
||||
span_lint(cx, WRITE_LITERAL, span, "writing a literal with an empty format string");
|
||||
});
|
||||
|
||||
if_chain! {
|
||||
if args_args.len() >= 2;
|
||||
if let ExprAddrOf(_, ref match_expr) = args_args[1].node;
|
||||
if let ExprMatch(ref args, _, _) = match_expr.node;
|
||||
if let ExprTup(ref args) = args.node;
|
||||
if let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]);
|
||||
then {
|
||||
match name {
|
||||
"write" => if has_newline_end(args, fmtstr, fmtlen) {
|
||||
span_lint(cx, WRITE_WITH_NEWLINE, span,
|
||||
"using `write!()` with a format string that ends in a \
|
||||
newline, consider using `writeln!()` instead");
|
||||
},
|
||||
"writeln" => if has_empty_arg(cx, span, fmtstr, fmtlen) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
WRITE_WITH_NEWLINE,
|
||||
span,
|
||||
"using `writeln!(v, \"\")`",
|
||||
"replace it with",
|
||||
"writeln!(v)".to_string(),
|
||||
);
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_print_variants<'a, 'tcx>(
|
||||
cx: &LateContext<'a, 'tcx>,
|
||||
expr: &'tcx Expr,
|
||||
fun_id: def_id::DefId,
|
||||
args: &ptr::P<[Expr]>,
|
||||
) {
|
||||
// Search for `std::io::_print(..)` which is unique in a
|
||||
// `print!` expansion.
|
||||
if match_def_path(cx.tcx, fun_id, &paths::IO_PRINT) {
|
||||
if let Some(span) = is_expn_of(expr.span, "print") {
|
||||
// `println!` uses `print!`.
|
||||
let (span, name) = match is_expn_of(span, "println") {
|
||||
Some(span) => (span, "println"),
|
||||
None => (span, "print"),
|
||||
};
|
||||
|
||||
span_lint(cx, PRINT_STDOUT, span, &format!("use of `{}!`", name));
|
||||
|
||||
if_chain! {
|
||||
// ensure we're calling Arguments::new_v1
|
||||
if args.len() == 1;
|
||||
if let ExprCall(ref args_fun, ref args_args) = args[0].node;
|
||||
then {
|
||||
// Check for literals in the print!/println! args
|
||||
check_fmt_args_for_literal(cx, args_args, |span| {
|
||||
span_lint(cx, PRINT_LITERAL, span, "printing a literal with an empty format string");
|
||||
});
|
||||
|
||||
if_chain! {
|
||||
if let ExprPath(ref qpath) = args_fun.node;
|
||||
if let Some(const_def_id) = opt_def_id(resolve_node(cx, qpath, args_fun.hir_id));
|
||||
if match_def_path(cx.tcx, const_def_id, &paths::FMT_ARGUMENTS_NEWV1);
|
||||
if args_args.len() == 2;
|
||||
if let ExprAddrOf(_, ref match_expr) = args_args[1].node;
|
||||
if let ExprMatch(ref args, _, _) = match_expr.node;
|
||||
if let ExprTup(ref args) = args.node;
|
||||
if let Some((fmtstr, fmtlen)) = get_argument_fmtstr_parts(&args_args[0]);
|
||||
then {
|
||||
match name {
|
||||
"print" =>
|
||||
if has_newline_end(args, fmtstr, fmtlen) {
|
||||
span_lint(cx, PRINT_WITH_NEWLINE, span,
|
||||
"using `print!()` with a format string that ends in a \
|
||||
newline, consider using `println!()` instead");
|
||||
},
|
||||
"println" =>
|
||||
if has_empty_arg(cx, span, fmtstr, fmtlen) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
PRINT_WITH_NEWLINE,
|
||||
span,
|
||||
"using `println!(\"\")`",
|
||||
"replace it with",
|
||||
"println!()".to_string(),
|
||||
);
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Search for something like
|
||||
// `::std::fmt::ArgumentV1::new(__arg0, ::std::fmt::Debug::fmt)`
|
||||
else if args.len() == 2 && match_def_path(cx.tcx, fun_id, &paths::FMT_ARGUMENTV1_NEW) {
|
||||
if let ExprPath(ref qpath) = args[1].node {
|
||||
if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, args[1].hir_id)) {
|
||||
if match_def_path(cx.tcx, def_id, &paths::DEBUG_FMT_METHOD) && !is_in_debug_impl(cx, expr)
|
||||
&& is_expn_of(expr.span, "panic").is_none()
|
||||
{
|
||||
span_lint(cx, USE_DEBUG, args[0].span, "use of `Debug`-based formatting");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check for literals in write!/writeln! and print!/println! args
|
||||
// ensuring the format string for the literal is `DISPLAY_FMT_METHOD`
|
||||
// e.g., `writeln!(buf, "... {} ...", "foo")`
|
||||
// ^ literal in `writeln!`
|
||||
// e.g., `println!("... {} ...", "foo")`
|
||||
// ^ literal in `println!`
|
||||
fn check_fmt_args_for_literal<'a, 'tcx, F>(cx: &LateContext<'a, 'tcx>, args: &HirVec<Expr>, lint_fn: F)
|
||||
where
|
||||
F: Fn(Span),
|
||||
{
|
||||
if_chain! {
|
||||
if args.len() > 1;
|
||||
if let ExprAddrOf(_, ref match_expr) = args[1].node;
|
||||
if let ExprMatch(ref matchee, ref arms, _) = match_expr.node;
|
||||
if let ExprTup(ref tup) = matchee.node;
|
||||
if arms.len() == 1;
|
||||
if let ExprArray(ref arm_body_exprs) = arms[0].body.node;
|
||||
then {
|
||||
// it doesn't matter how many args there are in the `write!`/`writeln!`,
|
||||
// if there's one literal, we should warn the user
|
||||
for (idx, tup_arg) in tup.iter().enumerate() {
|
||||
if_chain! {
|
||||
// first, make sure we're dealing with a literal (i.e., an ExprLit)
|
||||
if let ExprAddrOf(_, ref tup_val) = tup_arg.node;
|
||||
if let ExprLit(_) = tup_val.node;
|
||||
|
||||
// next, check the corresponding match arm body to ensure
|
||||
// this is "{}", or DISPLAY_FMT_METHOD
|
||||
if let ExprCall(_, ref body_args) = arm_body_exprs[idx].node;
|
||||
if body_args.len() == 2;
|
||||
if let ExprPath(ref body_qpath) = body_args[1].node;
|
||||
if let Some(fun_def_id) = opt_def_id(resolve_node(cx, body_qpath, body_args[1].hir_id));
|
||||
if match_def_path(cx.tcx, fun_def_id, &paths::DISPLAY_FMT_METHOD) ||
|
||||
match_def_path(cx.tcx, fun_def_id, &paths::DEBUG_FMT_METHOD);
|
||||
then {
|
||||
lint_fn(tup_val.span);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Check for fmtstr = "... \n"
|
||||
fn has_newline_end(args: &HirVec<Expr>, fmtstr: InternedString, fmtlen: usize) -> bool {
|
||||
if_chain! {
|
||||
// check the final format string part
|
||||
if let Some('\n') = fmtstr.chars().last();
|
||||
|
||||
// "foo{}bar" is made into two strings + one argument,
|
||||
// if the format string starts with `{}` (eg. "{}foo"),
|
||||
// the string array is prepended an empty string "".
|
||||
// We only want to check the last string after any `{}`:
|
||||
if args.len() < fmtlen;
|
||||
then {
|
||||
return true
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Check for writeln!(v, "") / println!("")
|
||||
fn has_empty_arg<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, fmtstr: InternedString, fmtlen: usize) -> bool {
|
||||
if_chain! {
|
||||
// check that the string is empty
|
||||
if fmtlen == 1;
|
||||
if fmtstr.deref() == "\n";
|
||||
|
||||
// check the presence of that string
|
||||
if let Ok(snippet) = cx.sess().codemap().span_to_snippet(span);
|
||||
if snippet.contains("\"\"");
|
||||
then {
|
||||
return true
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Returns the slice of format string parts in an `Arguments::new_v1` call.
|
||||
fn get_argument_fmtstr_parts(expr: &Expr) -> Option<(InternedString, usize)> {
|
||||
if_chain! {
|
||||
if let ExprAddrOf(_, ref expr) = expr.node; // &["…", "…", …]
|
||||
if let ExprArray(ref exprs) = expr.node;
|
||||
if let Some(expr) = exprs.last();
|
||||
if let ExprLit(ref lit) = expr.node;
|
||||
if let LitKind::Str(ref lit, _) = lit.node;
|
||||
then {
|
||||
return Some((lit.as_str(), exprs.len()));
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
fn is_in_debug_impl(cx: &LateContext, expr: &Expr) -> bool {
|
||||
let map = &cx.tcx.hir;
|
||||
|
||||
// `fmt` method
|
||||
if let Some(NodeImplItem(item)) = map.find(map.get_parent(expr.id)) {
|
||||
// `Debug` impl
|
||||
if let Some(NodeItem(item)) = map.find(map.get_parent(item.id)) {
|
||||
if let ItemImpl(_, _, _, _, Some(ref tr), _, _) = item.node {
|
||||
return match_path(&tr.path, &["Debug"]);
|
||||
}
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
@ -1,6 +1,6 @@
|
||||
|
||||
|
||||
#![allow(print_literal)]
|
||||
#![allow(print_literal, write_literal)]
|
||||
#![warn(print_stdout, use_debug)]
|
||||
|
||||
use std::fmt::{Debug, Display, Formatter, Result};
|
||||
|
35
tests/ui/write_literal.rs
Normal file
35
tests/ui/write_literal.rs
Normal file
@ -0,0 +1,35 @@
|
||||
#![allow(unused_must_use)]
|
||||
#![warn(write_literal)]
|
||||
|
||||
use std::io::Write;
|
||||
|
||||
fn main() {
|
||||
let mut v = Vec::new();
|
||||
|
||||
// These should be fine
|
||||
write!(&mut v, "Hello");
|
||||
writeln!(&mut v, "Hello");
|
||||
let world = "world";
|
||||
writeln!(&mut v, "Hello {}", world);
|
||||
writeln!(&mut v, "3 in hex is {:X}", 3);
|
||||
|
||||
// These should throw warnings
|
||||
write!(&mut v, "Hello {}", "world");
|
||||
writeln!(&mut v, "Hello {} {}", world, "world");
|
||||
writeln!(&mut v, "Hello {}", "world");
|
||||
writeln!(&mut v, "10 / 4 is {}", 2.5);
|
||||
writeln!(&mut v, "2 + 1 = {}", 3);
|
||||
writeln!(&mut v, "2 + 1 = {:.4}", 3);
|
||||
writeln!(&mut v, "2 + 1 = {:5.4}", 3);
|
||||
writeln!(&mut v, "Debug test {:?}", "hello, world");
|
||||
|
||||
// positional args don't change the fact
|
||||
// that we're using a literal -- this should
|
||||
// throw a warning
|
||||
writeln!(&mut v, "{0} {1}", "hello", "world");
|
||||
writeln!(&mut v, "{1} {0}", "hello", "world");
|
||||
|
||||
// named args shouldn't change anything either
|
||||
writeln!(&mut v, "{foo} {bar}", foo = "hello", bar = "world");
|
||||
writeln!(&mut v, "{bar} {foo}", foo = "hello", bar = "world");
|
||||
}
|
100
tests/ui/write_literal.stderr
Normal file
100
tests/ui/write_literal.stderr
Normal file
@ -0,0 +1,100 @@
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:17:32
|
||||
|
|
||||
17 | write!(&mut v, "Hello {}", "world");
|
||||
| ^^^^^^^
|
||||
|
|
||||
= note: `-D write-literal` implied by `-D warnings`
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:18:44
|
||||
|
|
||||
18 | writeln!(&mut v, "Hello {} {}", world, "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:19:34
|
||||
|
|
||||
19 | writeln!(&mut v, "Hello {}", "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:20:38
|
||||
|
|
||||
20 | writeln!(&mut v, "10 / 4 is {}", 2.5);
|
||||
| ^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:21:36
|
||||
|
|
||||
21 | writeln!(&mut v, "2 + 1 = {}", 3);
|
||||
| ^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:22:39
|
||||
|
|
||||
22 | writeln!(&mut v, "2 + 1 = {:.4}", 3);
|
||||
| ^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:23:40
|
||||
|
|
||||
23 | writeln!(&mut v, "2 + 1 = {:5.4}", 3);
|
||||
| ^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:24:41
|
||||
|
|
||||
24 | writeln!(&mut v, "Debug test {:?}", "hello, world");
|
||||
| ^^^^^^^^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:29:33
|
||||
|
|
||||
29 | writeln!(&mut v, "{0} {1}", "hello", "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:29:42
|
||||
|
|
||||
29 | writeln!(&mut v, "{0} {1}", "hello", "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:30:33
|
||||
|
|
||||
30 | writeln!(&mut v, "{1} {0}", "hello", "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:30:42
|
||||
|
|
||||
30 | writeln!(&mut v, "{1} {0}", "hello", "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:33:43
|
||||
|
|
||||
33 | writeln!(&mut v, "{foo} {bar}", foo = "hello", bar = "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:33:58
|
||||
|
|
||||
33 | writeln!(&mut v, "{foo} {bar}", foo = "hello", bar = "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:34:43
|
||||
|
|
||||
34 | writeln!(&mut v, "{bar} {foo}", foo = "hello", bar = "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: writing a literal with an empty format string
|
||||
--> $DIR/write_literal.rs:34:58
|
||||
|
|
||||
34 | writeln!(&mut v, "{bar} {foo}", foo = "hello", bar = "world");
|
||||
| ^^^^^^^
|
||||
|
||||
error: aborting due to 16 previous errors
|
||||
|
25
tests/ui/write_with_newline.rs
Normal file
25
tests/ui/write_with_newline.rs
Normal file
@ -0,0 +1,25 @@
|
||||
#![allow(write_literal)]
|
||||
#![warn(write_with_newline)]
|
||||
|
||||
use std::io::Write;
|
||||
|
||||
fn main() {
|
||||
let mut v = Vec::new();
|
||||
|
||||
// These should fail
|
||||
write!(&mut v, "Hello\n");
|
||||
write!(&mut v, "Hello {}\n", "world");
|
||||
write!(&mut v, "Hello {} {}\n\n", "world", "#2");
|
||||
write!(&mut v, "{}\n", 1265);
|
||||
|
||||
// These should be fine
|
||||
write!(&mut v, "");
|
||||
write!(&mut v, "Hello");
|
||||
writeln!(&mut v, "Hello");
|
||||
writeln!(&mut v, "Hello\n");
|
||||
writeln!(&mut v, "Hello {}\n", "world");
|
||||
write!(&mut v, "Issue\n{}", 1265);
|
||||
write!(&mut v, "{}", 1265);
|
||||
write!(&mut v, "\n{}", 1275);
|
||||
|
||||
}
|
28
tests/ui/write_with_newline.stderr
Normal file
28
tests/ui/write_with_newline.stderr
Normal file
@ -0,0 +1,28 @@
|
||||
error: using `write!()` with a format string that ends in a newline, consider using `writeln!()` instead
|
||||
--> $DIR/write_with_newline.rs:10:5
|
||||
|
|
||||
10 | write!(&mut v, "Hello/n");
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D write-with-newline` implied by `-D warnings`
|
||||
|
||||
error: using `write!()` with a format string that ends in a newline, consider using `writeln!()` instead
|
||||
--> $DIR/write_with_newline.rs:11:5
|
||||
|
|
||||
11 | write!(&mut v, "Hello {}/n", "world");
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: using `write!()` with a format string that ends in a newline, consider using `writeln!()` instead
|
||||
--> $DIR/write_with_newline.rs:12:5
|
||||
|
|
||||
12 | write!(&mut v, "Hello {} {}/n/n", "world", "#2");
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: using `write!()` with a format string that ends in a newline, consider using `writeln!()` instead
|
||||
--> $DIR/write_with_newline.rs:13:5
|
||||
|
|
||||
13 | write!(&mut v, "{}/n", 1265);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 4 previous errors
|
||||
|
16
tests/ui/writeln_empty_string.rs
Normal file
16
tests/ui/writeln_empty_string.rs
Normal file
@ -0,0 +1,16 @@
|
||||
#![allow(unused_must_use)]
|
||||
#![warn(writeln_empty_string)]
|
||||
use std::io::Write;
|
||||
|
||||
fn main() {
|
||||
let mut v = Vec::new();
|
||||
|
||||
// This should fail
|
||||
writeln!(&mut v, "");
|
||||
|
||||
// These should be fine
|
||||
writeln!(&mut v);
|
||||
writeln!(&mut v, " ");
|
||||
write!(&mut v, "");
|
||||
|
||||
}
|
10
tests/ui/writeln_empty_string.stderr
Normal file
10
tests/ui/writeln_empty_string.stderr
Normal file
@ -0,0 +1,10 @@
|
||||
error: using `writeln!(v, "")`
|
||||
--> $DIR/writeln_empty_string.rs:9:5
|
||||
|
|
||||
9 | writeln!(&mut v, "");
|
||||
| ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `writeln!(v)`
|
||||
|
|
||||
= note: `-D write-with-newline` implied by `-D warnings`
|
||||
|
||||
error: aborting due to previous error
|
||||
|
Loading…
Reference in New Issue
Block a user