From c645a9febe4451bec13eda67fb808f3471d5d00f Mon Sep 17 00:00:00 2001 From: llogiq Date: Mon, 14 Dec 2015 22:16:56 +0100 Subject: [PATCH] adding missing doc comments --- src/cyclomatic_complexity.rs | 7 +++++++ src/lifetimes.rs | 7 +++++++ src/loops.rs | 7 +++++++ src/methods.rs | 30 ++++++++++++++++++++++++++++++ src/misc_early.rs | 7 +++++++ src/needless_features.rs | 14 ++++++++++++++ src/needless_update.rs | 7 +++++++ src/no_effect.rs | 7 +++++++ src/temporary_assignment.rs | 7 +++++++ 9 files changed, 93 insertions(+) diff --git a/src/cyclomatic_complexity.rs b/src/cyclomatic_complexity.rs index cb3917692..c2cc6a8c4 100644 --- a/src/cyclomatic_complexity.rs +++ b/src/cyclomatic_complexity.rs @@ -11,6 +11,13 @@ use rustc_front::intravisit::{Visitor, walk_expr}; use utils::{in_macro, LimitStack}; +/// **What it does:** It `Warn`s on methods with high cyclomatic complexity +/// +/// **Why is this bad?** Methods of high cyclomatic complexity tend to be badly readable. Also LLVM will usually optimize small methods better. +/// +/// **Known problems:** Sometimes it's hard to find a way to reduce the complexity +/// +/// **Example:** No. You'll see it when you get the warning. declare_lint! { pub CYCLOMATIC_COMPLEXITY, Warn, "finds functions that should be split up into multiple functions" } diff --git a/src/lifetimes.rs b/src/lifetimes.rs index 743dc366e..6ae10c094 100644 --- a/src/lifetimes.rs +++ b/src/lifetimes.rs @@ -19,6 +19,13 @@ declare_lint!(pub NEEDLESS_LIFETIMES, Warn, "using explicit lifetimes for references in function arguments when elision rules \ would allow omitting them"); +/// **What it does:** This lint checks for lifetimes in generics that are never used anywhere else. It is `Warn` by default. +/// +/// **Why is this bad?** The additional lifetimes make the code look more complicated, while there is nothing out of the ordinary going on. Removing them leads to more readable code. +/// +/// **Known problems:** None +/// +/// **Example:** `fn unused_lifetime<'a>(x: u8) { .. }` declare_lint!(pub UNUSED_LIFETIMES, Warn, "unused lifetimes in function definitions"); diff --git a/src/loops.rs b/src/loops.rs index 8295e1861..5c552d2ce 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -114,6 +114,13 @@ declare_lint!{ pub EXPLICIT_COUNTER_LOOP, Warn, /// **Example:** `loop {}` declare_lint!{ pub EMPTY_LOOP, Warn, "empty `loop {}` detected" } +/// **What it does:** This lint checks for `while let` expressions on iterators. It is `Warn` by default. +/// +/// **Why is this bad?** Readability. A simple `for` loop is shorter and conveys the intent better. +/// +/// **Known problems:** None +/// +/// **Example:** `while let Some(val) = iter() { .. }` declare_lint!{ pub WHILE_LET_ON_ITERATOR, Warn, "using a while-let loop instead of a for loop on an iterator" } #[derive(Copy, Clone)] diff --git a/src/methods.rs b/src/methods.rs index 6c629f29c..ef7a58959 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -24,6 +24,7 @@ pub struct MethodsPass; /// **Example:** `x.unwrap()` declare_lint!(pub OPTION_UNWRAP_USED, Allow, "using `Option.unwrap()`, which should at least get a better message using `expect()`"); + /// **What it does:** This lint checks for `.unwrap()` calls on `Result`s. It is `Allow` by default. /// /// **Why is this bad?** `result.unwrap()` will let the thread panic on `Err` values. Normally, you want to implement more sophisticated error handling, and propagate errors upwards with `try!`. @@ -35,6 +36,7 @@ declare_lint!(pub OPTION_UNWRAP_USED, Allow, /// **Example:** `x.unwrap()` declare_lint!(pub RESULT_UNWRAP_USED, Allow, "using `Result.unwrap()`, which might be better handled"); + /// **What it does:** This lint checks for `.to_string()` method calls on values of type `&str`. It is `Warn` by default. /// /// **Why is this bad?** This uses the whole formatting machinery just to clone a string. Using `.to_owned()` is lighter on resources. You can also consider using a [`Cow<'a, str>`](http://doc.rust-lang.org/std/borrow/enum.Cow.html) instead in some cases. @@ -44,6 +46,7 @@ declare_lint!(pub RESULT_UNWRAP_USED, Allow, /// **Example:** `s.to_string()` where `s: &str` declare_lint!(pub STR_TO_STRING, Warn, "using `to_string()` on a str, which should be `to_owned()`"); + /// **What it does:** This lint checks for `.to_string()` method calls on values of type `String`. It is `Warn` by default. /// /// **Why is this bad?** As our string is already owned, this whole operation is basically a no-op, but still creates a clone of the string (which, if really wanted, should be done with `.clone()`). @@ -53,6 +56,7 @@ declare_lint!(pub STR_TO_STRING, Warn, /// **Example:** `s.to_string()` where `s: String` declare_lint!(pub STRING_TO_STRING, Warn, "calling `String.to_string()` which is a no-op"); + /// **What it does:** This lint checks for methods that should live in a trait implementation of a `std` trait (see [llogiq's blog post](http://llogiq.github.io/2015/07/30/traits.html) for further information) instead of an inherent implementation. It is `Warn` by default. /// /// **Why is this bad?** Implementing the traits improve ergonomics for users of the code, often with very little cost. Also people seeing a `mul(..)` method may expect `*` to work equally, so you should have good reason to disappoint them. @@ -68,6 +72,7 @@ declare_lint!(pub STRING_TO_STRING, Warn, /// ``` declare_lint!(pub SHOULD_IMPLEMENT_TRAIT, Warn, "defining a method that should be implementing a std trait"); + /// **What it does:** This lint checks for methods with certain name prefixes and `Warn`s (by default) if the prefix doesn't match how self is taken. The actual rules are: /// /// |Prefix |`self` taken | @@ -92,6 +97,7 @@ declare_lint!(pub SHOULD_IMPLEMENT_TRAIT, Warn, declare_lint!(pub WRONG_SELF_CONVENTION, Warn, "defining a method named with an established prefix (like \"into_\") that takes \ `self` with the wrong convention"); + /// **What it does:** This is the same as [`wrong_self_convention`](#wrong_self_convention), but for public items. This lint is `Allow` by default. /// /// **Why is this bad?** See [`wrong_self_convention`](#wrong_self_convention). @@ -107,12 +113,36 @@ declare_lint!(pub WRONG_SELF_CONVENTION, Warn, declare_lint!(pub WRONG_PUB_SELF_CONVENTION, Allow, "defining a public method named with an established prefix (like \"into_\") that takes \ `self` with the wrong convention"); + +/// **What it does:** This lint `Warn`s on using `ok().expect(..)`. +/// +/// **Why is this bad?** Because you usually call `expect()` on the `Result` directly to get a good error message. +/// +/// **Known problems:** None. +/// +/// **Example:** `x.ok().expect("why did I do this again?")` declare_lint!(pub OK_EXPECT, Warn, "using `ok().expect()`, which gives worse error messages than \ calling `expect` directly on the Result"); + +/// **What it does:** This lint `Warn`s on `_.map(_).unwrap_or(_)`. +/// +/// **Why is this bad?** Readability, this can be written more concisely as `_.map_or(_, _)`. +/// +/// **Known problems:** None. +/// +/// **Example:** `x.map(|a| a + 1).unwrap_or(0)` declare_lint!(pub OPTION_MAP_UNWRAP_OR, Warn, "using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as \ `map_or(a, f)`)"); + +/// **What it does:** This lint `Warn`s on `_.map(_).unwrap_or_else(_)`. +/// +/// **Why is this bad?** Readability, this can be written more concisely as `_.map_or_else(_, _)`. +/// +/// **Known problems:** None. +/// +/// **Example:** `x.map(|a| a + 1).unwrap_or_else(some_function)` declare_lint!(pub OPTION_MAP_UNWRAP_OR_ELSE, Warn, "using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as \ `map_or_else(g, f)`)"); diff --git a/src/misc_early.rs b/src/misc_early.rs index 8adff7523..1520e9c0e 100644 --- a/src/misc_early.rs +++ b/src/misc_early.rs @@ -6,6 +6,13 @@ use syntax::ast::*; use utils::span_lint; +/// **What it does:** This lint `Warn`s on struct field patterns bound to wildcards. +/// +/// **Why is this bad?** Using `..` instead is shorter and leaves the focus on the fields that are actually bound. +/// +/// **Known problems:** None. +/// +/// **Example:** `let { a: _, b: ref b, c: _ } = ..` declare_lint!(pub UNNEEDED_FIELD_PATTERN, Warn, "Struct fields are bound to a wildcard instead of using `..`"); diff --git a/src/needless_features.rs b/src/needless_features.rs index 44db5e922..2dd53c2d7 100644 --- a/src/needless_features.rs +++ b/src/needless_features.rs @@ -8,6 +8,13 @@ use rustc_front::hir::*; use utils::span_lint; use utils; +/// **What it does:** This lint `Warn`s on use of the `as_slice(..)` function, which is unstable. +/// +/// **Why is this bad?** Using this function doesn't make your code better, but it will preclude it from building with stable Rust. +/// +/// **Known problems:** None. +/// +/// **Example:** `x.as_slice(..)` declare_lint! { pub UNSTABLE_AS_SLICE, Warn, @@ -15,6 +22,13 @@ declare_lint! { see https://github.com/rust-lang/rust/issues/27729" } +/// **What it does:** This lint `Warn`s on use of the `as_mut_slice(..)` function, which is unstable. +/// +/// **Why is this bad?** Using this function doesn't make your code better, but it will preclude it from building with stable Rust. +/// +/// **Known problems:** None. +/// +/// **Example:** `x.as_mut_slice(..)` declare_lint! { pub UNSTABLE_AS_MUT_SLICE, Warn, diff --git a/src/needless_update.rs b/src/needless_update.rs index c65d0c9e7..e1e0f4818 100644 --- a/src/needless_update.rs +++ b/src/needless_update.rs @@ -4,6 +4,13 @@ use rustc_front::hir::{Expr, ExprStruct}; use utils::span_lint; +/// **What it does:** This lint `Warn`s on needlessly including a base struct on update when all fields are changed anyway. +/// +/// **Why is this bad?** This will cost resources (because the base has to be somewhere), and make the code less readable. +/// +/// **Known problems:** None. +/// +/// **Example:** `Point { x: 1, y: 0, ..zero_point }`` declare_lint! { pub NEEDLESS_UPDATE, Warn, diff --git a/src/no_effect.rs b/src/no_effect.rs index 82fcf92fd..b51b4235d 100644 --- a/src/no_effect.rs +++ b/src/no_effect.rs @@ -6,6 +6,13 @@ use rustc_front::hir::{Stmt, StmtSemi}; use utils::in_macro; use utils::span_lint; +/// **What it does:** This lint `Warn`s on statements which have no effect. +/// +/// **Why is this bad?** Similar to dead code, these statements are actually executed. However, as they have no effect, all they do is make the code less readable. +/// +/// **Known problems:** None. +/// +/// **Example:** `0;` declare_lint! { pub NO_EFFECT, Warn, diff --git a/src/temporary_assignment.rs b/src/temporary_assignment.rs index 6cfcb711f..622bf5bc2 100644 --- a/src/temporary_assignment.rs +++ b/src/temporary_assignment.rs @@ -4,6 +4,13 @@ use rustc_front::hir::{Expr, ExprAssign, ExprField, ExprStruct, ExprTup, ExprTup use utils::is_adjusted; use utils::span_lint; +/// **What it does:** This lint `Warn`s on creating a struct or tuple just to assign a value in it. +/// +/// **Why is this bad?** Readability. If the struct is only created to be updated, why not write the struct you want in the first place? +/// +/// **Known problems:** None. +/// +/// **Example:** `(0, 0).0 = 1` declare_lint! { pub TEMPORARY_ASSIGNMENT, Warn,