Rollup merge of #126604 - kadiwa4:uplift_double_negation, r=nnethercote

Uplift `clippy::double_neg` lint as `double_negations`

Warns about cases like this:
```rust
fn main() {
    let x = 1;
    let _b = --x; //~ WARN use of a double negation
}
```

The intent is to keep people from thinking that `--x` is a prefix decrement operator. `++x`, `x++` and `x--` are invalid expressions and already have a helpful diagnostic.

I didn't add a machine-applicable suggestion to the lint because it's not entirely clear what the programmer was trying to achieve with the `--x` operation. The code that triggers the lint should always be reviewed manually.

Closes #82987

Author: fmease

compiler/rustc_lint/messages.ftl

@@ -76,6 +76,11 @@ lint_builtin_deprecated_attr_link = use of deprecated attribute `{$name}`: {$rea
 lint_builtin_deref_nullptr = dereferencing a null pointer
     .label = this code causes undefined behavior when executed
 
+lint_builtin_double_negations = use of a double negation
+    .note = the prefix `--` could be misinterpreted as a decrement operator which exists in other languages
+    .note_decrement = use `-= 1` if you meant to decrement the value
+    .add_parens_suggestion = add parentheses for clarity
+
 lint_builtin_ellipsis_inclusive_range_patterns = `...` range patterns are deprecated
     .suggestion = use `..=` for an inclusive range
 

compiler/rustc_lint/src/builtin.rs

@@ -49,16 +49,16 @@ use rustc_trait_selection::traits::{self};
 use crate::errors::BuiltinEllipsisInclusiveRangePatterns;
 use crate::lints::{
     BuiltinAnonymousParams, BuiltinConstNoMangle, BuiltinDeprecatedAttrLink,
-    BuiltinDeprecatedAttrLinkSuggestion, BuiltinDerefNullptr,
-    BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives,
-    BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures,
-    BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents,
-    BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinMutablesTransmutes,
-    BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed,
-    BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
-    BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
-    BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
-    BuiltinWhileTrue, InvalidAsmLabel,
+    BuiltinDeprecatedAttrLinkSuggestion, BuiltinDerefNullptr, BuiltinDoubleNegations,
+    BuiltinDoubleNegationsAddParens, BuiltinEllipsisInclusiveRangePatternsLint,
+    BuiltinExplicitOutlives, BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote,
+    BuiltinIncompleteFeatures, BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures,
+    BuiltinKeywordIdents, BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc,
+    BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns,
+    BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasBounds,
+    BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub,
+    BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment,
+    BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel,
 };
 use crate::nonstandard_style::{MethodLateContext, method_context};
 use crate::{
@@ -90,19 +90,11 @@ declare_lint! {
 
 declare_lint_pass!(WhileTrue => [WHILE_TRUE]);
 
-/// Traverse through any amount of parenthesis and return the first non-parens expression.
-fn pierce_parens(mut expr: &ast::Expr) -> &ast::Expr {
-    while let ast::ExprKind::Paren(sub) = &expr.kind {
-        expr = sub;
-    }
-    expr
-}
-
 impl EarlyLintPass for WhileTrue {
     #[inline]
     fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
         if let ast::ExprKind::While(cond, _, label) = &e.kind
-            && let ast::ExprKind::Lit(token_lit) = pierce_parens(cond).kind
+            && let ast::ExprKind::Lit(token_lit) = cond.peel_parens().kind
             && let token::Lit { kind: token::Bool, symbol: kw::True, .. } = token_lit
             && !cond.span.from_expansion()
         {
@@ -1576,6 +1568,58 @@ impl<'tcx> LateLintPass<'tcx> for TrivialConstraints {
     }
 }
 
+declare_lint! {
+    /// The `double_negations` lint detects expressions of the form `--x`.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// fn main() {
+    ///     let x = 1;
+    ///     let _b = --x;
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Negating something twice is usually the same as not negating it at all.
+    /// However, a double negation in Rust can easily be confused with the
+    /// prefix decrement operator that exists in many languages derived from C.
+    /// Use `-(-x)` if you really wanted to negate the value twice.
+    ///
+    /// To decrement a value, use `x -= 1` instead.
+    pub DOUBLE_NEGATIONS,
+    Warn,
+    "detects expressions of the form `--x`"
+}
+
+declare_lint_pass!(
+    /// Lint for expressions of the form `--x` that can be confused with C's
+    /// prefix decrement operator.
+    DoubleNegations => [DOUBLE_NEGATIONS]
+);
+
+impl EarlyLintPass for DoubleNegations {
+    #[inline]
+    fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
+        // only lint on the innermost `--` in a chain of `-` operators,
+        // even if there are 3 or more negations
+        if let ExprKind::Unary(UnOp::Neg, ref inner) = expr.kind
+            && let ExprKind::Unary(UnOp::Neg, ref inner2) = inner.kind
+            && !matches!(inner2.kind, ExprKind::Unary(UnOp::Neg, _))
+        {
+            cx.emit_span_lint(DOUBLE_NEGATIONS, expr.span, BuiltinDoubleNegations {
+                add_parens: BuiltinDoubleNegationsAddParens {
+                    start_span: inner.span.shrink_to_lo(),
+                    end_span: inner.span.shrink_to_hi(),
+                },
+            });
+        }
+    }
+}
+
 declare_lint_pass!(
     /// Does nothing as a lint pass, but registers some `Lint`s
     /// which are used by other parts of the compiler.
@@ -1594,7 +1638,8 @@ declare_lint_pass!(
         UNSTABLE_FEATURES,
         UNREACHABLE_PUB,
         TYPE_ALIAS_BOUNDS,
-        TRIVIAL_BOUNDS
+        TRIVIAL_BOUNDS,
+        DOUBLE_NEGATIONS
     ]
 );
 
@@ -2651,7 +2696,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
 }
 
 declare_lint! {
-    /// The `deref_nullptr` lint detects when an null pointer is dereferenced,
+    /// The `deref_nullptr` lint detects when a null pointer is dereferenced,
     /// which causes [undefined behavior].
     ///
     /// ### Example

compiler/rustc_lint/src/lib.rs

@@ -181,6 +181,7 @@ early_lint_methods!(
             UnusedDocComment: UnusedDocComment,
             Expr2024: Expr2024,
             Precedence: Precedence,
+            DoubleNegations: DoubleNegations,
         ]
     ]
 );

compiler/rustc_lint/src/lints.rs

@@ -331,6 +331,24 @@ pub(crate) struct BuiltinTrivialBounds<'a> {
     pub predicate: Clause<'a>,
 }
 
