Further improve diagnostics for expressions in pattern position #121697
Open
Description
opened on Feb 27, 2024
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:
- 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 likex.f
ands.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 checksp0.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 utilizingSourceMap
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 flaggingx.0e4
as an expression (x.0.1
gets lexed as [Ident, Dot, FloatLit]). Seeparse_expr_tuple_field_access_float
for prior art.
- Firstly we need to remove the extra check for parentheses located after the
- We should consider the keyword
as
to be a “herald” next to.
to recover0 as usize
etc. in pattern position (addressed in #123877). - We should somehow recover
f().g()
andetc. meaning more method calls / field accesses where the “basis” is a more complex expression. Rephrased: Recognize more “(0).f()
$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.
- Recovering from
- 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). - At some point we should provide a structured multipart suggestion for wrapping expressions in pattern position in a
const {}
block (featureinline_const_pat
). We could provide the suggestion ifself.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 featureinline_const_pat
is enabled from inside the parserand 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. - Various FIXMEs added in Further improve diagnostics for expressions in pattern position #123877
Metadata
Assignees
Labels
Area: Messages for errors, warnings, and lintsArea: The parsing of Rust source code to an ASTRelating to patterns and pattern matchingDiagnostics: Confusing error or lint; hard to understand for new users.Diagnostics: An error or lint that doesn't give enough information about the problem at hand.Relevant to the compiler team, which will review and decide on the PR/issue.
Activity