Merge remote-tracking branch 'upstream/master'

Author: fuerstenau

CHANGELOG.md

@@ -760,6 +760,7 @@ All notable changes to this project will be documented in this file.
 [`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
 [`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
 [`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception
+[`module_name_repetitions`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions
 [`modulo_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_one
 [`multiple_crate_versions`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions
 [`multiple_inherent_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl
@@ -850,7 +851,6 @@ All notable changes to this project will be documented in this file.
 [`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
 [`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
 [`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
-[`stutter`]: https://rust-lang.github.io/rust-clippy/master/index.html#stutter
 [`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl
 [`suspicious_assignment_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_assignment_formatting
 [`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting

CONTRIBUTING.md

@@ -19,8 +19,8 @@ All contributors are expected to follow the [Rust Code of Conduct](http://www.ru
   * [Running test suite](#running-test-suite)
   * [Running rustfmt](#running-rustfmt)
   * [Testing manually](#testing-manually)
-  * [How Clippy works](#how-clippy-works)
-  * [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
+* [How Clippy works](#how-clippy-works)
+* [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
 * [Issue and PR Triage](#issue-and-pr-triage)
 * [Bors and Homu](#bors-and-homu)
 * [Contributions](#contributions)
@@ -42,6 +42,13 @@ Some issues are easier than others. The [`good first issue`](https://github.com/
 label can be used to find the easy issues. If you want to work on an issue, please leave a comment
 so that we can assign it to you!
 
+There are also some abandoned PRs, marked with
+[`S-inactive-closed`](https://github.com/rust-lang/rust-clippy/pulls?q=is%3Aclosed+label%3AS-inactive-closed).
+Pretty often these PRs are nearly completed and just need some extra steps
+(formatting, addressing review comments, ...) to be merged. If you want to
+complete such a PR, please leave a comment in the PR and open a new one based
+on it.
+
 Issues marked [`T-AST`](https://github.com/rust-lang/rust-clippy/labels/T-AST) involve simple
 matching of the syntax tree structure, and are generally easier than
 [`T-middle`](https://github.com/rust-lang/rust-clippy/labels/T-middle) issues, which involve types
@@ -168,7 +175,7 @@ Manually testing against an example file is useful if you have added some
 local modifications, run `env CLIPPY_TESTS=true cargo run --bin clippy-driver -- -L ./target/debug input.rs`
 from the working copy root.
 
-### How Clippy works
+## How Clippy works
 
 Clippy is a [rustc compiler plugin][compiler_plugin]. The main entry point is at [`src/lib.rs`][main_entry]. In there, the lint registration is delegated to the [`clippy_lints`][lint_crate] crate.
 
@@ -218,7 +225,7 @@ The difference between `EarlyLintPass` and `LateLintPass` is that the methods of
 
 That's why the `else_if_without_else` example uses the `register_early_lint_pass` function. Because the [actual lint logic][else_if_without_else] does not depend on any type information.
 
-### Fixing build failures caused by Rust
+## Fixing build failures caused by Rust
 
 Clippy will sometimes fail to build from source because building it depends on unstable internal Rust features. Most of the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures caused by Rust updates, can be a good way to learn about Rust internals.
 

README.md

@@ -79,7 +79,7 @@ To have cargo compile your crate with Clippy without Clippy installation
 in your code, you can use:
 
 ```terminal
-cargo run --bin cargo-clippy --manifest-path=path_to_clippys_Cargo.toml
+RUSTFLAGS=--sysroot=`rustc --print sysroot` cargo run --bin cargo-clippy --manifest-path=path_to_clippys_Cargo.toml
 ```
 
 *[Note](https://github.com/rust-lang/rust-clippy/wiki#a-word-of-warning):*
@@ -151,6 +151,10 @@ Note: `deny` produces errors instead of warnings.
 
 If you do not want to include your lint levels in your code, you can globally enable/disable lints by passing extra flags to Clippy during the run: `cargo clippy -- -A clippy::lint_name` will run Clippy with `lint_name` disabled and `cargo clippy -- -W clippy::lint_name` will run it with that enabled. This also works with lint groups. For example you can run Clippy with warnings for all lints enabled: `cargo clippy -- -W clippy::pedantic`
 
+## Contributing
+
+If you want to contribute to Clippy, you can find more information in [CONTRIBUTING.md](https://github.com/rust-lang/rust-clippy/blob/master/CONTRIBUTING.md).
+
 ## License
 
 Copyright 2014-2018 The Rust Project Developers

ci/base-tests.sh

@@ -26,23 +26,38 @@ cd clippy_lints && cargo test && cd ..
 cd rustc_tools_util && cargo test && cd ..
 cd clippy_dev && cargo test && cd ..
 
+# make sure clippy can be called via ./path/to/cargo-clippy
+cd clippy_workspace_tests
+../target/debug/cargo-clippy
+cd ..
+
 # Perform various checks for lint registration
 ./util/dev update_lints --check
 cargo +nightly fmt --all -- --check
 
-
-#avoid loop spam
-set +x
 # make sure tests are formatted
 
 # some lints are sensitive to formatting, exclude some files
-needs_formatting=false
+tests_need_reformatting="false"
+# switch to nightly
+rustup default nightly
+# avoid loop spam and allow cmds with exit status != 0
+set +ex
+
 for file in `find tests -not -path "tests/ui/methods.rs" -not -path "tests/ui/format.rs" -not -path "tests/ui/formatting.rs" -not -path "tests/ui/empty_line_after_outer_attribute.rs" -not -path "tests/ui/double_parens.rs" -not -path "tests/ui/doc.rs" -not -path "tests/ui/unused_unit.rs" | grep "\.rs$"` ; do
-rustfmt ${file} --check  || echo "${file} needs reformatting!" ; needs_formatting=true
+  rustfmt ${file} --check
+  if [ $? -ne 0 ]; then
+    echo "${file} needs reformatting!"
+    tests_need_reformatting="true"
+  fi
 done
 
-if [ "${needs_reformatting}" = true ] ; then
+set -ex # reset
+
+if [ "${tests_need_reformatting}" == "true" ] ; then
     echo "Tests need reformatting!"
     exit 2
 fi
-set -x
+
+# switch back to master
+rustup default master

ci/integration-tests.sh

@@ -21,7 +21,7 @@ function check() {
 # run clippy on a project, try to be verbose and trigger as many warnings as possible for greater coverage
   RUST_BACKTRACE=full cargo clippy --all-targets --all-features -- --cap-lints warn -W clippy::pedantic -W clippy::nursery  &> clippy_output
   cat clippy_output
-  ! cat clippy_output | grep -q "internal compiler error\|query stack during panic"
+  ! cat clippy_output | grep -q "internal compiler error\|query stack during panic\|E0463"
   if [[ $? != 0 ]]; then
     return 1
   fi

clippy_dev/src/lib.rs

@@ -432,7 +432,7 @@ fn test_gen_deprecated() {
             "should_assert_eq",
             "group1",
             "abc",
-            Some("has been superseeded by should_assert_eq2"),
+            Some("has been superseded by should_assert_eq2"),
             "module_name",
         ),
         Lint::new(
@@ -447,7 +447,7 @@ fn test_gen_deprecated() {
     let expected: Vec<String> = vec![
         "    store.register_removed(",
         "        \"should_assert_eq\",",
-        "        \"has been superseeded by should_assert_eq2\",",
+        "        \"has been superseded by should_assert_eq2\",",
         "    );",
         "    store.register_removed(",
         "        \"another_deprecated\",",

clippy_lints/src/attrs.rs

@@ -337,7 +337,9 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
                                 &name_lower,
                                 Some(tool_name.as_str())
                             ) {
-                                CheckLintNameResult::NoLint => (),
+                                // FIXME: can we suggest similar lint names here?
+                                // https://github.com/rust-lang/rust/pull/56992
+                                CheckLintNameResult::NoLint(None) => (),
                                 _ => {
                                     db.span_suggestion(lint.span,
                                                        "lowercase the lint name",

clippy_lints/src/double_comparison.rs

@@ -17,7 +17,7 @@ use crate::syntax::source_map::Span;
 
 use crate::utils::{snippet_with_applicability, span_lint_and_sugg, SpanlessEq};
 
-/// **What it does:** Checks for double comparions that could be simpified to a single expression.
+/// **What it does:** Checks for double comparions that could be simplified to a single expression.
 ///
 ///
 /// **Why is this bad?** Readability.

clippy_lints/src/enum_variants.rs

@@ -75,7 +75,7 @@ declare_clippy_lint! {
 /// }
 /// ```
 declare_clippy_lint! {
-    pub STUTTER,
+    pub MODULE_NAME_REPETITIONS,
     pedantic,
     "type names prefixed/postfixed with their containing module's name"
 }
@@ -126,7 +126,12 @@ impl EnumVariantNames {
 
 impl LintPass for EnumVariantNames {
     fn get_lints(&self) -> LintArray {
-        lint_array!(ENUM_VARIANT_NAMES, PUB_ENUM_VARIANT_NAMES, STUTTER, MODULE_INCEPTION)
+        lint_array!(
+            ENUM_VARIANT_NAMES,
+            PUB_ENUM_VARIANT_NAMES,
+            MODULE_NAME_REPETITIONS,
+            MODULE_INCEPTION
+        )
     }
 }
 
@@ -277,7 +282,7 @@ impl EarlyLintPass for EnumVariantNames {
                             match item_camel.chars().nth(nchars) {
                                 Some(c) if is_word_beginning(c) => span_lint(
                                     cx,
-                                    STUTTER,
+                                    MODULE_NAME_REPETITIONS,
                                     item.span,
                                     "item name starts with its containing module's name",
                                 ),
@@ -287,7 +292,7 @@ impl EarlyLintPass for EnumVariantNames {
                         if rmatching == nchars {
                             span_lint(
                                 cx,
-                                STUTTER,
+                                MODULE_NAME_REPETITIONS,
                                 item.span,
                                 "item name ends with its containing module's name",
                             );

clippy_lints/src/formatting.rs

@@ -31,8 +31,8 @@ declare_clippy_lint! {
     "suspicious formatting of `*=`, `-=` or `!=`"
 }
 
-/// **What it does:** Checks for formatting of `else if`. It lints if the `else`
-/// and `if` are not on the same line or the `else` seems to be missing.
+/// **What it does:** Checks for formatting of `else`. It lints if the `else`
+/// is followed immediately by a newline or the `else` seems to be missing.
 ///
 /// **Why is this bad?** This is probably some refactoring remnant, even if the
 /// code is correct, it might look confusing.
@@ -42,19 +42,29 @@ declare_clippy_lint! {
 /// **Example:**
 /// ```rust,ignore
 /// if foo {
+/// } { // looks like an `else` is missing here
+/// }
+///
+/// if foo {
 /// } if bar { // looks like an `else` is missing here
 /// }
 ///
 /// if foo {
 /// } else
 ///
+/// { // this is the `else` block of the previous `if`, but should it be?
+/// }
+///
+/// if foo {
+/// } else
+///
 /// if bar { // this is the `else` block of the previous `if`, but should it be?
 /// }
 /// ```
 declare_clippy_lint! {
     pub SUSPICIOUS_ELSE_FORMATTING,
     style,
-    "suspicious formatting of `else if`"
+    "suspicious formatting of `else`"
 }
 
 /// **What it does:** Checks for possible missing comma in an array. It lints if
@@ -96,7 +106,7 @@ impl EarlyLintPass for Formatting {
             match (&w[0].node, &w[1].node) {
                 (&ast::StmtKind::Expr(ref first), &ast::StmtKind::Expr(ref second))
                 | (&ast::StmtKind::Expr(ref first), &ast::StmtKind::Semi(ref second)) => {
-                    check_consecutive_ifs(cx, first, second);
+                    check_missing_else(cx, first, second);
                 },
                 _ => (),
             }
@@ -105,7 +115,7 @@ impl EarlyLintPass for Formatting {
 
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
         check_assign(cx, expr);
-        check_else_if(cx, expr);
+        check_else(cx, expr);
         check_array(cx, expr);
     }
 }
@@ -139,10 +149,18 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &ast::Expr) {
     }
 }
 
-/// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else if`.
-fn check_else_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
+/// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else`.
+fn check_else(cx: &EarlyContext<'_>, expr: &ast::Expr) {
     if let Some((then, &Some(ref else_))) = unsugar_if(expr) {
-        if unsugar_if(else_).is_some() && !differing_macro_contexts(then.span, else_.span) && !in_macro(then.span) {
+        if (is_block(else_) || unsugar_if(else_).is_some())
+            && !differing_macro_contexts(then.span, else_.span)
+            && !in_macro(then.span)
+        {
+            // workaround for rust-lang/rust#43081
+            if expr.span.lo().0 == 0 && expr.span.hi().0 == 0 {
+                return;
+            }
+
             // this will be a span from the closing ‘}’ of the “then” block (excluding) to
             // the
             // “if” of the “else if” block (excluding)
@@ -154,14 +172,19 @@ fn check_else_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
                 let else_pos = else_snippet.find("else").expect("there must be a `else` here");
 
                 if else_snippet[else_pos..].contains('\n') {
+                    let else_desc = if unsugar_if(else_).is_some() { "if" } else { "{..}" };
+
                     span_note_and_lint(
                         cx,
                         SUSPICIOUS_ELSE_FORMATTING,
                         else_span,
-                        "this is an `else if` but the formatting might hide it",
+                        &format!("this is an `else {}` but the formatting might hide it", else_desc),
                         else_span,
-                        "to remove this lint, remove the `else` or remove the new line between `else` \
-                         and `if`",
+                        &format!(
+                            "to remove this lint, remove the `else` or remove the new line between \
+                             `else` and `{}`",
+                            else_desc,
+                        ),
                     );
                 }
             }
@@ -200,32 +223,47 @@ fn check_array(cx: &EarlyContext<'_>, expr: &ast::Expr) {
     }
 }
 
-/// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for consecutive ifs.
-fn check_consecutive_ifs(cx: &EarlyContext<'_>, first: &ast::Expr, second: &ast::Expr) {
+fn check_missing_else(cx: &EarlyContext<'_>, first: &ast::Expr, second: &ast::Expr) {
     if !differing_macro_contexts(first.span, second.span)
         && !in_macro(first.span)
         && unsugar_if(first).is_some()
-        && unsugar_if(second).is_some()
+        && (is_block(second) || unsugar_if(second).is_some())
     {
         // where the else would be
         let else_span = first.span.between(second.span);
 
         if let Some(else_snippet) = snippet_opt(cx, else_span) {
             if !else_snippet.contains('\n') {
+                let (looks_like, next_thing) = if unsugar_if(second).is_some() {
+                    ("an `else if`", "the second `if`")
+                } else {
+                    ("an `else {..}`", "the next block")
+                };
+
                 span_note_and_lint(
                     cx,
                     SUSPICIOUS_ELSE_FORMATTING,
                     else_span,
-                    "this looks like an `else if` but the `else` is missing",
+                    &format!("this looks like {} but the `else` is missing", looks_like),
                     else_span,
-                    "to remove this lint, add the missing `else` or add a new line before the second \
-                     `if`",
+                    &format!(
+                        "to remove this lint, add the missing `else` or add a new line before {}",
+                        next_thing,
+                    ),
                 );
             }
         }
     }
 }
 
+fn is_block(expr: &ast::Expr) -> bool {
+    if let ast::ExprKind::Block(..) = expr.node {
+        true
+    } else {
+        false
+    }
+}
+
 /// Match `if` or `if let` expressions and return the `then` and `else` block.
 fn unsugar_if(expr: &ast::Expr) -> Option<(&P<ast::Block>, &Option<P<ast::Expr>>)> {
     match expr.node {

clippy_lints/src/implicit_return.rs

@@ -12,7 +12,7 @@ use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use crate::rustc::{declare_tool_lint, lint_array};
 use crate::rustc_errors::Applicability;
 use crate::syntax::{ast::NodeId, source_map::Span};
-use crate::utils::{snippet_opt, span_lint_and_then};
+use crate::utils::{in_macro, snippet_opt, span_lint_and_then};
 
 /// **What it does:** Checks for missing return statements at the end of a block.
 ///
@@ -116,14 +116,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         _: FnKind<'tcx>,
         _: &'tcx FnDecl,
         body: &'tcx Body,
-        _: Span,
+        span: Span,
         _: NodeId,
     ) {
         let def_id = cx.tcx.hir().body_owner_def_id(body.id());
         let mir = cx.tcx.optimized_mir(def_id);
 
         // checking return type through MIR, HIR is not able to determine inferred closure return types
-        if !mir.return_ty().is_unit() {
+        // make sure it's not a macro
+        if !mir.return_ty().is_unit() && !in_macro(span) {
             Self::expr_match(cx, &body.value);
         }
     }