squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Merge pull request #795 from mcarton/deprecated

Browse source 
Deprecates 4 lints
This commit is contained in:
Manish Goregaokar 2016-03-25 00:40:37 +05:30
parent 4796ea0558 15e55f5df5
commit 7e65493599
11 changed files with 115 additions and 171 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
README.md
View file

@ -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

44
src/deprecated_lints.rs Normal file
View file

@ -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
src/lib.rs
View file

@ -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,

53
src/methods.rs
View file

@ -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::<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

68
src/needless_features.rs
View file

@ -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<T>"])
}

1
tests/compile-fail/cmp_owned.rs
View file

@ -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

6
tests/compile-fail/methods.rs
View file

@ -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`

28
tests/compile-fail/needless_features.rs
View file

@ -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();
}

12
tests/run-pass/deprecated.rs Normal file
View file

@ -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() {}

34
util/update_lints.py
View file

@ -13,14 +13,20 @@ declare_lint_re = re.compile(r'''
pub \s+ (?P<name>[A-Z_][A-Z_0-9]*) \s*,\s*
(?P<level>Forbid|Deny|Warn|Allow) \s*,\s*
" (?P<desc>(?:[^"\\]+|\\.)*) " \s* [})]
''', re.X | re.S)
''', re.VERBOSE | re.DOTALL)
declare_deprecated_lint_re = re.compile(r'''
declare_deprecated_lint! \s* [{(] \s*
pub \s+ (?P<name>[A-Z_][A-Z_0-9]*) \s*,\s*
" (?P<desc>(?:[^"\\]+|\\.)*) " \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'\]\);',

21
util/update_wiki.py
View file

@ -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 @@ conf_template = """
"""
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))