all: DRY for lint descriptions

* use the rustc style for lint descriptions
* add a script to parse all lint descriptions
  and put the generated table into README
This commit is contained in:
Georg Brandl 2015-08-13 10:32:35 +02:00
parent 8dfa02938d
commit 2c2716f045
22 changed files with 180 additions and 70 deletions

View File

@ -6,35 +6,42 @@ A collection of lints that give helpful tips to newbies and catch oversights.
##Lints
Lints included in this crate:
- `single_match`: Warns when a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used, and recommends `if let` instead.
- `box_vec`: Warns on usage of `Box<Vec<T>>`
- `linkedlist`: Warns on usage of `LinkedList`
- `str_to_string`: Warns on usage of `str::to_string()`
- `toplevel_ref_arg`: Warns when a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`)
- `eq_op`: Warns on equal operands on both sides of a comparison or bitwise combination
- `bad_bit_mask`: Denies expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
- `ineffective_bit_mask`: Warns on expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`
- `needless_bool` : Warns on if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
- `ptr_arg`: Warns on fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively
- `approx_constant`: Warns if the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found and suggests to use the constant
- `cmp_nan`: Denies comparisons to NAN (which will always return false, which is probably not intended)
- `float_cmp`: Warns on `==` or `!=` comparisons of floaty typed values. As floating-point operations usually involve rounding errors, it is always better to check for approximate equality within some small bounds
- `precedence`: Warns on expressions where precedence may trip up the unwary reader of the source and suggests adding parenthesis, e.g. `x << 2 + y` will be parsed as `x << (2 + y)`
- `redundant_closure`: Warns on usage of eta-reducible closures like `|a| foo(a)` (which can be written as just `foo`)
- `identity_op`: Warns on identity operations like `x + 0` or `y / 1` (which can be reduced to `x` and `y`, respectively)
- `mut_mut`: Warns on `&mut &mut` which is either a copy'n'paste error, or shows a fundamental misunderstanding of references
- `len_zero`: Warns on `_.len() == 0` and suggests using `_.is_empty()` (or similar comparisons with `>` or `!=`)
- `len_without_is_empty`: Warns on traits or impls that have a `.len()` but no `.is_empty()` method
- `cmp_owned`: Warns on creating owned instances for comparing with others, e.g. `x == "foo".to_string()`
- `inline_always`: Warns on `#[inline(always)]`, because in most cases it is a bad idea
- `collapsible_if`: Warns on cases where two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }`
- `zero_width_space`: Warns on encountering a unicode zero-width space
- `string_add_assign`: Warns on `x = x + ..` where `x` is a `String` and suggests using `push_str(..)` instead.
- `needless_return`: Warns on using `return expr;` when a simple `expr` would suffice.
- `let_and_return`: Warns on doing `let x = expr; x` at the end of a function.
- `option_unwrap_used`: Warns when `Option.unwrap()` is used, and suggests `.expect()`.
- `result_unwrap_used`: Warns when `Result.unwrap()` is used (silent by default).
- `modulo_one`: Warns on taking a number modulo 1, which always has a result of 0.
name | default | meaning
---------------------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
approx_constant | warn | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant
bad_bit_mask | deny | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
box_vec | warn | usage of `Box<Vec<T>>`, vector elements are already on the heap
cmp_nan | deny | comparisons to NAN (which will always return false, which is probably not intended)
cmp_owned | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()`
collapsible_if | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }`
eq_op | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)
float_cmp | warn | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds)
identity_op | warn | using identity operations, e.g. `x + 0` or `y / 1`
ineffective_bit_mask | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`
inline_always | warn | `#[inline(always)]` is a bad idea in most cases
len_without_is_empty | warn | traits and impls that have `.len()` but not `.is_empty()`
len_zero | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead
let_and_return | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a function
let_unit_value | warn | creating a let binding to a value of unit type, which usually can't be used afterwards
linkedlist | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a RingBuf
modulo_one | warn | taking a number modulo 1, which always returns 0
mut_mut | warn | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references)
needless_bool | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
needless_lifetimes | warn | using explicit lifetimes for references in function arguments when elision rules would allow omitting them
needless_range_loop | warn | for-looping over a range of indices where an iterator over items would do
needless_return | warn | using a return statement like `return expr;` where an expression would suffice
non_ascii_literal | allow | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead
option_unwrap_used | warn | using `Option.unwrap()`, which should at least get a better message using `expect()`
precedence | warn | expressions where precedence may trip up the unwary reader of the source; suggests adding parentheses, e.g. `x << 2 + y` will be parsed as `x << (2 + y)`
ptr_arg | allow | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively
redundant_closure | warn | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)
result_unwrap_used | allow | using `Result.unwrap()`, which might be better handled
single_match | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead
str_to_string | warn | using `to_string()` on a str, which should be `to_owned()`
string_add_assign | warn | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead
string_to_string | warn | calling `String.to_string()` which is a no-op
toplevel_ref_arg | warn | a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`)
zero_width_space | deny | using a zero-width space in a string literal, which is confusing
To use, add the following lines to your Cargo.toml:

View File

@ -12,7 +12,8 @@ use utils::span_lint;
declare_lint! {
pub APPROX_CONSTANT,
Warn,
"Warn if a user writes an approximate known constant in their code"
"the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) \
is found; suggests to use the constant"
}
const KNOWN_CONSTS : &'static [(f64, &'static str)] = &[(f64::E, "E"), (f64::FRAC_1_PI, "FRAC_1_PI"),

View File

@ -9,7 +9,7 @@ use syntax::parse::token::InternedString;
use utils::{in_macro, match_path, span_lint};
declare_lint! { pub INLINE_ALWAYS, Warn,
"#[inline(always)] is usually a bad idea."}
"`#[inline(always)]` is a bad idea in most cases" }
#[derive(Copy,Clone)]

