Skip to content

Commit

Permalink
submodules: update clippy from 5afdf8b to b1d0343
Browse files Browse the repository at this point in the history
Changes:
````
new_ret_no_self: add sample from rust-lang#3313 to Known Problems section.
Support multiline comments and hopefully fix panic
Check for comments in collapsible ifs
Resolve ICE in needless range loop lint
RIIR update_lints: Update changelog links
Rename if_let_redundant_pattern_matching to redundant_pattern_matching
Add lint for redundant pattern matching for explicit return boolean
Fix issue rust-lang#3322: reword help message for len_zero
Simplify manual_memcpy suggestion in some cases
Fix dogfood
Update known problems for unnecessary_fold
RIIR update_lints: Replace lint count in README.md
Rename `active_lints` to `usable_lints`
Add comment on WalkDir vs. fs::read_dir
sort_by -> sort_by_key
Some more documentation for clippy_dev
Use `WalkDir` to also gather from subdirectories
Avoid linting `boxed_local` on trait implementations.
Website: Make lint categories linkable
Restore clippy_dummy's placeholder name
Swap order of methods in `needless_range_loop` suggestion in some cases
Revert "Exclude pattern guards from unnecessary_fold lint"
Exclude pattern guards from unnecessary_fold lint
````
  • Loading branch information
