Auto merge of #6787 - matthiaskrgr:lint_msgs, r=llogiq

tests: add test that roughly ensures that our lint messages conform with the diagnostics convention of the rustc dev guide

lint message should not start with uppercase letters
lint messages should not have punctuation at the end of the last line

https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-structure

The test reads through all the .stderr files in the testsuit and checks lint messages that start with "help: ", "error: " etc.
There is also an exception list for special messages that are deemed acceptable.

changelog: make sure lint messages conform with the rustc dev guide and add test

Author: bors

Cargo.toml

@@ -42,6 +42,7 @@ tester = "0.9"
 clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }
 serde = { version = "1.0", features = ["derive"] }
 derive-new = "0.5"
+regex = "1.4"
 
 # A noop dependency that changes in the Rust repository, it's a bit of a hack.
 # See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`

clippy_lints/src/assign_ops.rs

@@ -209,7 +209,7 @@ fn lint_misrefactored_assign_op(
                 diag.span_suggestion(
                     expr.span,
                     &format!(
-                        "Did you mean `{} = {} {} {}` or `{}`? Consider replacing it with",
+                        "did you mean `{} = {} {} {}` or `{}`? Consider replacing it with",
                         snip_a,
                         snip_a,
                         op.node.as_str(),

clippy_lints/src/attrs.rs

@@ -639,7 +639,7 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
                     diag.span_suggestion(span, "try", sugg, Applicability::MaybeIncorrect);
 
                     if !unix_suggested && is_unix(os) {
-                        diag.help("Did you mean `unix`?");
+                        diag.help("did you mean `unix`?");
                         unix_suggested = true;
                     }
                 }

clippy_lints/src/await_holding_invalid.rs

@@ -116,7 +116,7 @@ fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorType
                     cx,
                     AWAIT_HOLDING_LOCK,
                     ty_cause.span,
-                    "this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await.",
+                    "this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await",
                     ty_cause.scope_span.or(Some(span)),
                     "these are all the await points this lock is held through",
                 );
@@ -126,7 +126,7 @@ fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorType
                         cx,
                         AWAIT_HOLDING_REFCELL_REF,
                         ty_cause.span,
-                        "this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.",
+                        "this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await",
                         ty_cause.scope_span.or(Some(span)),
                         "these are all the await points this ref is held through",
                     );

clippy_lints/src/comparison_chain.rs

@@ -117,7 +117,7 @@ impl<'tcx> LateLintPass<'tcx> for ComparisonChain {
             expr.span,
             "`if` chain can be rewritten with `match`",
             None,
-            "Consider rewriting the `if` chain to use `cmp` and `match`.",
+            "consider rewriting the `if` chain to use `cmp` and `match`",
         )
     }
 }

clippy_lints/src/drop_forget_ref.rs

@@ -98,13 +98,13 @@ declare_clippy_lint! {
 }
 
 const DROP_REF_SUMMARY: &str = "calls to `std::mem::drop` with a reference instead of an owned value. \
-                                Dropping a reference does nothing.";
+                                Dropping a reference does nothing";
 const FORGET_REF_SUMMARY: &str = "calls to `std::mem::forget` with a reference instead of an owned value. \
-                                  Forgetting a reference does nothing.";
+                                  Forgetting a reference does nothing";
 const DROP_COPY_SUMMARY: &str = "calls to `std::mem::drop` with a value that implements `Copy`. \
-                                 Dropping a copy leaves the original intact.";
+                                 Dropping a copy leaves the original intact";
 const FORGET_COPY_SUMMARY: &str = "calls to `std::mem::forget` with a value that implements `Copy`. \
-                                   Forgetting a copy leaves the original intact.";
+                                   Forgetting a copy leaves the original intact";
 
 declare_lint_pass!(DropForgetRef => [DROP_REF, FORGET_REF, DROP_COPY, FORGET_COPY]);
 

clippy_lints/src/fallible_impl_from.rs