View File

@ -11,15 +11,14 @@ use utils::span_lint;
declare_lint! {
pub BAD_BIT_MASK,
Deny,
"Deny the use of incompatible bit masks in comparisons, e.g. \
'(a & 1) == 2'"
"expressions of the form `_ & mask == select` that will only ever return `true` or `false` \
(because in the example `select` containing bits that `mask` doesn't have)"
}
declare_lint! {
pub INEFFECTIVE_BIT_MASK,
Warn,
"Warn on the use of an ineffective bit mask in comparisons, e.g. \
'(a & 1) > 2'"
"expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`"
}
/// Checks for incompatible bit masks in comparisons, e.g. `x & 1 == 2`.

View File

@ -23,7 +23,8 @@ use utils::{in_macro, span_help_and_lint, snippet};
declare_lint! {
pub COLLAPSIBLE_IF,
Warn,
"Warn on if expressions that can be collapsed"
"two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` \
can be written as `if x && y { foo() }`"
}
#[derive(Copy,Clone)]

View File

@ -8,7 +8,7 @@ use utils::span_lint;
declare_lint! {
pub EQ_OP,
Warn,
"warn about comparing equal expressions (e.g. x == x)"
"equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)"
}
#[derive(Copy,Clone)]

View File

@ -11,7 +11,7 @@ pub struct EtaPass;
declare_lint!(pub REDUNDANT_CLOSURE, Warn,
"Warn on usage of redundant closures, i.e. `|a| foo(a)`");
"using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)");
impl LintPass for EtaPass {
fn get_lints(&self) -> LintArray {

View File

@ -10,7 +10,7 @@ use syntax::codemap::Span;
use utils::{span_lint, snippet};
declare_lint! { pub IDENTITY_OP, Warn,
"Warn on identity operations, e.g. '_ + 0'"}
"using identity operations, e.g. `x + 0` or `y / 1`" }
#[derive(Copy,Clone)]
pub struct IdentityOp;

View File

@ -12,10 +12,11 @@ use syntax::ast::*;
use utils::{span_lint, walk_ptrs_ty, snippet};
declare_lint!(pub LEN_ZERO, Warn,
"Warn when .is_empty() could be used instead of checking .len()");
"checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` \
could be used instead");
declare_lint!(pub LEN_WITHOUT_IS_EMPTY, Warn,
"Warn on traits and impls that have .len() but not .is_empty()");
"traits and impls that have `.len()` but not `.is_empty()`");
#[derive(Copy,Clone)]
pub struct LenZero;

View File

