This adds shared tests for sending workspace/applyEdit reverse-requests that run for both the LSP and Legacy servers.
It involved moving some code out of the base LSP test onto mixins to be used by LSP-over-Legacy tests and I extracted some mixins that can be used in tests to provide a common interface to both servers for writing shared tests.
Change-Id: I8c6d09f220b2593680547311a4fce66013f86e2e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404420
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This adds a new field to the existing (legacy) setClientCapabilities parameters that accepts an LSP ClientCapabilities.
This will allow a legacy client to indicate that it supports things like the `workspace/applyEdit` reverse-request.
Change-Id: Ia3b75c701f1699c92f902e058daec4844ce664fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404106
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Parse errors for spec types were being reported twice - once by the containing object (in canParse()), and once by the nested canParse() call for the nested type.
This skips reporting the error for nested calls to a canParse() method, which will always report the error itself.
Change-Id: I03e4a9638fd6a3fc77eac918f6fd16def93327d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404105
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Many fields in this class are initialized in the class body, which is
unfortunate but seems necessary. However, such fields can often be
marked final, which makes the life cycle of such fields more clear:
contextManager, analysisDriverScheduler, pubPackageService.
Also, searchEngine can be a late final field wtih an initializer.
The analysisPerformanceLogger field is unused outside of the
constructor body in which it is declared; can be deleted.
isFirstAnalysisSinceContextsBuilt can be made private.
`_getByteStorePath` is also moved out to be an extension getter; it was
an instance method, but only uses the one OverlayResourceProvider.
Logically the function is just extracting a value from an
OverlayResourceProvider.
Change-Id: Ib08693ef16221560a1f14279725d9d44f9f5f99c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403920
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Usually when adding arguments, we'd always put them at the end. However there is convention (and a lint) for putting child/children last - so if the arguments in the last position are child/children, we should insert before them.
Change-Id: Ia0e0cd6b10f16cef30aa0cfe4fdedecc223f08c4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403584
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Originally we put those with arguments first so that in the property editor they'd be at the top. However the results in the order changing if you add/remove them, and across widget instances.
Returning them in source order keeps the consistent (and allows the "more important" ones to be at the top), though nothing prevents having a different sort (or an option to change it) on the client side.
Change-Id: I4808f0cb2ae5fcbb6d5496aa736528e60ac7c935
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403582
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
The original code tried to predict the "best" kind of quotes based on the string value (for example changing to double quotes when the string contained a single quote).
Since, we decided it would be better to try to preserve the existing quotes the user picked (this results in less of the string changing, and preserves user preferences/lints).
In some cases we can't preserve the exact delimeters, because if the string is a raw string but the new value contains the delimeter, we can't escape it, so we drop the `r` and escape anything that requires it.
Change-Id: I9a224ade5edd5bd558310b78be132fc9addd3ed3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398885
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
The `handleExpectedRequest` test method sends a request to the server and expects a request back (for example executing a command that will trigger server-to-client applyEdit). There is a timeout (since this request may never come), but in the case where the timeout is caused by the outbound request failing, we should report that failure instead (since it is certainly the cause of the timeout).
This changes the code to capture the error from the outbound request, and _if_ the expected request never comes, throws that in preference to the timeout.
Before:
```
00:05 +0 -1: ApplyAllFixesInWorkspace | test_partFile_issue59572 [E]
TimeoutException after 0:00:05.000000: Future not completed
test\lsp\server_abstract.dart 1143:15 LspAnalysisServerTestMixin.expectRequest.<fn>
dart:async/zone.dart 1517:47 _rootRun
dart:async/zone.dart 1422:19 _CustomZone.run
dart:async/future_impl.dart 1036:34 Future.timeout.<fn>
===== asynchronous gap ===========================
test\lsp\server_abstract.dart 1139:29 LspAnalysisServerTestMixin.expectRequest
===== asynchronous gap ===========================
test\lsp\server_abstract.dart 1190:27 LspAnalysisServerTestMixin.handleExpectedRequest
===== asynchronous gap ===========================
test\lsp\server_abstract.dart 155:27 AbstractLspAnalysisServerTest.executeForEdits
```
After:
```
00:05 +0 -1: ApplyAllFixesInWorkspace | test_partFile_issue59572 [E]
{
"code": -32006,
"message": "dart.edit.fixAllInWorkspace requires a single Map argument"
}
test\lsp\server_abstract.dart 1211:7 LspAnalysisServerTestMixin.handleExpectedRequest.<fn>
dart:async/zone.dart 1538:47 _rootRunUnary
dart:async/zone.dart 1429:19 _CustomZone.runUnary
dart:async/future_impl.dart 229:22 _FutureListener.handleError
dart:async/future_impl.dart 944:47 Future._propagateToListeners.handleError
dart:async/future_impl.dart 965:13 Future._propagateToListeners
dart:async/future_impl.dart 730:5 Future._completeError
```
Additionally, if the timeout is not because of a failed outbound request, the error message is now like "Did not receive the expected workspace/applyEdit request from the server in the timeout period" instead of "Future not completed".
Fixes https://github.com/dart-lang/sdk/issues/59780
Change-Id: I8b8cc25194390ffffbe034eae313504792a42211
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403460
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
The completion code was no longer using this (we use `ElementLocation2` directly in the completion handler now), and the navigation check was just to ensure we didn't include MultiplyDefinedElements in navigation results.
Change-Id: I8412ec60d3d56386a7ced3d78c3a7428313df5d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/402101
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
* If there are no return statements, suggest a `void` or `Future<void>`
return type.
* Visit yield statements as well, to suggest proper return types in
generators.
* Combine the 'return type computers' in extract_method.dart and
add_return_type.dart int one.
Change-Id: I223a50b11e7e77183f166c193fa74b8a2800127e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401862
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This change simplifies working with `@Native`-annotated functions by allowing the native type to be omitted when it can be inferred from the Dart function's signature. While this was previously supported for `@Native` fields, it now applies to functions as well.
Before this change, you needed to specify the native type explicitly:
```
@Native<Void Function(Pointer)>()
external void free(Pointer p);
```
After this change, the native type can now be omitted if it's clear from the Dart signature:
```
@Native()
external void free(Pointer p);
```
TEST=tests/ffi/native_assets/*
CoreLibraryReviewExempt: VM only
Closes: https://github.com/dart-lang/sdk/issues/54810
Change-Id: Ied5407fcd2f49d85284cb7817f0c8cad2a73626b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400840
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Moritz Sümmermann <mosum@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This moves completion resolution off `ElementLocation` onto the class recently created as `TypeHierarchyItemLocation`, which is now renamed to `ElementLocation2` and extracted to its own file.
Change-Id: I1f0b831ded7b08d6c09f97fcfd66f38f1dd4750e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401021
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
pq