Skip to content

Commit

Permalink
Merge pull request #795 from mcarton/deprecated
Browse files Browse the repository at this point in the history
Deprecates 4 lints
  • Loading branch information
Manishearth committed Mar 24, 2016
2 parents 4796ea0 + 15e55f5 commit 7e65493
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 171 deletions.
6 changes: 1 addition & 5 deletions README.md
Expand Up @@ -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
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
44 changes: 44 additions & 0 deletions 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"
}
13 changes: 7 additions & 6 deletions src/lib.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
53 changes: 2 additions & 51 deletions src/methods.rs
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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"]) {
Expand Down Expand Up @@ -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::<String>(), 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
Expand Down
68 changes: 0 additions & 68 deletions src/needless_features.rs

This file was deleted.

1 change: 0 additions & 1 deletion tests/compile-fail/cmp_owned.rs
Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions tests/compile-fail/methods.rs
Expand Up @@ -296,12 +296,6 @@ fn main() {
let res: Result<i32, ()> = 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`
Expand Down
28 changes: 0 additions & 28 deletions tests/compile-fail/needless_features.rs

This file was deleted.

12 changes: 12 additions & 0 deletions 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() {}

0 comments on commit 7e65493

Please sign in to comment.