From 7877a42308a05597840f212d10d2abb27086669f Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 19 Mar 2016 15:06:56 +0100 Subject: [PATCH 1/6] Fix some spelling mistakes here and there --- src/copies.rs | 2 +- src/derive.rs | 2 +- src/items_after_statements.rs | 6 +++--- src/lib.rs | 4 ++-- src/misc_early.rs | 4 ++-- src/needless_bool.rs | 4 ++-- src/utils/hir.rs | 2 +- tests/compile-fail/bool_comparison.rs | 4 ++-- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/copies.rs b/src/copies.rs index 789f852c6..04f8aaa37 100644 --- a/src/copies.rs +++ b/src/copies.rs @@ -47,7 +47,7 @@ declare_lint! { /// match foo { /// Bar => bar(), /// Quz => quz(), -/// Baz => bar(), // <= oups +/// Baz => bar(), // <= oops /// } /// ``` declare_lint! { diff --git a/src/derive.rs b/src/derive.rs index 4a20d8018..ab4f73eaf 100644 --- a/src/derive.rs +++ b/src/derive.rs @@ -14,7 +14,7 @@ use utils::{match_path, span_lint_and_then}; /// /// **Why is this bad?** The implementation of these traits must agree (for example for use with /// `HashMap`) so it’s probably a bad idea to use a default-generated `Hash` implementation with -/// an explicitely defined `PartialEq`. In particular, the following must hold for any type: +/// an explicitly defined `PartialEq`. In particular, the following must hold for any type: /// /// ```rust /// k1 == k2 ⇒ hash(k1) == hash(k2) diff --git a/src/items_after_statements.rs b/src/items_after_statements.rs index a2ab72469..952dcb7ed 100644 --- a/src/items_after_statements.rs +++ b/src/items_after_statements.rs @@ -32,15 +32,15 @@ declare_lint! { "finds blocks where an item comes after a statement" } -pub struct ItemsAfterStatemets; +pub struct ItemsAfterStatements; -impl LintPass for ItemsAfterStatemets { +impl LintPass for ItemsAfterStatements { fn get_lints(&self) -> LintArray { lint_array!(ITEMS_AFTER_STATEMENTS) } } -impl EarlyLintPass for ItemsAfterStatemets { +impl EarlyLintPass for ItemsAfterStatements { fn check_block(&mut self, cx: &EarlyContext, item: &Block) { if in_macro(cx, item.span) { return; diff --git a/src/lib.rs b/src/lib.rs index 7d24b2c89..75b5ec4cc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -134,7 +134,7 @@ pub fn plugin_registrar(reg: &mut Registry) { } Err((err, span)) => { reg.sess.struct_span_err(span, err) - .span_note(span, "Clippy will use defaulf configuration") + .span_note(span, "Clippy will use default configuration") .emit(); utils::conf::Conf::default() } @@ -163,7 +163,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_early_lint_pass(box precedence::Precedence); reg.register_late_lint_pass(box eta_reduction::EtaPass); reg.register_late_lint_pass(box identity_op::IdentityOp); - reg.register_early_lint_pass(box items_after_statements::ItemsAfterStatemets); + reg.register_early_lint_pass(box items_after_statements::ItemsAfterStatements); reg.register_late_lint_pass(box mut_mut::MutMut); reg.register_late_lint_pass(box mut_reference::UnnecessaryMutPassed); reg.register_late_lint_pass(box len_zero::LenZero); diff --git a/src/misc_early.rs b/src/misc_early.rs index 60e175d63..b1c584a4b 100644 --- a/src/misc_early.rs +++ b/src/misc_early.rs @@ -110,10 +110,10 @@ impl EarlyLintPass for MiscEarly { let arg_name = sp_ident.node.to_string(); if arg_name.starts_with('_') { - if let Some(correspondance) = registered_names.get(&arg_name[1..]) { + if let Some(correspondence) = registered_names.get(&arg_name[1..]) { span_lint(cx, DUPLICATE_UNDERSCORE_ARGUMENT, - *correspondance, + *correspondence, &format!("`{}` already exists, having another argument having almost the same \ name makes code comprehension and documentation more difficult", arg_name[1..].to_owned()));; diff --git a/src/needless_bool.rs b/src/needless_bool.rs index 43e7cfdda..ab5a1e26b 100644 --- a/src/needless_bool.rs +++ b/src/needless_bool.rs @@ -105,7 +105,7 @@ impl LateLintPass for BoolComparison { span_lint_and_then(cx, BOOL_COMPARISON, e.span, - "equality checks against true are unnecesary", + "equality checks against true are unnecessary", |db| { db.span_suggestion(e.span, "try simplifying it as shown:", hint); }); @@ -115,7 +115,7 @@ impl LateLintPass for BoolComparison { span_lint_and_then(cx, BOOL_COMPARISON, e.span, - "equality checks against true are unnecesary", + "equality checks against true are unnecessary", |db| { db.span_suggestion(e.span, "try simplifying it as shown:", hint); }); diff --git a/src/utils/hir.rs b/src/utils/hir.rs index ed2a49b3c..20c7e33fb 100644 --- a/src/utils/hir.rs +++ b/src/utils/hir.rs @@ -13,7 +13,7 @@ use utils::differing_macro_contexts; pub struct SpanlessEq<'a, 'tcx: 'a> { /// Context used to evaluate constant expressions. cx: &'a LateContext<'a, 'tcx>, - /// If is true, never consider as equal expressions containing fonction calls. + /// If is true, never consider as equal expressions containing function calls. ignore_fn: bool, } diff --git a/tests/compile-fail/bool_comparison.rs b/tests/compile-fail/bool_comparison.rs index 8a792931d..836759455 100644 --- a/tests/compile-fail/bool_comparison.rs +++ b/tests/compile-fail/bool_comparison.rs @@ -5,7 +5,7 @@ fn main() { let x = true; if x == true { "yes" } else { "no" }; - //~^ ERROR equality checks against true are unnecesary + //~^ ERROR equality checks against true are unnecessary //~| HELP try simplifying it as shown: //~| SUGGESTION if x { "yes" } else { "no" }; if x == false { "yes" } else { "no" }; @@ -13,7 +13,7 @@ fn main() { //~| HELP try simplifying it as shown: //~| SUGGESTION if !x { "yes" } else { "no" }; if true == x { "yes" } else { "no" }; - //~^ ERROR equality checks against true are unnecesary + //~^ ERROR equality checks against true are unnecessary //~| HELP try simplifying it as shown: //~| SUGGESTION if x { "yes" } else { "no" }; if false == x { "yes" } else { "no" }; From cfb1bc3723f42e98d08e24001696abc5a17bc8a6 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 19 Mar 2016 15:09:24 +0100 Subject: [PATCH 2/6] `chmod -x` test files --- tests/compile-fail/blacklisted_name.rs | 0 tests/compile-fail/conf_french_blacklisted_name.rs | 0 tests/compile-fail/copies.rs | 0 tests/compile-fail/derive.rs | 0 tests/compile-fail/format.rs | 0 tests/compile-fail/formatting.rs | 0 tests/compile-fail/functions.rs | 0 tests/compile-fail/new_without_default.rs | 0 tests/compile-fail/print.rs | 0 tests/compile-fail/swap.rs | 0 tests/compile-fail/unused_labels.rs | 0 tests/compile-fail/vec.rs | 0 tests/run-pass/ice-700.rs | 0 13 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 tests/compile-fail/blacklisted_name.rs mode change 100755 => 100644 tests/compile-fail/conf_french_blacklisted_name.rs mode change 100755 => 100644 tests/compile-fail/copies.rs mode change 100755 => 100644 tests/compile-fail/derive.rs mode change 100755 => 100644 tests/compile-fail/format.rs mode change 100755 => 100644 tests/compile-fail/formatting.rs mode change 100755 => 100644 tests/compile-fail/functions.rs mode change 100755 => 100644 tests/compile-fail/new_without_default.rs mode change 100755 => 100644 tests/compile-fail/print.rs mode change 100755 => 100644 tests/compile-fail/swap.rs mode change 100755 => 100644 tests/compile-fail/unused_labels.rs mode change 100755 => 100644 tests/compile-fail/vec.rs mode change 100755 => 100644 tests/run-pass/ice-700.rs diff --git a/tests/compile-fail/blacklisted_name.rs b/tests/compile-fail/blacklisted_name.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/conf_french_blacklisted_name.rs b/tests/compile-fail/conf_french_blacklisted_name.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/copies.rs b/tests/compile-fail/copies.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/derive.rs b/tests/compile-fail/derive.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/format.rs b/tests/compile-fail/format.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/formatting.rs b/tests/compile-fail/formatting.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/functions.rs b/tests/compile-fail/functions.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/new_without_default.rs b/tests/compile-fail/new_without_default.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/print.rs b/tests/compile-fail/print.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/swap.rs b/tests/compile-fail/swap.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/unused_labels.rs b/tests/compile-fail/unused_labels.rs old mode 100755 new mode 100644 diff --git a/tests/compile-fail/vec.rs b/tests/compile-fail/vec.rs old mode 100755 new mode 100644 diff --git a/tests/run-pass/ice-700.rs b/tests/run-pass/ice-700.rs old mode 100755 new mode 100644 From 941ec6e4f5cfc54078f5fe65702ec46b6c59b8d2 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 19 Mar 2016 17:48:29 +0100 Subject: [PATCH 3/6] Beautify more docs --- src/bit_mask.rs | 36 +++++++++++++------------- src/consts.rs | 2 +- src/formatting.rs | 4 +-- src/len_zero.rs | 6 ++--- src/loops.rs | 8 +++--- src/matches.rs | 2 +- src/non_expressive_names.rs | 2 +- src/ptr_arg.rs | 4 +-- src/strings.rs | 2 +- src/utils/mod.rs | 10 +++---- src/zero_div_zero.rs | 2 +- tests/compile-fail/methods.rs | 10 +++---- tests/used_underscore_binding_macro.rs | 2 +- 13 files changed, 44 insertions(+), 46 deletions(-) diff --git a/src/bit_mask.rs b/src/bit_mask.rs index f96927f15..15fbabf9f 100644 --- a/src/bit_mask.rs +++ b/src/bit_mask.rs @@ -13,14 +13,14 @@ use utils::span_lint; /// The formula for detecting if an expression of the type `_ m c` (where `` /// is one of {`&`, `|`} and `` is one of {`!=`, `>=`, `>`, `!=`, `>=`, `>`}) can be determined from the following table: /// -/// |Comparison |Bit-Op|Example |is always|Formula | -/// |------------|------|------------|---------|----------------------| -/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | -/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | -/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | -/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | -/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | -/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | +/// |Comparison |Bit Op |Example |is always|Formula | +/// |------------|-------|------------|---------|----------------------| +/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | +/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | +/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | +/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | +/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | +/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | /// /// **Why is this bad?** If the bits that the comparison cares about are always set to zero or one by the bit mask, the comparison is constant `true` or `false` (depending on mask, compared value, and operators). /// @@ -38,7 +38,7 @@ declare_lint! { /// **What it does:** This lint checks for bit masks in comparisons which can be removed without changing the outcome. The basic structure can be seen in the following table: /// -/// |Comparison|Bit-Op |Example |equals | +/// |Comparison| Bit Op |Example |equals | /// |----------|---------|-----------|-------| /// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`| /// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`| @@ -61,21 +61,21 @@ declare_lint! { /// is one of {`&`, '|'} and `` is one of {`!=`, `>=`, `>` , /// `!=`, `>=`, `>`}) can be determined from the following table: /// -/// |Comparison |Bit-Op|Example |is always|Formula | -/// |------------|------|------------|---------|----------------------| -/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | -/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | -/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | -/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | -/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | -/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | +/// |Comparison |Bit Op |Example |is always|Formula | +/// |------------|-------|------------|---------|----------------------| +/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | +/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | +/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | +/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | +/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | +/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | /// /// This lint is **deny** by default /// /// There is also a lint that warns on ineffective masks that is *warn* /// by default. /// -/// |Comparison|Bit-Op |Example |equals |Formula| +/// |Comparison| Bit Op |Example |equals |Formula| /// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`|`¹ && m <= c`| /// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`|`¹ && m < c` | /// diff --git a/src/consts.rs b/src/consts.rs index 67da1216c..97a99dda4 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -30,7 +30,7 @@ impl From for FloatWidth { } } -/// a Lit_-like enum to fold constant `Expr`s into +/// A `LitKind`-like enum to fold constant `Expr`s into. #[derive(Debug, Clone)] pub enum Constant { /// a String "abc" diff --git a/src/formatting.rs b/src/formatting.rs index 091280de2..aa6dd46cf 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -82,7 +82,7 @@ impl EarlyLintPass for Formatting { } } -/// Implementation of the SUSPICIOUS_ASSIGNMENT_FORMATTING lint. +/// Implementation of the `SUSPICIOUS_ASSIGNMENT_FORMATTING` lint. fn check_assign(cx: &EarlyContext, expr: &ast::Expr) { if let ast::ExprKind::Assign(ref lhs, ref rhs) = expr.node { if !differing_macro_contexts(lhs.span, rhs.span) && !in_macro(cx, lhs.span) { @@ -108,7 +108,7 @@ fn check_assign(cx: &EarlyContext, expr: &ast::Expr) { } } -/// Implementation of the SUSPICIOUS_ELSE_FORMATTING lint for weird `else if`. +/// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else if`. fn check_else_if(cx: &EarlyContext, expr: &ast::Expr) { if let Some((then, &Some(ref else_))) = unsugar_if(expr) { if unsugar_if(else_).is_some() && !differing_macro_contexts(then.span, else_.span) && !in_macro(cx, then.span) { diff --git a/src/len_zero.rs b/src/len_zero.rs index 6dad86843..1a097820e 100644 --- a/src/len_zero.rs +++ b/src/len_zero.rs @@ -164,9 +164,9 @@ fn check_len_zero(cx: &LateContext, span: Span, name: &Name, args: &[P], l } } -/// check if this type has an is_empty method +/// Check if this type has an `is_empty` method. fn has_is_empty(cx: &LateContext, expr: &Expr) -> bool { - /// get a ImplOrTraitItem and return true if it matches is_empty(self) + /// Get an `ImplOrTraitItem` and return true if it matches `is_empty(self)`. fn is_is_empty(cx: &LateContext, id: &ImplOrTraitItemId) -> bool { if let MethodTraitItemId(def_id) = *id { if let ty::MethodTraitItem(ref method) = cx.tcx.impl_or_trait_item(def_id) { @@ -179,7 +179,7 @@ fn has_is_empty(cx: &LateContext, expr: &Expr) -> bool { } } - /// check the inherent impl's items for an is_empty(self) method + /// Check the inherent impl's items for an `is_empty(self)` method. fn has_is_empty_impl(cx: &LateContext, id: &DefId) -> bool { let impl_items = cx.tcx.impl_items.borrow(); cx.tcx.inherent_impls.borrow().get(id).map_or(false, |ids| { diff --git a/src/loops.rs b/src/loops.rs index c0d399374..546f07a65 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -571,7 +571,7 @@ fn check_for_loop_explicit_counter(cx: &LateContext, arg: &Expr, body: &Expr, ex } } -/// Check for the FOR_KV_MAP lint. +/// Check for the `FOR_KV_MAP` lint. fn check_for_loop_over_map_kv(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Expr, expr: &Expr) { if let PatKind::Tup(ref pat) = pat.node { if pat.len() == 2 { @@ -607,7 +607,7 @@ fn check_for_loop_over_map_kv(cx: &LateContext, pat: &Pat, arg: &Expr, body: &Ex } -/// Return true if the pattern is a `PatWild` or an ident prefixed with '_'. +/// Return true if the pattern is a `PatWild` or an ident prefixed with `'_'`. fn pat_is_wild(pat: &PatKind, body: &Expr) -> bool { match *pat { PatKind::Wild => true, @@ -750,8 +750,8 @@ impl<'v, 't> Visitor<'v> for VarUsedAfterLoopVisitor<'v, 't> { } -/// Return true if the type of expr is one that provides IntoIterator impls -/// for &T and &mut T, such as Vec. +/// Return true if the type of expr is one that provides `IntoIterator` impls +/// for `&T` and `&mut T`, such as `Vec`. #[cfg_attr(rustfmt, rustfmt_skip)] fn is_ref_iterable_type(cx: &LateContext, e: &Expr) -> bool { // no walk_ptrs_ty: calling iter() on a reference can make sense because it diff --git a/src/matches.rs b/src/matches.rs index bc3b45b32..7bffd445f 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -330,7 +330,7 @@ fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: Match } } -/// Get all arms that are unbounded PatRange-s. +/// Get all arms that are unbounded `PatRange`s. fn all_ranges(cx: &LateContext, arms: &[Arm]) -> Vec> { arms.iter() .filter_map(|arm| { diff --git a/src/non_expressive_names.rs b/src/non_expressive_names.rs index b7d2ac80a..d7cb6fc5d 100644 --- a/src/non_expressive_names.rs +++ b/src/non_expressive_names.rs @@ -251,7 +251,7 @@ impl EarlyLintPass for NonExpressiveNames { } } -/// precondition: a_name.chars().count() < b_name.chars().count() +/// Precondition: `a_name.chars().count() < b_name.chars().count()`. fn levenstein_not_1(a_name: &str, b_name: &str) -> bool { debug_assert!(a_name.chars().count() < b_name.chars().count()); let mut a_chars = a_name.chars(); diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs index b7882bfda..6498db66e 100644 --- a/src/ptr_arg.rs +++ b/src/ptr_arg.rs @@ -1,6 +1,4 @@ -//! Checks for usage of &Vec[_] and &String -//! -//! This lint is **warn** by default +//! Checks for usage of `&Vec[_]` and `&String`. use rustc::front::map::NodeItem; use rustc::lint::*; diff --git a/src/strings.rs b/src/strings.rs index aa9cdcce5..9f68175b2 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -1,4 +1,4 @@ -//! This LintPass catches both string addition and string addition + assignment +//! This lint catches both string addition and string addition + assignment //! //! Note that since we have two lints where one subsumes the other, we try to //! disable the subsumed lint unless it has a higher level diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 017890dc4..34404f4c2 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -129,8 +129,8 @@ pub fn in_macro(cx: &T, span: Span) -> bool { /// Returns true if the macro that expanded the crate was outside of the current crate or was a /// compiler plugin. pub fn in_external_macro(cx: &T, span: Span) -> bool { - /// Invokes in_macro with the expansion info of the given span slightly heavy, try to use this - /// after other checks have already happened. + /// Invokes `in_macro` with the expansion info of the given span slightly heavy, try to use + /// this after other checks have already happened. fn in_macro_ext(cx: &T, opt_info: Option<&ExpnInfo>) -> bool { // no ExpnInfo = no macro opt_info.map_or(false, |info| { @@ -657,7 +657,7 @@ pub fn is_direct_expn_of(cx: &LateContext, span: Span, name: &str) -> Option usize { let mut iter = s.char_indices(); if let Some((_, first)) = iter.next() { @@ -690,7 +690,7 @@ pub fn camel_case_until(s: &str) -> usize { } } -/// Returns index of last CamelCase component of `s`. +/// Return index of the last camel-case component of `s`. pub fn camel_case_from(s: &str) -> usize { let mut iter = s.char_indices().rev(); if let Some((_, first)) = iter.next() { @@ -719,7 +719,7 @@ pub fn camel_case_from(s: &str) -> usize { last_i } -/// Represents a range akin to `ast::ExprKind::Range`. +/// Represent a range akin to `ast::ExprKind::Range`. #[derive(Debug, Copy, Clone)] pub struct UnsugaredRange<'a> { pub start: Option<&'a Expr>, diff --git a/src/zero_div_zero.rs b/src/zero_div_zero.rs index 1d119b051..f58b0e695 100644 --- a/src/zero_div_zero.rs +++ b/src/zero_div_zero.rs @@ -4,7 +4,7 @@ use rustc_front::hir::*; use utils::span_help_and_lint; /// `ZeroDivZeroPass` is a pass that checks for a binary expression that consists -/// `of 0.0/0.0`, which is always NaN. It is more clear to replace instances of +/// `of 0.0/0.0`, which is always `NaN`. It is more clear to replace instances of /// `0.0/0.0` with `std::f32::NaN` or `std::f64::NaN`, depending on the precision. pub struct ZeroDivZeroPass; diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 74f262b05..9d938ebb1 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -85,8 +85,8 @@ macro_rules! opt_map { } /// Checks implementation of the following lints: -/// OPTION_MAP_UNWRAP_OR -/// OPTION_MAP_UNWRAP_OR_ELSE +/// * `OPTION_MAP_UNWRAP_OR` +/// * `OPTION_MAP_UNWRAP_OR_ELSE` fn option_methods() { let opt = Some(1); @@ -154,7 +154,7 @@ impl IteratorFalsePositives { } } -/// Checks implementation of FILTER_NEXT lint +/// Checks implementation of `FILTER_NEXT` lint fn filter_next() { let v = vec![3, 2, 1, 0, -1, -2, -3]; @@ -174,7 +174,7 @@ fn filter_next() { let _ = foo.filter().next(); } -/// Checks implementation of SEARCH_IS_SOME lint +/// Checks implementation of `SEARCH_IS_SOME` lint fn search_is_some() { let v = vec![3, 2, 1, 0, -1, -2, -3]; @@ -218,7 +218,7 @@ fn search_is_some() { let _ = foo.rposition().is_some(); } -/// Checks implementation of the OR_FUN_CALL lint +/// Checks implementation of the `OR_FUN_CALL` lint fn or_fun_call() { struct Foo; diff --git a/tests/used_underscore_binding_macro.rs b/tests/used_underscore_binding_macro.rs index 4170f907b..7a8faa627 100644 --- a/tests/used_underscore_binding_macro.rs +++ b/tests/used_underscore_binding_macro.rs @@ -3,7 +3,7 @@ extern crate rustc_serialize; -/// Test that we do not lint for unused underscores in a MacroAttribute expansion +/// Test that we do not lint for unused underscores in a `MacroAttribute` expansion #[deny(used_underscore_binding)] #[derive(RustcEncodable)] struct MacroAttributesTest { From 42bf37f49f49829507be4f2dfd6c5db9b8234b66 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sat, 19 Mar 2016 17:59:12 +0100 Subject: [PATCH 4/6] Add a lint for bad documentation formatting --- README.md | 1 + src/doc.rs | 112 ++++++++++++++++++++++++++++++++++++++ src/lib.rs | 3 + tests/compile-fail/doc.rs | 26 +++++++++ 4 files changed, 142 insertions(+) create mode 100644 src/doc.rs create mode 100755 tests/compile-fail/doc.rs diff --git a/README.md b/README.md index 95ba6b085..e4d18c28b 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,7 @@ name [cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | finds functions that should be split up into multiple functions [deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | `Warn` on `#[deprecated(since = "x")]` where x is not semver [derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly +[doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown) | warn | checks for the presence of the `_` character outside ticks in documentation [drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value [duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected diff --git a/src/doc.rs b/src/doc.rs new file mode 100644 index 000000000..610aa34ab --- /dev/null +++ b/src/doc.rs @@ -0,0 +1,112 @@ +use rustc::lint::*; +use std::borrow::Cow; +use syntax::ast; +use syntax::codemap::Span; +use utils::span_lint; + +/// **What it does:** This lint checks for the presence of the `_` character outside ticks in +/// documentation. +/// +/// **Why is this bad?** *Rustdoc* supports markdown formatting, the `_` character probably +/// indicates some code which should be included between ticks. +/// +/// **Known problems:** Lots of bad docs won’t be fixed, the lint only checks for `_`. +/// +/// **Examples:** +/// ```rust +/// /// Do something with the foo_bar parameter. +/// fn doit(foo_bar) { .. } +/// ``` +declare_lint! { + pub DOC_MARKDOWN, Warn, + "checks for the presence of the `_` character outside ticks in documentation" +} + +#[derive(Copy,Clone)] +pub struct Doc; + +impl LintPass for Doc { + fn get_lints(&self) -> LintArray { + lint_array![DOC_MARKDOWN] + } +} + +impl EarlyLintPass for Doc { + fn check_crate(&mut self, cx: &EarlyContext, krate: &ast::Crate) { + check_attrs(cx, &krate.attrs, krate.span); + } + + fn check_item(&mut self, cx: &EarlyContext, item: &ast::Item) { + check_attrs(cx, &item.attrs, item.span); + } +} + +/// Collect all doc attributes. Multiple `///` are represented in different attributes. `rustdoc` +/// has a pass to merge them, but we probably don’t want to invoke that here. +fn collect_doc(attrs: &[ast::Attribute]) -> (Cow, Option) { + fn doc_and_span(attr: &ast::Attribute) -> Option<(&str, Span)> { + if attr.node.is_sugared_doc { + if let ast::MetaItemKind::NameValue(_, ref doc) = attr.node.value.node { + if let ast::LitKind::Str(ref doc, _) = doc.node { + return Some((&doc[..], attr.span)); + } + } + } + + None + } + let doc_and_span: fn(_) -> _ = doc_and_span; + + let mut doc_attrs = attrs.iter().filter_map(doc_and_span); + + let count = doc_attrs.clone().take(2).count(); + + match count { + 0 => ("".into(), None), + 1 => { + let (doc, span) = doc_attrs.next().unwrap_or_else(|| unreachable!()); + (doc.into(), Some(span)) + } + _ => (doc_attrs.map(|s| s.0).collect::().into(), None), + } +} + +fn check_attrs<'a>(cx: &EarlyContext, attrs: &'a [ast::Attribute], default_span: Span) { + let (doc, span) = collect_doc(attrs); + let span = span.unwrap_or(default_span); + + let mut in_ticks = false; + for word in doc.split_whitespace() { + let ticks = word.bytes().filter(|&b| b == b'`').count(); + + if ticks == 2 { // likely to be “`foo`” + continue; + } else if ticks % 2 == 1 { + in_ticks = !in_ticks; + continue; // let’s assume no one will ever write something like “`foo`_bar” + } + + if !in_ticks { + check_word(cx, word, span); + } + } +} + +fn check_word(cx: &EarlyContext, word: &str, span: Span) { + /// Checks if a string a camel-case, ie. contains at least two uppercase letter (`Clippy` is + /// ok) and one lower-case letter (`NASA` is ok). Plural are also excluded (`IDs` is ok). + fn is_camel_case(s: &str) -> bool { + let s = if s.ends_with('s') { + &s[..s.len()-1] + } else { + s + }; + + s.chars().filter(|&c| c.is_uppercase()).take(2).count() > 1 && + s.chars().filter(|&c| c.is_lowercase()).take(1).count() > 0 + } + + if word.contains('_') || is_camel_case(word) { + span_lint(cx, DOC_MARKDOWN, span, &format!("you should put `{}` between ticks in the documentation", word)); + } +} diff --git a/src/lib.rs b/src/lib.rs index 75b5ec4cc..0a51385f6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,6 +54,7 @@ pub mod collapsible_if; pub mod copies; pub mod cyclomatic_complexity; pub mod derive; +pub mod doc; pub mod drop_ref; pub mod entry; pub mod enum_clike; @@ -223,6 +224,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box new_without_default::NewWithoutDefault); reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names)); reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold)); + reg.register_early_lint_pass(box doc::Doc); reg.register_lint_group("clippy_pedantic", vec![ array_indexing::INDEXING_SLICING, @@ -265,6 +267,7 @@ pub fn plugin_registrar(reg: &mut Registry) { cyclomatic_complexity::CYCLOMATIC_COMPLEXITY, derive::DERIVE_HASH_XOR_EQ, derive::EXPL_IMPL_CLONE_ON_COPY, + doc::DOC_MARKDOWN, drop_ref::DROP_REF, entry::MAP_ENTRY, enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT, diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs new file mode 100755 index 000000000..9eb949a3b --- /dev/null +++ b/tests/compile-fail/doc.rs @@ -0,0 +1,26 @@ +//! This file tests for the DOC_MARKDOWN lint +//~^ ERROR: you should put `DOC_MARKDOWN` between ticks + +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(doc_markdown)] + +/// The foo_bar function does nothing. +//~^ ERROR: you should put `foo_bar` between ticks +fn foo_bar() { +} + +/// That one tests multiline ticks. +/// ```rust +/// foo_bar FOO_BAR +/// ``` +fn multiline_ticks() { +} + +/// The `main` function is the entry point of the program. Here it only calls the `foo_bar` and +/// `multiline_ticks` functions. +fn main() { + foo_bar(); + multiline_ticks(); +} From b1d1f095f12d6640aac2ab7e8ad29fc36de90b39 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 28 Mar 2016 18:00:24 +0200 Subject: [PATCH 5/6] Improve the DOC_MARKDOWN lint `_` can be used for emphasize text. `::` is equality as bad outside ticks. --- README.md | 2 +- src/doc.rs | 37 +++++++++++++++++++++++++++++-------- tests/compile-fail/doc.rs | 8 ++++++-- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index e4d18c28b..264bc6f01 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ name [cyclomatic_complexity](https://github.com/Manishearth/rust-clippy/wiki#cyclomatic_complexity) | warn | finds functions that should be split up into multiple functions [deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver) | warn | `Warn` on `#[deprecated(since = "x")]` where x is not semver [derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq) | warn | deriving `Hash` but implementing `PartialEq` explicitly -[doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown) | warn | checks for the presence of the `_` character outside ticks in documentation +[doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown) | warn | checks for the presence of `_`, `::` or camel-case outside ticks in documentation [drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value [duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected diff --git a/src/doc.rs b/src/doc.rs index 610aa34ab..a4a0448c1 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -4,22 +4,23 @@ use syntax::ast; use syntax::codemap::Span; use utils::span_lint; -/// **What it does:** This lint checks for the presence of the `_` character outside ticks in -/// documentation. +/// **What it does:** This lint checks for the presence of `_`, `::` or camel-case words outside +/// ticks in documentation. /// -/// **Why is this bad?** *Rustdoc* supports markdown formatting, the `_` character probably +/// **Why is this bad?** *Rustdoc* supports markdown formatting, `_`, `::` and camel-case probably /// indicates some code which should be included between ticks. /// -/// **Known problems:** Lots of bad docs won’t be fixed, the lint only checks for `_`. +/// **Known problems:** Lots of bad docs won’t be fixed, what the lint checks for is limited. /// /// **Examples:** /// ```rust -/// /// Do something with the foo_bar parameter. +/// /// Do something with the foo_bar parameter. See also that::other::module::foo. +/// // ^ `foo_bar` and `that::other::module::foo` should be ticked. /// fn doit(foo_bar) { .. } /// ``` declare_lint! { pub DOC_MARKDOWN, Warn, - "checks for the presence of the `_` character outside ticks in documentation" + "checks for the presence of `_`, `::` or camel-case outside ticks in documentation" } #[derive(Copy,Clone)] @@ -71,10 +72,21 @@ fn collect_doc(attrs: &[ast::Attribute]) -> (Cow, Option) { } } -fn check_attrs<'a>(cx: &EarlyContext, attrs: &'a [ast::Attribute], default_span: Span) { +pub fn check_attrs<'a>(cx: &EarlyContext, attrs: &'a [ast::Attribute], default_span: Span) { let (doc, span) = collect_doc(attrs); let span = span.unwrap_or(default_span); + // In markdown, `_` can be used to emphasize something, or, is a raw `_` depending on context. + // There really is no markdown specification that would disambiguate this properly. This is + // what GitHub and Rustdoc do: + // + // foo_bar test_quz → foo_bar test_quz + // foo_bar_baz → foo_bar_baz (note that the “official” spec says this should be emphasized) + // _foo bar_ test_quz_ → foo bar test_quz_ + // \_foo bar\_ → _foo bar_ + // (_baz_) → (baz) + // foo _ bar _ baz → foo _ bar _ baz + let mut in_ticks = false; for word in doc.split_whitespace() { let ticks = word.bytes().filter(|&b| b == b'`').count(); @@ -106,7 +118,16 @@ fn check_word(cx: &EarlyContext, word: &str, span: Span) { s.chars().filter(|&c| c.is_lowercase()).take(1).count() > 0 } - if word.contains('_') || is_camel_case(word) { + fn has_underscore(s: &str) -> bool { + s != "_" && !s.contains("\\_") && s.contains('_') + } + + // Trim punctuation as in `some comment (see foo::bar).` + // ^^ + // Or even as `_foo bar_` which is emphasized. + let word = word.trim_matches(|c: char| !c.is_alphanumeric()); + + if has_underscore(word) || word.contains("::") || is_camel_case(word) { span_lint(cx, DOC_MARKDOWN, span, &format!("you should put `{}` between ticks in the documentation", word)); } } diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs index 9eb949a3b..35b5857d9 100755 --- a/tests/compile-fail/doc.rs +++ b/tests/compile-fail/doc.rs @@ -6,9 +6,13 @@ #![deny(doc_markdown)] -/// The foo_bar function does nothing. -//~^ ERROR: you should put `foo_bar` between ticks +/// The foo_bar function does _nothing_. See also foo::bar. (note the dot there) +/// Markdown is _weird_. I mean _really weird_. This \_ is ok. So is `_`. But not Foo::some_fun +/// which should be reported only once despite being __doubly bad__. fn foo_bar() { +//~^ ERROR: you should put `foo_bar` between ticks +//~| ERROR: you should put `foo::bar` between ticks +//~| ERROR: you should put `Foo::some_fun` between ticks } /// That one tests multiline ticks. From 371a5537eb0435fef43763d1bbbc21007ac707e4 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 28 Mar 2016 21:23:21 +0200 Subject: [PATCH 6/6] Address nits in DOC_MARKDOWN --- README.md | 2 +- src/bit_mask.rs | 34 +++++++++++++++++----------------- src/doc.rs | 4 +++- tests/compile-fail/doc.rs | 8 ++++++++ 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 264bc6f01..43caa8054 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Table of contents: * [License](#license) ##Lints -There are 136 lints included in this crate: +There are 137 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ diff --git a/src/bit_mask.rs b/src/bit_mask.rs index 15fbabf9f..28fc31150 100644 --- a/src/bit_mask.rs +++ b/src/bit_mask.rs @@ -13,14 +13,14 @@ use utils::span_lint; /// The formula for detecting if an expression of the type `_ m c` (where `` /// is one of {`&`, `|`} and `` is one of {`!=`, `>=`, `>`, `!=`, `>=`, `>`}) can be determined from the following table: /// -/// |Comparison |Bit Op |Example |is always|Formula | -/// |------------|-------|------------|---------|----------------------| -/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | -/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | -/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | -/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | -/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | -/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | +/// |Comparison |Bit Op|Example |is always|Formula | +/// |------------|------|------------|---------|----------------------| +/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | +/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | +/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | +/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | +/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | +/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | /// /// **Why is this bad?** If the bits that the comparison cares about are always set to zero or one by the bit mask, the comparison is constant `true` or `false` (depending on mask, compared value, and operators). /// @@ -61,21 +61,21 @@ declare_lint! { /// is one of {`&`, '|'} and `` is one of {`!=`, `>=`, `>` , /// `!=`, `>=`, `>`}) can be determined from the following table: /// -/// |Comparison |Bit Op |Example |is always|Formula | -/// |------------|-------|------------|---------|----------------------| -/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | -/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | -/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | -/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | -/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | -/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | +/// |Comparison |Bit Op|Example |is always|Formula | +/// |------------|------|------------|---------|----------------------| +/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | +/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | +/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | +/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | +/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | +/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | /// /// This lint is **deny** by default /// /// There is also a lint that warns on ineffective masks that is *warn* /// by default. /// -/// |Comparison| Bit Op |Example |equals |Formula| +/// |Comparison|Bit Op |Example |equals |Formula| /// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`|`¹ && m <= c`| /// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`|`¹ && m < c` | /// diff --git a/src/doc.rs b/src/doc.rs index a4a0448c1..5637fb2ce 100644 --- a/src/doc.rs +++ b/src/doc.rs @@ -8,7 +8,8 @@ use utils::span_lint; /// ticks in documentation. /// /// **Why is this bad?** *Rustdoc* supports markdown formatting, `_`, `::` and camel-case probably -/// indicates some code which should be included between ticks. +/// indicates some code which should be included between ticks. `_` can also be used for empasis in +/// markdown, this lint tries to consider that. /// /// **Known problems:** Lots of bad docs won’t be fixed, what the lint checks for is limited. /// @@ -114,6 +115,7 @@ fn check_word(cx: &EarlyContext, word: &str, span: Span) { s }; + s.chars().all(char::is_alphanumeric) && s.chars().filter(|&c| c.is_uppercase()).take(2).count() > 1 && s.chars().filter(|&c| c.is_lowercase()).take(1).count() > 0 } diff --git a/tests/compile-fail/doc.rs b/tests/compile-fail/doc.rs index 35b5857d9..eecf5e0b2 100755 --- a/tests/compile-fail/doc.rs +++ b/tests/compile-fail/doc.rs @@ -18,13 +18,21 @@ fn foo_bar() { /// That one tests multiline ticks. /// ```rust /// foo_bar FOO_BAR +/// _foo bar_ /// ``` fn multiline_ticks() { } +/// This _is a test for +/// multiline +/// emphasis_. +fn test_emphasis() { +} + /// The `main` function is the entry point of the program. Here it only calls the `foo_bar` and /// `multiline_ticks` functions. fn main() { foo_bar(); multiline_ticks(); + test_emphasis(); }