matthiaskrgr committed Oct 21, 2018
1 parent 7da731a commit 4c2181b
Show file tree
Hide file tree
Showing 34 changed files with 886 additions and 203 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ All notable changes to this project will be documented in this file.
[`redundant_closure_call`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#redundant_closure_call
[`redundant_field_names`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#redundant_field_names
[`redundant_pattern`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#redundant_pattern
[`redundant_pattern_matching`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#redundant_pattern_matching
[`ref_in_deref`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#ref_in_deref
[`regex_macro`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#regex_macro
[`replace_consts`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#replace_consts
Expand Down
1 change: 1 addition & 0 deletions clippy_dev/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ clap = "~2.32"
itertools = "0.7"
regex = "1"
lazy_static = "1.0"
walkdir = "2"
177 changes: 167 additions & 10 deletions clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use itertools::Itertools;
use lazy_static::lazy_static;
use regex::Regex;
use walkdir::WalkDir;
use std::collections::HashMap;
use std::ffi::OsStr;
use std::fs;
Expand All @@ -35,6 +36,7 @@ lazy_static! {
pub static ref DOCS_LINK: String = "https://rust-lang-nursery.github.io/rust-clippy/master/index.html".to_string();
}

/// Lint data parsed from the Clippy source code.
#[derive(Clone, PartialEq, Debug)]
pub struct Lint {
pub name: String,
Expand All @@ -55,22 +57,39 @@ impl Lint {
}
}

/// Returns all non-deprecated lints
pub fn active_lints(lints: impl Iterator<Item=Self>) -> impl Iterator<Item=Self> {
lints.filter(|l| l.deprecation.is_none())
/// Returns all non-deprecated lints and non-internal lints
pub fn usable_lints(lints: impl Iterator<Item=Self>) -> impl Iterator<Item=Self> {
lints.filter(|l| l.deprecation.is_none() && !l.is_internal())
}

/// Returns the lints in a HashMap, grouped by the different lint groups
pub fn by_lint_group(lints: &[Self]) -> HashMap<String, Vec<Self>> {
lints.iter().map(|lint| (lint.group.to_string(), lint.clone())).into_group_map()
}

pub fn is_internal(&self) -> bool {
self.group.starts_with("internal")
}
}

pub fn gen_changelog_lint_list(lints: Vec<Lint>) -> Vec<String> {
let mut lint_list_sorted: Vec<Lint> = lints;
lint_list_sorted.sort_by_key(|l| l.name.clone());
lint_list_sorted
.iter()
.filter(|l| !l.is_internal())
.map(|l| {
format!("[`{}`]: {}#{}", l.name, DOCS_LINK.clone(), l.name)
})
.collect()
}

/// Gathers all files in `src/clippy_lints` and gathers all lints inside
pub fn gather_all() -> impl Iterator<Item=Lint> {
lint_files().flat_map(|f| gather_from_file(&f))
}

fn gather_from_file(dir_entry: &fs::DirEntry) -> impl Iterator<Item=Lint> {
fn gather_from_file(dir_entry: &walkdir::DirEntry) -> impl Iterator<Item=Lint> {
let mut file = fs::File::open(dir_entry.path()).unwrap();
let mut content = String::new();
file.read_to_string(&mut content).unwrap();
Expand All @@ -89,13 +108,98 @@ fn parse_contents(content: &str, filename: &str) -> impl Iterator<Item=Lint> {
}

/// Collects all .rs files in the `clippy_lints/src` directory
fn lint_files() -> impl Iterator<Item=fs::DirEntry> {
fs::read_dir("../clippy_lints/src")
.unwrap()
fn lint_files() -> impl Iterator<Item=walkdir::DirEntry> {
// We use `WalkDir` instead of `fs::read_dir` here in order to recurse into subdirectories.
// Otherwise we would not collect all the lints, for example in `clippy_lints/src/methods/`.
WalkDir::new("../clippy_lints/src")
.into_iter()
.filter_map(|f| f.ok())
.filter(|f| f.path().extension() == Some(OsStr::new("rs")))
}

/// Replace a region in a file delimited by two lines matching regexes.
///
/// `path` is the relative path to the file on which you want to perform the replacement.
///
/// See `replace_region_in_text` for documentation of the other options.
#[allow(clippy::expect_fun_call)]
pub fn replace_region_in_file<F>(path: &str, start: &str, end: &str, replace_start: bool, replacements: F) where F: Fn() -> Vec<String> {
let mut f = fs::File::open(path).expect(&format!("File not found: {}", path));
let mut contents = String::new();
f.read_to_string(&mut contents).expect("Something went wrong reading the file");
let replaced = replace_region_in_text(&contents, start, end, replace_start, replacements);

let mut f = fs::File::create(path).expect(&format!("File not found: {}", path));
f.write_all(replaced.as_bytes()).expect("Unable to write file");
// Ensure we write the changes with a trailing newline so that
// the file has the proper line endings.
f.write_all(b"\n").expect("Unable to write file");
}

/// Replace a region in a text delimited by two lines matching regexes.
///
/// * `text` is the input text on which you want to perform the replacement
/// * `start` is a `&str` that describes the delimiter line before the region you want to replace.
/// As the `&str` will be converted to a `Regex`, this can contain regex syntax, too.
/// * `end` is a `&str` that describes the delimiter line until where the replacement should
/// happen. As the `&str` will be converted to a `Regex`, this can contain regex syntax, too.
/// * If `replace_start` is true, the `start` delimiter line is replaced as well.
/// The `end` delimiter line is never replaced.
/// * `replacements` is a closure that has to return a `Vec<String>` which contains the new text.
///
/// If you want to perform the replacement on files instead of already parsed text,
/// use `replace_region_in_file`.
///
/// # Example
///
/// ```
/// let the_text = "replace_start\nsome text\nthat will be replaced\nreplace_end";
/// let result = clippy_dev::replace_region_in_text(
/// the_text,
/// r#"replace_start"#,
/// r#"replace_end"#,
/// false,
/// || {
/// vec!["a different".to_string(), "text".to_string()]
/// }
/// );
/// assert_eq!("replace_start\na different\ntext\nreplace_end", result);
/// ```
pub fn replace_region_in_text<F>(text: &str, start: &str, end: &str, replace_start: bool, replacements: F) -> String where F: Fn() -> Vec<String> {
let lines = text.lines();
let mut in_old_region = false;
let mut found = false;
let mut new_lines = vec![];
let start = Regex::new(start).unwrap();
let end = Regex::new(end).unwrap();

for line in lines {
if in_old_region {
if end.is_match(&line) {
in_old_region = false;
new_lines.extend(replacements());
new_lines.push(line.to_string());
}
} else if start.is_match(&line) {
if !replace_start {
new_lines.push(line.to_string());
}
in_old_region = true;
found = true;
} else {
new_lines.push(line.to_string());
}
}

if !found {
// This happens if the provided regex in `clippy_dev/src/main.rs` is not found in the
// given text or file. Most likely this is an error on the programmer's side and the Regex
// is incorrect.
println!("regex {:?} not found. You may have to update it.", start);
}
new_lines.join("\n")
}

#[test]
fn test_parse_contents() {
let result: Vec<Lint> = parse_contents(
Expand Down Expand Up @@ -136,15 +240,54 @@ declare_deprecated_lint! {
}

#[test]
fn test_active_lints() {
fn test_replace_region() {
let text = r#"
abc
123
789
def
ghi"#;
let expected = r#"
abc
hello world
def
ghi"#;
let result = replace_region_in_text(text, r#"^\s*abc$"#, r#"^\s*def"#, false, || {
vec!["hello world".to_string()]
});
assert_eq!(expected, result);
}

#[test]
fn test_replace_region_with_start() {
let text = r#"
abc
123
789
def
ghi"#;
let expected = r#"
hello world
def
ghi"#;
let result = replace_region_in_text(text, r#"^\s*abc$"#, r#"^\s*def"#, true, || {
vec!["hello world".to_string()]
});
assert_eq!(expected, result);
}

#[test]
fn test_usable_lints() {
let lints = vec![
Lint::new("should_assert_eq", "Deprecated", "abc", Some("Reason"), "module_name"),
Lint::new("should_assert_eq2", "Not Deprecated", "abc", None, "module_name")
Lint::new("should_assert_eq2", "Not Deprecated", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "internal", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "internal_style", "abc", None, "module_name")
];
let expected = vec![
Lint::new("should_assert_eq2", "Not Deprecated", "abc", None, "module_name")
];
assert_eq!(expected, Lint::active_lints(lints.into_iter()).collect::<Vec<Lint>>());
assert_eq!(expected, Lint::usable_lints(lints.into_iter()).collect::<Vec<Lint>>());
}

#[test]
Expand All @@ -164,3 +307,17 @@ fn test_by_lint_group() {
]);
assert_eq!(expected, Lint::by_lint_group(&lints));
}

#[test]
fn test_gen_changelog_lint_list() {
let lints = vec![
Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"),
Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"),
];
let expected = vec![
format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK.to_string()),
format!("[`should_assert_eq2`]: {}#should_assert_eq2", DOCS_LINK.to_string())
];
assert_eq!(expected, gen_changelog_lint_list(lints));
}
38 changes: 34 additions & 4 deletions clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,54 @@ fn main() {
if let Some(matches) = matches.subcommand_matches("update_lints") {
if matches.is_present("print-only") {
print_lints();
} else {
update_lints();
}
}
}

fn print_lints() {
let lint_list = gather_all().collect::<Vec<Lint>>();
let grouped_by_lint_group = Lint::by_lint_group(&lint_list);
let lint_list = gather_all();
let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list).collect();
let lint_count = usable_lints.len();
let grouped_by_lint_group = Lint::by_lint_group(&usable_lints);

for (lint_group, mut lints) in grouped_by_lint_group {
if lint_group == "Deprecated" { continue; }
println!("\n## {}", lint_group);

lints.sort_by(|a, b| a.name.cmp(&b.name));
lints.sort_by_key(|l| l.name.clone());

for lint in lints {
println!("* [{}]({}#{}) ({})", lint.name, clippy_dev::DOCS_LINK.clone(), lint.name, lint.desc);
}
}

println!("there are {} lints", Lint::active_lints(lint_list.into_iter()).count());
println!("there are {} lints", lint_count);
}

fn update_lints() {
let lint_list: Vec<Lint> = gather_all().collect();
let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list.clone().into_iter()).collect();
let lint_count = usable_lints.len();

replace_region_in_file(
"../README.md",
r#"\[There are \d+ lints included in this crate!\]\(https://rust-lang-nursery.github.io/rust-clippy/master/index.html\)"#,
"",
true,
|| {
vec![
format!("[There are {} lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)", lint_count)
]
}
);

replace_region_in_file(
"../CHANGELOG.md",
"<!-- begin autogenerated links to lint list -->",
"<!-- end autogenerated links to lint list -->",
false,
|| { gen_changelog_lint_list(lint_list.clone()) }
);
}
2 changes: 1 addition & 1 deletion clippy_dummy/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[package]
name = "clippy" # rename to clippy before publishing
name = "clippy_dummy" # rename to clippy before publishing
version = "0.0.302"
authors = ["Manish Goregaokar <[email protected]>"]
edition = "2018"
Expand Down
9 changes: 9 additions & 0 deletions clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,17 @@ fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
}
}

fn block_starts_with_comment(cx: &EarlyContext<'_>, expr: &ast::Block) -> bool {
// We trim all opening braces and whitespaces and then check if the next string is a comment.
let trimmed_block_text =
snippet_block(cx, expr.span, "..").trim_left_matches(|c: char| c.is_whitespace() || c == '{').to_owned();
trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*")
}

fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
if_chain! {
if let ast::ExprKind::Block(ref block, _) = else_.node;
if !block_starts_with_comment(cx, block);
if let Some(else_) = expr_block(block);
if !in_macro(else_.span);
then {
Expand All @@ -135,6 +143,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {

fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &ast::Expr, then: &ast::Block) {
if_chain! {
if !block_starts_with_comment(cx, then);
if let Some(inner) = expr_block(then);
if let ast::ExprKind::If(ref check_inner, ref content, None) = inner.node;
then {
Expand Down
12 changes: 11 additions & 1 deletion clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ macro_rules! declare_deprecated_lint {

/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** This used to check for `assert!(a == b)` and recommend
/// **Deprecation reason:** This used to check for `assert!(a == b)` and recommend
/// replacement with `assert_eq!(a, b)`, but this is no longer needed after RFC 2011.
declare_deprecated_lint! {
pub SHOULD_ASSERT_EQ,
Expand Down Expand Up @@ -102,3 +102,13 @@ declare_deprecated_lint! {
pub ASSIGN_OPS,
"using compound assignment operators (e.g. `+=`) is harmless"
}

/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** The original rule will only lint for `if let`. After
/// making it support to lint `match`, naming as `if let` is not suitable for it.
/// So, this lint is deprecated.
declare_deprecated_lint! {
pub IF_LET_REDUNDANT_PATTERN_MATCHING,
"this lint has been changed to redundant_pattern_matching"
}
13 changes: 12 additions & 1 deletion clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ impl LintPass for Pass {
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {

fn check_fn(
&mut self,
cx: &LateContext<'a, 'tcx>,
Expand All @@ -74,13 +75,23 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
_: Span,
node_id: NodeId,
) {
let fn_def_id = cx.tcx.hir.local_def_id(node_id);
// If the method is an impl for a trait, don't warn
let parent_id = cx.tcx.hir.get_parent(node_id);
let parent_node = cx.tcx.hir.find(parent_id);

if let Some(Node::Item(item)) = parent_node {
if let ItemKind::Impl(_, _, _, _, Some(..), _, _) = item.node {
return;
}
}

let mut v = EscapeDelegate {
cx,
set: NodeSet(),
too_large_for_stack: self.too_large_for_stack,
};

let fn_def_id = cx.tcx.hir.local_def_id(node_id);
let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
ExprUseVisitor::new(&mut v, cx.tcx, cx.param_env, region_scope_tree, cx.tables, None).consume_body(body);

Expand Down
Loading

0 comments on commit 4c2181b

Please sign in to comment.