Removes nearly all of the special handling of `uniqueName` in
constructors in the `DiagnosticCode` class hierarchy.
With three exceptions, the value passed for the named parameter
`uniqueName` is the same as the value that will eventually be stored
in the `DiagnosticCode.uniqueName` field. The three exceptions are the
constructors for `LintCode` and `SecurityLintCode`, and the unnamed
constructor for `LinterLintCode`. These constructors allow
`uniqueName` to be `null`, and if `null` is supplied, they replace it
with `LintCode.$name`.
These exceptions are needed for analyzer API compatibility in the case
of `LintCode`, and for compatibility with google3 code in the case of
`SecurityLintCode` and `LinterLintCode`.
In follow-up CLs, I will be updating google3 code to make the
exceptions in `SecurityLintCode` and `LinterLintCode` unnecessary.
Change-Id: I6a6a6964d828c3a8db6563d47347d5ca8f550ac3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459001
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Adds the constructor parameter `uniqueNameCheck` to `DiagnosticCode`
and derived classes. If a non-null value is supplied for this
parameter, an assertion checks that it's equal to
`uniqueName`. (Passing a `null` value disables the assertion; this is
necessary to preserve analyzer API compatibility).
The reason for this change is that the value supplied for `uniqueName`
when calling constructors in the `DiagnosticCode` class hierarchy is
not necessarily the same as the value that is eventually stored in the
`uniqueName` field; it goes through a sequence of manipulations while
being passed up the superclass chain that make its actual behavior
difficult to follow. The `uniqueNameCheck` field allows me to test the
theory that the actual values that wind up in `uniqueName` are simply
the analyzer codes, with the exception that `LinterLintCode.` is
replaced with `LintCode.`.
In a follow-up CL, I plan to eliminate nearly all the manipulations of
`uniqueName` that occur while being passed up the superclass chain,
which will simplify the code generation logic for computing
`uniqueName` values. Adding `uniqueNameCheck` now allows me to test
out the new, simpler logic, and verify that it will produce the same
result as today's more complex logic.
The `uniqueNameCheck` parameter is marked as `@deprecated` to prevent
clients from trying to use it; this ensures that when I remove it in a
follow-up CL, it won't be necessary to bump the analyzer major
version.
Change-Id: I6a6a696440bce62a31b988d12064244e54e37b6b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458820
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Replaces imports of `pkg/analyzer/lib/src/lint/linter.dart` with
direct imports of the files it re-exports:
- `package:analyzer/analysis_rule/analysis_rule.dart`
- `package:analyzer/analysis_rule/rule_context.dart`
Also inlines the typedefs `LintRule` = `AnalysisRule` and
`LinterContext` = `RuleContext`.
The rationale for this change is that implementers of lints inside of
google3 tend to use existing lint rules as a starting point; this has
resulted in a lot of internal code importing from
`package:analyzer/src`, which makes modifying analyzer internals more
difficult. Changing existing lint rules to use proper analyzer public
APIs should help curb this trend.
Change-Id: I6a6a6964136534b393c9504bea11d22eafb11c05
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459180
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This change adds a new parameter `typeParametersInScope` to `DartEditBuilder.writeType`, and related methods to deprecate the previous `methodBeingCopied` parameter, which limited our ability to pass in some type parameters that are in scope.
Also fixes a TODO about testing if the `Record` feature is enabled or not.
And as a bonus, we now always write down type parameters by looking for the closest public type or falling back to `Object?` or `dynamic`.
Bug: https://github.com/dart-lang/sdk/issues/61186
Change-Id: I7a7b1c62adb81aecfdf61ebbaa0d95f1d33bced8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/442282
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Auto-Submit: Felipe Morschel <git@fmorschel.dev>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This library mostly exports _public_ API, and the to-be-deprecated
NodeLintRegistry class. So we just move private RuleContext classes
into a new library, and remove the exports.
The lint rules then are adjusted, mostly just renaming
NodeLintRegistry -> RuleVisitorRegistry. But also, this removes a lot
of implementation imports; most rules now have _no_ private imports!
Change-Id: I023f7efa979f94d208a6ae7421902145e8761c9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443364
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This removes `useLineEndingsForPlatform = false` from a few more tests, and as part of fixing them, ensures that `LspChangeVerifier` always normalises the expected text, and asserts that all modifies files were modified.
This highlighted many more tests that were not normalizing their content (but didn't use `useLineEndingsForPlatform` because they didn't previously call any shared methods that required/asserting normalization. Those tests were also updated.
Change-Id: Ica914b6207b3c9fd440de4e0470e50a896dd976d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443561
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This was mostly a rename/move of the folder (and the analysis server updated all references), but I also had to:
- add `integration_test/analysis_options.yaml` to import from `../test` to get the same lint ignores
- update paths of exclusions in `verify_sorted_test.dart`
By moving all of the tests that start the server out-of-process out of test, we can:
1. Use "dart test" to just run the faster tests ("dart test test") and get functionality of the pkg:test runner (for example running tests concurrently and JSON output)
2. Allow VS Code to spawn different debug sessions for the "test" and "integration_test" folder, which means we can use a `preLaunchTask` to trigger compilation of the analysis server from source whenever running integration tests (avoiding having to compile manually, or run from source in a way that compiles a new server for each test suite)
Change-Id: I37cc03dc32d08c5b51a2eab79f6338bb079b32ac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/434801
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
The related issue asked for a new priority for the `Create class` fix
that would be lower if the undefined name was lowercase, giving the
`Create method` and `Create function` fixes a higher relative priority.
This change also adds a new abstract class to test the relative priority
between fix kinds. It is also used to test agains the merge combinators
fixes.
Fixes: https://github.com/dart-lang/sdk/issues/60523
Change-Id: I938f52a577ecf1b6bb8dd66c94fd45395a011ffa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422321
Auto-Submit: Felipe Morschel <git@fmorschel.dev>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This CL contains breaking changes for package:dtd and prepares both package:dart_service_protocol_shared and package:dtd for publish.
This CL also fixes https://github.com/dart-lang/sdk/issues/60757 so that DTD-registered services are sent over the `Service` stream upon initial subscription like what is done for client-registered services.
Change-Id: I619af816e64af01864c7ed9b98743c6691bf7e0b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429161
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Kenzie Davisson <kenzieschmoll@google.com>
There are a few things in this CL that were difficult to separate testing of (because they're all part of the same feature):
1. sets `requiresTrustedCaller=false` on the `executeCommand` handler so that commands can be called over DTD. It adds the same flag to the command handlers themselves, so a command can control whether it can be called over DTD or not (right now we allow everything except `logAction` and `sendWorkspaceEdit` which are both commands used internally and not appropriate for DTD clients to call).
2. Removes the allow-list on DTD methods, allowing all LSP shared methods to be available over DTD
3. Extends the integration test classes to support reverse-requests so we can verify the edits being sent back to the editor when calling the code actions commands over DTD
4. It also fixes a few bugs where we read the callers capabilities instead of the editors capabilities (which until now would always be the same in those places, but with this change are not).
Change-Id: I6d271ddad6dc1b00a98b10b735763a368c91af7a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428784
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
A minor refactor towards supporting CodeActions over DTD.
The (existing) `LspRequestHelpersMixin` is a collection of helper methods like `getHover()` that allow sharing strongly-typed access to LSP methods across different kinds of tests (LSP, LSP-over-Legacy, and in-process vs out-of-process).
This adds a class (`DtdHelper`) that allows those same helpers to be used to call LSP over DTD (it has to be a separate instance because these helpers already exist on the base test classes to communicate with the server directly without DTD).
As part of this, the mixin was split up to separate request, reverse-requests and notifications, since only outbound requests apply over DTD and it cannot provide an implementation for the others.
This change also adds `textDocument/hover` to the allow-list so that some of these tests can be unskipped (since in an upcoming change the allow-list will be removed entirely anyway).
Change-Id: Id7bf40020cb5397e4cb8bbdefeed71745f58c4cc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428520
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This is some more final (🙃) cleanup before having CodeActions work for LSP-over-Legacy and run all the same tests. Includes:
- Adding some additional signatures to the SharedTestInterface
- Moving some setup code (that uses those methods) into the shared test mixins
- Add a field for `testPackageName` to allow tests to handle differences between the test package name between LSP/Legacy (something that should probably consolidated but is probably not for this CL)
- Move plugin tests back out of the shared code action tests (these require some additional abstraction to work the same across both kinds of tests - TODO added)
Change-Id: Iad25f1ff1e141d3315e9b972a6d0850e71d4a4bd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427443
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
The next CL will make CodeActions a shared handler (so it can run for both LSP and legacy servers). This moves the tests to a shared mixin (without any changes) to keep that change smaller and easier to review if the tests do end up requiring changes.
Not all tests are moved yet - plugins are not (because the plugin code is not the same across server types), not "Fix All" (because it will require some additional changes to not be LSP-specific compared to others).
Change-Id: Ib4727ef1b1cc5b96d98cdbd6e17bf4b7b2791e3e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428003
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This updates the `ApplyCodeActionCommandHandler` to be a "shared" command that can work for either kind of server (although it does not yet allow calling over DTD - the executeCommand handler still requires trusted callers).
It also moves all of the tests into a shared mixin so they will be run for both server kinds.
Change-Id: I831cc5a1a9feadc528e5b33b82e3469c60111dfa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427440
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Now that we have `defaultValue` against each argument, there was some redundancy here.
`isDefault` is redundant because if `hasArgument=false` it's always default, and if `hasArgument=true`, then `value` and `defaultValue` can be compared
When `hasArgument=false`, there's no value in duplicating `defaultValue` into `value`
Change-Id: I5cba055e56175565842efef815e08dd46e9ed36a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406621
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
If we get an editArgument request (usually via DDS), we should verify the connected editor supports edits and return an explicit message if not.
Also adds a `setUp()` declaration to `SharedTestInterface` so shared tests can set client capabilities instead of them being duplicated in each test class (and moves the same for ApplyEdit tests to this).
Change-Id: Ie6a85620b3b53663ce92e55d93e3aefcaf4dc980
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405342
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This is some minor refactoring to support the next CL that will enable the EditArgument request (the one that actually edits arguments, not the one that gets the list of arguments) over legacy.
It moves some code for sending LSP reverse-requests through the legacy server from test code into the actual server (and fixes that they weren't correctly wrapped in the 'lsp.handle' protocol classes) and updates the tests to use the shared interface ready.
Change-Id: I06492138645538072fb12f2fc2d424e214f8055b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404824
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
The legacy protocol does not currently version documents, so this adds some additional tests for that.
The tests are in the Legacy class and not the shared class because in LSP versions are not optional for opening/changing documents (they are optional in some of the other APIs, but not for open/change).
Change-Id: I7e75b813025b3f67ed143e1b3a8b717224bfa80f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404820
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
This moves all tests (without any changes) to a shared mixin, and then applies that mixin to the original test class. It also adds a new test class for LSP-over-Legacy, meaning the tests will now run for both protocols.
There are two failing tests when using LSP-over-Legacy so they are overridden with `@FailingTest()` temporarily.
Change-Id: I8c7efcf8f5d85d874b3483f4f3953d4d8b68a31a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404621
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
This is refactor extracted from an upcoming change (to make property editor tests run against both servers) to make that change smaller and easier to review.
There are some existing shared tests that run for both LSP and Legacy servers, but they currently do not touch much server API (one is for DTD and one tests reverse-requests). Migrating other tests (such as EditableArguments) requires some additional API be the same between the different test/server base classes.
This change adds an `abstract interface class SharedTestInterface` to serve as a common interface for methods that shared tests need to use that have different implementations between LSP and Legacy. For example, updating the overlays in an LSP-over-Legacy test needs to use the Legacy APIs for updating the overlay and not the LSP ones (so we can't just use the LSP methods like we would for calling something like getHover for LSP-over-Legacy).
It also:
- adds some new futures to the LSP test base to match the behaviour of the legacy one (wait for in-progress analysis)
- replaces the shared mixins with real base classes that implement the shared interface (for ex. `abstract class SharedLspOverLegacyTest extends LspOverLegacyTest implements SharedTestInterface`) to make it easier to create shared tests
- renames `sendLspRequest` to `sendLspRequestToClient` to make it clearer what direction this method is for
Change-Id: I070c2c005b11b9afd8a87aa22b04972a9dde2320
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404680
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
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>
Robert Nystrom