From 5a7307aed51b9b0037e8376f1ef5cde2860ca228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Wed, 14 Nov 2018 14:49:40 +0100 Subject: [PATCH] submodules: update clippy from d8b42690 to f5d868c9 Fixes clippy toolstate. Changes: ```` rustup https://github.com/rust-lang/rust/pull/55852/ Fix "too" -> "foo" typo in format.rs Fix `use_self` violation Fix wrong suggestion for `redundant_closure_call` Check for common metadata Fix `use_self` false positive on `use` statements Fix `use_self` false positive Remove `+` from `has_unary_equivalent` Fix dogfood Update println! formatting Fix false positive in check mode caused by `gen_deprecated` RIIR update lints: Add check mode (update_lints.py rewrite complete) changed into_iter to iter and fixed a lint check Fix `collapsible_if` error Fix `possible_missing_comma` false positives format code fix comment spacing change single char str to char add lint to lintarray macro Revert "small fix" small fix added float support for mistyped literal lints tmp progress ```` --- CHANGELOG.md | 1 + CONTRIBUTING.md | 2 +- Cargo.toml | 2 +- README.md | 2 +- ci/base-tests.sh | 5 +- clippy_dev/src/lib.rs | 122 ++++++----- clippy_dev/src/main.rs | 59 +++-- clippy_lints/Cargo.toml | 2 +- clippy_lints/src/cargo_common_metadata.rs | 115 ++++++++++ clippy_lints/src/format.rs | 2 +- clippy_lints/src/formatting.rs | 9 +- clippy_lints/src/lib.rs | 15 +- clippy_lints/src/literal_representation.rs | 156 +++++++------ clippy_lints/src/misc_early.rs | 63 ++++-- clippy_lints/src/use_self.rs | 44 ++-- tests/ui/formatting.rs | 12 + tests/ui/literals.rs | 10 +- tests/ui/literals.stderr | 146 ++++++++----- tests/ui/redundant_closure_call.rs | 5 + tests/ui/use_self.rs | 25 +++ util/update_lints.py | 242 +-------------------- 21 files changed, 568 insertions(+), 471 deletions(-) create mode 100644 clippy_lints/src/cargo_common_metadata.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index bdd01bfeecdd..3237f76b4aee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -628,6 +628,7 @@ All notable changes to this project will be documented in this file. [`box_vec`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#box_vec [`boxed_local`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#boxed_local [`builtin_type_shadow`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#builtin_type_shadow +[`cargo_common_metadata`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cargo_common_metadata [`cast_lossless`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cast_lossless [`cast_possible_truncation`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cast_possible_truncation [`cast_possible_wrap`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#cast_possible_wrap diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c4080a5fa9c4..50f61eb1e670 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -180,7 +180,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { The [`rustc_plugin::PluginRegistry`][plugin_registry] provides two methods to register lints: [register_early_lint_pass][reg_early_lint_pass] and [register_late_lint_pass][reg_late_lint_pass]. Both take an object that implements an [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass] respectively. This is done in every single lint. -It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `util/update_lints.py` and you don't have to add anything by hand. When you are writing your own lint, you can use that script to save you some time. +It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `util/dev update_lints` and you don't have to add anything by hand. When you are writing your own lint, you can use that script to save you some time. ```rust // ./clippy_lints/src/else_if_without_else.rs diff --git a/Cargo.toml b/Cargo.toml index 2293913f38e7..0a1f3dabb933 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,7 @@ rustc_tools_util = { version = "0.1.0", path = "rustc_tools_util"} [dev-dependencies] clippy_dev = { version = "0.0.1", path = "clippy_dev" } -cargo_metadata = "0.6" +cargo_metadata = "0.6.2" compiletest_rs = "0.3.16" lazy_static = "1.0" serde_derive = "1.0" diff --git a/README.md b/README.md index 0525788e64e1..47db3a358e3f 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 287 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html) +[There are 288 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/ci/base-tests.sh b/ci/base-tests.sh index 0ed1494be821..88cc20842e82 100755 --- a/ci/base-tests.sh +++ b/ci/base-tests.sh @@ -23,8 +23,9 @@ cargo test --features debugging cd clippy_lints && cargo test && cd .. cd rustc_tools_util && cargo test && cd .. cd clippy_dev && cargo test && cd .. -# check that the lint lists are up-to-date -./util/update_lints.py -c + +# Perform various checks for lint registration +./util/dev update_lints --check CLIPPY="`pwd`/target/debug/cargo-clippy clippy" # run clippy on its own codebase... diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 2ae8423381db..5e1a454195e4 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -114,19 +114,22 @@ pub fn gen_changelog_lint_list(lints: Vec) -> Vec { /// Generates the `register_removed` code in `./clippy_lints/src/lib.rs`. pub fn gen_deprecated(lints: &[Lint]) -> Vec { - lints.iter() - .filter_map(|l| { - l.clone().deprecation.and_then(|depr_text| { - Some( - format!( - " store.register_removed(\n \"{}\",\n \"{}\",\n );", - l.name, - depr_text + itertools::flatten( + lints + .iter() + .filter_map(|l| { + l.clone().deprecation.and_then(|depr_text| { + Some( + vec![ + " store.register_removed(".to_string(), + format!(" \"{}\",", l.name), + format!(" \"{}\",", depr_text), + " );".to_string() + ] ) - ) + }) }) - }) - .collect() + ).collect() } /// Gathers all files in `src/clippy_lints` and gathers all lints inside @@ -168,23 +171,33 @@ fn lint_files() -> impl Iterator { .filter(|f| f.path().extension() == Some(OsStr::new("rs"))) } +/// Whether a file has had its text changed or not +#[derive(PartialEq, Debug)] +pub struct FileChange { + pub changed: bool, + pub new_lines: String, +} + /// 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(path: &str, start: &str, end: &str, replace_start: bool, replacements: F) where F: Fn() -> Vec { +pub fn replace_region_in_file(path: &str, start: &str, end: &str, replace_start: bool, write_back: bool, replacements: F) -> FileChange where F: Fn() -> Vec { 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"); + let file_change = replace_region_in_text(&contents, start, end, replace_start, replacements); + + if write_back { + let mut f = fs::File::create(path).expect(&format!("File not found: {}", path)); + f.write_all(file_change.new_lines.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"); + } + file_change } /// Replace a region in a text delimited by two lines matching regexes. @@ -213,10 +226,10 @@ pub fn replace_region_in_file(path: &str, start: &str, end: &str, replace_sta /// || { /// vec!["a different".to_string(), "text".to_string()] /// } -/// ); +/// ).new_lines; /// assert_eq!("replace_start\na different\ntext\nreplace_end", result); /// ``` -pub fn replace_region_in_text(text: &str, start: &str, end: &str, replace_start: bool, replacements: F) -> String where F: Fn() -> Vec { +pub fn replace_region_in_text(text: &str, start: &str, end: &str, replace_start: bool, replacements: F) -> FileChange where F: Fn() -> Vec { let lines = text.lines(); let mut in_old_region = false; let mut found = false; @@ -224,7 +237,7 @@ pub fn replace_region_in_text(text: &str, start: &str, end: &str, replace_sta let start = Regex::new(start).unwrap(); let end = Regex::new(end).unwrap(); - for line in lines { + for line in lines.clone() { if in_old_region { if end.is_match(&line) { in_old_region = false; @@ -248,7 +261,11 @@ pub fn replace_region_in_text(text: &str, start: &str, end: &str, replace_sta // is incorrect. eprintln!("error: regex `{:?}` not found. You may have to update it.", start); } - new_lines.join("\n") + + FileChange { + changed: lines.ne(new_lines.clone()), + new_lines: new_lines.join("\n") + } } #[test] @@ -292,17 +309,11 @@ declare_deprecated_lint! { #[test] fn test_replace_region() { - let text = r#" -abc -123 -789 -def -ghi"#; - let expected = r#" -abc -hello world -def -ghi"#; + let text = "\nabc\n123\n789\ndef\nghi"; + let expected = FileChange { + changed: true, + new_lines: "\nabc\nhello world\ndef\nghi".to_string() + }; let result = replace_region_in_text(text, r#"^\s*abc$"#, r#"^\s*def"#, false, || { vec!["hello world".to_string()] }); @@ -311,22 +322,30 @@ ghi"#; #[test] fn test_replace_region_with_start() { - let text = r#" -abc -123 -789 -def -ghi"#; - let expected = r#" -hello world -def -ghi"#; + let text = "\nabc\n123\n789\ndef\nghi"; + let expected = FileChange { + changed: true, + new_lines: "\nhello world\ndef\nghi".to_string() + }; 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_replace_region_no_changes() { + let text = "123\n456\n789"; + let expected = FileChange { + changed: false, + new_lines: "123\n456\n789".to_string() + }; + let result = replace_region_in_text(text, r#"^\s*123$"#, r#"^\s*456"#, false, || { + vec![] + }); + assert_eq!(expected, result); +} + #[test] fn test_usable_lints() { let lints = vec![ @@ -377,14 +396,19 @@ fn test_gen_changelog_lint_list() { fn test_gen_deprecated() { let lints = vec![ Lint::new("should_assert_eq", "group1", "abc", Some("has been superseeded by should_assert_eq2"), "module_name"), + Lint::new("another_deprecated", "group2", "abc", Some("will be removed"), "module_name"), Lint::new("should_assert_eq2", "group2", "abc", None, "module_name") ]; let expected: Vec = vec![ - r#" store.register_removed( - "should_assert_eq", - "has been superseeded by should_assert_eq2", - );"#.to_string() - ]; + " store.register_removed(", + " \"should_assert_eq\",", + " \"has been superseeded by should_assert_eq2\",", + " );", + " store.register_removed(", + " \"another_deprecated\",", + " \"will be removed\",", + " );" + ].into_iter().map(String::from).collect(); assert_eq!(expected, gen_deprecated(&lints)); } diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 288fb7c58b4c..bfd98968c422 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -15,6 +15,12 @@ extern crate regex; use clap::{App, Arg, SubCommand}; use clippy_dev::*; +#[derive(PartialEq)] +enum UpdateMode { + Check, + Change +} + fn main() { let matches = App::new("Clippy developer tooling") .subcommand( @@ -28,17 +34,23 @@ fn main() { .arg( Arg::with_name("print-only") .long("print-only") - .short("p") - .help("Print a table of lints to STDOUT. This does not include deprecated and internal lints. (Does not modify any files)"), + .help("Print a table of lints to STDOUT. This does not include deprecated and internal lints. (Does not modify any files)") + ) + .arg( + Arg::with_name("check") + .long("check") + .help("Checks that util/dev update_lints has been run. Used on CI."), ) - ) - .get_matches(); + ) + .get_matches(); if let Some(matches) = matches.subcommand_matches("update_lints") { if matches.is_present("print-only") { print_lints(); + } else if matches.is_present("check") { + update_lints(&UpdateMode::Check); } else { - update_lints(); + update_lints(&UpdateMode::Change); } } } @@ -63,53 +75,58 @@ fn print_lints() { println!("there are {} lints", lint_count); } -fn update_lints() { +fn update_lints(update_mode: &UpdateMode) { let lint_list: Vec = gather_all().collect(); let usable_lints: Vec = Lint::usable_lints(lint_list.clone().into_iter()).collect(); let lint_count = usable_lints.len(); - replace_region_in_file( + let mut file_change = 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, + update_mode == &UpdateMode::Change, || { vec![ format!("[There are {} lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)", lint_count) ] } - ); + ).changed; - replace_region_in_file( + file_change |= replace_region_in_file( "../CHANGELOG.md", "", "", false, + update_mode == &UpdateMode::Change, || { gen_changelog_lint_list(lint_list.clone()) } - ); + ).changed; - replace_region_in_file( + file_change |= replace_region_in_file( "../clippy_lints/src/lib.rs", "begin deprecated lints", "end deprecated lints", false, + update_mode == &UpdateMode::Change, || { gen_deprecated(&lint_list) } - ); + ).changed; - replace_region_in_file( + file_change |= replace_region_in_file( "../clippy_lints/src/lib.rs", "begin lints modules", "end lints modules", false, + update_mode == &UpdateMode::Change, || { gen_modules_list(lint_list.clone()) } - ); + ).changed; // Generate lists of lints in the clippy::all lint group - replace_region_in_file( + file_change |= replace_region_in_file( "../clippy_lints/src/lib.rs", r#"reg.register_lint_group\("clippy::all""#, r#"\]\);"#, false, + update_mode == &UpdateMode::Change, || { // clippy::all should only include the following lint groups: let all_group_lints = usable_lints.clone().into_iter().filter(|l| { @@ -121,16 +138,22 @@ fn update_lints() { gen_lint_group_list(all_group_lints) } - ); + ).changed; // Generate the list of lints for all other lint groups for (lint_group, lints) in Lint::by_lint_group(&usable_lints) { - replace_region_in_file( + file_change |= replace_region_in_file( "../clippy_lints/src/lib.rs", &format!("reg.register_lint_group\\(\"clippy::{}\"", lint_group), r#"\]\);"#, false, + update_mode == &UpdateMode::Change, || { gen_lint_group_list(lints.clone()) } - ); + ).changed; + } + + if update_mode == &UpdateMode::Check && file_change { + println!("Not all lints defined properly. Please run `util/dev update_lints` to make sure all lints are defined properly."); + std::process::exit(1); } } diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 8907dd3659c8..fa1b22784bde 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -17,7 +17,7 @@ keywords = ["clippy", "lint", "plugin"] edition = "2018" [dependencies] -cargo_metadata = "0.6" +cargo_metadata = "0.6.2" itertools = "0.7" lazy_static = "1.0.2" matches = "0.1.7" diff --git a/clippy_lints/src/cargo_common_metadata.rs b/clippy_lints/src/cargo_common_metadata.rs new file mode 100644 index 000000000000..1a053de27d13 --- /dev/null +++ b/clippy_lints/src/cargo_common_metadata.rs @@ -0,0 +1,115 @@ +// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! lint on missing cargo common metadata + +use crate::rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass}; +use crate::rustc::{declare_tool_lint, lint_array}; +use crate::syntax::{ast::*, source_map::DUMMY_SP}; +use crate::utils::span_lint; + +use cargo_metadata; + +/// **What it does:** Checks to see if all common metadata is defined in +/// `Cargo.toml`. See: https://rust-lang-nursery.github.io/api-guidelines/documentation.html#cargotoml-includes-all-common-metadata-c-metadata +/// +/// **Why is this bad?** It will be more difficult for users to discover the +/// purpose of the crate, and key information related to it. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```toml +/// # This `Cargo.toml` is missing an authors field: +/// [package] +/// name = "clippy" +/// version = "0.0.212" +/// description = "A bunch of helpful lints to avoid common pitfalls in Rust" +/// repository = "https://github.com/rust-lang-nursery/rust-clippy" +/// readme = "README.md" +/// license = "MIT/Apache-2.0" +/// keywords = ["clippy", "lint", "plugin"] +/// categories = ["development-tools", "development-tools::cargo-plugins"] +/// ``` +declare_clippy_lint! { + pub CARGO_COMMON_METADATA, + cargo, + "common metadata is defined in `Cargo.toml`" +} + +fn warning(cx: &EarlyContext<'_>, message: &str) { + span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, message); +} + +fn missing_warning(cx: &EarlyContext<'_>, package: &cargo_metadata::Package, field: &str) { + let message = format!("package `{}` is missing `{}` metadata", package.name, field); + warning(cx, &message); +} + +fn is_empty_str(value: &Option) -> bool { + match value { + None => true, + Some(value) if value.is_empty() => true, + _ => false + } +} + +fn is_empty_vec(value: &[String]) -> bool { + // This works because empty iterators return true + value.iter().all(|v| v.is_empty()) +} + +pub struct Pass; + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array!(CARGO_COMMON_METADATA) + } +} + +impl EarlyLintPass for Pass { + fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) { + let metadata = if let Ok(metadata) = cargo_metadata::metadata_deps(None, true) { + metadata + } else { + warning(cx, "could not read cargo metadata"); + return; + }; + + for package in metadata.packages { + if is_empty_vec(&package.authors) { + missing_warning(cx, &package, "package.authors"); + } + + if is_empty_str(&package.description) { + missing_warning(cx, &package, "package.description"); + } + + if is_empty_str(&package.license) { + missing_warning(cx, &package, "package.license"); + } + + if is_empty_str(&package.repository) { + missing_warning(cx, &package, "package.repository"); + } + + if is_empty_str(&package.readme) { + missing_warning(cx, &package, "package.readme"); + } + + if is_empty_vec(&package.keywords) { + missing_warning(cx, &package, "package.keywords"); + } + + if is_empty_vec(&package.categories) { + missing_warning(cx, &package, "package.categories"); + } + } + } +} diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 41046a98a34c..bd9fa6f80be3 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -21,7 +21,7 @@ use crate::rustc_errors::Applicability; /// **What it does:** Checks for the use of `format!("string literal with no /// argument")` and `format!("{}", foo)` where `foo` is a string. /// -/// **Why is this bad?** There is no point of doing that. `format!("too")` can +/// **Why is this bad?** There is no point of doing that. `format!("foo")` can /// be replaced by `"foo".to_owned()` if you really need a `String`. The even /// worse `&format!("foo")` is often encountered in the wild. `format!("{}", /// foo)` can be replaced by `foo.clone()` if `foo: String` or `foo.to_owned()` diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 7f5715ef6913..d06183cb52f1 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -173,12 +173,19 @@ fn check_else_if(cx: &EarlyContext<'_>, expr: &ast::Expr) { } } +fn has_unary_equivalent(bin_op: ast::BinOpKind) -> bool { + // &, *, - + bin_op == ast::BinOpKind::And + || bin_op == ast::BinOpKind::Mul + || bin_op == ast::BinOpKind::Sub +} + /// Implementation of the `POSSIBLE_MISSING_COMMA` lint for array fn check_array(cx: &EarlyContext<'_>, expr: &ast::Expr) { if let ast::ExprKind::Array(ref array) = expr.node { for element in array { if let ast::ExprKind::Binary(ref op, ref lhs, _) = element.node { - if !differing_macro_contexts(lhs.span, op.span) { + if has_unary_equivalent(op.node) && !differing_macro_contexts(lhs.span, op.span) { let space_span = lhs.span.between(op.span); if let Some(space_snippet) = snippet_opt(cx, space_span) { let lint_span = lhs.span.with_lo(lhs.span.hi()); diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 06ae78a6509c..6ebe9df6a1ef 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -96,6 +96,7 @@ pub mod blacklisted_name; pub mod block_in_if_condition; pub mod booleans; pub mod bytecount; +pub mod cargo_common_metadata; pub mod collapsible_if; pub mod const_static_lifetime; pub mod copies; @@ -444,6 +445,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box double_comparison::Pass); reg.register_late_lint_pass(box question_mark::Pass); reg.register_late_lint_pass(box suspicious_trait_impl::SuspiciousImpl); + reg.register_early_lint_pass(box cargo_common_metadata::Pass); reg.register_early_lint_pass(box multiple_crate_versions::Pass); reg.register_early_lint_pass(box wildcard_dependencies::Pass); reg.register_late_lint_pass(box map_unit_fn::Pass); @@ -548,8 +550,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { bytecount::NAIVE_BYTECOUNT, collapsible_if::COLLAPSIBLE_IF, const_static_lifetime::CONST_STATIC_LIFETIME, - copies::IF_SAME_THEN_ELSE, copies::IFS_SAME_COND, + copies::IF_SAME_THEN_ELSE, cyclomatic_complexity::CYCLOMATIC_COMPLEXITY, derive::DERIVE_HASH_XOR_EQ, double_comparison::DOUBLE_COMPARISONS, @@ -743,12 +745,12 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { unused_io_amount::UNUSED_IO_AMOUNT, unused_label::UNUSED_LABEL, vec::USELESS_VEC, + write::PRINTLN_EMPTY_STRING, write::PRINT_LITERAL, write::PRINT_WITH_NEWLINE, - write::PRINTLN_EMPTY_STRING, + write::WRITELN_EMPTY_STRING, write::WRITE_LITERAL, write::WRITE_WITH_NEWLINE, - write::WRITELN_EMPTY_STRING, zero_div_zero::ZERO_DIVIDED_BY_ZERO, ]); @@ -831,12 +833,12 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { types::IMPLICIT_HASHER, types::LET_UNIT_VALUE, unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, + write::PRINTLN_EMPTY_STRING, write::PRINT_LITERAL, write::PRINT_WITH_NEWLINE, - write::PRINTLN_EMPTY_STRING, + write::WRITELN_EMPTY_STRING, write::WRITE_LITERAL, write::WRITE_WITH_NEWLINE, - write::WRITELN_EMPTY_STRING, ]); reg.register_lint_group("clippy::complexity", Some("clippy_complexity"), vec![ @@ -916,8 +918,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { bit_mask::BAD_BIT_MASK, bit_mask::INEFFECTIVE_BIT_MASK, booleans::LOGIC_BUG, - copies::IF_SAME_THEN_ELSE, copies::IFS_SAME_COND, + copies::IF_SAME_THEN_ELSE, derive::DERIVE_HASH_XOR_EQ, drop_forget_ref::DROP_COPY, drop_forget_ref::DROP_REF, @@ -985,6 +987,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { ]); reg.register_lint_group("clippy::cargo", Some("clippy_cargo"), vec![ + cargo_common_metadata::CARGO_COMMON_METADATA, multiple_crate_versions::MULTIPLE_CRATE_VERSIONS, wildcard_dependencies::WILDCARD_DEPENDENCIES, ]); diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index f029dd65b395..bd54c0684862 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -7,16 +7,15 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. - //! Lints concerned with the grouping of digits with underscores in integral or //! floating-point literal expressions. -use crate::rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass, in_external_macro, LintContext}; +use crate::rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintContext, LintPass}; use crate::rustc::{declare_tool_lint, lint_array}; -use if_chain::if_chain; use crate::syntax::ast::*; use crate::syntax_pos; use crate::utils::{snippet_opt, span_lint_and_sugg}; +use if_chain::if_chain; /// **What it does:** Warns if a long integral or floating-point constant does /// not contain underscores. @@ -41,9 +40,9 @@ declare_clippy_lint! { /// **Why is this bad?** This is most probably a typo /// /// **Known problems:** -/// - Recommends a signed suffix, even though the number might be too big and an unsigned +/// - Recommends a signed suffix, even though the number might be too big and an unsigned /// suffix is required -/// - Does not match on `_128` since that is a valid grouping for decimal and octal numbers +/// - Does not match on `_128` since that is a valid grouping for decimal and octal numbers /// /// **Example:** /// @@ -168,21 +167,21 @@ impl<'a> DigitInfo<'a> { let len = sans_prefix.len(); let mut last_d = '\0'; for (d_idx, d) in sans_prefix.char_indices() { - let suffix_start = if last_d == '_' { - d_idx - 1 - } else { - d_idx - }; - if float && (d == 'f' || d == 'e' || d == 'E') || - !float && (d == 'i' || d == 'u' || is_possible_suffix_index(&sans_prefix, suffix_start, len)) { - let (digits, suffix) = sans_prefix.split_at(suffix_start); - return Self { - digits, - radix, - prefix, - suffix: Some(suffix), - float, - }; + let suffix_start = if last_d == '_' { d_idx - 1 } else { d_idx }; + if float + && (d == 'f' + || is_possible_float_suffix_index(&sans_prefix, suffix_start, len) + || ((d == 'E' || d == 'e') && !has_possible_float_suffix(&sans_prefix))) + || !float && (d == 'i' || d == 'u' || is_possible_suffix_index(&sans_prefix, suffix_start, len)) + { + let (digits, suffix) = sans_prefix.split_at(suffix_start); + return Self { + digits, + radix, + prefix, + suffix: Some(suffix), + float, + }; } last_d = d } @@ -224,18 +223,44 @@ impl<'a> DigitInfo<'a> { .map(|chunk| chunk.iter().collect()) .collect::>() .join("_"); + let suffix_hint = match self.suffix { + Some(suffix) if is_mistyped_float_suffix(suffix) => format!("_f{}", &suffix[1..]), + Some(suffix) => suffix.to_string(), + None => String::new(), + }; + format!("{}.{}{}", int_part_hint, frac_part_hint, suffix_hint) + } else if self.float && (self.digits.contains('E') || self.digits.contains('e')) { + let which_e = if self.digits.contains('E') { 'E' } else { 'e' }; + let parts: Vec<&str> = self.digits.split(which_e).collect(); + let filtered_digits_vec_0 = parts[0].chars().filter(|&c| c != '_').rev().collect::>(); + let filtered_digits_vec_1 = parts[1].chars().filter(|&c| c != '_').rev().collect::>(); + let before_e_hint = filtered_digits_vec_0 + .chunks(group_size) + .map(|chunk| chunk.iter().rev().collect()) + .rev() + .collect::>() + .join("_"); + let after_e_hint = filtered_digits_vec_1 + .chunks(group_size) + .map(|chunk| chunk.iter().rev().collect()) + .rev() + .collect::>() + .join("_"); + let suffix_hint = match self.suffix { + Some(suffix) if is_mistyped_float_suffix(suffix) => format!("_f{}", &suffix[1..]), + Some(suffix) => suffix.to_string(), + None => String::new(), + }; format!( - "{}.{}{}", - int_part_hint, - frac_part_hint, - self.suffix.unwrap_or("") + "{}{}{}{}{}", + self.prefix.unwrap_or(""), + before_e_hint, + which_e, + after_e_hint, + suffix_hint ) } else { - let filtered_digits_vec = self.digits - .chars() - .filter(|&c| c != '_') - .rev() - .collect::>(); + let filtered_digits_vec = self.digits.chars().filter(|&c| c != '_').rev().collect::>(); let mut hint = filtered_digits_vec .chunks(group_size) .map(|chunk| chunk.iter().rev().collect()) @@ -248,18 +273,11 @@ impl<'a> DigitInfo<'a> { hint = format!("{:0>4}{}", &hint[..nb_digits_to_fill], &hint[nb_digits_to_fill..]); } let suffix_hint = match self.suffix { - Some(suffix) if is_mistyped_suffix(suffix) => { - format!("_i{}", &suffix[1..]) - }, + Some(suffix) if is_mistyped_suffix(suffix) => format!("_i{}", &suffix[1..]), Some(suffix) => suffix.to_string(), - None => String::new() + None => String::new(), }; - format!( - "{}{}{}", - self.prefix.unwrap_or(""), - hint, - suffix_hint - ) + format!("{}{}{}", self.prefix.unwrap_or(""), hint, suffix_hint) } } } @@ -269,22 +287,20 @@ enum WarningType { InconsistentDigitGrouping, LargeDigitGroups, DecimalRepresentation, - MistypedLiteralSuffix + MistypedLiteralSuffix, } impl WarningType { crate fn display(&self, grouping_hint: &str, cx: &EarlyContext<'_>, span: syntax_pos::Span) { match self { - WarningType::MistypedLiteralSuffix => { - span_lint_and_sugg( - cx, - MISTYPED_LITERAL_SUFFIXES, - span, - "mistyped literal suffix", - "did you mean to write", - grouping_hint.to_string() - ) - }, + WarningType::MistypedLiteralSuffix => span_lint_and_sugg( + cx, + MISTYPED_LITERAL_SUFFIXES, + span, + "mistyped literal suffix", + "did you mean to write", + grouping_hint.to_string(), + ), WarningType::UnreadableLiteral => span_lint_and_sugg( cx, UNREADABLE_LITERAL, @@ -380,7 +396,7 @@ impl LiteralDigitGrouping { // Lint integral and fractional parts separately, and then check consistency of digit // groups if both pass. - let _ = Self::do_lint(parts[0], None) + let _ = Self::do_lint(parts[0], digit_info.suffix) .map(|integral_group_size| { if parts.len() > 1 { // Lint the fractional part of literal just like integral part, but reversed. @@ -391,11 +407,11 @@ impl LiteralDigitGrouping { fractional_group_size, parts[0].len(), parts[1].len()); - if !consistent { - WarningType::InconsistentDigitGrouping.display(&digit_info.grouping_hint(), - cx, - lit.span); - } + if !consistent { + WarningType::InconsistentDigitGrouping.display(&digit_info.grouping_hint(), + cx, + lit.span); + } }) .map_err(|warning_type| warning_type.display(&digit_info.grouping_hint(), cx, @@ -494,9 +510,7 @@ impl EarlyLintPass for LiteralRepresentation { impl LiteralRepresentation { pub fn new(threshold: u64) -> Self { - Self { - threshold, - } + Self { threshold } } fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) { // Lint integral literals. @@ -529,7 +543,12 @@ impl LiteralRepresentation { fn do_lint(digits: &str) -> Result<(), WarningType> { if digits.len() == 1 { // Lint for 1 digit literals, if someone really sets the threshold that low - if digits == "1" || digits == "2" || digits == "4" || digits == "8" || digits == "3" || digits == "7" + if digits == "1" + || digits == "2" + || digits == "4" + || digits == "8" + || digits == "3" + || digits == "7" || digits == "F" { return Err(WarningType::DecimalRepresentation); @@ -538,6 +557,7 @@ impl LiteralRepresentation { // Lint for Literals with a hex-representation of 2 or 3 digits let f = &digits[0..1]; // first digit let s = &digits[1..]; // suffix + // Powers of 2 if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && s.chars().all(|c| c == '0')) // Powers of 2 minus 1 @@ -550,6 +570,7 @@ impl LiteralRepresentation { let f = &digits[0..1]; // first digit let m = &digits[1..digits.len() - 1]; // middle digits, except last let s = &digits[1..]; // suffix + // Powers of 2 with a margin of +15/-16 if ((f.eq("1") || f.eq("2") || f.eq("4") || f.eq("8")) && m.chars().all(|c| c == '0')) || ((f.eq("1") || f.eq("3") || f.eq("7") || f.eq("F")) && m.chars().all(|c| c == 'F')) @@ -570,6 +591,17 @@ fn is_mistyped_suffix(suffix: &str) -> bool { } fn is_possible_suffix_index(lit: &str, idx: usize, len: usize) -> bool { - ((len > 3 && idx == len - 3) || (len > 2 && idx == len - 2)) && - is_mistyped_suffix(lit.split_at(idx).1) + ((len > 3 && idx == len - 3) || (len > 2 && idx == len - 2)) && is_mistyped_suffix(lit.split_at(idx).1) +} + +fn is_mistyped_float_suffix(suffix: &str) -> bool { + ["_32", "_64"].contains(&suffix) +} + +fn is_possible_float_suffix_index(lit: &str, idx: usize, len: usize) -> bool { + (len > 3 && idx == len - 3) && is_mistyped_float_suffix(lit.split_at(idx).1) +} + +fn has_possible_float_suffix(lit: &str) -> bool { + lit.ends_with("_32") || lit.ends_with("_64") } diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index a2fd487078e8..4973db4a5e83 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -15,7 +15,7 @@ use if_chain::if_chain; use std::char; use crate::syntax::ast::*; use crate::syntax::source_map::Span; -use crate::syntax::visit::FnKind; +use crate::syntax::visit::{FnKind, Visitor, walk_expr}; use crate::utils::{constants, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_then}; use crate::rustc_errors::Applicability; @@ -199,6 +199,31 @@ impl LintPass for MiscEarly { } } +// Used to find `return` statements or equivalents e.g. `?` +struct ReturnVisitor { + found_return: bool, +} + +impl ReturnVisitor { + fn new() -> Self { + Self { + found_return: false, + } + } +} + +impl<'ast> Visitor<'ast> for ReturnVisitor { + fn visit_expr(&mut self, ex: &'ast Expr) { + if let ExprKind::Ret(_) = ex.node { + self.found_return = true; + } else if let ExprKind::Try(_) = ex.node { + self.found_return = true; + } + + walk_expr(self, ex) + } +} + impl EarlyLintPass for MiscEarly { fn check_generics(&mut self, cx: &EarlyContext<'_>, gen: &Generics) { for param in &gen.params { @@ -216,7 +241,7 @@ impl EarlyLintPass for MiscEarly { } } - fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) { + fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat, _: &mut bool) { if let PatKind::Struct(ref npat, ref pfields, _) = pat.node { let mut wilds = 0; let type_name = npat.segments @@ -311,21 +336,25 @@ impl EarlyLintPass for MiscEarly { match expr.node { ExprKind::Call(ref paren, _) => if let ExprKind::Paren(ref closure) = paren.node { if let ExprKind::Closure(_, _, _, ref decl, ref block, _) = closure.node { - span_lint_and_then( - cx, - REDUNDANT_CLOSURE_CALL, - expr.span, - "Try not to call a closure in the expression where it is declared.", - |db| if decl.inputs.is_empty() { - let hint = snippet(cx, block.span, "..").into_owned(); - db.span_suggestion_with_applicability( - expr.span, - "Try doing something like: ", - hint, - Applicability::MachineApplicable, // snippet - ); - }, - ); + let mut visitor = ReturnVisitor::new(); + visitor.visit_expr(block); + if !visitor.found_return { + span_lint_and_then( + cx, + REDUNDANT_CLOSURE_CALL, + expr.span, + "Try not to call a closure in the expression where it is declared.", + |db| if decl.inputs.is_empty() { + let hint = snippet(cx, block.span, "..").into_owned(); + db.span_suggestion_with_applicability( + expr.span, + "Try doing something like: ", + hint, + Applicability::MachineApplicable, // snippet + ); + }, + ); + } } }, ExprKind::Unary(UnOp::Neg, ref inner) => if let ExprKind::Unary(UnOp::Neg, _) = inner.node { diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index b997d76d0e77..ad4ced995ba8 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -16,6 +16,7 @@ use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use crate::rustc::ty; use crate::rustc::{declare_tool_lint, lint_array}; use crate::syntax_pos::symbol::keywords::SelfType; +use crate::syntax::ast::NodeId; /// **What it does:** Checks for unnecessary repetition of structure name when a /// replacement with `Self` is applicable. @@ -73,7 +74,7 @@ fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path) { } struct TraitImplTyVisitor<'a, 'tcx: 'a> { - item_path: &'a Path, + item_type: ty::Ty<'tcx>, cx: &'a LateContext<'a, 'tcx>, trait_type_walker: ty::walk::TypeWalker<'tcx>, impl_type_walker: ty::walk::TypeWalker<'tcx>, @@ -85,21 +86,28 @@ impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> { let impl_ty = self.impl_type_walker.next(); if let TyKind::Path(QPath::Resolved(_, path)) = &t.node { - if self.item_path.def == path.def { - let is_self_ty = if let def::Def::SelfTy(..) = path.def { - true - } else { - false - }; - if !is_self_ty && impl_ty != trait_ty { - // The implementation and trait types don't match which means that - // the concrete type was specified by the implementation but - // it didn't use `Self` - span_use_self_lint(self.cx, path); + // The implementation and trait types don't match which means that + // the concrete type was specified by the implementation + if impl_ty != trait_ty { + + if let Some(impl_ty) = impl_ty { + if self.item_type == impl_ty { + let is_self_ty = if let def::Def::SelfTy(..) = path.def { + true + } else { + false + }; + + if !is_self_ty { + span_use_self_lint(self.cx, path); + } + } } + } } + walk_ty(self, t) } @@ -110,7 +118,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> { fn check_trait_method_impl_decl<'a, 'tcx: 'a>( cx: &'a LateContext<'a, 'tcx>, - item_path: &'a Path, + item_type: ty::Ty<'tcx>, impl_item: &ImplItem, impl_decl: &'tcx FnDecl, impl_trait_ref: &ty::TraitRef<'_>, @@ -151,7 +159,7 @@ fn check_trait_method_impl_decl<'a, 'tcx: 'a>( ) { let mut visitor = TraitImplTyVisitor { cx, - item_path, + item_type, trait_type_walker: trait_ty.walk(), impl_type_walker: impl_ty.walk(), }; @@ -192,7 +200,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf { let impl_item = cx.tcx.hir.impl_item(impl_item_ref.id); if let ImplItemKind::Method(MethodSig{ decl: impl_decl, .. }, impl_body_id) = &impl_item.node { - check_trait_method_impl_decl(cx, item_path, impl_item, impl_decl, &impl_trait_ref); + let item_type = cx.tcx.type_of(impl_def_id); + check_trait_method_impl_decl(cx, item_type, impl_item, impl_decl, &impl_trait_ref); + let body = cx.tcx.hir.body(*impl_body_id); visitor.visit_body(body); } else { @@ -225,6 +235,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> { walk_path(self, path); } + fn visit_use(&mut self, _path: &'tcx Path, _id: NodeId, _hir_id: HirId) { + // Don't check use statements + } + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { NestedVisitorMap::All(&self.cx.tcx.hir) } diff --git a/tests/ui/formatting.rs b/tests/ui/formatting.rs index 0dca8c8585b3..88f6e497d12e 100644 --- a/tests/ui/formatting.rs +++ b/tests/ui/formatting.rs @@ -112,4 +112,16 @@ fn main() { 1 + 2, 3 + 4, 5 + 6, ]; + + // don't lint for bin op without unary equiv + // issue 3244 + vec![ + 1 + / 2, + ]; + // issue 3396 + vec![ + true + | false, + ]; } diff --git a/tests/ui/literals.rs b/tests/ui/literals.rs index 4db7ce957120..c08c4b693b80 100644 --- a/tests/ui/literals.rs +++ b/tests/ui/literals.rs @@ -7,9 +7,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. - - - #![warn(clippy::mixed_case_hex_literals)] #![warn(clippy::unseparated_literal_suffix)] #![warn(clippy::zero_prefixed_literal)] @@ -64,4 +61,11 @@ fn main() { let fail21 = 4___16; let fail22 = 3__4___23; let fail23 = 3__16___23; + + let fail24 = 12.34_64; + let fail25 = 1E2_32; + let fail26 = 43E7_64; + let fail27 = 243E17_32; + let fail28 = 241251235E723_64; + let fail29 = 42279.911_32; } diff --git a/tests/ui/literals.stderr b/tests/ui/literals.stderr index 4e26b9dd3216..d2a50e2ded52 100644 --- a/tests/ui/literals.stderr +++ b/tests/ui/literals.stderr @@ -1,182 +1,218 @@ error: inconsistent casing in hexadecimal literal - --> $DIR/literals.rs:24:17 + --> $DIR/literals.rs:21:17 | -24 | let fail1 = 0xabCD; +21 | let fail1 = 0xabCD; | ^^^^^^ | = note: `-D clippy::mixed-case-hex-literals` implied by `-D warnings` error: inconsistent casing in hexadecimal literal - --> $DIR/literals.rs:25:17 + --> $DIR/literals.rs:22:17 | -25 | let fail2 = 0xabCD_u32; +22 | let fail2 = 0xabCD_u32; | ^^^^^^^^^^ error: inconsistent casing in hexadecimal literal - --> $DIR/literals.rs:26:17 + --> $DIR/literals.rs:23:17 | -26 | let fail2 = 0xabCD_isize; +23 | let fail2 = 0xabCD_isize; | ^^^^^^^^^^^^ error: integer type suffix should be separated by an underscore - --> $DIR/literals.rs:27:27 + --> $DIR/literals.rs:24:27 | -27 | let fail_multi_zero = 000_123usize; +24 | let fail_multi_zero = 000_123usize; | ^^^^^^^^^^^^ | = note: `-D clippy::unseparated-literal-suffix` implied by `-D warnings` error: this is a decimal constant - --> $DIR/literals.rs:27:27 + --> $DIR/literals.rs:24:27 | -27 | let fail_multi_zero = 000_123usize; +24 | let fail_multi_zero = 000_123usize; | ^^^^^^^^^^^^ | = note: `-D clippy::zero-prefixed-literal` implied by `-D warnings` help: if you mean to use a decimal constant, remove the `0` to remove confusion | -27 | let fail_multi_zero = 123usize; +24 | let fail_multi_zero = 123usize; | ^^^^^^^^ help: if you mean to use an octal constant, use `0o` | -27 | let fail_multi_zero = 0o123usize; +24 | let fail_multi_zero = 0o123usize; | ^^^^^^^^^^ error: integer type suffix should be separated by an underscore - --> $DIR/literals.rs:32:17 + --> $DIR/literals.rs:29:17 | -32 | let fail3 = 1234i32; +29 | let fail3 = 1234i32; | ^^^^^^^ error: integer type suffix should be separated by an underscore - --> $DIR/literals.rs:33:17 + --> $DIR/literals.rs:30:17 | -33 | let fail4 = 1234u32; +30 | let fail4 = 1234u32; | ^^^^^^^ error: integer type suffix should be separated by an underscore - --> $DIR/literals.rs:34:17 + --> $DIR/literals.rs:31:17 | -34 | let fail5 = 1234isize; +31 | let fail5 = 1234isize; | ^^^^^^^^^ error: integer type suffix should be separated by an underscore - --> $DIR/literals.rs:35:17 + --> $DIR/literals.rs:32:17 | -35 | let fail6 = 1234usize; +32 | let fail6 = 1234usize; | ^^^^^^^^^ error: float type suffix should be separated by an underscore - --> $DIR/literals.rs:36:17 + --> $DIR/literals.rs:33:17 | -36 | let fail7 = 1.5f32; +33 | let fail7 = 1.5f32; | ^^^^^^ error: this is a decimal constant - --> $DIR/literals.rs:40:17 + --> $DIR/literals.rs:37:17 | -40 | let fail8 = 0123; +37 | let fail8 = 0123; | ^^^^ help: if you mean to use a decimal constant, remove the `0` to remove confusion | -40 | let fail8 = 123; +37 | let fail8 = 123; | ^^^ help: if you mean to use an octal constant, use `0o` | -40 | let fail8 = 0o123; +37 | let fail8 = 0o123; | ^^^^^ error: long literal lacking separators - --> $DIR/literals.rs:51:17 + --> $DIR/literals.rs:48:17 | -51 | let fail9 = 0xabcdef; +48 | let fail9 = 0xabcdef; | ^^^^^^^^ help: consider: `0x00ab_cdef` | = note: `-D clippy::unreadable-literal` implied by `-D warnings` error: long literal lacking separators - --> $DIR/literals.rs:52:18 + --> $DIR/literals.rs:49:18 | -52 | let fail10 = 0xBAFEBAFE; +49 | let fail10 = 0xBAFEBAFE; | ^^^^^^^^^^ help: consider: `0xBAFE_BAFE` error: long literal lacking separators - --> $DIR/literals.rs:53:18 + --> $DIR/literals.rs:50:18 | -53 | let fail11 = 0xabcdeff; +50 | let fail11 = 0xabcdeff; | ^^^^^^^^^ help: consider: `0x0abc_deff` error: long literal lacking separators - --> $DIR/literals.rs:54:18 + --> $DIR/literals.rs:51:18 | -54 | let fail12 = 0xabcabcabcabcabcabc; +51 | let fail12 = 0xabcabcabcabcabcabc; | ^^^^^^^^^^^^^^^^^^^^ help: consider: `0x00ab_cabc_abca_bcab_cabc` error: digit groups should be smaller - --> $DIR/literals.rs:55:18 + --> $DIR/literals.rs:52:18 | -55 | let fail13 = 0x1_23456_78901_usize; +52 | let fail13 = 0x1_23456_78901_usize; | ^^^^^^^^^^^^^^^^^^^^^ help: consider: `0x0123_4567_8901_usize` | = note: `-D clippy::large-digit-groups` implied by `-D warnings` error: mistyped literal suffix - --> $DIR/literals.rs:57:18 + --> $DIR/literals.rs:54:18 | -57 | let fail14 = 2_32; +54 | let fail14 = 2_32; | ^^^^ help: did you mean to write: `2_i32` | = note: #[deny(clippy::mistyped_literal_suffixes)] on by default error: mistyped literal suffix - --> $DIR/literals.rs:58:18 + --> $DIR/literals.rs:55:18 | -58 | let fail15 = 4_64; +55 | let fail15 = 4_64; | ^^^^ help: did you mean to write: `4_i64` error: mistyped literal suffix - --> $DIR/literals.rs:59:18 + --> $DIR/literals.rs:56:18 | -59 | let fail16 = 7_8; +56 | let fail16 = 7_8; | ^^^ help: did you mean to write: `7_i8` error: mistyped literal suffix - --> $DIR/literals.rs:60:18 + --> $DIR/literals.rs:57:18 | -60 | let fail17 = 23_16; +57 | let fail17 = 23_16; | ^^^^^ help: did you mean to write: `23_i16` error: digits grouped inconsistently by underscores - --> $DIR/literals.rs:62:18 + --> $DIR/literals.rs:59:18 | -62 | let fail19 = 12_3456_21; +59 | let fail19 = 12_3456_21; | ^^^^^^^^^^ help: consider: `12_345_621` | = note: `-D clippy::inconsistent-digit-grouping` implied by `-D warnings` error: mistyped literal suffix - --> $DIR/literals.rs:63:18 + --> $DIR/literals.rs:60:18 | -63 | let fail20 = 2__8; +60 | let fail20 = 2__8; | ^^^^ help: did you mean to write: `2_i8` error: mistyped literal suffix - --> $DIR/literals.rs:64:18 + --> $DIR/literals.rs:61:18 | -64 | let fail21 = 4___16; +61 | let fail21 = 4___16; | ^^^^^^ help: did you mean to write: `4_i16` error: digits grouped inconsistently by underscores - --> $DIR/literals.rs:65:18 + --> $DIR/literals.rs:62:18 | -65 | let fail22 = 3__4___23; +62 | let fail22 = 3__4___23; | ^^^^^^^^^ help: consider: `3_423` error: digits grouped inconsistently by underscores - --> $DIR/literals.rs:66:18 + --> $DIR/literals.rs:63:18 | -66 | let fail23 = 3__16___23; +63 | let fail23 = 3__16___23; | ^^^^^^^^^^ help: consider: `31_623` -error: aborting due to 25 previous errors +error: mistyped literal suffix + --> $DIR/literals.rs:65:18 + | +65 | let fail24 = 12.34_64; + | ^^^^^^^^ help: did you mean to write: `12.34_f64` + +error: mistyped literal suffix + --> $DIR/literals.rs:66:18 + | +66 | let fail25 = 1E2_32; + | ^^^^^^ help: did you mean to write: `1E2_f32` + +error: mistyped literal suffix + --> $DIR/literals.rs:67:18 + | +67 | let fail26 = 43E7_64; + | ^^^^^^^ help: did you mean to write: `43E7_f64` + +error: mistyped literal suffix + --> $DIR/literals.rs:68:18 + | +68 | let fail27 = 243E17_32; + | ^^^^^^^^^ help: did you mean to write: `243E17_f32` + +error: mistyped literal suffix + --> $DIR/literals.rs:69:18 + | +69 | let fail28 = 241251235E723_64; + | ^^^^^^^^^^^^^^^^ help: did you mean to write: `241_251_235E723_f64` + +error: mistyped literal suffix + --> $DIR/literals.rs:70:18 + | +70 | let fail29 = 42279.911_32; + | ^^^^^^^^^^^^ help: did you mean to write: `42_279.911_f32` + +error: aborting due to 31 previous errors diff --git a/tests/ui/redundant_closure_call.rs b/tests/ui/redundant_closure_call.rs index 4912e5fc1b49..e68cdc2c1d17 100644 --- a/tests/ui/redundant_closure_call.rs +++ b/tests/ui/redundant_closure_call.rs @@ -28,4 +28,9 @@ fn main() { i = closure(3); i = closure(4); + + #[allow(clippy::needless_return)] + (|| return 2)(); + (|| -> Option { None? })(); + (|| -> Result { r#try!(Err(2)) })(); } diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index 073d64d5a4bf..60dc2d54d05d 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -219,3 +219,28 @@ mod existential { } } } + +mod issue3410 { + + struct A; + struct B; + + trait Trait { + fn a(v: T); + } + + impl Trait> for Vec { + fn a(_: Vec) {} + } +} + +mod issue3425 { + enum Enum { + A, + } + impl Enum { + fn a () { + use self::Enum::*; + } + } +} diff --git a/util/update_lints.py b/util/update_lints.py index 221069d353cb..4467b5c0cf7d 100755 --- a/util/update_lints.py +++ b/util/update_lints.py @@ -10,245 +10,11 @@ # option. This file may not be copied, modified, or distributed # except according to those terms. - -# Generate a Markdown table of all lints, and put it in README.md. -# With -n option, only print the new table to stdout. -# With -c option, print a warning and set exit status to 1 if a file would be -# changed. - -import os -import re import sys -from subprocess import call - -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) - -declare_clippy_lint_re = re.compile(r''' - declare_clippy_lint! \s* [{(] \s* - pub \s+ (?P[A-Z_][A-Z_0-9]*) \s*,\s* - (?P[a-z_]+) \s*,\s* - " (?P(?:[^"\\]+|\\.)*) " \s* [})] -''', re.VERBOSE | re.DOTALL) - -nl_escape_re = re.compile(r'\\\n\s*') - -docs_link = 'https://rust-lang-nursery.github.io/rust-clippy/master/index.html' - - -def collect(deprecated_lints, clippy_lints, fn): - """Collect all lints from a file. - - Adds entries to the lints list as `(module, name, level, desc)`. - """ - with open(fn) as fp: - code = fp.read() - - 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('\\"', '"'))) - - for match in declare_clippy_lint_re.finditer(code): - # remove \-newline escapes from description string - desc = nl_escape_re.sub('', match.group('desc')) - cat = match.group('cat') - if cat in ('internal', 'internal_warn'): - continue - module_name = os.path.splitext(os.path.basename(fn))[0] - if module_name == 'mod': - module_name = os.path.basename(os.path.dirname(fn)) - clippy_lints[cat].append((module_name, - match.group('name').lower(), - "allow", - desc.replace('\\"', '"'))) - - -def gen_group(lints): - """Write lint group (list of all lints in the form module::NAME).""" - for (module, name, _, _) in sorted(lints): - yield ' %s::%s,\n' % (module, name.upper()) - - -def gen_mods(lints): - """Declare modules""" - - for module in sorted(set(lint[0] for lint in lints)): - yield 'pub mod %s;\n' % module - - -def gen_deprecated(lints): - """Declare deprecated lints""" - - for lint in lints: - yield ' store.register_removed(\n' - yield ' "%s",\n' % lint[1] - yield ' "%s",\n' % lint[2] - yield ' );\n' - - -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. - - A callback is called to write the new region. If `replace_start` is true, - the start delimiter line is replaced as well. The end delimiter line is - never replaced. - """ - # read current content - with open(fn) as fp: - lines = list(fp) - - found = False - - # replace old region with new region - new_lines = [] - in_old_region = False - for line in lines: - if in_old_region: - if re.search(region_end, line): - in_old_region = False - new_lines.extend(callback()) - new_lines.append(line) - elif re.search(region_start, line): - if not replace_start: - new_lines.append(line) - # old region starts here - in_old_region = True - found = True - else: - new_lines.append(line) - - if not found: - print("regex " + region_start + " not found") - - # write back to file - if write_back: - with open(fn, 'w') as fp: - fp.writelines(new_lines) - - # if something changed, return true - return lines != new_lines - - -def main(print_only=False, check=False): - deprecated_lints = [] - clippy_lints = { - "correctness": [], - "style": [], - "complexity": [], - "perf": [], - "restriction": [], - "pedantic": [], - "cargo": [], - "nursery": [], - } - - # check directory - if not os.path.isfile('clippy_lints/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('clippy_lints/src'): - for fn in files: - if fn.endswith('.rs'): - collect(deprecated_lints, clippy_lints, - os.path.join(root, fn)) - - # determine version - with open('Cargo.toml') as fp: - for line in fp: - if line.startswith('version ='): - clippy_version = line.split()[2].strip('"') - break - else: - print('Error: version not found in Cargo.toml!') - return - - all_lints = [] - clippy_lint_groups = [ - "correctness", - "style", - "complexity", - "perf", - ] - clippy_lint_list = [] - for x in clippy_lint_groups: - clippy_lint_list += clippy_lints[x] - for _, value in clippy_lints.iteritems(): - all_lints += value - - if print_only: - call(["./util/dev", "update_lints", "--print-only"]) - return - - # update the lint counter in README.md - changed = replace_region( - 'README.md', - r'^\[There are \d+ lints included in this crate!\]\(https://rust-lang-nursery.github.io/rust-clippy/master/index.html\)$', "", - lambda: ['[There are %d lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)\n' % - (len(all_lints))], - write_back=not check) - - # update the links in the CHANGELOG - changed |= replace_region( - 'CHANGELOG.md', - "", - "", - lambda: ["[`{0}`]: {1}#{0}\n".format(l[1], docs_link) for l in - sorted(all_lints + deprecated_lints, - key=lambda l: l[1])], - replace_start=False, write_back=not check) - - # update version of clippy_lints in Cargo.toml - changed |= replace_region( - 'Cargo.toml', r'# begin automatic update', '# end automatic update', - lambda: ['clippy_lints = { version = "%s", path = "clippy_lints" }\n' % - clippy_version], - replace_start=False, write_back=not check) - - # update version of clippy_lints in Cargo.toml - changed |= replace_region( - 'clippy_lints/Cargo.toml', r'# begin automatic update', '# end automatic update', - lambda: ['version = "%s"\n' % clippy_version], - replace_start=False, write_back=not check) - - # update the `pub mod` list - changed |= replace_region( - 'clippy_lints/src/lib.rs', r'begin lints modules', r'end lints modules', - lambda: gen_mods(all_lints), - replace_start=False, write_back=not check) - - # same for "clippy::*" lint collections - changed |= replace_region( - 'clippy_lints/src/lib.rs', r'reg.register_lint_group\("clippy::all"', r'\]\);', - lambda: gen_group(clippy_lint_list), - replace_start=False, write_back=not check) - - for key, value in clippy_lints.iteritems(): - # same for "clippy::*" lint collections - changed |= replace_region( - 'clippy_lints/src/lib.rs', r'reg.register_lint_group\("clippy::' + key + r'"', r'\]\);', - lambda: gen_group(value), - replace_start=False, write_back=not check) - - # same for "deprecated" lint collection - changed |= replace_region( - 'clippy_lints/src/lib.rs', r'begin deprecated lints', r'end deprecated lints', - lambda: gen_deprecated(deprecated_lints), - replace_start=False, - write_back=not check) - - if check and changed: - print('Please run util/update_lints.py to regenerate lints lists.') - return 1 +def main(): + print('Error: Please use `util/dev` to update lints') + return 1 if __name__ == '__main__': - sys.exit(main(print_only='-n' in sys.argv, check='-c' in sys.argv)) + sys.exit(main())