@@ -133,7 +133,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[h
                         move |diag| {
                             diag.help(
                                 "`From` is intended for infallible conversions only. \
-                                Use `TryFrom` if there's a possibility for the conversion to fail.");
+                                Use `TryFrom` if there's a possibility for the conversion to fail");
                             diag.span_note(fpu.result, "potential failure(s)");
                         });
                 }

clippy_lints/src/indexing_slicing.rs

@@ -132,13 +132,13 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
                 }
 
                 let help_msg = match (range.start, range.end) {
-                    (None, Some(_)) => "Consider using `.get(..n)`or `.get_mut(..n)` instead",
-                    (Some(_), None) => "Consider using `.get(n..)` or .get_mut(n..)` instead",
-                    (Some(_), Some(_)) => "Consider using `.get(n..m)` or `.get_mut(n..m)` instead",
+                    (None, Some(_)) => "consider using `.get(..n)`or `.get_mut(..n)` instead",
+                    (Some(_), None) => "consider using `.get(n..)` or .get_mut(n..)` instead",
+                    (Some(_), Some(_)) => "consider using `.get(n..m)` or `.get_mut(n..m)` instead",
                     (None, None) => return, // [..] is ok.
                 };
 
-                span_lint_and_help(cx, INDEXING_SLICING, expr.span, "slicing may panic.", None, help_msg);
+                span_lint_and_help(cx, INDEXING_SLICING, expr.span, "slicing may panic", None, help_msg);
             } else {
                 // Catchall non-range index, i.e., [n] or [n << m]
                 if let ty::Array(..) = ty.kind() {
@@ -153,9 +153,9 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
                     cx,
                     INDEXING_SLICING,
                     expr.span,
-                    "indexing may panic.",
+                    "indexing may panic",
                     None,
-                    "Consider using `.get(n)` or `.get_mut(n)` instead",
+                    "consider using `.get(n)` or `.get_mut(n)` instead",
                 );
             }
         }

clippy_lints/src/integer_division.rs

@@ -39,7 +39,7 @@ impl<'tcx> LateLintPass<'tcx> for IntegerDivision {
                 expr.span,
                 "integer division",
                 None,
-                "division of integers may cause loss of precision. consider using floats.",
+                "division of integers may cause loss of precision. consider using floats",
             );
         }
     }

clippy_lints/src/loops.rs

@@ -1625,10 +1625,7 @@ fn check_for_loop_range<'tcx>(
                         cx,
                         NEEDLESS_RANGE_LOOP,
                         expr.span,
