Skip to content

Uplift clippy::double_neg lint as double_negations #126604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
87 changes: 66 additions & 21 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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.
Expand All @@ -1594,7 +1638,8 @@ declare_lint_pass!(
UNSTABLE_FEATURES,
UNREACHABLE_PUB,
TYPE_ALIAS_BOUNDS,
TRIVIAL_BOUNDS
TRIVIAL_BOUNDS,
DOUBLE_NEGATIONS
]
);

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ early_lint_methods!(
UnusedDocComment: UnusedDocComment,
Expr2024: Expr2024,
Precedence: Precedence,
DoubleNegations: DoubleNegations,
]
]
);
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/.github/driver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions src/tools/clippy/book/src/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
```

Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes were necessary to get the clippy dev tool to compile

let mut iter = iter
.by_ref()
.filter(|t| !matches!(t.token_kind, TokenKind::Whitespace | TokenKind::LineComment { .. }));
Expand All @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""]
Expand Down
18 changes: 0 additions & 18 deletions src/tools/clippy/clippy_lints/src/misc_early/double_neg.rs

This file was deleted.

22 changes: 0 additions & 22 deletions src/tools/clippy/clippy_lints/src/misc_early/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod builtin_type_shadow;
mod double_neg;
mod literal_suffix;
mod mixed_case_hex_literals;
mod redundant_at_rest_pattern;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}

Expand Down
10 changes: 0 additions & 10 deletions src/tools/clippy/tests/ui/double_neg.rs

This file was deleted.

11 changes: 0 additions & 11 deletions src/tools/clippy/tests/ui/double_neg.stderr

This file was deleted.

Loading
Loading