+#[derive(LintDiagnostic)]
+#[diag(lint_builtin_double_negations)]
+#[note(lint_note)]
+#[note(lint_note_decrement)]
+pub(crate) struct BuiltinDoubleNegations {
+    #[subdiagnostic]
+    pub add_parens: BuiltinDoubleNegationsAddParens,
+}
+
+#[derive(Subdiagnostic)]
+#[multipart_suggestion(lint_add_parens_suggestion, applicability = "maybe-incorrect")]
+pub(crate) struct BuiltinDoubleNegationsAddParens {
+    #[suggestion_part(code = "(")]
+    pub start_span: Span,
+    #[suggestion_part(code = ")")]
+    pub end_span: Span,
+}
+
 #[derive(LintDiagnostic)]
 pub(crate) enum BuiltinEllipsisInclusiveRangePatternsLint {
     #[diag(lint_builtin_ellipsis_inclusive_range_patterns)]

src/tools/clippy/.github/driver.sh

@@ -47,9 +47,9 @@ unset CARGO_MANIFEST_DIR
 
 # Run a lint and make sure it produces the expected output. It's also expected to exit with code 1
 # FIXME: How to match the clippy invocation in compile-test.rs?
-./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/double_neg.rs 2>double_neg.stderr && exit 1
-sed -e "/= help: for/d" double_neg.stderr > normalized.stderr
-diff -u normalized.stderr tests/ui/double_neg.stderr
+./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/box_default.rs 2>box_default.stderr && exit 1
+sed -e "/= help: for/d" box_default.stderr > normalized.stderr
+diff -u normalized.stderr tests/ui/box_default.stderr
 
 # make sure "clippy-driver --rustc --arg" and "rustc --arg" behave the same
 SYSROOT=$(rustc --print sysroot)

src/tools/clippy/book/src/usage.md

@@ -33,7 +33,7 @@ You can configure lint levels on the command line by adding
 `-A/W/D clippy::lint_name` like this:
 
 ```bash
-cargo clippy -- -Aclippy::style -Wclippy::double_neg -Dclippy::perf
+cargo clippy -- -Aclippy::style -Wclippy::box_default -Dclippy::perf
 ```
 
 For [CI] all warnings can be elevated to errors which will in turn fail
@@ -101,11 +101,10 @@ You can configure lint levels in source code the same way you can configure
 ```rust,ignore
 #![allow(clippy::style)]
 
-#[warn(clippy::double_neg)]
+#[warn(clippy::box_default)]
 fn main() {
-    let x = 1;
-    let y = --x;
-    //      ^^ warning: double negation
+    let _ = Box::<String>::new(Default::default());
+    // ^ warning: `Box::new(_)` of default value
 }
 ```
 

src/tools/clippy/clippy_dev/src/new_lint.rs

@@ -455,7 +455,7 @@ fn setup_mod_file(path: &Path, lint: &LintData<'_>) -> io::Result<&'static str>
     });
 
     // Find both the last lint declaration (declare_clippy_lint!) and the lint pass impl