-                        &format!(
-                            "the loop variable `{}` is only used to index `{}`.",
-                            ident.name, indexed
-                        ),
+                        &format!("the loop variable `{}` is only used to index `{}`", ident.name, indexed),
                         |diag| {
                             multispan_sugg(
                                 diag,
@@ -1763,7 +1760,7 @@ fn check_arg_type(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
             arg.span,
             &format!(
                 "for loop over `{0}`, which is an `Option`. This is more readably written as an \
-                `if let` statement.",
+                `if let` statement",
                 snippet(cx, arg.span, "_")
             ),
             None,
@@ -1780,7 +1777,7 @@ fn check_arg_type(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
             arg.span,
             &format!(
                 "for loop over `{0}`, which is a `Result`. This is more readably written as an \
-                `if let` statement.",
+                `if let` statement",
                 snippet(cx, arg.span, "_")
             ),
             None,
@@ -1826,7 +1823,7 @@ fn check_for_loop_explicit_counter<'tcx>(
                         cx,
                         EXPLICIT_COUNTER_LOOP,
                         for_span.with_hi(arg.span.hi()),
-                        &format!("the variable `{}` is used as a loop counter.", name),
+                        &format!("the variable `{}` is used as a loop counter", name),
                         "consider using",
                         format!(
                             "for ({}, {}) in {}.enumerate()",
@@ -3055,16 +3052,16 @@ impl IterFunction {
     fn get_suggestion_text(&self) -> &'static str {
         match &self.func {
             IterFunctionKind::IntoIter => {
-                "Use the original Iterator instead of collecting it and then producing a new one"
+                "use the original Iterator instead of collecting it and then producing a new one"
             },
             IterFunctionKind::Len => {
-                "Take the original Iterator's count instead of collecting it and finding the length"
+                "take the original Iterator's count instead of collecting it and finding the length"
             },
             IterFunctionKind::IsEmpty => {
-                "Check if the original Iterator has anything instead of collecting it and seeing if it's empty"
+                "check if the original Iterator has anything instead of collecting it and seeing if it's empty"
             },
             IterFunctionKind::Contains(_) => {
-                "Check if the original Iterator contains an element instead of collecting then checking"
+                "check if the original Iterator contains an element instead of collecting then checking"
             },
         }
     }

clippy_lints/src/matches.rs

@@ -1173,9 +1173,9 @@ fn check_wild_in_or_pats(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
                     cx,
                     WILDCARD_IN_OR_PATTERNS,
                     arm.pat.span,
-                    "wildcard pattern covers any other pattern as it will match anyway.",
+                    "wildcard pattern covers any other pattern as it will match anyway",
                     None,
-                    "Consider handling `_` separately.",
+                    "consider handling `_` separately",
                 );
             }
         }

clippy_lints/src/methods/mod.rs

@@ -2569,7 +2569,7 @@ fn lint_step_by<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, args: &'tcx
                 cx,
                 ITERATOR_STEP_BY_ZERO,
                 expr.span,
-                "Iterator::step_by(0) will panic at runtime",
+                "`Iterator::step_by(0)` will panic at runtime",
             );
         }
     }
@@ -3081,7 +3081,7 @@ fn lint_filter_next<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, fil
     // lint if caller of `.filter().next()` is an Iterator
     if match_trait_method(cx, expr, &paths::ITERATOR) {
         let msg = "called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
-                   `.find(..)` instead.";
+                   `.find(..)` instead";
         let filter_snippet = snippet(cx, filter_args[1].span, "..");
         if filter_snippet.lines().count() <= 1 {
             let iter_snippet = snippet(cx, filter_args[0].span, "..");
@@ -3209,7 +3209,7 @@ fn lint_filter_map_next<'tcx>(
         }
 
         let msg = "called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
-                   `.find_map(..)` instead.";
+                   `.find_map(..)` instead";
         let filter_snippet = snippet(cx, filter_args[1].span, "..");
         if filter_snippet.lines().count() <= 1 {
             let iter_snippet = snippet(cx, filter_args[0].span, "..");

clippy_lints/src/methods/unnecessary_lazy_eval.rs

@@ -50,7 +50,7 @@ pub(super) fn lint<'tcx>(
                     UNNECESSARY_LAZY_EVALUATIONS,
                     expr.span,
                     msg,
-                    &format!("Use `{}` instead", simplify_using),
+                    &format!("use `{}` instead", simplify_using),
                     format!(
                         "{0}.{1}({2})",
                         snippet(cx, args[0].span, ".."),

clippy_lints/src/misc.rs

@@ -292,7 +292,7 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
                     TOPLEVEL_REF_ARG,
                     arg.pat.span,
                     "`ref` directly on a function argument is ignored. \
-                    Consider using a reference type instead.",
+                    Consider using a reference type instead",
                 );
             }
         }
@@ -422,7 +422,7 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
                 expr.span,
                 &format!(
                     "used binding `{}` which is prefixed with an underscore. A leading \
-                     underscore signals that a binding will not be used.",
+                     underscore signals that a binding will not be used",
                     binding
                 ),
             );

clippy_lints/src/needless_question_mark.rs

