This warning is similar to the existing `UNREACHABLE_SWITCH_CASE`
warning, except that it warns if the `default` clause of a switch
statement is unreachable due to all the `case` clasuses fully
exhausting the switched type.
To make the implementation easier, I changed the API for the
`reportExhaustiveness` method in `_fe_analyzer_shared` (which is the
primary entry point to the shared exhaustiveness checker). Previously,
this method returned a list of `ExhaustivenessError`, where each list
element was either an `UnreachableCaseError` (indicating that a
certain case was unreachable) or a `NonExhaustiveError` (indicating
that the entire switch statement was not exhaustive). If the caller
passed in `false` for `computeUnreachable`, `UnreachableCaseError`s
would not be returned, so the returned list would either be empty or
contain a single `NonExhaustiveError`.
The new API renames the types for clarity:
- `NonExhaustiveError` becomes `NonExhaustiveness`, to highlight the
fact that it's not necessarily an error for the switch's cases to be
non-exhaustive; it's only an error if the scrutinee's static type is
an "always exhaustive" type and there is no `default` clause.
- `UnreachableCaseError` becomes `CaseUnreachability`, to highlight
the fact that it's not an error for a case to be unreachable; it's a
warning.
Also, the new API adds instances of `CaseUnreachability` to an
optional user-provided list instead of returning a newly created list;
this allows callers to communicate that they don't need to see
`CaseUnreachability` information by passing `null`. This frees up the
return type to simply be an instance of `NonExhaustiveness` (if the
cases are not exhaustive) or `null` (if they are exhaustive). This
makes it easier for the analyzer to decide whether to issue the new
warning, because it doesn't have to dig around the list looking for an
instance of `NonExhaustiveness`.
The new warning has an associated quick fix (remove the unreachable
`default` clause). This quick fix uses the same `RemoveDeadCode` logic
in the analysis server that the existing `UNREACHABLE_SWITCH_CASE`
warning uses.
Fixes https://github.com/dart-lang/sdk/issues/54575.
Bug: https://github.com/dart-lang/sdk/issues/54575
Change-Id: I18b6b7c5249d77d28ead7488b4aae4ea65c4b664
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378960
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Most DTD tests are currently skipped because we're not currently exposing the LSP handlers, however I temporarily added them and verified with this change that those tests pass (both success and error requests).
Change-Id: I001b7b07c99be18a3711a434bdd081a631e4b4c2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/383842
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Previously any pending imports added by DartFileEditBuilder would just be sorted by their text ignoring the kind of import. If there were existing imports for dart, package, relative then they might be inserted in the right places, but if there were not (or not the right kind) of existing imports, they would be just be added in alphabetical order (rather than dart, then package, then relative).
This change extracts some of the rules for sorting directives from analysis_server's ImportOrganizer into analyzer_plugin/src so that they can be reused by the DartFileEditBuilder that lives there.
Fixes https://github.com/dart-lang/sdk/issues/56657
Change-Id: I6dc5476add2b7b1804080ffdc8270d0bb80597db
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384284
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Prior to this change, all LSP handlers would ask the server for the client capabilities when they would affect their output. For callers over DTD, this could result in the same requests getting different results depending on the capabilities of the editor.
This change adds LspClientCapabilities to the MessageInfo that goes along with each message (usually a request) and then passes in the servers clientCapabilities for requests originating from the editor client, and a fixed set of capabilities for requests that come from DTD.
Most handlers will now use the callers capabilities, however there are a few handlers that build edits to send to the editor (instead of _returning_ the edits), which will use the editors capabilities.
This is essentially a no-op for now, because we're just providing the same capabilities via a different route - but when we come to supporting LSP handlers over DTD (that are affected by client capabilities), they will get consisted results based on our fixed set of capabilities (which as of yet is probably incomplete and may be updated a little as that progresses).
Change-Id: I5bcef8c3ebb7b94c83d85e56a303333d302abdee
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382881
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Before exposing existing LSP methods we need to provide a way for DTD clients to always get the same format responses regardless of the client capabilities provided by the original server. Until then, add an allow-list for LSP methods that we can use to selectively enable specific methods that are not affected by client capabilities.
Change-Id: Ia1c994a50c9b94a91f4085c09f9d9d94c0fad79e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382401
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
This adds a new request `dart/connectToDtd` that lets a client pass a DTD URI to the server. The server will connect to DTD and provide a subset of LSP methods (those that are currently available over LSP-Over-Legacy, minus the `connectToDtd` method itself) to DTD clients.
This works for both LSP and Legacy servers, and the shared tests run against both (where for legacy, the `dart/connectToDtd` request uses the LSP-over-Legacy support).
This is not currently behind a flag, however it won't do much on its own because no clients will make this request. Once we're happy the functioanlity is good we can either agree an SDK version number for IDEs to gate calling this method on, or we can expose a flag in server capabilities (and some equiv for legacy) so the client knows the functionality is available.
Change-Id: I911d37745a1b2685249dd88e4b152c3196ba627c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/379303
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
This adds failing tests for https://github.com/dart-lang/sdk/issues/56475.
If this CL is applied to the 3.5 release and the `singleOptionContexts` flag set back to `true` (matching 3.4 behaviour), the tests will pass. Each package in both tests will get its own context root (because they have analysis_options to enable the plugins) and each one will read the correct plugins that are enabled.
However if `singleOptionContexts` is `false` (as shipped in 3.5) or this code is applied to bleeding-edge (where the flag is gone, but behaves the same as `false`) both tests fail as follows:
`test_sentToPlugins_inNestedPackages_withPackageConfigs` fails because we read the child analysis_options for the root and try to incorrectly load plugins from the root (and include duplicates and allow more than one plugin):
```
Expected: {
'package1': ['plugin1'],
'package2': ['plugin2'],
'package3': ['plugin1']
}
Actual: {
'home': ['plugin1', 'plugin2', 'plugin1'],
'package1': ['plugin1', 'plugin2', 'plugin1'],
'package2': ['plugin1', 'plugin2', 'plugin1'],
'package3': ['plugin1']
}
```
`test_sentToPlugins_inNestedPackages_withoutPackageConfigs` fails because we now only have a context root for the root which enables all plugins from the children:
```
Expected: {
'package1': ['plugin1'],
'package2': ['plugin2'],
'package3': ['plugin1']
}
Actual: {
'home': ['plugin1', 'plugin2', 'plugin1']
}
```
Change-Id: I12f45eeb5c1848352af26ac3fdb690e51833c22a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381460
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Previously, `HintCode.UNREACHABLE_SWITCH_CASE` was marked as
deprecated, and `WarningCode.UNREACHABLE_SWITCH_CASE` was an alias to
it. This created a slightly confusing situation, because it meant that
all code referring to the diagnostic had to refer to it as
`WarningCode.UNREACHABLE_SWITCH_CASE` (to avoid a deprecation lint),
but the diagnostic still _behaved_ like it was a hint, and therefore
test runner expectations still had to treat it as a hint.
It turns out that it's not really necessary to go through the
deprecation dance when changing the kind of a diagnostic, since (a)
members of `HintCode` and `WarningCode` aren't exposed through the
analyzer public API, and (b) ignore comments don't have to specify
whether something is a hint or a warning.
So the easiest way to clear up the confusion is to just remove
`HintCode.UNREACHABLE_SWITCH_CASE` entirely, and move its implemention
into `WarningCode.UNREACHABLE_SWITCH_CASE` (so that the latter is no
longer an alias).
Change-Id: I9ff7901ad38a2c168c5e54cbe0c1c52bf7c50186
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381103
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Work towards https://github.com/dart-lang/sdk/issues/56467
Thw two assists do the following:
* Add a digit separators every 3 digits in a decimal int, a decimal
double, and a scientific notation double.
* Add a digit separators every 2 digits in a hexadecimal int.
No assists are introduced which remove digit separators.
Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Change-Id: I0920fb279285963c33a78d9288317213739ae83b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380601
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This lint is the complement of 'omit_obvious_local_variable_types'
in that it flags every declaration that does _not_ specify the type
of a local variable, when it has an initializing expression whose type
is not obvious (defined as in 'omit_obvious_local_variable_types').
Change-Id: If80a961e47fed0412d0ea0bac5ac010e26fea7bc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/379160
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Parker Lougheed