diff --git a/clippy_lints/src/methods/manual_str_repeat.rs b/clippy_lints/src/methods/manual_str_repeat.rs index 4b8a9316b33d..e4570308d0b2 100644 --- a/clippy_lints/src/methods/manual_str_repeat.rs +++ b/clippy_lints/src/methods/manual_str_repeat.rs @@ -1,22 +1,22 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_with_context; +use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; +use clippy_utils::sugg::Sugg; use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type}; use clippy_utils::{is_expr_path_def_path, paths}; use if_chain::if_chain; -use rustc_ast::util::parser::PREC_POSTFIX; use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, LangItem}; use rustc_lint::LateContext; use rustc_middle::ty::{self, Ty, TyS}; use rustc_span::symbol::{sym, Symbol}; +use std::borrow::Cow; use super::MANUAL_STR_REPEAT; enum RepeatKind { - Str, String, - Char, + Char(char), } fn get_ty_param(ty: Ty<'_>) -> Option> { @@ -30,8 +30,8 @@ fn get_ty_param(ty: Ty<'_>) -> Option> { fn parse_repeat_arg(cx: &LateContext<'_>, e: &Expr<'_>) -> Option { if let ExprKind::Lit(lit) = &e.kind { match lit.node { - LitKind::Str(..) => Some(RepeatKind::Str), - LitKind::Char(_) => Some(RepeatKind::Char), + LitKind::Str(..) => Some(RepeatKind::String), + LitKind::Char(c) => Some(RepeatKind::Char(c)), _ => None, } } else { @@ -43,7 +43,7 @@ fn parse_repeat_arg(cx: &LateContext<'_>, e: &Expr<'_>) -> Option { Some(RepeatKind::String) } else { let ty = ty.peel_refs(); - (ty.is_str() || is_type_diagnostic_item(cx, ty, sym::string_type)).then(|| RepeatKind::Str) + (ty.is_str() || is_type_diagnostic_item(cx, ty, sym::string_type)).then(|| RepeatKind::String) } } } @@ -68,17 +68,19 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol, if ctxt == repeat_expr.span.ctxt(); then { let mut app = Applicability::MachineApplicable; - let (val_snip, val_is_mac) = snippet_with_context(cx, repeat_arg.span, ctxt, "..", &mut app); let count_snip = snippet_with_context(cx, take_arg.span, ctxt, "..", &mut app).0; let val_str = match repeat_kind { - RepeatKind::Str if !val_is_mac && repeat_arg.precedence().order() < PREC_POSTFIX => { - format!("({})", val_snip) - }, - RepeatKind::Str | RepeatKind::String => val_snip.into(), - RepeatKind::Char if val_snip == r#"'"'"# => r#""\"""#.into(), - RepeatKind::Char if val_snip == r#"'\''"# => r#""'""#.into(), - RepeatKind::Char => format!("\"{}\"", &val_snip[1..val_snip.len() - 1]), + RepeatKind::Char(_) if repeat_arg.span.ctxt() != ctxt => return, + RepeatKind::Char('\'') => r#""'""#.into(), + RepeatKind::Char('"') => r#""\"""#.into(), + RepeatKind::Char(_) => + match snippet_with_applicability(cx, repeat_arg.span, "..", &mut app) { + Cow::Owned(s) => Cow::Owned(format!("\"{}\"", &s[1..s.len() - 1])), + s => s, + }, + RepeatKind::String => + Sugg::hir_with_context(cx, repeat_arg, ctxt, "..", &mut app).maybe_par().to_string().into(), }; span_lint_and_sugg( diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index 0c9506617577..1af566ae2a77 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -2,7 +2,7 @@ #![deny(clippy::missing_docs_in_private_items)] use crate::higher; -use crate::source::{snippet, snippet_opt, snippet_with_macro_callsite}; +use crate::source::{snippet, snippet_opt, snippet_with_context, snippet_with_macro_callsite}; use rustc_ast::util::parser::AssocOp; use rustc_ast::{ast, token}; use rustc_ast_pretty::pprust::token_kind_to_string; @@ -10,7 +10,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::{EarlyContext, LateContext, LintContext}; use rustc_span::source_map::{CharPos, Span}; -use rustc_span::{BytePos, Pos}; +use rustc_span::{BytePos, Pos, SyntaxContext}; use std::borrow::Cow; use std::convert::TryInto; use std::fmt::Display; @@ -90,6 +90,29 @@ impl<'a> Sugg<'a> { Self::hir_from_snippet(expr, snippet) } + /// Same as `hir`, but first walks the span up to the given context. This will result in the + /// macro call, rather then the expansion, if the span is from a child context. If the span is + /// not from a child context, it will be used directly instead. + /// + /// e.g. Given the expression `&vec![]`, getting a snippet from the span for `vec![]` as a HIR + /// node would result in `box []`. If given the context of the address of expression, this + /// function will correctly get a snippet of `vec![]`. + pub fn hir_with_context( + cx: &LateContext<'_>, + expr: &hir::Expr<'_>, + ctxt: SyntaxContext, + default: &'a str, + applicability: &mut Applicability, + ) -> Self { + let (snippet, in_macro) = snippet_with_context(cx, expr.span, ctxt, default, applicability); + + if in_macro { + Sugg::NonParen(snippet) + } else { + Self::hir_from_snippet(expr, snippet) + } + } + /// Generate a suggestion for an expression with the given snippet. This is used by the `hir_*` /// function variants of `Sugg`, since these use different snippet functions. fn hir_from_snippet(expr: &hir::Expr<'_>, snippet: Cow<'a, str>) -> Self {