Merge branch 'master' into fix-3514

Author: Michael Wright

.travis.yml

@@ -18,15 +18,6 @@ env:
  global:
    - RUST_BACKTRACE=1
 
-before_install:
- - |
-    # work-around for issue https://github.com/travis-ci/travis-ci/issues/6307
-    # might not be necessary in the future
-    if [ "$TRAVIS_OS_NAME" == "osx" ]; then
-     command curl -sSL https://rvm.io/mpapis.asc | gpg --import -
-     rvm get stable
-    fi
-
 install:
   - |
     if [ -z ${INTEGRATION} ]; then

CONTRIBUTING.md

@@ -19,7 +19,6 @@ 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)
-  * [Linting Clippy with your local changes](#linting-clippy-with-your-local-changes)
   * [How Clippy works](#how-clippy-works)
   * [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
 * [Issue and PR Triage](#issue-and-pr-triage)

README.md

@@ -106,8 +106,8 @@ script:
   # etc.
 ```
 
-It might happen that clippy is not available for a certain nightly release.
-In this case you can try to conditionally install clippy from the git repo.
+It might happen that Clippy is not available for a certain nightly release.
+In this case you can try to conditionally install Clippy from the git repo.
 
 ```yaml
 language: rust
@@ -149,21 +149,7 @@ You can add options to your code to `allow`/`warn`/`deny` Clippy lints:
 
 Note: `deny` produces errors instead of warnings.
 
-Note: To use the new `clippy::lint_name` syntax, a recent compiler has to be used
-currently. If you want to compile your code with the stable toolchain you can use a `cfg_attr` to
-activate the `tool_lints` feature:
-```rust
-#![cfg_attr(feature = "cargo-clippy", allow(clippy::lint_name))]
-```
-
-For this to work you have to use Clippy on the nightly toolchain: `cargo +nightly clippy`. If you
-want to use Clippy with the stable toolchain, you can stick to the old unscoped method to
-enable/disable Clippy lints until `tool_lints` are stable:
-```rust
-#![cfg_attr(feature = "cargo-clippy", allow(clippy_lint))]
-```
-
-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 lint_name` will run clippy with `lint_name` disabled and `cargo clippy -- -W lint_name` will run it with that enabled. On newer compilers you may need to use `clippy::lint_name` instead.
+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`
 
 ## License
 

ci/base-tests.sh

@@ -29,3 +29,20 @@ cd clippy_dev && cargo test && 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
+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
+done
+
+if [ "${needs_reformatting}" = true ] ; then
+    echo "Tests need reformatting!"
+    exit 2
+fi
+set -x

clippy_lints/src/doc.rs

@@ -238,7 +238,7 @@ fn check_doc<'a, Events: Iterator<Item = (usize, pulldown_cmark::Event<'a>)>>(
 }
 
 fn check_text(cx: &EarlyContext<'_>, valid_idents: &[String], text: &str, span: Span) {
-    for word in text.split_whitespace() {
+    for word in text.split(|c: char| c.is_whitespace() || c == '\'') {
         // Trim punctuation as in `some comment (see foo::bar).`
         //                                                   ^^
         // Or even as in `_foo bar_` which is emphasized.
@@ -281,6 +281,10 @@ fn check_word(cx: &EarlyContext<'_>, word: &str, span: Span) {
         s != "_" && !s.contains("\\_") && s.contains('_')
     }
 
+    fn has_hyphen(s: &str) -> bool {
+        s != "-" && s.contains('-')
+    }
+
     if let Ok(url) = Url::parse(word) {
         // try to get around the fact that `foo::bar` parses as a valid URL
         if !url.cannot_be_a_base() {
@@ -295,6 +299,11 @@ fn check_word(cx: &EarlyContext<'_>, word: &str, span: Span) {
         }
     }
 
+    // We assume that mixed-case words are not meant to be put inside bacticks. (Issue #2343)
+    if has_underscore(word) && has_hyphen(word) {
+        return;
+    }
+
     if has_underscore(word) || word.contains("::") || is_camel_case(word) {
         span_lint(
             cx,

clippy_lints/src/explicit_write.rs

@@ -10,8 +10,9 @@
 use crate::rustc::hir::*;
 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use crate::rustc::{declare_tool_lint, lint_array};
-use crate::utils::opt_def_id;
-use crate::utils::{is_expn_of, match_def_path, resolve_node, span_lint};
+use crate::rustc_errors::Applicability;
+use crate::syntax::ast::LitKind;
+use crate::utils::{is_expn_of, match_def_path, opt_def_id, resolve_node, span_lint, span_lint_and_sugg};
 use if_chain::if_chain;
 
 /// **What it does:** Checks for usage of `write!()` / `writeln()!` which can be
@@ -81,32 +82,85 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                 } else {
                     ""
                 };
-                if let Some(macro_name) = calling_macro {
-                    span_lint(
-                        cx,
-                        EXPLICIT_WRITE,
-                        expr.span,
-                        &format!(
-                            "use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead",
-                            macro_name,
-                            dest_name,
-                            prefix,
-                            macro_name.replace("write", "print")
-                        )
-                    );
+
+                // We need to remove the last trailing newline from the string because the
+                // underlying `fmt::write` function doesn't know whether `println!` or `print!` was
+                // used.
+                if let Some(mut write_output) = write_output_string(write_args) {
+                    if write_output.ends_with('\n') {
+                        write_output.pop();
+                    }
+
+                    if let Some(macro_name) = calling_macro {
+                        span_lint_and_sugg(
+                            cx,
+                            EXPLICIT_WRITE,
+                            expr.span,
+                            &format!(
+                                "use of `{}!({}(), ...).unwrap()`",
+                                macro_name,
+                                dest_name
+                            ),
+                            "try this",
+                            format!("{}{}!(\"{}\")", prefix, macro_name.replace("write", "print"), write_output.escape_default()),
+                            Applicability::MachineApplicable
+                        );
+                    } else {
+                        span_lint_and_sugg(
+                            cx,
+                            EXPLICIT_WRITE,
+                            expr.span,
+                            &format!("use of `{}().write_fmt(...).unwrap()`", dest_name),
+                            "try this",
+                            format!("{}print!(\"{}\")", prefix, write_output.escape_default()),
+                            Applicability::MachineApplicable
+                        );
+                    }
                 } else {
-                    span_lint(
-                        cx,
-                        EXPLICIT_WRITE,
-                        expr.span,
-                        &format!(
-                            "use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead",
-                            dest_name,
-                            prefix,
-                        )
-                    );
+                    // We don't have a proper suggestion
+                    if let Some(macro_name) = calling_macro {
+                        span_lint(
+                            cx,
+                            EXPLICIT_WRITE,
+                            expr.span,
+                            &format!(
+                                "use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead",
+                                macro_name,
+                                dest_name,
+                                prefix,
+                                macro_name.replace("write", "print")
+                            )
+                        );
+                    } else {
+                        span_lint(
+                            cx,
+                            EXPLICIT_WRITE,
+                            expr.span,
+                            &format!("use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead", dest_name, prefix),
+                        );
+                    }
                 }
+
             }
         }
     }
 }
+
+// Extract the output string from the given `write_args`.
+fn write_output_string(write_args: &HirVec<Expr>) -> Option<String> {
+    if_chain! {
+        // Obtain the string that should be printed
+        if write_args.len() > 1;
+        if let ExprKind::Call(_, ref output_args) = write_args[1].node;
+        if output_args.len() > 0;
+        if let ExprKind::AddrOf(_, ref output_string_expr) = output_args[0].node;
+        if let ExprKind::Array(ref string_exprs) = output_string_expr.node;
+        if string_exprs.len() > 0;
+        if let ExprKind::Lit(ref lit) = string_exprs[0].node;
+        if let LitKind::Str(ref write_output, _) = lit.node;
+        then {
+            return Some(write_output.to_string())
+        }
+    }
+    None
+}

clippy_lints/src/lib.rs

@@ -14,6 +14,7 @@
 #![feature(slice_patterns)]
 #![feature(stmt_expr_attributes)]
 #![feature(range_contains)]
+#![feature(str_escape)]
 #![allow(clippy::missing_docs_in_private_items)]
 #![recursion_limit = "256"]
 #![warn(rust_2018_idioms, trivial_casts, trivial_numeric_casts)]

clippy_lints/src/new_without_default.rs

@@ -17,7 +17,7 @@ use crate::rustc_errors::Applicability;
 use crate::syntax::source_map::Span;
 use crate::utils::paths;
 use crate::utils::sugg::DiagnosticBuilderExt;
-use crate::utils::{get_trait_def_id, implements_trait, return_ty, same_tys, span_lint_and_then};
+use crate::utils::{get_trait_def_id, implements_trait, return_ty, same_tys, span_lint_node_and_then};
 use if_chain::if_chain;
 
 /// **What it does:** Checks for types with a `fn new() -> Self` method and no
@@ -165,9 +165,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
                                     }
 
                                     if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) {
-                                        span_lint_and_then(
+                                        span_lint_node_and_then(
                                             cx,
                                             NEW_WITHOUT_DEFAULT_DERIVE,
+                                            id,
                                             impl_item.span,
                                             &format!(
                                                 "you should consider deriving a `Default` implementation for `{}`",
@@ -183,9 +184,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
                                                 );
                                             });
                                     } else {
-                                        span_lint_and_then(
+                                        span_lint_node_and_then(
                                             cx,
                                             NEW_WITHOUT_DEFAULT,
+                                            id,
                                             impl_item.span,
                                             &format!(
                                                 "you should consider adding a `Default` implementation for `{}`",

clippy_lints/src/partialeq_ne_impl.rs

@@ -10,7 +10,7 @@
 use crate::rustc::hir::*;
 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use crate::rustc::{declare_tool_lint, lint_array};
-use crate::utils::{is_automatically_derived, span_lint};
+use crate::utils::{is_automatically_derived, span_lint_node};
 use if_chain::if_chain;
 
 /// **What it does:** Checks for manual re-implementations of `PartialEq::ne`.
@@ -56,10 +56,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
             then {
                 for impl_item in impl_items {
                     if impl_item.ident.name == "ne" {
-                        span_lint(cx,
-                                  PARTIALEQ_NE_IMPL,
-                                  impl_item.span,
-                                  "re-implementing `PartialEq::ne` is unnecessary")
+                        span_lint_node(
+                            cx,
+                            PARTIALEQ_NE_IMPL,
+                            impl_item.id.node_id,
+                            impl_item.span,
+                            "re-implementing `PartialEq::ne` is unnecessary",
+                        );
                     }
                 }
             }

clippy_lints/src/question_mark.rs

@@ -17,7 +17,7 @@ use if_chain::if_chain;
 
 use crate::rustc_errors::Applicability;
 use crate::utils::paths::*;
-use crate::utils::{match_def_path, match_type, span_lint_and_then};
+use crate::utils::{match_def_path, match_type, span_lint_and_then, SpanlessEq};
 
 /// **What it does:** Checks for expressions that could be replaced by the question mark operator
 ///
@@ -64,14 +64,40 @@ impl Pass {
     /// If it matches, it will suggest to use the question mark operator instead
     fn check_is_none_and_early_return_none(cx: &LateContext<'_, '_>, expr: &Expr) {
         if_chain! {
-            if let ExprKind::If(ref if_expr, ref body, _) = expr.node;
-            if let ExprKind::MethodCall(ref segment, _, ref args) = if_expr.node;
+            if let ExprKind::If(if_expr, body, else_) = &expr.node;
+            if let ExprKind::MethodCall(segment, _, args) = &if_expr.node;
             if segment.ident.name == "is_none";
             if Self::expression_returns_none(cx, body);
             if let Some(subject) = args.get(0);
             if Self::is_option(cx, subject);
 
             then {
+                if let Some(else_) = else_ {
+                    if_chain! {
+                        if let ExprKind::Block(block, None) = &else_.node;
+                        if block.stmts.len() == 0;
+                        if let Some(block_expr) = &block.expr;
+                        if SpanlessEq::new(cx).ignore_fn().eq_expr(subject, block_expr);
+                        then {
+                            span_lint_and_then(
+                                cx,
+                                QUESTION_MARK,
+                                expr.span,
+                                "this block may be rewritten with the `?` operator",
+                                |db| {
+                                    db.span_suggestion_with_applicability(
+                                        expr.span,
+                                        "replace_it_with",
+                                        format!("Some({}?)", Sugg::hir(cx, subject, "..")),
+                                        Applicability::MaybeIncorrect, // snippet
+                                    );
+                                }
+                            )
+                        }
+                    }
+                    return;
+                }
+
                 span_lint_and_then(
                     cx,
                     QUESTION_MARK,
@@ -84,7 +110,7 @@ impl Pass {
                             expr.span,
                             "replace_it_with",
                             format!("{}?;", receiver_str),
-                            Applicability::MachineApplicable, // snippet
+                            Applicability::MaybeIncorrect, // snippet
                         );
                     }
                 )
@@ -133,9 +159,13 @@ impl Pass {
             }
         }
 
-        // Check if the block has an implicit return expression
-        if let Some(ref ret_expr) = block.expr {
-            return Some(ret_expr.clone());
+        // Check for `return` without a semicolon.
+        if_chain! {
+            if block.stmts.len() == 0;
+            if let Some(ExprKind::Ret(Some(ret_expr))) = block.expr.as_ref().map(|e| &e.node);
+            then {
+                return Some(ret_expr.clone());
+            }
         }
 
         None

clippy_lints/src/redundant_field_names.rs

@@ -57,7 +57,10 @@ impl EarlyLintPass for RedundantFieldNames {
                     continue;
                 }
                 if let ExprKind::Path(None, path) = &field.expr.node {
-                    if path.segments.len() == 1 && path.segments[0].ident == field.ident {
+                    if path.segments.len() == 1
+                        && path.segments[0].ident == field.ident
+                        && path.segments[0].args.is_none()
+                    {
                         span_lint_and_sugg(
                             cx,
                             REDUNDANT_FIELD_NAMES,

clippy_lints/src/utils/conf.rs

@@ -137,6 +137,7 @@ define_Conf! {
         "iOS", "macOS",
         "TeX", "LaTeX", "BibTeX", "BibLaTeX",
         "MinGW",
+        "CamelCase",
     ] => Vec<String>),
     /// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have
     (too_many_arguments_threshold, "too_many_arguments_threshold", 7 => u64),