@ -64,16 +64,21 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_lint_pass(box loops::LoopsPass as LintPassObject);
reg.register_lint_pass(box lifetimes::LifetimePass as LintPassObject);
reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
reg.register_lint_group("clippy", vec![types::BOX_VEC,
types::LINKEDLIST,
types::LET_UNIT_VALUE,
misc::SINGLE_MATCH,
misc::TOPLEVEL_REF_ARG, eq_op::EQ_OP,
misc::TOPLEVEL_REF_ARG,
misc::CMP_NAN,
misc::FLOAT_CMP,
misc::PRECEDENCE,
misc::CMP_OWNED,
eq_op::EQ_OP,
bit_mask::BAD_BIT_MASK,
bit_mask::INEFFECTIVE_BIT_MASK,
ptr_arg::PTR_ARG,
needless_bool::NEEDLESS_BOOL,
approx_const::APPROX_CONSTANT,
misc::CMP_NAN, misc::FLOAT_CMP,
misc::PRECEDENCE, misc::CMP_OWNED,
eta_reduction::REDUNDANT_CLOSURE,
identity_op::IDENTITY_OP,
mut_mut::MUT_MUT,
@ -85,12 +90,12 @@ pub fn plugin_registrar(reg: &mut Registry) {
unicode::NON_ASCII_LITERAL,
strings::STRING_ADD_ASSIGN,
returns::NEEDLESS_RETURN,
returns::LET_AND_RETURN,
misc::MODULO_ONE,
methods::OPTION_UNWRAP_USED,
methods::RESULT_UNWRAP_USED,
methods::STR_TO_STRING,
methods::STRING_TO_STRING,
types::LET_UNIT_VALUE,
lifetimes::NEEDLESS_LIFETIMES,
loops::NEEDLESS_RANGE_LOOP,
]);

View File

@ -7,7 +7,8 @@ use std::collections::HashSet;
use std::iter::FromIterator;
declare_lint!(pub NEEDLESS_LIFETIMES, Warn,
"Warn on explicit lifetimes when elision rules would apply");
"using explicit lifetimes for references in function arguments when elision rules \
would allow omitting them");
#[derive(Copy,Clone)]
pub struct LifetimePass;

View File

@ -6,7 +6,7 @@ use std::collections::HashSet;
use utils::{span_lint, get_parent_expr};
declare_lint!{ pub NEEDLESS_RANGE_LOOP, Warn,
"Warn about looping over a range of indices if a normal iterator would do" }
"for-looping over a range of indices where an iterator over items would do" }
#[derive(Copy, Clone)]
pub struct LoopsPass;

View File