@@ -142,7 +142,7 @@ fn emit_lint(cx: &LateContext<'_>, expr: &SomeOkCall<'_>) {
         cx,
         NEEDLESS_QUESTION_MARK,
         entire_expr.span,
-        "Question mark operator is useless here",
+        "question mark operator is useless here",
         "try",
         format!("{}", utils::snippet(cx, inner_expr.span, r#""...""#)),
         Applicability::MachineApplicable,

clippy_lints/src/ptr.rs

@@ -188,7 +188,7 @@ fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id:
                         PTR_ARG,
                         arg.span,
                         "writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used \
-                         with non-Vec-based slices.",
+                         with non-Vec-based slices",
                         |diag| {
                             if let Some(ref snippet) = get_only_generic_arg_snippet(cx, arg) {
                                 diag.span_suggestion(
@@ -217,7 +217,7 @@ fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id:
                         cx,
                         PTR_ARG,
                         arg.span,
-                        "writing `&String` instead of `&str` involves a new object where a slice will do.",
+                        "writing `&String` instead of `&str` involves a new object where a slice will do",
                         |diag| {
                             diag.span_suggestion(arg.span, "change this to", "&str".into(), Applicability::Unspecified);
                             for (clonespan, suggestion) in spans {
@@ -239,7 +239,7 @@ fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id:
                         cx,
                         PTR_ARG,
                         arg.span,
-                        "writing `&PathBuf` instead of `&Path` involves a new object where a slice will do.",
+                        "writing `&PathBuf` instead of `&Path` involves a new object where a slice will do",
                         |diag| {
                             diag.span_suggestion(
                                 arg.span,
@@ -278,7 +278,7 @@ fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id:
                                 cx,
                                 PTR_ARG,
                                 arg.span,
-                                "using a reference to `Cow` is not recommended.",
+                                "using a reference to `Cow` is not recommended",
                                 "change this to",
                                 "&".to_owned() + &r,
                                 Applicability::Unspecified,

clippy_lints/src/suspicious_operation_groupings.rs

@@ -261,8 +261,8 @@ fn emit_suggestion(cx: &EarlyContext<'_>, span: Span, sugg: String, applicabilit
         cx,
         SUSPICIOUS_OPERATION_GROUPINGS,
         span,
-        "This sequence of operators looks suspiciously like a bug.",
-        "I think you meant",
+        "this sequence of operators looks suspiciously like a bug",
+        "did you mean",
         sugg,
         applicability,
     )

clippy_lints/src/transmuting_null.rs

@@ -27,7 +27,7 @@ declare_clippy_lint! {
 
 declare_lint_pass!(TransmutingNull => [TRANSMUTING_NULL]);
 
-const LINT_MSG: &str = "transmuting a known null pointer into a reference.";
+const LINT_MSG: &str = "transmuting a known null pointer into a reference";
 
 impl<'tcx> LateLintPass<'tcx> for TransmutingNull {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {

clippy_lints/src/types.rs

@@ -388,7 +388,7 @@ impl Types {
                                 hir_ty.span,
                                 "you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`",
                                 None,
-                                "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.",
+                                "`Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation",
                             );
                             return; // don't recurse into the type
                         }
@@ -554,7 +554,7 @@ impl Types {
                                     cx,
                                     VEC_BOX,
                                     hir_ty.span,
-                                    "`Vec<T>` is already on the heap, the boxing is unnecessary.",
+                                    "`Vec<T>` is already on the heap, the boxing is unnecessary",
                                     "try",
                                     format!("Vec<{}>", snippet(cx, boxed_ty.span, "..")),
                                     Applicability::MachineApplicable,
@@ -578,7 +578,7 @@ impl Types {
                             cx,
                             LINKEDLIST,
                             hir_ty.span,
-                            "I see you're using a LinkedList! Perhaps you meant some other data structure?",
+                            "you seem to be using a `LinkedList`! Perhaps you meant some other data structure?",
                             None,
                             "a `VecDeque` might work",
                         );

clippy_lints/src/zero_div_zero.rs

@@ -55,7 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for ZeroDiv {
                     "constant division of `0.0` with `0.0` will always result in NaN",
                     None,
                     &format!(
-                        "Consider using `{}::NAN` if you would like a constant representing NaN",
+                        "consider using `{}::NAN` if you would like a constant representing NaN",
                         float_type,
                     ),
                 );