[CP] [_fe_analyzer_shared] Handle private names in exhaustiveness checking

[stereotype441]

Commit(s) to merge

3a5b06a

Target

Beta Stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/301180 https://dart-review.googlesource.com/c/sdk/+/302845

Issue Description

When a record pattern in a switch refers to a private getter, the CFE implementation of exhaustiveness gets confused and fails to recognise the coverage of the pattern. As a result, users can get a bogus error stating that their switch isn't exhaustive.

What is the fix

The CFE's implementation of the exhaustiveness callback getFieldTypes was modified so that it considers private getters as legitimate targets for a field in an object pattern, provided that those getters are defined in the same library as the pattern. Previously, it ignored private getters entirely (essentially treating all private getters as inaccessible for the purpose of exhaustiveness checking).

The analyzer's implementation of getFieldTypes was also updated to match the CFE fix. Previously, the analyzer considered all private getters as accessible for purpose of exhaustiveness checking; this was incorrect, but it would require a very contrived piece of code to illustrate the flaw.

Why cherry-pick

Since sealed classes are designed to allow exhaustiveness checking on a class hierarchy that's entirely defined within a single library, it seems likely that users will create switch statements within that library and expect to be able to use object patterns that refer to private getters. (Indeed, this is how I discovered the bug in the first place).

Working around the error requires either adding additional cases (which won't ever be reached at runtime) or rewriting the switch statement so that it doesn't refer to the private getter (e.g. checking the getter in a when clause). Neither of these workarounds is particularly attractive.

The analyzer doesn't have this issue, so a user using an IDE doesn't see the problem until they try to run their code. At that point, if they try to fix their code by navigating to their IDE's "problems" view (in the hopes of then navigating to the place in the source code that experienced the problem), they won't find any errors in the "problems" view (since this view is populated with errors from the analyzer, not the CFE). This could lead to user confusion and frustration.

The fix is fairly low risk, since it is confined to the treatment of private getters in object patterns.

Risk

low

Issue link(s)

#52041

Extra Info

Since the cherry-pick window for beta has essentially closed, I'm not expecting this to actually get approved for a cherry-pick to beta. But I can't follow the cherry-pick process for stable yet (because the stable branch of Dart 3.0 hasn't been created yet). So I'm filing this as a cherry-pick-to-beta request and marking it with the cherry-pick-hold tag. I'm assuming that once the stable branch has been created, all cherry-picks with that tag will get re-evaluated for possibly cherry-picking to stable.

[itsjustkevin]

Thanks @stereotype441, noting that this is for the first stable hotfix to avoid losing track of it.

[itsjustkevin]

@stereotype441 we have the stable branch available now, please rebase to the stable branch and we can get this pushed through the queue.

[stereotype441]

@stereotype441 we have the stable branch available now, please rebase to the stable branch and we can get this pushed through the queue.

Thanks @itsjustkevin! For some reason Gerrit decided to assign a fresh CL id when I rebased it (https://dart-review.googlesource.com/c/sdk/+/302845), so I've updated the issue here to point to it, and I've sent it for code review again.

[itsjustkevin]

Thanks @stereotype441, that happened on another cherry-pick CL also.

[stereotype441]

Ok, the CL has been reviewed. I'm just waiting for cherry-pick approval now.