@ -8,13 +8,13 @@ use utils::{span_lint, match_def_path, walk_ptrs_ty};
pub struct MethodsPass;
declare_lint!(pub OPTION_UNWRAP_USED, Warn,
"Warn on using unwrap() on an Option value");
"using `Option.unwrap()`, which should at least get a better message using `expect()`");
declare_lint!(pub RESULT_UNWRAP_USED, Allow,
"Warn on using unwrap() on a Result value");
"using `Result.unwrap()`, which might be better handled");
declare_lint!(pub STR_TO_STRING, Warn,
"Warn when a String could use to_owned() instead of to_string()");
"using `to_string()` on a str, which should be `to_owned()`");
declare_lint!(pub STRING_TO_STRING, Warn,
"Warn when calling String.to_string()");
"calling `String.to_string()` which is a no-op");
impl LintPass for MethodsPass {
fn get_lints(&self) -> LintArray {

View File

@ -16,7 +16,8 @@ pub struct MiscPass;
declare_lint!(pub SINGLE_MATCH, Warn,
"Warn on usage of matches with a single nontrivial arm");
"a match statement with a single nontrivial arm (i.e, where the other arm \
is `_ => {}`) is used; recommends `if let` instead");
impl LintPass for MiscPass {
fn get_lints(&self) -> LintArray {
@ -59,7 +60,9 @@ impl LintPass for MiscPass {
}
declare_lint!(pub TOPLEVEL_REF_ARG, Warn, "Warn about pattern matches with top-level `ref` bindings");
declare_lint!(pub TOPLEVEL_REF_ARG, Warn,
"a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not \
`fn foo((ref x, ref y): (u8, u8))`)");
#[allow(missing_copy_implementations)]
pub struct TopLevelRefPass;
@ -82,7 +85,8 @@ impl LintPass for TopLevelRefPass {
}
}
declare_lint!(pub CMP_NAN, Deny, "Deny comparisons to std::f32::NAN or std::f64::NAN");
declare_lint!(pub CMP_NAN, Deny,
"comparisons to NAN (which will always return false, which is probably not intended)");
#[derive(Copy,Clone)]
pub struct CmpNan;
@ -114,7 +118,9 @@ fn check_nan(cx: &Context, path: &Path, span: Span) {
}
declare_lint!(pub FLOAT_CMP, Warn,
"Warn on ==/!= comparison of floaty values");
"using `==` or `!=` on float values (as floating-point operations \
usually involve rounding errors, it is always better to check for approximate \
equality within small bounds)");
#[derive(Copy,Clone)]
pub struct FloatCmp;
@ -147,7 +153,8 @@ fn is_float(cx: &Context, expr: &Expr) -> bool {
}
declare_lint!(pub PRECEDENCE, Warn,
"Warn on mixing bit ops with integer arithmetic without parentheses");
"expressions where precedence may trip up the unwary reader of the source; \
suggests adding parentheses, e.g. `x << 2 + y` will be parsed as `x << (2 + y)`");
#[derive(Copy,Clone)]
pub struct Precedence;
@ -190,7 +197,7 @@ fn is_arith_op(op : BinOp_) -> bool {
}
declare_lint!(pub CMP_OWNED, Warn,
"Warn on creating an owned string just for comparison");
"creating owned instances for comparing with others, e.g. `x == \"foo\".to_string()`");
#[derive(Copy,Clone)]
pub struct CmpOwned;
@ -242,7 +249,7 @@ fn is_str_arg(cx: &Context, args: &[P<Expr>]) -> bool {
walk_ptrs_ty(cx.tcx.expr_ty(&*args[0])).sty { true } else { false }
}
declare_lint!(pub MODULO_ONE, Warn, "Warn on expressions that include % 1, which is always 0");
declare_lint!(pub MODULO_ONE, Warn, "taking a number modulo 1, which always returns 0");
#[derive(Copy,Clone)]
pub struct ModuloOne;

View File

@ -6,7 +6,8 @@ use syntax::codemap::{BytePos, ExpnInfo, Span};
use utils::{in_macro, span_lint};
declare_lint!(pub MUT_MUT, Warn,
"Warn on usage of double-mut refs, e.g. '&mut &mut ...'");
"usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, \
or shows a fundamental misunderstanding of references)");
#[derive(Copy,Clone)]
pub struct MutMut;

View File

@ -15,7 +15,8 @@ use utils::{de_p, span_lint};
declare_lint! {
pub NEEDLESS_BOOL,
Warn,
"Warn on needless use of if x { true } else { false } (or vice versa)"
"if-statements with plain booleans in the then- and else-clause, e.g. \
`if p { true } else { false }`"
}
#[derive(Copy,Clone)]

View File

@ -16,7 +16,8 @@ use utils::span_lint;
declare_lint! {
pub PTR_ARG,
Allow,
"Warn on declaration of a &Vec- or &String-typed method argument"
"fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` \
instead, respectively"
}
#[derive(Copy,Clone)]

View File

@ -7,9 +7,10 @@ use rustc::lint::{Context, LintPass, LintArray, Level};
use utils::{span_lint, snippet, match_path};
declare_lint!(pub NEEDLESS_RETURN, Warn,
"Warn on using a return statement where an expression would be enough");
"using a return statement like `return expr;` where an expression would suffice");
declare_lint!(pub LET_AND_RETURN, Warn,
"Warn on creating a let-binding and then immediately returning it");
"creating a let-binding and then immediately returning it like `let x = expr; x` at \
the end of a function");
#[derive(Copy,Clone)]
pub struct ReturnPass;

View File

@ -14,7 +14,7 @@ use utils::{match_def_path, span_lint, walk_ptrs_ty};
declare_lint! {
pub STRING_ADD_ASSIGN,
Warn,
"Warn on `x = x + ..` where x is a `String`"
"using `x = x + ..` where x is a `String`; suggests using `push_str()` instead"
}
#[derive(Copy,Clone)]