-    while let Some(LintDeclSearchResult { content, .. }) = iter.find(|result| result.token == TokenKind::Ident) {
+    while let Some(LintDeclSearchResult { content, .. }) = iter.find(|result| result.token_kind == TokenKind::Ident) {
         let mut iter = iter
             .by_ref()
             .filter(|t| !matches!(t.token_kind, TokenKind::Whitespace | TokenKind::LineComment { .. }));
@@ -465,7 +465,7 @@ fn setup_mod_file(path: &Path, lint: &LintData<'_>) -> io::Result<&'static str>
                 // matches `!{`
                 match_tokens!(iter, Bang OpenBrace);
                 if let Some(LintDeclSearchResult { range, .. }) =
-                    iter.find(|result| result.token == TokenKind::CloseBrace)
+                    iter.find(|result| result.token_kind == TokenKind::CloseBrace)
                 {
                     last_decl_curly_offset = Some(range.end);
                 }

src/tools/clippy/clippy_lints/src/declared_lints.rs

@@ -507,7 +507,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::misc::USED_UNDERSCORE_BINDING_INFO,
     crate::misc::USED_UNDERSCORE_ITEMS_INFO,
     crate::misc_early::BUILTIN_TYPE_SHADOW_INFO,
-    crate::misc_early::DOUBLE_NEG_INFO,
     crate::misc_early::DUPLICATE_UNDERSCORE_ARGUMENT_INFO,
     crate::misc_early::MIXED_CASE_HEX_LITERALS_INFO,
     crate::misc_early::REDUNDANT_AT_REST_PATTERN_INFO,

src/tools/clippy/clippy_lints/src/deprecated_lints.rs

@@ -129,6 +129,8 @@ declare_with_version! { RENAMED(RENAMED_VERSION): &[(&str, &str)] = &[
     ("clippy::clone_double_ref", "suspicious_double_ref_op"),
     #[clippy::version = ""]
     ("clippy::cmp_nan", "invalid_nan_comparisons"),
+    #[clippy::version = "1.86.0"]
+    ("clippy::double_neg", "double_negations"),
     #[clippy::version = ""]
     ("clippy::drop_bounds", "drop_bounds"),
     #[clippy::version = ""]

src/tools/clippy/clippy_lints/src/misc_early/double_neg.rs

@@ -1,5 +1,4 @@
 mod builtin_type_shadow;
-mod double_neg;
 mod literal_suffix;
 mod mixed_case_hex_literals;
 mod redundant_at_rest_pattern;
@@ -85,25 +84,6 @@ declare_clippy_lint! {
     "function arguments having names which only differ by an underscore"
 }
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// Detects expressions of the form `--x`.
-    ///
-    /// ### Why is this bad?
-    /// It can mislead C/C++ programmers to think `x` was
-    /// decremented.
-    ///
-    /// ### Example
-    /// ```no_run
-    /// let mut x = 3;
-    /// --x;
-    /// ```
-    #[clippy::version = "pre 1.29.0"]
-    pub DOUBLE_NEG,
-    style,
-    "`--x`, which is a double negation of `x` and not a pre-decrement as in C/C++"
-}
-
 declare_clippy_lint! {
     /// ### What it does
     /// Warns on hexadecimal literals with mixed-case letter
@@ -352,7 +332,6 @@ declare_clippy_lint! {
 declare_lint_pass!(MiscEarlyLints => [
     UNNEEDED_FIELD_PATTERN,
     DUPLICATE_UNDERSCORE_ARGUMENT,
-    DOUBLE_NEG,
     MIXED_CASE_HEX_LITERALS,
     UNSEPARATED_LITERAL_SUFFIX,
     SEPARATED_LITERAL_SUFFIX,
@@ -415,7 +394,6 @@ impl EarlyLintPass for MiscEarlyLints {
         if let ExprKind::Lit(lit) = expr.kind {
             MiscEarlyLints::check_lit(cx, lit, expr.span);
         }
-        double_neg::check(cx, expr);
     }
 }