X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Futils%2Fsugg.rs;h=d1e4db6264f74ad3dc2e5e6ddbdf6bdee4d2257f;hb=c67d2b121adb614eb21b56020d730081cdff0abb;hp=a8bc0f3fca1804b84680f62508b7d734e3f1a83e;hpb=6cba3da727f8e084c5124c77901b7ffba2264076;p=rust.git diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index a8bc0f3fca1..d1e4db6264f 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -1,16 +1,7 @@ -// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - //! Contains utility functions to generate suggestions. #![deny(clippy::missing_docs_in_private_items)] -use crate::utils::{higher, in_macro, snippet, snippet_opt}; +use crate::utils::{higher, in_macro_or_desugar, snippet, snippet_opt, snippet_with_macro_callsite}; use matches::matches; use rustc::hir; use rustc::lint::{EarlyContext, LateContext, LintContext}; @@ -55,38 +46,7 @@ impl<'a> Sugg<'a> { pub fn hir_opt(cx: &LateContext<'_, '_>, expr: &hir::Expr) -> Option { snippet_opt(cx, expr.span).map(|snippet| { let snippet = Cow::Owned(snippet); - match expr.node { - hir::ExprKind::AddrOf(..) - | hir::ExprKind::Box(..) - | hir::ExprKind::Closure(.., _) - | hir::ExprKind::If(..) - | hir::ExprKind::Unary(..) - | hir::ExprKind::Match(..) => Sugg::MaybeParen(snippet), - hir::ExprKind::Continue(..) - | hir::ExprKind::Yield(..) - | hir::ExprKind::Array(..) - | hir::ExprKind::Block(..) - | hir::ExprKind::Break(..) - | hir::ExprKind::Call(..) - | hir::ExprKind::Field(..) - | hir::ExprKind::Index(..) - | hir::ExprKind::InlineAsm(..) - | hir::ExprKind::Lit(..) - | hir::ExprKind::Loop(..) - | hir::ExprKind::MethodCall(..) - | hir::ExprKind::Path(..) - | hir::ExprKind::Repeat(..) - | hir::ExprKind::Ret(..) - | hir::ExprKind::Struct(..) - | hir::ExprKind::Tup(..) - | hir::ExprKind::While(..) - | hir::ExprKind::Err => Sugg::NonParen(snippet), - hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), - hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet), - hir::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(higher::binop(op.node)), snippet), - hir::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet), - hir::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet), - } + Self::hir_from_snippet(expr, snippet) }) } @@ -109,7 +69,7 @@ pub fn hir_with_applicability( default: &'a str, applicability: &mut Applicability, ) -> Self { - if *applicability != Applicability::Unspecified && in_macro(expr.span) { + if *applicability != Applicability::Unspecified && in_macro_or_desugar(expr.span) { *applicability = Applicability::MaybeIncorrect; } Self::hir_opt(cx, expr).unwrap_or_else(|| { @@ -120,6 +80,50 @@ pub fn hir_with_applicability( }) } + /// Same as `hir`, but will use the pre expansion span if the `expr` was in a macro. + pub fn hir_with_macro_callsite(cx: &LateContext<'_, '_>, expr: &hir::Expr, default: &'a str) -> Self { + let snippet = snippet_with_macro_callsite(cx, expr.span, default); + + 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 { + match expr.node { + hir::ExprKind::AddrOf(..) + | hir::ExprKind::Box(..) + | hir::ExprKind::Closure(.., _) + | hir::ExprKind::Unary(..) + | hir::ExprKind::Match(..) => Sugg::MaybeParen(snippet), + hir::ExprKind::Continue(..) + | hir::ExprKind::Yield(..) + | hir::ExprKind::Array(..) + | hir::ExprKind::Block(..) + | hir::ExprKind::Break(..) + | hir::ExprKind::Call(..) + | hir::ExprKind::Field(..) + | hir::ExprKind::Index(..) + | hir::ExprKind::InlineAsm(..) + | hir::ExprKind::Lit(..) + | hir::ExprKind::Loop(..) + | hir::ExprKind::MethodCall(..) + | hir::ExprKind::Path(..) + | hir::ExprKind::Repeat(..) + | hir::ExprKind::Ret(..) + | hir::ExprKind::Struct(..) + | hir::ExprKind::Tup(..) + | hir::ExprKind::While(..) + | hir::ExprKind::DropTemps(_) + | hir::ExprKind::Err => Sugg::NonParen(snippet), + hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), + hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet), + hir::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(higher::binop(op.node)), snippet), + hir::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet), + hir::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet), + } + } + /// Prepare a suggestion from an expression. pub fn ast(cx: &EarlyContext<'_>, expr: &ast::Expr, default: &'a str) -> Self { use syntax::ast::RangeLimits; @@ -132,7 +136,6 @@ pub fn ast(cx: &EarlyContext<'_>, expr: &ast::Expr, default: &'a str) -> Self { | ast::ExprKind::Closure(..) | ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) - | ast::ExprKind::ObsoleteInPlace(..) | ast::ExprKind::Unary(..) | ast::ExprKind::Match(..) => Sugg::MaybeParen(snippet), ast::ExprKind::Async(..) @@ -160,6 +163,7 @@ pub fn ast(cx: &EarlyContext<'_>, expr: &ast::Expr, default: &'a str) -> Self { | ast::ExprKind::Array(..) | ast::ExprKind::While(..) | ast::ExprKind::WhileLet(..) + | ast::ExprKind::Await(..) | ast::ExprKind::Err => Sugg::NonParen(snippet), ast::ExprKind::Range(.., RangeLimits::HalfOpen) => Sugg::BinOp(AssocOp::DotDot, snippet), ast::ExprKind::Range(.., RangeLimits::Closed) => Sugg::BinOp(AssocOp::DotDotEq, snippet), @@ -215,6 +219,17 @@ pub fn mut_addr_deref(self) -> Sugg<'static> { make_unop("&mut *", self) } + /// Convenience method to transform suggestion into a return call + pub fn make_return(self) -> Sugg<'static> { + Sugg::NonParen(Cow::Owned(format!("return {}", self))) + } + + /// Convenience method to transform suggestion into a block + /// where the suggestion is a trailing expression + pub fn blockify(self) -> Sugg<'static> { + Sugg::NonParen(Cow::Owned(format!("{{ {} }}", self))) + } + /// Convenience method to create the `..` or `...` /// suggestion. #[allow(dead_code)] @@ -225,13 +240,13 @@ pub fn range(self, end: &Self, limit: ast::RangeLimits) -> Sugg<'static> { } } - /// Add parenthesis to any expression that might need them. Suitable to the - /// `self` argument of - /// a method call (eg. to build `bar.foo()` or `(1 + 2).foo()`). + /// Adds parenthesis to any expression that might need them. Suitable to the + /// `self` argument of a method call + /// (e.g., to build `bar.foo()` or `(1 + 2).foo()`). pub fn maybe_par(self) -> Self { match self { Sugg::NonParen(..) => self, - // (x) and (x).y() both don't need additional parens + // `(x)` and `(x).y()` both don't need additional parens. Sugg::MaybeParen(sugg) => { if sugg.starts_with('(') && sugg.ends_with(')') { Sugg::MaybeParen(sugg) @@ -267,14 +282,14 @@ fn not(self) -> Sugg<'static> { /// Helper type to display either `foo` or `(foo)`. struct ParenHelper { - /// Whether parenthesis are needed. + /// `true` if parentheses are needed. paren: bool, /// The main thing to display. wrapped: T, } impl ParenHelper { - /// Build a `ParenHelper`. + /// Builds a `ParenHelper`. fn new(paren: bool, wrapped: T) -> Self { Self { paren, wrapped } } @@ -290,7 +305,7 @@ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { } } -/// Build the string for `` adding parenthesis when necessary. +/// Builds the string for `` adding parenthesis when necessary. /// /// For convenience, the operator is taken as a string because all unary /// operators have the same @@ -299,18 +314,19 @@ pub fn make_unop(op: &str, expr: Sugg<'_>) -> Sugg<'static> { Sugg::MaybeParen(format!("{}{}", op, expr.maybe_par()).into()) } -/// Build the string for ` ` adding parenthesis when necessary. +/// Builds the string for ` ` adding parenthesis when necessary. /// /// Precedence of shift operator relative to other arithmetic operation is /// often confusing so /// parenthesis will always be added for a mix of these. pub fn make_assoc(op: AssocOp, lhs: &Sugg<'_>, rhs: &Sugg<'_>) -> Sugg<'static> { - /// Whether the operator is a shift operator `<<` or `>>`. + /// Returns `true` if the operator is a shift operator `<<` or `>>`. fn is_shift(op: &AssocOp) -> bool { matches!(*op, AssocOp::ShiftLeft | AssocOp::ShiftRight) } - /// Whether the operator is a arithmetic operator (`+`, `-`, `*`, `/`, `%`). + /// Returns `true` if the operator is a arithmetic operator + /// (i.e., `+`, `-`, `*`, `/`, `%`). fn is_arith(op: &AssocOp) -> bool { matches!( *op, @@ -318,9 +334,8 @@ fn is_arith(op: &AssocOp) -> bool { ) } - /// Whether the operator `op` needs parenthesis with the operator `other` - /// in the direction - /// `dir`. + /// Returns `true` if the operator `op` needs parenthesis with the operator + /// `other` in the direction `dir`. fn needs_paren(op: &AssocOp, other: &AssocOp, dir: Associativity) -> bool { other.precedence() < op.precedence() || (other.precedence() == op.precedence() @@ -369,7 +384,6 @@ fn needs_paren(op: &AssocOp, other: &AssocOp, dir: Associativity) -> bool { rhs ), AssocOp::Assign => format!("{} = {}", lhs, rhs), - AssocOp::ObsoleteInPlace => format!("in ({}) {}", lhs, rhs), AssocOp::AssignOp(op) => format!("{} {}= {}", lhs, token_to_string(&token::BinOp(op)), rhs), AssocOp::As => format!("{} as {}", lhs, rhs), AssocOp::DotDot => format!("{}..{}", lhs, rhs), @@ -398,10 +412,9 @@ enum Associativity { Right, } -/// Return the associativity/fixity of an operator. The difference with -/// `AssocOp::fixity` is that -/// an operator can be both left and right associative (such as `+`: -/// `a + b + c == (a + b) + c == a + (b + c)`. +/// Returns the associativity/fixity of an operator. The difference with +/// `AssocOp::fixity` is that an operator can be both left and right associative +/// (such as `+`: `a + b + c == (a + b) + c == a + (b + c)`. /// /// Chained `as` and explicit `:` type coercion never need inner parenthesis so /// they are considered @@ -410,7 +423,7 @@ fn associativity(op: &AssocOp) -> Associativity { use syntax::util::parser::AssocOp::*; match *op { - ObsoleteInPlace | Assign | AssignOp(_) => Associativity::Right, + Assign | AssignOp(_) => Associativity::Right, Add | BitAnd | BitOr | BitXor | LAnd | LOr | Multiply | As | Colon => Associativity::Both, Divide | Equal | Greater | GreaterEqual | Less | LessEqual | Modulus | NotEqual | ShiftLeft | ShiftRight | Subtract => Associativity::Left, @@ -418,7 +431,7 @@ fn associativity(op: &AssocOp) -> Associativity { } } -/// Convert a `hir::BinOp` to the corresponding assigning binary operator. +/// Converts a `hir::BinOp` to the corresponding assigning binary operator. fn hirbinop2assignop(op: hir::BinOp) -> AssocOp { use syntax::parse::token::BinOpToken::*; @@ -445,7 +458,7 @@ fn hirbinop2assignop(op: hir::BinOp) -> AssocOp { }) } -/// Convert an `ast::BinOp` to the corresponding assigning binary operator. +/// Converts an `ast::BinOp` to the corresponding assigning binary operator. fn astbinop2assignop(op: ast::BinOp) -> AssocOp { use syntax::ast::BinOpKind::*; use syntax::parse::token::BinOpToken; @@ -465,13 +478,13 @@ fn astbinop2assignop(op: ast::BinOp) -> AssocOp { }) } -/// Return the indentation before `span` if there are nothing but `[ \t]` +/// Returns the indentation before `span` if there are nothing but `[ \t]` /// before it on its line. fn indentation<'a, T: LintContext<'a>>(cx: &T, span: Span) -> Option { let lo = cx.sess().source_map().lookup_char_pos(span.lo()); if let Some(line) = lo.file.get_line(lo.line - 1 /* line numbers in `Loc` are 1-based */) { if let Some((pos, _)) = line.char_indices().find(|&(_, c)| c != ' ' && c != '\t') { - // we can mix char and byte positions here because we only consider `[ \t]` + // We can mix char and byte positions here because we only consider `[ \t]`. if lo.col == CharPos(pos) { Some(line[..pos].into()) } else { @@ -545,7 +558,7 @@ fn suggest_item_with_attr( if let Some(indent) = indentation(cx, item) { let span = item.with_hi(item.lo()); - self.span_suggestion_with_applicability(span, msg, format!("{}\n{}", attr, indent), applicability); + self.span_suggestion(span, msg, format!("{}\n{}", attr, indent), applicability); } } @@ -566,7 +579,7 @@ fn suggest_prepend_item(&mut self, cx: &T, item: Span, msg: &str, new_item: &str }) .collect::(); - self.span_suggestion_with_applicability(span, msg, format!("{}\n{}", new_item, indent), applicability); + self.span_suggestion(span, msg, format!("{}\n{}", new_item, indent), applicability); } } @@ -584,6 +597,24 @@ fn suggest_remove_item(&mut self, cx: &T, item: Span, msg: &str, applicability: } } - self.span_suggestion_with_applicability(remove_span, msg, String::new(), applicability); + self.span_suggestion(remove_span, msg, String::new(), applicability); + } +} + +#[cfg(test)] +mod test { + use super::Sugg; + use std::borrow::Cow; + + const SUGGESTION: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("function_call()")); + + #[test] + fn make_return_transform_sugg_into_a_return_call() { + assert_eq!("return function_call()", SUGGESTION.make_return().to_string()); + } + + #[test] + fn blockify_transforms_sugg_into_a_block() { + assert_eq!("{ function_call() }", SUGGESTION.blockify().to_string()); } }