View File

@ -12,9 +12,10 @@ use utils::{in_macro, snippet, span_lint, span_help_and_lint};
pub struct TypePass;
declare_lint!(pub BOX_VEC, Warn,
"Warn on usage of Box<Vec<T>>");
"usage of `Box<Vec<T>>`, vector elements are already on the heap");
declare_lint!(pub LINKEDLIST, Warn,
"Warn on usage of LinkedList");
"usage of LinkedList, usually a vector is faster, or a more specialized data \
structure like a RingBuf");
/// Matches a type with a provided string, and returns its type parameters if successful
pub fn match_ty_unwrap<'a>(ty: &'a Ty, segments: &[&str]) -> Option<&'a [P<Ty>]> {
@ -81,7 +82,7 @@ impl LintPass for TypePass {
pub struct LetPass;
declare_lint!(pub LET_UNIT_VALUE, Warn,
"Warn on let-binding a value of unit type");
"creating a let binding to a value of unit type, which usually can't be used afterwards");
fn check_let_unit(cx: &Context, decl: &Decl, info: Option<&ExpnInfo>) {

View File

@ -3,8 +3,11 @@ use syntax::ast::*;
use syntax::codemap::{BytePos, Span};
use utils::span_lint;
declare_lint!{ pub ZERO_WIDTH_SPACE, Deny, "Zero-width space is confusing" }
declare_lint!{ pub NON_ASCII_LITERAL, Allow, "Lint literal non-ASCII chars in literals" }
declare_lint!{ pub ZERO_WIDTH_SPACE, Deny,
"using a zero-width space in a string literal, which is confusing" }
declare_lint!{ pub NON_ASCII_LITERAL, Allow,
"using any literal non-ASCII chars in a string literal; suggests \
using the \\u escape instead" }
#[derive(Copy, Clone)]
pub struct Unicode;

80
util/update_readme.py Normal file
View File

@ -0,0 +1,80 @@
# Generate a Markdown table of all lints, and put it in README.md.
# With -n option, only print the new table to stdout.
import os
import re
import sys
declare_lint_re = re.compile(r'''
declare_lint! \s* [{(] \s*
pub \s+ (?P<name>[A-Z_]+) \s*,\s*
(?P<level>Forbid|Deny|Warn|Allow) \s*,\s*
" (?P<desc>(?:[^"\\]+|\\.)*) " \s* [})]
''', re.X | re.S)
nl_escape_re = re.compile(r'\\\n\s*')
def collect(lints, fp):
code = fp.read()
for match in declare_lint_re.finditer(code):
# remove \-newline escapes from description string
desc = nl_escape_re.sub('', match.group('desc'))
lints.append((match.group('name').lower(),
match.group('level').lower(),
desc.replace('\\"', '"')))
def write_tbl(lints, fp):
# first and third column widths
w_name = max(len(l[0]) for l in lints)
w_desc = max(len(l[2]) for l in lints)
# header and underline
fp.write('%-*s | default | meaning\n' % (w_name, 'name'))
fp.write('%s-|-%s-|-%s\n' % ('-' * w_name, '-' * 7, '-' * w_desc))
# one table row per lint
for (name, default, meaning) in sorted(lints):
fp.write('%-*s | %-7s | %s\n' % (w_name, name, default, meaning))
def main(print_only=False):
lints = []
# check directory
if not os.path.isfile('src/lib.rs'):
print('Error: call this script from clippy checkout directory!')
return
# collect all lints from source files
for root, dirs, files in os.walk('src'):
for fn in files:
if fn.endswith('.rs'):
with open(os.path.join(root, fn)) as fp:
collect(lints, fp)
if print_only:
write_tbl(lints, sys.stdout)
return
# read current README.md content
with open('README.md') as fp:
lines = list(fp)
# replace old table with new table
with open('README.md', 'w') as fp:
in_old_tbl = False
for line in lines:
if line.replace(' ', '').strip() == 'name|default|meaning':
# old table starts here
write_tbl(lints, fp)
in_old_tbl = True
if in_old_tbl:
# the old table is finished by an empty line
if line.strip():
continue
in_old_tbl = False
fp.write(line)
if __name__ == '__main__':
main(print_only='-n' in sys.argv)