Suggest array indexing when tuple indexing on an array #54292
Conversation
rust-highfive
added
the
S-waiting-on-review
src/librustc_typeck/check/mod.rs
Outdated
| let base = self.tcx.hir.node_to_pretty_string(base.id); | ||
| let msg = format!("attempting to use tuple indexing on an array; try"); | ||
| let suggestion = format!("{}[{}]", base, field); | ||
| err.span_suggestion(field.span, &msg, suggestion); |
There was a problem hiding this comment.
Please use span_suggestion_with_applicability and Applicability::MaybeIncorrect.
src/librustc_typeck/check/mod.rs
Outdated
| @@ -3344,6 +3344,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
| } | |||
| }; | |||
| } | |||
| ty::Array(ty, _) if ty.is_numeric() => { | |||
There was a problem hiding this comment.
I wonder if you can get the numeric value of field and check wether it is smaller than ty::Array(_, len). That would give us a great degree of confidence that the suggestion is correct, we could even switch between Applicability levels given that... 🤔
src/librustc_typeck/check/mod.rs
Outdated
| @@ -3344,6 +3344,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
| } | |||
| }; | |||
| } | |||
| ty::Array(ty, _) if ty.is_numeric() => { | |||
There was a problem hiding this comment.
Is there any reason to limit this suggestion to only [{integer}; _] and [{float}; _]? I'm not against it, as being conservative in suggestions is not bad, I just want to hear what was your reasoning.
src/librustc_typeck/check/mod.rs
Outdated
| @@ -3344,6 +3344,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { | |||
| } | |||
| }; | |||
| } | |||
| ty::Array(ty, _) if ty.is_numeric() => { | |||
| let base = self.tcx.hir.node_to_pretty_string(base.id); | |||
| let msg = format!("attempting to use tuple indexing on an array; try"); | |||
There was a problem hiding this comment.
Let's change the wording to something along the lines of instead of using tuple indexing use array indexing.
|
@estebank Limiting to only numeric types was definitely overly conservative, and I should’ve been checking if the field was numeric instead :) Good thinking on the array length and applicability levels. I’ll try it out! |
src/librustc_typeck/check/mod.rs
Outdated
| Applicability::MaybeIncorrect | ||
| }; | ||
| err.span_suggestion_with_applicability( | ||
| field.span, help, suggestion, applicability |
There was a problem hiding this comment.
You probably want to use expr.span here instead of field.span.
| @@ -2,7 +2,7 @@ error[E0609]: no field `0` on type `[{integer}; 5]` | |||
| --> $DIR/issue-53712.rs:5:9 | |||
| | | |||
| LL | arr.0; | |||
| | ^ help: attempting to use tuple indexing on an array; try: `arr[0]` | |||
| | ^ help: instead of using tuple indexing, use array indexing: `arr[0]` | |||
There was a problem hiding this comment.
I just realized that the suggestion span is slightly incorrect. The suggestion span should cover the entire expression, otherwise when applying the suggestion with rustfix, for example, you'd end up with the following code
arr.arr[0];
There was a problem hiding this comment.
Ah, right you are!
|
@bors r+ rollup |
|
📌 Commit 73fdc81 has been approved by |
bors
added
S-waiting-on-bors
Suggest array indexing when tuple indexing on an array Closes rust-lang#53712 r? @varkor cc @estebank
Rollup of 15 pull requests Successful merges: - #52813 (Duration div mul extras) - #53470 (Warn about metadata loader errors) - #54233 (Remove LLVM 3.9 workaround.) - #54257 (Switch wasm math symbols to their original names) - #54258 (Enable fatal warnings for the wasm32 linker) - #54266 (Update LLVM to fix "bool" arguments on PPC32) - #54290 (Switch linker for aarch64-pc-windows-msvc from LLD to MSVC) - #54292 (Suggest array indexing when tuple indexing on an array) - #54295 (A few cleanups and minor improvements to rustc/traits) - #54298 (miri: correctly compute expected alignment for field) - #54333 (Update The Book to latest) - #54337 (Remove unneeded clone() from tests in librustdoc) - #54346 (rustc: future-proof error reporting for polymorphic constants in types.) - #54362 (Pass --batch to gdb) - #54367 (Add regression test for thread local static mut borrows)
Closes #53712
r? @varkor
cc @estebank