Skip to content

Further improve diagnostics for expressions in pattern position #121697

Open

Description

I've had this on my TODO list for a while but never had the time to write a PR for it. Therefore I'm just going to open an issue for it instead.

Since PR #118625 we try to reparse something in pattern position as an expression under certain conditions and provide a smarter diagnostic of the form “expected a pattern, found an expression // arbitrary expressions are not allowed in patterns” which is already amazing.

However, the current heuristic has a few limitations I'd like to see addressed which should be relatively simple to implement:

  1. The heuristic doesn't cover field accesses (of the form $literal.$ident and so forth) but only things that look like method calls (e.g., 0.f()). I argue we should also detect expressions like x.f and s.0. This would've prevented #104996 from getting opened! (addressed in #123877)
    • Firstly we need to remove the extra check for parentheses located after the .-“herald” check.
    • Then, we should in my opinion check the Span to make sure that there's no whitespace/comment before & after the . to avoid overzealously emitting this diagnostic when the user accidentally typoed a comma as a dot (as in (x. y)(x, y)) which isn't too farfetched given the keys are right next to each other on most keyboards (just need to check sp0.hi == sp1.lo). Of course, this would rule out multi-line method / field chains but that should be rare. We could (as a follow-up) check if the would-be expression is “appropriately” indented (by utilizing SourceMap methods) to address that, too. (NOTE: this isn't implemented yet, unclear if we actually want this)
    • The following is a bit opinionated: I think we should remove the capitalization check (at the moment, we don't consider x.Foo() to be a method call). If you disagree, we should at least also consider _ to be a valid start of a field or method name (x._foo() doesn't get recognized at the time of writing).
    • We should support sequences of numeric field accesses like x.0.1 without accidentally flagging x.0e4 as an expression (x.0.1 gets lexed as [Ident, Dot, FloatLit]). See parse_expr_tuple_field_access_float for prior art.
  2. We should consider the keyword as to be a “herald” next to . to recover 0 as usize etc. in pattern position (addressed in #123877).
  3. We should somehow recover f().g() and (0).f() etc. meaning more method calls / field accesses where the “basis” is a more complex expression. Rephrased: Recognize more “$expr.$ident” over just “$lit_or_ident.$ident” (the latter is the status quo, roughly, if I'm not mistaken).
    • Recovering from (0).f() was addressed in #123877.
  4. We should add a help message (structured or text-based) instructing the user to extract the expression to a const item: “extract the expression to a const and refer to that” (addressed in #123877).
  5. At some point we should provide a structured multipart suggestion for wrapping expressions in pattern position in a const {} block (feature inline_const_pat). We could provide the suggestion if self.sess.unstable_features.is_nightly_build() holds but I don't know if others like the idea of that. Note that we cannot check if feature inline_const_pat is enabled from inside the parser and diagnostic stashing/stealing doesn't help here either since we want to “stash” a subdiagnostic (a structured suggestion) not true actually, should be possible to impl (addressed in #123877.
  6. Various FIXMEs added in Further improve diagnostics for expressions in pattern position #123877
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Labels

A-diagnosticsArea: Messages for errors, warnings, and lintsArea: Messages for errors, warnings, and lintsA-parserArea: The parsing of Rust source code to an ASTArea: The parsing of Rust source code to an ASTA-patternsRelating to patterns and pattern matchingRelating to patterns and pattern matchingD-newcomer-roadblockDiagnostics: Confusing error or lint; hard to understand for new users.Diagnostics: Confusing error or lint; hard to understand for new users.D-terseDiagnostics: An error or lint that doesn't give enough information about the problem at hand.Diagnostics: An error or lint that doesn't give enough information about the problem at hand.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions