From d11b958faf4e1b9fe791eff9485a4abec28ca541 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Wed, 4 Dec 2019 21:50:28 +0100 Subject: [PATCH 1/2] Fix false positive in `string_add`. --- clippy_lints/src/strings.rs | 8 +++++++- tests/ui/string_add.rs | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 5261d8585669..c1d441c39358 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -8,7 +8,9 @@ use syntax::source_map::Spanned; use if_chain::if_chain; use crate::utils::SpanlessEq; -use crate::utils::{get_parent_expr, is_allowed, match_type, paths, span_lint, span_lint_and_sugg, walk_ptrs_ty}; +use crate::utils::{ + get_parent_expr, in_macro, is_allowed, match_type, paths, span_lint, span_lint_and_sugg, walk_ptrs_ty, +}; declare_clippy_lint! { /// **What it does:** Checks for string appends of the form `x = x + y` (without @@ -80,6 +82,10 @@ declare_lint_pass!(StringAdd => [STRING_ADD, STRING_ADD_ASSIGN]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { + if in_macro(e.span) { + return; + } + if let ExprKind::Binary( Spanned { node: BinOpKind::Add, .. diff --git a/tests/ui/string_add.rs b/tests/ui/string_add.rs index c9dd13eea8a3..55f44432bb2f 100644 --- a/tests/ui/string_add.rs +++ b/tests/ui/string_add.rs @@ -16,4 +16,13 @@ fn main() { let mut x = 1; x = x + 1; assert_eq!(2, x); + + macro_rules! mac { + () => { + let y = "".to_owned(); + let z = y + "..."; + }; + } + + mac!(); } From 946961d19ec48c26142907796f8e2b7567fb8a6c Mon Sep 17 00:00:00 2001 From: daxpedda Date: Thu, 5 Dec 2019 11:06:13 +0100 Subject: [PATCH 2/2] Change to only detect in external macros. --- clippy_lints/src/strings.rs | 8 +++----- tests/ui/auxiliary/macro_rules.rs | 8 ++++++++ tests/ui/string_add.rs | 14 ++++++-------- tests/ui/string_add.stderr | 8 ++++---- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index c1d441c39358..d0f0bd6d94c1 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -1,6 +1,6 @@ use rustc::declare_lint_pass; use rustc::hir::*; -use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; use rustc_errors::Applicability; use rustc_session::declare_tool_lint; use syntax::source_map::Spanned; @@ -8,9 +8,7 @@ use syntax::source_map::Spanned; use if_chain::if_chain; use crate::utils::SpanlessEq; -use crate::utils::{ - get_parent_expr, in_macro, is_allowed, match_type, paths, span_lint, span_lint_and_sugg, walk_ptrs_ty, -}; +use crate::utils::{get_parent_expr, is_allowed, match_type, paths, span_lint, span_lint_and_sugg, walk_ptrs_ty}; declare_clippy_lint! { /// **What it does:** Checks for string appends of the form `x = x + y` (without @@ -82,7 +80,7 @@ declare_lint_pass!(StringAdd => [STRING_ADD, STRING_ADD_ASSIGN]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { - if in_macro(e.span) { + if in_external_macro(cx.sess(), e.span) { return; } diff --git a/tests/ui/auxiliary/macro_rules.rs b/tests/ui/auxiliary/macro_rules.rs index 002b05b588d7..504d6733abfe 100644 --- a/tests/ui/auxiliary/macro_rules.rs +++ b/tests/ui/auxiliary/macro_rules.rs @@ -31,3 +31,11 @@ macro_rules! try_err { } }; } + +#[macro_export] +macro_rules! string_add { + () => { + let y = "".to_owned(); + let z = y + "..."; + }; +} diff --git a/tests/ui/string_add.rs b/tests/ui/string_add.rs index 55f44432bb2f..30fd17c59e51 100644 --- a/tests/ui/string_add.rs +++ b/tests/ui/string_add.rs @@ -1,3 +1,8 @@ +// aux-build:macro_rules.rs + +#[macro_use] +extern crate macro_rules; + #[warn(clippy::string_add)] #[allow(clippy::string_add_assign, unused)] fn main() { @@ -17,12 +22,5 @@ fn main() { x = x + 1; assert_eq!(2, x); - macro_rules! mac { - () => { - let y = "".to_owned(); - let z = y + "..."; - }; - } - - mac!(); + string_add!(); } diff --git a/tests/ui/string_add.stderr b/tests/ui/string_add.stderr index 8345c50f9710..3987641c75a3 100644 --- a/tests/ui/string_add.stderr +++ b/tests/ui/string_add.stderr @@ -1,5 +1,5 @@ error: manual implementation of an assign operation - --> $DIR/string_add.rs:8:9 + --> $DIR/string_add.rs:13:9 | LL | x = x + "."; | ^^^^^^^^^^^ help: replace it with: `x += "."` @@ -7,7 +7,7 @@ LL | x = x + "."; = note: `-D clippy::assign-op-pattern` implied by `-D warnings` error: you added something to a string. Consider using `String::push_str()` instead - --> $DIR/string_add.rs:8:13 + --> $DIR/string_add.rs:13:13 | LL | x = x + "."; | ^^^^^^^ @@ -15,13 +15,13 @@ LL | x = x + "."; = note: `-D clippy::string-add` implied by `-D warnings` error: you added something to a string. Consider using `String::push_str()` instead - --> $DIR/string_add.rs:12:13 + --> $DIR/string_add.rs:17:13 | LL | let z = y + "..."; | ^^^^^^^^^ error: manual implementation of an assign operation - --> $DIR/string_add.rs:17:5 + --> $DIR/string_add.rs:22:5 | LL | x = x + 1; | ^^^^^^^^^ help: replace it with: `x += 1`