diff --git a/README.md b/README.md index 57dedbdb39e7..b3e6937604db 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Table of contents: * [License](#license) ##Lints -There are 138 lints included in this crate: +There are 134 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -124,11 +124,9 @@ name [single_char_pattern](https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern) | warn | using a single-character str where a char could be used, e.g. `_.split("x")` [single_match](https://github.com/Manishearth/rust-clippy/wiki#single_match) | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead [single_match_else](https://github.com/Manishearth/rust-clippy/wiki#single_match_else) | allow | a match statement with a two arms where the second arm's pattern is a wildcard; recommends `if let` instead -[str_to_string](https://github.com/Manishearth/rust-clippy/wiki#str_to_string) | warn | using `to_string()` on a str, which should be `to_owned()` [string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add) | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead [string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign) | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead [string_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes) | warn | calling `as_bytes` on a string literal; suggests using a byte string literal instead -[string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String::to_string` which is inefficient [suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting) | warn | suspicious formatting of `*=`, `-=` or `!=` [suspicious_else_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting) | warn | suspicious formatting of `else if` [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries @@ -140,8 +138,6 @@ name [unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp) | warn | comparing unit values (which is always `true` or `false`, respectively) [unnecessary_mut_passed](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_mut_passed) | warn | an argument is passed as a mutable reference although the function/method only demands an immutable reference [unneeded_field_pattern](https://github.com/Manishearth/rust-clippy/wiki#unneeded_field_pattern) | warn | Struct fields are bound to a wildcard instead of using `..` -[unstable_as_mut_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_mut_slice) | warn | as_mut_slice is not stable and can be replaced by &mut v[..]see https://github.com/rust-lang/rust/issues/27729 -[unstable_as_slice](https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice) | warn | as_slice is not stable and can be replaced by & v[..]see https://github.com/rust-lang/rust/issues/27729 [unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect) | warn | `collect()`ing an iterator without using the result; this is usually better written as a for loop [unused_label](https://github.com/Manishearth/rust-clippy/wiki#unused_label) | warn | unused label [unused_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes) | warn | unused lifetimes in function definitions diff --git a/src/deprecated_lints.rs b/src/deprecated_lints.rs new file mode 100644 index 000000000000..abdb6297b9ee --- /dev/null +++ b/src/deprecated_lints.rs @@ -0,0 +1,44 @@ +macro_rules! declare_deprecated_lint { + (pub $name: ident, $_reason: expr) => { + declare_lint!(pub $name, Allow, "deprecated lint") + } +} + +/// **What it does:** Nothing. This lint has been deprecated. +/// +/// **Deprecation reason:** This used to check for `Vec::as_slice`, which was unstable with good +/// stable alternatives. `Vec::as_slice` has now been stabilized. +declare_deprecated_lint! { + pub UNSTABLE_AS_SLICE, + "`Vec::as_slice` has been stabilized in 1.7" +} + + +/// **What it does:** Nothing. This lint has been deprecated. +/// +/// **Deprecation reason:** This used to check for `Vec::as_mut_slice`, which was unstable with good +/// stable alternatives. `Vec::as_mut_slice` has now been stabilized. +declare_deprecated_lint! { + pub UNSTABLE_AS_MUT_SLICE, + "`Vec::as_mut_slice` has been stabilized in 1.7" +} + +/// **What it does:** Nothing. This lint has been deprecated. +/// +/// **Deprecation reason:** This used to check for `.to_string()` method calls on values +/// of type `&str`. This is not unidiomatic and with specialization coming, `to_string` could be +/// specialized to be as efficient as `to_owned`. +declare_deprecated_lint! { + pub STR_TO_STRING, + "using `str::to_string` is common even today and specialization will likely happen soon" +} + +/// **What it does:** Nothing. This lint has been deprecated. +/// +/// **Deprecation reason:** This used to check for `.to_string()` method calls on values +/// of type `String`. This is not unidiomatic and with specialization coming, `to_string` could be +/// specialized to be as efficient as `clone`. +declare_deprecated_lint! { + pub STRING_TO_STRING, + "using `string::to_string` is common even today and specialization will likely happen soon" +} diff --git a/src/lib.rs b/src/lib.rs index f6179f4f9932..ffa66fd581b0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,7 +81,6 @@ pub mod mut_mut; pub mod mut_reference; pub mod mutex_atomic; pub mod needless_bool; -pub mod needless_features; pub mod needless_update; pub mod new_without_default; pub mod no_effect; @@ -141,6 +140,13 @@ pub fn plugin_registrar(reg: &mut Registry) { } }; + let mut store = reg.sess.lint_store.borrow_mut(); + store.register_removed("unstable_as_slice", "`Vec::as_slice` has been stabilized in 1.7"); + store.register_removed("unstable_as_mut_slice", "`Vec::as_mut_slice` has been stabilized in 1.7"); + store.register_removed("str_to_string", "using `str::to_string` is common even today and specialization will likely happen soon"); + store.register_removed("string_to_string", "using `string::to_string` is common even today and specialization will likely happen soon"); + // end deprecated lints, do not remove this comment, it’s used in `update_lints` + reg.register_late_lint_pass(box types::TypePass); reg.register_late_lint_pass(box misc::TopLevelRefPass); reg.register_late_lint_pass(box misc::CmpNan); @@ -185,7 +191,6 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box open_options::NonSensicalOpenOptions); reg.register_late_lint_pass(box zero_div_zero::ZeroDivZeroPass); reg.register_late_lint_pass(box mutex_atomic::MutexAtomic); - reg.register_late_lint_pass(box needless_features::NeedlessFeaturesPass); reg.register_late_lint_pass(box needless_update::NeedlessUpdatePass); reg.register_late_lint_pass(box no_effect::NoEffectPass); reg.register_late_lint_pass(box map_clone::MapClonePass); @@ -308,8 +313,6 @@ pub fn plugin_registrar(reg: &mut Registry) { methods::SEARCH_IS_SOME, methods::SHOULD_IMPLEMENT_TRAIT, methods::SINGLE_CHAR_PATTERN, - methods::STR_TO_STRING, - methods::STRING_TO_STRING, methods::WRONG_SELF_CONVENTION, minmax::MIN_MAX, misc::CMP_NAN, @@ -326,8 +329,6 @@ pub fn plugin_registrar(reg: &mut Registry) { mutex_atomic::MUTEX_ATOMIC, needless_bool::BOOL_COMPARISON, needless_bool::NEEDLESS_BOOL, - needless_features::UNSTABLE_AS_MUT_SLICE, - needless_features::UNSTABLE_AS_SLICE, needless_update::NEEDLESS_UPDATE, new_without_default::NEW_WITHOUT_DEFAULT, no_effect::NO_EFFECT, diff --git a/src/methods.rs b/src/methods.rs index 6d33f31d45c5..95bbdd441c96 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -6,13 +6,13 @@ use rustc::middle::subst::{Subst, TypeSpace}; use rustc::middle::ty; use rustc_front::hir::*; use std::borrow::Cow; -use std::{fmt, iter}; +use std::fmt; use syntax::codemap::Span; use syntax::ptr::P; use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, match_path, match_trait_method, match_type, method_chain_args, return_ty, same_tys, snippet, snippet_opt, span_lint, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth}; -use utils::{BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH, RESULT_PATH, STRING_PATH, +use utils::{BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH, RESULT_PATH, VEC_PATH}; use utils::MethodArgs; @@ -45,31 +45,6 @@ declare_lint! { "using `Result.unwrap()`, which might be better handled" } -/// **What it does:** This lint checks for `.to_string()` method calls on values of type `&str`. -/// -/// **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. -/// -/// **Known problems:** None -/// -/// **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`. -/// -/// **Why is this bad?** This is an non-efficient way to clone a `String`, `.clone()` should be used -/// instead. `String` implements `ToString` mostly for generics. -/// -/// **Known problems:** None -/// -/// **Example:** `s.to_string()` where `s: String` -declare_lint! { - pub STRING_TO_STRING, Warn, - "calling `String::to_string` which is inefficient" -} - /// **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. /// /// **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. @@ -315,8 +290,6 @@ impl LintPass for MethodsPass { lint_array!(EXTEND_FROM_SLICE, OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, - STR_TO_STRING, - STRING_TO_STRING, SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, WRONG_PUB_SELF_CONVENTION, @@ -343,8 +316,6 @@ impl LateLintPass for MethodsPass { // Chain calls if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { lint_unwrap(cx, expr, arglists[0]); - } else if let Some(arglists) = method_chain_args(expr, &["to_string"]) { - lint_to_string(cx, expr, arglists[0]); } else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) { lint_ok_expect(cx, expr, arglists[0]); } else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) { @@ -640,26 +611,6 @@ fn lint_unwrap(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs) { } } -#[allow(ptr_arg)] -// Type of MethodArgs is potentially a Vec -/// lint use of `to_string()` for `&str`s and `String`s -fn lint_to_string(cx: &LateContext, expr: &Expr, to_string_args: &MethodArgs) { - let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&to_string_args[0])); - - if obj_ty.sty == ty::TyStr { - let mut arg_str = snippet(cx, to_string_args[0].span, "_"); - if ptr_depth > 1 { - arg_str = Cow::Owned(format!("({}{})", iter::repeat('*').take(ptr_depth - 1).collect::(), arg_str)); - } - span_lint(cx, STR_TO_STRING, expr.span, &format!("`{}.to_owned()` is faster", arg_str)); - } else if match_type(cx, obj_ty, &STRING_PATH) { - span_lint(cx, - STRING_TO_STRING, - expr.span, - "`String::to_string` is an inefficient way to clone a `String`; use `clone()` instead"); - } -} - #[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec /// lint use of `ok().expect()` for `Result`s diff --git a/src/needless_features.rs b/src/needless_features.rs deleted file mode 100644 index f80ac48320ec..000000000000 --- a/src/needless_features.rs +++ /dev/null @@ -1,68 +0,0 @@ -//! Checks for usage of nightly features that have simple stable equivalents -//! -//! This lint is **warn** by default - -use rustc::lint::*; -use rustc_front::hir::*; -use utils::span_lint; -use utils; - -/// **What it does:** This lint checks for usage 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, - "as_slice is not stable and can be replaced by & v[..]\ -see https://github.com/rust-lang/rust/issues/27729" -} - -/// **What it does:** This lint checks for usage 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, - "as_mut_slice is not stable and can be replaced by &mut v[..]\ -see https://github.com/rust-lang/rust/issues/27729" -} - -#[derive(Copy,Clone)] -pub struct NeedlessFeaturesPass; - -impl LintPass for NeedlessFeaturesPass { - fn get_lints(&self) -> LintArray { - lint_array!(UNSTABLE_AS_SLICE, UNSTABLE_AS_MUT_SLICE) - } -} - -impl LateLintPass for NeedlessFeaturesPass { - fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { - if let ExprMethodCall(ref name, _, _) = expr.node { - if name.node.as_str() == "as_slice" && check_paths(cx, expr) { - span_lint(cx, - UNSTABLE_AS_SLICE, - expr.span, - "used as_slice() from the 'convert' nightly feature. Use &[..] instead"); - } - if name.node.as_str() == "as_mut_slice" && check_paths(cx, expr) { - span_lint(cx, - UNSTABLE_AS_MUT_SLICE, - expr.span, - "used as_mut_slice() from the 'convert' nightly feature. Use &mut [..] instead"); - } - } - } -} - -fn check_paths(cx: &LateContext, expr: &Expr) -> bool { - utils::match_impl_method(cx, expr, &["collections", "vec", "Vec"]) -} diff --git a/tests/compile-fail/cmp_owned.rs b/tests/compile-fail/cmp_owned.rs index c06949eb01a0..eb4070d8fd6c 100644 --- a/tests/compile-fail/cmp_owned.rs +++ b/tests/compile-fail/cmp_owned.rs @@ -3,7 +3,6 @@ #[deny(cmp_owned)] fn main() { - #[allow(str_to_string)] fn with_to_string(x : &str) { x != "foo".to_string(); //~^ ERROR this creates an owned instance just for comparison. Consider using `x != "foo"` to compare without allocation diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index b1a8f6cf7767..edbdeb2e55a7 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -296,12 +296,6 @@ fn main() { let res: Result = Ok(0); let _ = res.unwrap(); //~ERROR used unwrap() on a Result - let _ = "str".to_string(); //~ERROR `"str".to_owned()` is faster - - let v = &"str"; - let string = v.to_string(); //~ERROR `(*v).to_owned()` is faster - let _again = string.to_string(); //~ERROR `String::to_string` is an inefficient way to clone a `String`; use `clone()` instead - res.ok().expect("disaster!"); //~ERROR called `ok().expect()` // the following should not warn, since `expect` isn't implemented unless // the error type implements `Debug` diff --git a/tests/compile-fail/needless_features.rs b/tests/compile-fail/needless_features.rs deleted file mode 100644 index c5c82c7072b7..000000000000 --- a/tests/compile-fail/needless_features.rs +++ /dev/null @@ -1,28 +0,0 @@ -#![feature(plugin)] -#![plugin(clippy)] - -#![deny(clippy)] - -fn test_as_slice() { - let v = vec![1]; - v.as_slice(); //~ERROR used as_slice() from the 'convert' nightly feature. Use &[..] - - let mut v2 = vec![1]; - v2.as_mut_slice(); //~ERROR used as_mut_slice() from the 'convert' nightly feature. Use &mut [..] -} - -struct ShouldWork; - -impl ShouldWork { - fn as_slice(&self) -> &ShouldWork { self } -} - -fn test_should_work() { - let sw = ShouldWork; - sw.as_slice(); -} - -fn main() { - test_as_slice(); - test_should_work(); -} diff --git a/tests/run-pass/deprecated.rs b/tests/run-pass/deprecated.rs new file mode 100644 index 000000000000..70223bfd8674 --- /dev/null +++ b/tests/run-pass/deprecated.rs @@ -0,0 +1,12 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#[warn(str_to_string)] +//~^WARNING: warning: lint str_to_string has been removed: using `str::to_string` +#[warn(string_to_string)] +//~^WARNING: warning: lint string_to_string has been removed: using `string::to_string` +#[warn(unstable_as_slice)] +//~^WARNING: warning: lint unstable_as_slice has been removed: `Vec::as_slice` has been stabilized +#[warn(unstable_as_mut_slice)] +//~^WARNING: warning: lint unstable_as_mut_slice has been removed: `Vec::as_mut_slice` has been stabilized +fn main() {} diff --git a/util/update_lints.py b/util/update_lints.py index 2eaa6ab62116..b16b4b67fad5 100755 --- a/util/update_lints.py +++ b/util/update_lints.py @@ -13,14 +13,20 @@ pub \s+ (?P[A-Z_][A-Z_0-9]*) \s*,\s* (?PForbid|Deny|Warn|Allow) \s*,\s* " (?P(?:[^"\\]+|\\.)*) " \s* [})] -''', re.X | re.S) +''', re.VERBOSE | re.DOTALL) + +declare_deprecated_lint_re = re.compile(r''' + declare_deprecated_lint! \s* [{(] \s* + pub \s+ (?P[A-Z_][A-Z_0-9]*) \s*,\s* + " (?P(?:[^"\\]+|\\.)*) " \s* [})] +''', re.VERBOSE | re.DOTALL) nl_escape_re = re.compile(r'\\\n\s*') wiki_link = 'https://github.com/Manishearth/rust-clippy/wiki' -def collect(lints, fn): +def collect(lints, deprecated_lints, fn): """Collect all lints from a file. Adds entries to the lints list as `(module, name, level, desc)`. @@ -35,6 +41,13 @@ def collect(lints, fn): match.group('level').lower(), desc.replace('\\"', '"'))) + for match in declare_deprecated_lint_re.finditer(code): + # remove \-newline escapes from description string + desc = nl_escape_re.sub('', match.group('desc')) + deprecated_lints.append((os.path.splitext(os.path.basename(fn))[0], + match.group('name').lower(), + desc.replace('\\"', '"'))) + def gen_table(lints, link=None): """Write lint table in Markdown format.""" @@ -67,6 +80,13 @@ def gen_mods(lints): yield 'pub mod %s;\n' % module +def gen_deprecated(lints): + """Declare deprecated lints""" + + for lint in lints: + yield ' store.register_removed("%s", "%s");\n' % (lint[1], lint[2]) + + def replace_region(fn, region_start, region_end, callback, replace_start=True, write_back=True): """Replace a region in a file delimited by two lines matching regexes. @@ -107,6 +127,7 @@ def replace_region(fn, region_start, region_end, callback, def main(print_only=False, check=False): lints = [] + deprecated_lints = [] # check directory if not os.path.isfile('src/lib.rs'): @@ -117,7 +138,7 @@ def main(print_only=False, check=False): for root, dirs, files in os.walk('src'): for fn in files: if fn.endswith('.rs'): - collect(lints, os.path.join(root, fn)) + collect(lints, deprecated_lints, os.path.join(root, fn)) if print_only: sys.stdout.writelines(gen_table(lints)) @@ -147,6 +168,13 @@ def main(print_only=False, check=False): lambda: gen_group(lints, levels=('warn', 'deny')), replace_start=False, write_back=not check) + # same for "deprecated" lint collection + changed |= replace_region( + 'src/lib.rs', r'let mut store', r'end deprecated lints', + lambda: gen_deprecated(deprecated_lints), + replace_start=False, + write_back=not check) + # same for "clippy_pedantic" lint collection changed |= replace_region( 'src/lib.rs', r'reg.register_lint_group\("clippy_pedantic"', r'\]\);', diff --git a/util/update_wiki.py b/util/update_wiki.py index cf3421a8228e..5040a28bca0a 100755 --- a/util/update_wiki.py +++ b/util/update_wiki.py @@ -51,6 +51,10 @@ def parse_file(d, f): last_comment.append(line[3:]) elif line.startswith("declare_lint!"): comment = False + deprecated = False + elif line.startswith("declare_deprecated_lint!"): + comment = False + deprecated = True else: last_comment = [] if not comment: @@ -60,7 +64,8 @@ def parse_file(d, f): if m: name = m.group(1).lower() - while True: + # Intentionally either a never looping or infinite loop + while not deprecated: m = re.search(level_re, line) if m: level = m.group(0) @@ -68,6 +73,9 @@ def parse_file(d, f): line = next(rs) + if deprecated: + level = "Deprecated" + print("found %s with level %s in %s" % (name, level, f)) d[name] = (level, last_comment) last_comment = [] @@ -107,14 +115,21 @@ def parse_file(d, f): """ +def level_message(level): + if level == "Deprecated": + return "\n**Those lints are deprecated**:\n\n" + else: + return "\n**Those lints are %s by default**:\n\n" % level + + def write_wiki_page(d, c, f): keys = list(d.keys()) keys.sort() with open(f, "w") as w: w.write(PREFIX) - for level in ('Deny', 'Warn', 'Allow'): - w.write("\n**Those lints are %s by default**:\n\n" % level) + for level in ('Deny', 'Warn', 'Allow', 'Deprecated'): + w.write(level_message(level)) for k in keys: if d[k][0] == level: w.write("[`%s`](#%s)\n" % (k, k))