`legacy_numeric_constants` spans are a bit off, synatx error due to misssing `)`

[matthiaskrgr]

Using the following flags

--force-warn clippy::legacy_numeric_constants

this code:

fn foo() {
    assert_eq!(0,-(i32::max_value()));
}

caused the following diagnostics:

    Checking _snippet_151 v0.1.0 (/tmp/icemaker_global_tempdir.VQhomPU3g4RT/icemaker_clippyfix_tempdir.JFZFEVyWdtjO/_snippet_151)
warning: usage of a legacy numeric method
 --> src/lib.rs:2:25
  |
2 |     assert_eq!(0,-(i32::max_value()));
  |                         ^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants
  = note: requested on the command line with `--force-warn clippy::legacy-numeric-constants`
help: use the associated constant instead
  |
2 -     assert_eq!(0,-(i32::max_value()));
2 +     assert_eq!(0,-(i32::MAX);
  |

warning: `_snippet_151` (lib) generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s

However after applying these diagnostics, the resulting code:

fn foo() {
    assert_eq!(0,-(i32::MAX);
}

no longer compiled:

    Checking _snippet_151 v0.1.0 (/tmp/icemaker_global_tempdir.VQhomPU3g4RT/icemaker_clippyfix_tempdir.JFZFEVyWdtjO/_snippet_151)
error: mismatched closing delimiter: `}`
 --> src/lib.rs:2:15
  |
1 | fn foo() {
  |          - closing delimiter possibly meant for this
2 |     assert_eq!(0,-(i32::MAX);
  |               ^ unclosed delimiter
3 | }
  | ^ mismatched closing delimiter

error: could not compile `_snippet_151` (lib test) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `_snippet_151` (lib) due to 1 previous error

Version:

rustc 1.89.0-nightly (44f415c1d 2025-06-06)
binary: rustc
commit-hash: 44f415c1d617ebc7b931a243b7b321ef8a6ca47c
commit-date: 2025-06-06
host: x86_64-unknown-linux-gnu
release: 1.89.0-nightly
LLVM version: 20.1.5

[donkomura]

@rustbot claim

[Centri3]

I guess parentheses are still sometimes in the span of an expression.

We're probably trimming the wrong span here. We get the parent expression's span here which linting on feels reallyyyy suspicious to me. We still need it to check it's a call, but we should use the last_segment I'm fairly sure.

@donkomura

[donkomura]

Thank you for your kind advice!

[donkomura]

I inserted a dbg! into the current code to inspect the expr.
It seems that the end of the span for the method call is incorrect even at compile time.

test patch

diff --git a/tests/ui/legacy_numeric_constants.rs b/tests/ui/legacy_numeric_constants.rs
index d38781990..36b0bc576 100644
--- a/tests/ui/legacy_numeric_constants.rs
+++ b/tests/ui/legacy_numeric_constants.rs
@@ -82,6 +82,10 @@ fn main() {
     [(0, "", std::i128::MAX)];
     //~^ ERROR: usage of a legacy numeric constant
     //~| HELP: use the associated constant instead
+
+    let _ = -(i32::max_value());
+    //~^ ERROR: usage of a legacy numeric method
+    //~| HELP: use the associated constant instead
 }

output

[clippy_lints/src/legacy_numeric_constants.rs:130:13] expr = Expr {
    hir_id: HirId(DefId(0:12 ~ legacy_numeric_constants[8fe9]::main).139),
    kind: Path(
        TypeRelative(
            Ty {
                hir_id: HirId(DefId(0:12 ~ legacy_numeric_constants[8fe9]::main).141),
                span: tests/ui/legacy_numeric_constants.rs:86:15: 86:18 (#0),
                kind: Path(
                    Resolved(
                        None,
                        Path {
                            span: tests/ui/legacy_numeric_constants.rs:86:15: 86:18 (#0),
                            res: PrimTy(
                                Int(
                                    I32,
                                ),
                            ),
                            segments: [
                                PathSegment {
                                    ident: i32#0,
                                    hir_id: HirId(DefId(0:12 ~ legacy_numeric_constants[8fe9]::main).140),
                                    res: PrimTy(
                                        Int(
                                            I32,
                                        ),
                                    ),
                                    args: None,
                                    infer_args: true,
                                },
                            ],
                        },
                    ),
                ),
            },
            PathSegment {
                ident: max_value#0,
                hir_id: HirId(DefId(0:12 ~ legacy_numeric_constants[8fe9]::main).142),
                res: Err,
                args: None,
                infer_args: true,
            },
        ),
    ),
    span: tests/ui/legacy_numeric_constants.rs:86:15: 86:29 (#0),
}
error: usage of a legacy numeric method
  --> tests/ui/legacy_numeric_constants.rs:86:20
   |
LL |     let _ = -(i32::max_value());
   |                    ^^^^^^^^^^^^
   |
help: use the associated constant instead
   |
LL -     let _ = -(i32::max_value());
LL +     let _ = -(i32::MAX;
   |

[donkomura]

@rustbot release-assignment

[ComputerDruid]

the parent expression is a call here, but its span includes one layer of parens:

error: par_expr.span
  --> tests/ui/legacy_numeric_constants.rs:125:20
   |
LL |     assert_eq!(0, -(i32::max_value()));
   |                    ^^^^^^^^^^^^^^^^^^

so yeah, doing last_segment.ident.span.with_hi(par_expr.span.hi()) doesn't work here.

I guess what's going on is the HIR doesn't need those parens, so it folded them into the expr. In theory we also don't mind including the pair of parens here; if we replaced that whole span with i32::MAX we'd get -i32::MAX, which I guess would be fine.

It looks like ip_constant has the same issue, too.