Skip to content
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

Suggest [tail @ ..] on [..tail] and [...tail] where tail is unresolved #120597

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

fmease
Copy link
Member

@fmease fmease commented Feb 3, 2024

Fixes #120591.
Will conflict with #120570 (rebased).

r? estebank or compiler

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 3, 2024
// where the user might have meant `[first, rest @ ..]`.
let (span, snippet) = match side {
Side::Start => (segment.ident.span.between(range.span), " @ ".into()),
Side::End => (range.span.to(segment.ident.span), format!("{} @ ..", segment.ident)),
Copy link
Member Author

@fmease fmease Feb 3, 2024

Choose a reason for hiding this comment

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

I briefly experimented with a multipart suggestion here but I didn't like how it got rendered:

LL -    [head, ..tail] => {}
LL +    [head, tail @ ..] => {}

Copy link
Member Author

@fmease fmease Feb 3, 2024

Choose a reason for hiding this comment

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

And thus I reverted to a single-piece suggestion:

LL |    [head, tail @ ..] => {}
   |           ~~~~~~~~~

Copy link
Member Author

Choose a reason for hiding this comment

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

However, I can use a multipart suggestion if you'd like me to, of course.

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally prefer the diff output, because it has less opportunity for confusion and behaves better in the face of longer pieces of code (think complex nested patterns, in this case it will always be an ident so it's fine), but don't care either way, as long as the users get the information.

@fmease fmease added the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Feb 3, 2024
@fmease fmease force-pushed the sugg-on-js-style-spread-op-in-pat branch from 4bdc63e to 285d8c2 Compare February 4, 2024 21:17
@fmease fmease removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Feb 4, 2024
Comment on lines +68 to +77
error[E0658]: exclusive range pattern syntax is experimental
--> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:11:13
|
LL | [_, ..tail] => println!("{tail}"),
| ^^^^^^
|
= note: see issue #37854 <https://github.com/rust-lang/rust/issues/37854> for more information
= help: add `#![feature(exclusive_range_pattern)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
= help: use an inclusive range pattern, like N..=M
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like it if we could bubble information about having given a suggestion so that we don't provide less useful messages, but whatever, it's not the end of the world and it could be that someone is trying to use this nightly feature.

|
= note: see issue #37854 <https://github.com/rust-lang/rust/issues/37854> for more information
= help: add `#![feature(exclusive_range_pattern)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
Copy link
Contributor

Choose a reason for hiding this comment

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

Hadn't seen this! Nice!

@estebank
Copy link
Contributor

estebank commented Feb 5, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 5, 2024

📌 Commit 285d8c2 has been approved by estebank

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 6, 2024
…in-pat, r=estebank

Suggest `[tail @ ..]` on `[..tail]` and `[...tail]` where `tail` is unresolved

Fixes rust-lang#120591.
~~Will conflict with rust-lang#120570~~ (rebased).

r? estebank or compiler
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 6, 2024
…in-pat, r=estebank

Suggest `[tail @ ..]` on `[..tail]` and `[...tail]` where `tail` is unresolved

Fixes rust-lang#120591.
~~Will conflict with rust-lang#120570~~ (rebased).

r? estebank or compiler
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#120302 (various const interning cleanups)
 - rust-lang#120520 (Some cleanups around diagnostic levels.)
 - rust-lang#120521 (Make `NonZero` constructors generic.)
 - rust-lang#120527 (Switch OwnedStore handle count to AtomicU32)
 - rust-lang#120564 (coverage: Split out counter increment sites from BCB node/edge counters)
 - rust-lang#120575 (Simplify codegen diagnostic handling)
 - rust-lang#120597 (Suggest `[tail @ ..]` on `[..tail]` and `[...tail]` where `tail` is unresolved)
 - rust-lang#120609 (hir: Stop keeping prefixes for most of `use` list stems)
 - rust-lang#120633 (pattern_analysis: gather up place-relevant info)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2024
…iaskrgr

Rollup of 12 pull requests

Successful merges:

 - rust-lang#120520 (Some cleanups around diagnostic levels.)
 - rust-lang#120575 (Simplify codegen diagnostic handling)
 - rust-lang#120597 (Suggest `[tail @ ..]` on `[..tail]` and `[...tail]` where `tail` is unresolved)
 - rust-lang#120602 (rustc_monomorphize: fix outdated comment in partition)
 - rust-lang#120609 (hir: Stop keeping prefixes for most of `use` list stems)
 - rust-lang#120631 (Emit a diagnostic for invalid target options)
 - rust-lang#120632 (For E0223, suggest associated functions that are similar to the path)
 - rust-lang#120670 (cleanup effect var handling)
 - rust-lang#120673 (rustc_metadata: fix typo)
 - rust-lang#120683 (miri: fix ICE with symbolic alignment check on extern static)
 - rust-lang#120690 (Remove b-naber from the compiler review rotation)
 - rust-lang#120713 (Make async closures test use async bound modifier)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 89aa85d into rust-lang:master Feb 6, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 6, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2024
Rollup merge of rust-lang#120597 - fmease:sugg-on-js-style-spread-op-in-pat, r=estebank

Suggest `[tail @ ..]` on `[..tail]` and `[...tail]` where `tail` is unresolved

Fixes rust-lang#120591.
~~Will conflict with rust-lang#120570~~ (rebased).

r? estebank or compiler
@fmease fmease deleted the sugg-on-js-style-spread-op-in-pat branch February 6, 2024 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Catch Javascript style "rest patterns"
4 participants