about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAlexander Regueiro <alexreg@me.com>2019-03-10 17:19:47 +0000
committerAlexander Regueiro <alexreg@me.com>2019-03-10 18:06:28 +0000
commitd2b85323ad3874f4aad0b56f6755bc987b5d8dad (patch)
treec3dbbebfa8c37601d0f873df3c740b194a4f57c0
parentd43966a1760069180fad2ec05f9c2e33391d6c42 (diff)
downloadrust-d2b85323ad3874f4aad0b56f6755bc987b5d8dad.tar.gz
rust-d2b85323ad3874f4aad0b56f6755bc987b5d8dad.zip
Addressed points raised in review.
-rw-r--r--clippy_lints/src/assertions_on_constants.rs3
-rw-r--r--clippy_lints/src/doc.rs5
-rw-r--r--clippy_lints/src/lifetimes.rs4
-rw-r--r--clippy_lints/src/loops.rs6
-rw-r--r--clippy_lints/src/matches.rs6
-rw-r--r--clippy_lints/src/misc.rs3
-rw-r--r--clippy_lints/src/question_mark.rs2
-rw-r--r--clippy_lints/src/slow_vector_initialization.rs2
-rw-r--r--clippy_lints/src/utils/camel_case.rs3
-rw-r--r--clippy_lints/src/utils/mod.rs22
-rw-r--r--clippy_lints/src/utils/sugg.rs23
-rw-r--r--tests/ui/block_in_if_condition.rs7
-rw-r--r--tests/ui/format.rs6
13 files changed, 44 insertions, 48 deletions
diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs
index 5522cd6e41e..e2651206e80 100644
--- a/clippy_lints/src/assertions_on_constants.rs
+++ b/clippy_lints/src/assertions_on_constants.rs
@@ -26,8 +26,7 @@ declare_clippy_lint! {
     /// ```
     pub ASSERTIONS_ON_CONSTANTS,
     style,
-    "`assert!(true)` / `assert!(false)` will be optimized out by the compiler, \
-     and should probably be replaced by a `panic!()` or `unreachable!()`"
+    "`assert!(true)` / `assert!(false)` will be optimized out by the compiler, and should probably be replaced by a `panic!()` or `unreachable!()`"
 }
 
 pub struct AssertionsOnConstants;
diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs
index 27e066375be..0e8d47384f1 100644
--- a/clippy_lints/src/doc.rs
+++ b/clippy_lints/src/doc.rs
@@ -258,9 +258,8 @@ fn check_text(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, text: &st
 
 fn check_word(cx: &EarlyContext<'_>, word: &str, span: Span) {
     /// Checks if a string is camel-case, i.e., contains at least two uppercase
-    /// letters (`Clippy` is
-    /// ok) and one lower-case letter (`NASA` is ok). Plural are also excluded
-    /// (`IDs` is ok).
+    /// letters (`Clippy` is ok) and one lower-case letter (`NASA` is ok).
+    /// Plurals are also excluded (`IDs` is ok).
     fn is_camel_case(s: &str) -> bool {
         if s.starts_with(|c: char| c.is_digit(10)) {
             return false;
diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs
index 9c07c9a94e0..4b2ba60090f 100644
--- a/clippy_lints/src/lifetimes.rs
+++ b/clippy_lints/src/lifetimes.rs
@@ -20,7 +20,7 @@ declare_clippy_lint! {
     /// them leads to more readable code.
     ///
     /// **Known problems:** Potential false negatives: we bail out if the function
-    /// has a where-clause where lifetimes are mentioned.
+    /// has a `where` clause where lifetimes are mentioned.
     ///
     /// **Example:**
     /// ```rust
@@ -385,7 +385,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RefVisitor<'a, 'tcx> {
     }
 }
 
-/// Are any lifetimes mentioned in the where-clause? If yes, we don't try to
+/// Are any lifetimes mentioned in the `where` clause? If so, we don't try to
 /// reason about elision.
 fn has_where_lifetimes<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, where_clause: &'tcx WhereClause) -> bool {
     for predicate in &where_clause.predicates {
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 186ffbc6450..0016625631c 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -1413,7 +1413,7 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat, arg: &Expr, expr: &Ex
     }
 }
 
-/// Checks for `for` loops over `Option`s and `Results`
+/// Checks for `for` loops over `Option`s and `Result`s.
 fn check_arg_type(cx: &LateContext<'_, '_>, pat: &Pat, arg: &Expr) {
     let ty = cx.tables.expr_ty(arg);
     if match_type(cx, ty, &paths::OPTION) {
@@ -1673,7 +1673,7 @@ fn check_for_mutation(
     delegate.mutation_span()
 }
 
-/// Returns `true` if the pattern is a `PatWild` or an ident prefixed with `'_'`.
+/// Returns `true` if the pattern is a `PatWild` or an ident prefixed with `_`.
 fn pat_is_wild<'tcx>(pat: &'tcx PatKind, body: &'tcx Expr) -> bool {
     match *pat {
         PatKind::Wild => true,
@@ -2336,7 +2336,7 @@ fn path_name(e: &Expr) -> Option<Name> {
 
 fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, expr: &'tcx Expr) {
     if constant(cx, cx.tables, cond).is_some() {
-        // A pure constant condition (e.g., while false) is not linted.
+        // A pure constant condition (e.g., `while false`) is not linted.
         return;
     }
 
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index e81bdd83426..e7e1df5cfff 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -322,7 +322,7 @@ fn check_single_match_opt_like(
     ty: Ty<'_>,
     els: Option<&Expr>,
 ) {
-    // list of candidate Enums we know will never get any more members
+    // list of candidate `Enum`s we know will never get any more members
     let candidates = &[
         (&paths::COW, "Borrowed"),
         (&paths::COW, "Cow::Borrowed"),
@@ -335,7 +335,7 @@ fn check_single_match_opt_like(
 
     let path = match arms[1].pats[0].node {
         PatKind::TupleStruct(ref path, ref inner, _) => {
-            // contains any non wildcard patterns? e.g., Err(err)
+            // Contains any non wildcard patterns (e.g., `Err(err)`)?
             if !inner.iter().all(is_wild) {
                 return;
             }
@@ -354,7 +354,7 @@ fn check_single_match_opt_like(
 }
 
 fn check_match_bool(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) {
-    // type of expression == bool
+    // Type of expression is `bool`.
     if cx.tables.expr_ty(ex).sty == ty::Bool {
         span_lint_and_then(
             cx,
diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 47969317524..46cb9ecc49b 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -592,8 +592,7 @@ fn is_used(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
 }
 
 /// Tests whether an expression is in a macro expansion (e.g., something
-/// generated by
-/// `#[derive(...)`] or the like).
+/// generated by `#[derive(...)]` or the like).
 fn in_attributes_expansion(expr: &Expr) -> bool {
     expr.span
         .ctxt()
diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs
index b9a8759da85..de11e3ff45b 100644
--- a/clippy_lints/src/question_mark.rs
+++ b/clippy_lints/src/question_mark.rs
@@ -52,7 +52,7 @@ impl Pass {
     ///
     /// ```ignore
     /// if option.is_none() {
-    ///    return `None`;
+    ///    return None;
     /// }
     /// ```
     ///
diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs
index 9a6676588cb..888405983fd 100644
--- a/clippy_lints/src/slow_vector_initialization.rs
+++ b/clippy_lints/src/slow_vector_initialization.rs
@@ -197,7 +197,7 @@ struct VectorInitializationVisitor<'a, 'tcx: 'a> {
     /// Contains the information.
     vec_alloc: VecAllocation<'tcx>,
 
-    /// Contains, the slow initialization expression, if one was found.
+    /// Contains the slow initialization expression, if one was found.
     slow_expression: Option<InitializationType<'tcx>>,
 
     /// `true` if the initialization of the vector has been found on the visited block.
diff --git a/clippy_lints/src/utils/camel_case.rs b/clippy_lints/src/utils/camel_case.rs
index 48fbc79298e..5b124dd96bf 100644
--- a/clippy_lints/src/utils/camel_case.rs
+++ b/clippy_lints/src/utils/camel_case.rs
@@ -1,5 +1,4 @@
-/// Returns the index of the character after the first camel-case component of
-/// `s`.
+/// Returns the index of the character after the first camel-case component of `s`.
 pub fn until(s: &str) -> usize {
     let mut iter = s.char_indices();
     if let Some((_, first)) = iter.next() {
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index 036c5306f25..bf94c8a0868 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -147,7 +147,7 @@ pub fn get_def_path(tcx: TyCtxt<'_, '_, '_>, def_id: DefId) -> Vec<&'static str>
         .collect()
 }
 
-/// Checks if type is struct, enum or union type with given def path.
+/// Checks if type is struct, enum or union type with the given def path.
 pub fn match_type(cx: &LateContext<'_, '_>, ty: Ty<'_>, path: &[&str]) -> bool {
     match ty.sty {
         ty::Adt(adt, _) => match_def_path(cx.tcx, adt.did, path),
@@ -155,7 +155,7 @@ pub fn match_type(cx: &LateContext<'_, '_>, ty: Ty<'_>, path: &[&str]) -> bool {
     }
 }
 
-/// Checks if the method call given in `expr` belongs to given trait.
+/// Checks if the method call given in `expr` belongs to the given trait.
 pub fn match_trait_method(cx: &LateContext<'_, '_>, expr: &Expr, path: &[&str]) -> bool {
     let method_call = cx.tables.type_dependent_defs()[expr.hir_id];
     let trt_id = cx.tcx.trait_of_item(method_call.def_id());
@@ -434,7 +434,7 @@ pub fn get_item_name(cx: &LateContext<'_, '_>, expr: &Expr) -> Option<Name> {
     }
 }
 
-/// Gets the name of a `Pat`, if any
+/// Gets the name of a `Pat`, if any.
 pub fn get_pat_name(pat: &Pat) -> Option<Name> {
     match pat.node {
         PatKind::Binding(.., ref spname, _) => Some(spname.name),
@@ -614,7 +614,7 @@ fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, ch: char) -> Cow<'_
     }
 }
 
-/// Gets a parent expressions if any – this is useful to constrain a lint.
+/// Gets the parent expression, if any –- this is useful to constrain a lint.
 pub fn get_parent_expr<'c>(cx: &'c LateContext<'_, '_>, e: &Expr) -> Option<&'c Expr> {
     let map = &cx.tcx.hir();
     let hir_id = e.hir_id;
@@ -727,7 +727,7 @@ pub fn is_expn_of(mut span: Span, name: &str) -> Option<Span> {
     }
 }
 
-/// Returns the pre-expansion span if is this directly comes from an expansion
+/// Returns the pre-expansion span if the span directly comes from an expansion
 /// of the macro `name`.
 /// The difference with `is_expn_of` is that in
 /// ```rust,ignore
@@ -749,7 +749,7 @@ pub fn is_direct_expn_of(span: Span, name: &str) -> Option<Span> {
     }
 }
 
-/// Convenience function to get the return type of a function
+/// Convenience function to get the return type of a function.
 pub fn return_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, fn_item: hir::HirId) -> Ty<'tcx> {
     let fn_def_id = cx.tcx.hir().local_def_id_from_hir_id(fn_item);
     let ret_ty = cx.tcx.fn_sig(fn_def_id).output();
@@ -759,9 +759,9 @@ pub fn return_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, fn_item: hir::HirId) -> T
 /// Checks if two types are the same.
 ///
 /// This discards any lifetime annotations, too.
-// FIXME: this works correctly for lifetimes bounds (`for <'a> Foo<'a>` == `for
-// <'b> Foo<'b>` but
-// not for type parameters.
+//
+// FIXME: this works correctly for lifetimes bounds (`for <'a> Foo<'a>` ==
+// `for <'b> Foo<'b>`, but not for type parameters).
 pub fn same_tys<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
     let a = cx.tcx.erase_late_bound_regions(&Binder::bind(a));
     let b = cx.tcx.erase_late_bound_regions(&Binder::bind(b));
@@ -871,8 +871,8 @@ pub fn iter_input_pats<'tcx>(decl: &FnDecl, body: &'tcx Body) -> impl Iterator<I
     (0..decl.inputs.len()).map(move |i| &body.arguments[i])
 }
 
-/// Checks if a given expression is a match expression
-/// expanded from `?` operator or `try` macro.
+/// Checks if a given expression is a match expression expanded from the `?`
+/// operator or the `try` macro.
 pub fn is_try<'a>(cx: &'_ LateContext<'_, '_>, expr: &'a Expr) -> Option<&'a Expr> {
     fn is_ok(cx: &'_ LateContext<'_, '_>, arm: &Arm) -> bool {
         if_chain! {
diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index 231d5ce90d6..72740cee500 100644
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -241,12 +241,12 @@ impl<'a> Sugg<'a> {
     }
 
     /// 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()`).
+    /// `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)
@@ -282,7 +282,7 @@ impl<'a> std::ops::Not for Sugg<'a> {
 
 /// Helper type to display either `foo` or `(foo)`.
 struct ParenHelper<T> {
-    /// Whether parenthesis are needed.
+    /// `true` if parentheses are needed.
     paren: bool,
     /// The main thing to display.
     wrapped: T,
@@ -320,12 +320,13 @@ pub fn make_unop(op: &str, expr: Sugg<'_>) -> Sugg<'static> {
 /// 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,
@@ -333,9 +334,8 @@ pub fn make_assoc(op: AssocOp, lhs: &Sugg<'_>, rhs: &Sugg<'_>) -> Sugg<'static>
         )
     }
 
-    /// 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()
@@ -414,9 +414,8 @@ enum Associativity {
 }
 
 /// 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)`.
+/// `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
diff --git a/tests/ui/block_in_if_condition.rs b/tests/ui/block_in_if_condition.rs
index 08fafc736af..10342ed28b5 100644
--- a/tests/ui/block_in_if_condition.rs
+++ b/tests/ui/block_in_if_condition.rs
@@ -48,9 +48,10 @@ fn predicate<F: FnOnce(T) -> bool, T>(pfn: F, val: T) -> bool {
 fn pred_test() {
     let v = 3;
     let sky = "blue";
-    // this is a sneaky case, where the block isn't directly in the condition, but is actually
-    , andadd some extra
-    // expressions to make sure linter isn't confused by them.
+    // This is a sneaky case, where the block isn't directly in the condition,
+    // but is actually nside a closure that the condition is using.
+    // The same principle applies -- add some extra expressions to make sure
+    // linter isn't confused by them.
     if v == 3
         && sky == "blue"
         && predicate(
diff --git a/tests/ui/format.rs b/tests/ui/format.rs
index f6f6258e82d..2ef31f0b948 100644
--- a/tests/ui/format.rs
+++ b/tests/ui/format.rs
@@ -6,7 +6,7 @@
 struct Foo(pub String);
 
 macro_rules! foo {
-  ($($t:tt)*) => (Foo(format!($($t)*)))
+    ($($t:tt)*) => (Foo(format!($($t)*)))
 }
 
 fn main() {
@@ -49,8 +49,8 @@ fn main() {
     foo!("should not warn");
 
     // Precision on string means slicing without panicking on size.
-    format!("{:.1}", "foo"); // could be `"foo"[..1]`
-    format!("{:.10}", "foo"); // could not be `"foo"[..10]`
+    format!("{:.1}", "foo"); // Could be `"foo"[..1]`
+    format!("{:.10}", "foo"); // Could not be `"foo"[..10]`
     format!("{:.prec$}", "foo", prec = 1);
     format!("{:.prec$}", "foo", prec = 10);