Path exclusions should ideally be defined inside a project's
`analysis_options.yaml` file, rather than being added programatically.
Plus, there's a bug with the constructor that causes this parameter to
be completely ignored anyways, so it's been obsolete and non-functional
for a while now. `getExcludedGlobs` in the `_ContextLocator` handles
parsing and adding excluded paths from the analysis server already, so
we should look into deprecating and removing this parameter.
Change-Id: I6c023041c7bb5fa4cb9dedc629afa4ea6ecb63d7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/511160
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
When we built the added the original interactive refactors support, we supported the client telling the server which field kinds it can prompt for, so that if there were questions the client didn't support that did not have default values, the refactor could be hidden.
However, we only implemented one such refactor (Move to File) and the only field (destination URI) has a default value, so the client capabilities never have any effect.
Since we're replacing this support with the new "Interactive Forms" and don't intend to create any new refactors using the old system, this code is all redundant and therefore can be deleted.
Change-Id: Ic940dc54e325ccab05b28f496dbedee76e17e0b4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/509460
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This adds support for the `command/resolve` request to support the new Interactive Forms functionality, and updates the refactor processes to use it instead of the original protocol when the client indicates support.
Currently only the "Move to File" refactor uses this functionality (matching the previous version) and it requires the `dart.experimentalInteractiveForms` setting be enabled for it to be used (in case we find bugs while developing the front-end that require incompatible changes).
Fixes https://github.com/dart-lang/sdk/issues/63371
Change-Id: I414a41fa2958ca9dcefe097f35ba28f3fa2fe367
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/508121
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This fixes a bug where an interactive form where all fields have defaults would be considered complete immediately, so we'd never present the fields to the client.
It also includes some minor refactoring extracted from a future CL that implements command/resolve and supports Interactive Forms in refactors in an attempt to reduce the size of that change to aid reviewing.
Change-Id: I176fe25dbb0b610d69617fa04562b0d3ce571642
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/508220
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This updates the interactive forms classes to reflect recent changes.
Previously, we would only send back the outstanding fields to the client, and it would only provide answers for the same.
Now, the server will always send all fields to the client, and the client will always provide all answers. Answers are looked up IDs (a new field on `FormField`, and `FormAnswer` which wraps the answer) rather than rely on indexes.
Fields can also now be required.
Change-Id: If4aa1f9a18fa873e83cb0ea1fd74c2e42cd2fa1f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/508103
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
We have comments in some generated files suggesting that the user use
this script to regenerate them, but the script previously only worked
on Macs. This CL fixes the script so that it work with Linux too.
Change-Id: I4b42b843513247b16bbbcd0b7c85f82d6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/507540
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
(Part of https://github.com/dart-lang/sdk/issues/63288)
This change migrates the packages owned by the developer experience
team to use the new constructor declaration syntax, described in
https://github.com/dart-lang/language/blob/main/accepted/future-releases/primary-constructors/feature-specification.md#abbreviations-of-in-body-constructor-declarations.
This change was performed in an automated fashion, by (a) bumping the
packages' SDK constraints to `3.13.0-0`, (b) enabling the lints
`unnecessary_type_name_in_constructor` and
`unnecessary_const_in_enum_constructor`, (c) fixing the resulting lint
failures using `dart fix`, and then (d) reformatting the affected
files.
To ease code review, I've reverted unrelated formatting changes.
Since this change requires bumping SDK constaints to `3.13.0-0`, it
was only performed on packages that are *not* published on
pub. (Packages that *are* published on pub should remain on lower
language versions until at least after the stable version of 3.13 is
released, so that we don't block users on the stable channel from
receiving updates to those packages.)
Change-Id: Ibb4daebafd239da58251e838ea6a3f336a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505046
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
SLSA-Policy-Verified: SLSA Policy Verification Service <devtools-gerritcodereview-exitgate@google.com>
Extension type augmentations do not have representation declarations, so
an `ExtensionTypeDeclaration` can no longer always expose its name
through a primary constructor. Add `ExtensionTypeDeclaration.namePart`
as the canonical API for the declared name and type parameters, and keep
`primaryConstructor` as a deprecated compatibility API for introductory
declarations.
Report `extensionTypeAugmentationHasRepresentation` when an augmentation
writes representation syntax. This keeps the parser recovery explicit:
the augmentation still gets a plain `namePart`, while the invalid
representation is diagnosed instead of being modeled as the
declaration's primary constructor.
Synthesize recovery representation and primary constructor fragments
only for the element model when an extension type has no introductory
declaration.
Migrate analyzer, analysis server, analyzer plugin, and linter clients
to read extension type names and type parameters from `namePart`.
Change-Id: I59dd957ac38f087c861b993caf246986dcdac713
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505067
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This adds some classes for processing interactive forms (I'm splitting this from the CL that updates refactorings to simplify reviewing).
The `InteractiveForm` class managers a whole form (all fields), and processes a set of fields/answers that come back from the client (which may be the full set, or a subset). It produces a list of the outstanding fields (which need to be go back to the client) as well as maintaining the full set of answers for the master field list (to use as arguments for a command, for example).
I've also commented out enums/lists from the protocol code for now, because I made the field kinds `sealed` so we get exhaustiveness checking in the validation, but want to delay fully implementing those for now.
See https://github.com/dart-lang/sdk/issues/63371
Change-Id: Icc4dcea8c9eab20ffbdd19ebbc69e1285464f7ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505683
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
In the new interactive forms, some of the enums can be combined (eg. `FileExistence.New | FileExisting.Existing`). This adds a flag that produces some helper functions for `hasFlag()` and `combine()`.
These helpers are not used outside of tests in this CL but will be used in a future CL (which I'm trying to avoid getting too big to simplify reviewing).
Change-Id: I1b5f06f05d96781c4bc6a246a0a9309a786c1987
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505320
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
The session log normalizer replaces known paths/URIs in JSON but doesn't take into account different URI encoding between the client and the server. For example VS Code will encode ampersands whereas Dart does not:
```
file:///c:/uri&encoding&quirks
file:///c:/uri%26encoding%26quirks
```
This means not all file URIs are correctly normalized.
Adding additional groups for each potentially-encoded characters make the regex many times slower (the benchmark test here goes from around 25ms to over 1s per iteration), so instead this change has the normalizer accept the original JSON map and uses jsonEncode()s `toEncodable` option to normalize any URIs (by converting them to their file paths and then encoding using Dart's Uri class) so they will always be consistent before the replacement.
(I tried doing the replacement also in `toEncodable`, but invoking the regex many times also slowed things down a lot).
There is a small time increase (2-3ms) for a payload of 2MB. The "before" times quoted here are slightly higher than previously quoted, but that's because `jsonEncode()` was previously done inside `SessionLoggerFileSink` (and therefore excluded from the timings before), but is now done inside the normalizer to allow normalizing the URI escaping.
Replacing 250 paths in payload of 2097152 bytes
Iteration #1, First: 57ms, Rest: 40ms
Iteration #2, First: 49ms, Rest: 41ms
Iteration #3, First: 45ms, Rest: 41ms
Iteration #4, First: 47ms, Rest: 40ms
Iteration #5, First: 40ms, Rest: 40ms
Replacing 250 paths in payload of 2097152 bytes
Iteration #1, First: 59ms, Rest: 43ms
Iteration #2, First: 53ms, Rest: 44ms
Iteration #3, First: 52ms, Rest: 44ms
Iteration #4, First: 49ms, Rest: 43ms
Iteration #5, First: 49ms, Rest: 43ms
Fixes https://github.com/dart-lang/sdk/issues/63331
Change-Id: Ice2dc7ceceaa6c08e2ff634d7564efe9f0f7de44
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/502940
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
The previous CL changes from just replacing paths, to handling URIs + filePaths separately.
This updates the committed scenario files so:
`file://{{workspaceFolder-0}}` becomes just `{{workspaceFolder-0}}` (since this token implies the file URI), and `{{workspaceFolder-0}}` (without the prefix) becomes `{{workspaceFolder-0:filePath}}` (to signify it was a file path).
Even with these changes, they fail for different reasons (invalid git hashes, mismatches in expected vs actual requests) so I there is still more work to do here.
Change-Id: Ic1543ab96ca002073540ef034adecb4c9ea57e58
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501521
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
For posix paths, replacing the file path during normalization and then swapping it back later works for both paths and URIs, because a file URI just contains the file path verbatim:
file:///foo/bar/baz
However that's not the case for Windows:
C:\foo\bar\baz
file:///c:/foo/bar/baz
So when normalizing, we need to know if we normalized a URI or a file path, so that we can reverse it later.
With this change, we'll use `{{workspaceFolder-0}}` for the URI, and `{{workspaceFolder-0:filePath}}` for the file path. Then when reversing, we can easily put the correct one back.
This also updates the log replace/scenarios to use the LogNormalizer to perform the denormalization so they don't have to have duplicated logic about what to restore.
I've also updated the existing committed scenarios (EDIT: moved this to a separate CL because Gerrit is falling over) - although even with those changes, they all fail for different reasons (invalid git hashes, mismatches in expected vs actual requests) so I think there is still more work to do here.
Fixes https://github.com/dart-lang/sdk/issues/63330
Change-Id: Ib4c4aabe2c7c0d089bd620bdf00de37acde25f52
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501600
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This some basic scaffolding for the migrate tool. We'll add a protocol to the analysis server with the following parameters and result:
Parameters
- `uris`: Workspaces/packages to be migrated
Result
- `summary`: Information about fixes that could not be applied automatically. (e.g. if there was a conflict) or information about what fixes were applied and what SDK version the pubspec has been changed to.
- `edit`: A list of edits to be applied.
There are no interesting tests yet, but I hope to have a suite of tests for the next change.
Fixes: https://github.com/dart-lang/sdk/issues/63247
Change-Id: I77508720acb17af5ec86675fd3f3045e2a610bf2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/496801
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
The Flutter Widget Preview feature is currently implemented within
Flutter Tools, which is responsible for detecting widget preview
annotations in the user's project. When previews are detected, the
Flutter Tool injects code generated based on the detected previews into an artificial widget_preview_scaffold project and performs a hot reload to render updates to the preview set in the scaffold application.
`package:analyzer` is currently being used to detect previews, but this comes with a significant amount of memory overhead. Since widget
previews are mostly being used from within IDEs which already have an
active analysis server, moving widget preview detection into the DAS
will remove the need for creating an additional analysis context in the Flutter Tool itself.
This change includes the initial work to move widget preview detection
into the DAS. It utilizes a pull-based mechanism, where the Flutter Tool listens for file system events and then queries the DAS using the `dart/textDocument/getFlutterWidgetPreviews` and `dart/workspace/getFlutterWidgetPreviews` LSP methods.
Each reported preview contains some generated code based on the annotation used to define the preview. This code has all constants from the original annotation evaluated to either primitive values or constant expressions with namespaces applied to each symbol, allowing for the Flutter Tool to inject this code directly when updating the generated code in the scaffold project.
Towards https://github.com/flutter/flutter/issues/179584
Change-Id: I043cb3235a66b25dda3f852ca7f147bff0e1e537
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/478100
Auto-Submit: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
The `Match.operator[]` does the same thing and is
generally recommended (and shorter).
(I want to deprecate `group` and `groups`)
Tested: Refactoring.
CoreLibraryReviewExempt: Calling equivalent function.
Change-Id: I4c758968ae622fe16b7322be1b29b05b91e7fcd9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489021
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
Add `ClassBody.members` and `EnumBody.constants`/`members` so callers
can access class and enum contents through the common body interfaces
instead of pattern matching on `BlockClassBody` and `BlockEnumBody`.
Implement empty node lists for empty class and enum bodies, update the
public API, and migrate analyzer, analysis_server, analyzer_plugin, and
linter code to use the new accessors directly. This removes the ad hoc
`members2` helper extensions and a large amount of repeated `tryCast`
and `switch` logic, making body traversal more uniform.
Change-Id: I51d75f2253c7e6f75efecae84bf0443ff5eb6788
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/488263
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Work towards https://github.com/dart-lang/sdk/issues/61868
This adds a new notification type, 'PluginPrint'. There are several
fields and variables then named 'pluginPrint' or 'print', and I am
definitely open to changing these names, but this is the best one that
I thought of.
PluginPrint has three fields: The name of the plugin that printed, the
message that was printed, and the timestamp.
We wrap each plugin's AnalysisRule invocations with a zone, so that
the `print` handler can know the name of the plugin. The prints are
caught and sent to the server isolate as Notifications. The
PluginIsolate then stores the collected prints. The Plugins Insights
page can then retrieve them and display them.
Manual testing, with 1000 libraries that get new lint reported once per
file, I did not observe a negative performance impact.
Change-Id: Id800ae41781c15ce89d06b563878c2b0edae43d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/486827
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This copies much of the logic from the normalize.dart standalone script.
It normalizes "workspace folders," context roots, and the Dart SDK path.
I tweaked the SessionLogger constructor to instantiate its own
`SessionLoggerInMemorySink` rather than the caller needing to call it.
And then the `SessionLoggerInMemorySink` instantiates the
`SessionLoggerFileSink` so that the caller doesn't need to worry about that. This lets the sink and the nextLogger each be final, and removes
details about what I consider "SessionLogger implementation details"
from the caller's concern.
When a SessionLogger is instantiated, it creates a LogNormalizer which
the SessionLoggerSinks use to normalize log entries.
I almost used
`WorkspacePackage.packagesAvailableTo(String libraryPath)`, but it
seemed unnecessary to require a file path (there is a TODO about
considering files in/out of the 'lib' folder, but not necessary for this
work). So I made `Workspace.packages`, as most of the subclasses
already implemented this concept.
Bug: https://github.com/dart-lang/sdk/issues/62697
Change-Id: Ic57fec490414cef7028c290614363623e88f5c15
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/485260
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This is the first CL in what will probably turn out to be a fairly large
number of CLs. The goal is to get the relevance table generator up to
date so that we could then use it to generate new tables, thereby,
hopefully, improving the ranking of completion suggestions.
It includes two sets of changes, and if it would be easier to review I
can split it into two CLs.
The first set of changes is to add visit methods for all of the AST
node classes. While the use of a recursive visitor doesn't allow the
compiler to let us know about missing visit methods, I temporarily
changed the superclass in order to ensure that all of them are
overridden. I want to do that so that we can know when a lack of data
collection is intentional vs. just an oversight.
Which is where the second set of changes comes in. This adds a
mechanism to report if the generator ever sees a keyword or identifier
for which data should have been collected but wasn't. It does this by
recording all such tokens before visiting the AST, and then removing
tokens for which data is collected and tokens that we've explicitly
decided we didn't need data for.
In future CLs I want to get it to a place where we can validate that
the generator and the code completion passes are using the same set of
keys for the generated tables. I also want to add reporting if we have
failed to record data for an expected key because that would indicate
that the corpus used to build the tables is incomplete in its coverate.
Change-Id: I9cb4b729f558884e7321abd741e7b8dc85c997ca
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/486260
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
My previous CL made some LSP tests run with String IDs instead of ints, but those tests don't appear to go through the session logger so did not fail with the casts here.
This change adds a base integration test that also uses string IDs, which did fail on the cast, so I've updated the session logger to use `Either2<int, String>` for IDs instead.
Fixes https://github.com/dart-lang/sdk/issues/62442
Change-Id: Iee582e9ce2b8b5a1127120c987670a679d2ca76c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/473260
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
I don't have a lot of confidence that the data being collected is what
needs to be collected in each of these cases, but I believe that I've
correctly identified the visit methods that need to be collecting data.
I think we need to have a test that ensures that the keys being used in
this table generator are the same as the keys being used by the
relevance feature computer (as computed by the in-scope pass). I don't
know exactly what that test would look like, but that would ensure that
(a) the keys are the same and (b) the table generator isn't missing
keys being used by the completion engine or collecting data that isn't
being used.
I also fixed a bug that prevented this generator from running and tested
to ensure that it could run over several example packages.
Change-Id: I0c0cff148d288942e39ee876e89cdf9d1468f59d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/473780
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This still isn't complete. It only removes file changes when a file is
closed and it ought to remove them when a file is saved, but I think
this is a minor enough hole that we can land this now and work on it
later.
It also still doesn't handle the legacy protocol.
Change-Id: If07d20fe61cc3d03167f9a930b67ddd59b8e97e1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471860
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Previously, the result of calling `toJson()` could still include some classes (like `List` and `EitherX<>` unions). This didn't cause issues because `jsonEncode()` would handle those types anyway (calling `toJson()` recursively), however changes to support the log replay are looking inside the returned Map and should see consistent types.
Change-Id: Ie52a7251ee10579de8ce605584feb82abc15eb8d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/472900
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
- Return messages that fail to parse with the error response.
- Add support for open subdirs to projects, speeds up SDK replays.
- Fix up normalize script when package deps are subdirs of the project.
- Re-record the sdk scenario with only pkgs/analysis_server open.
- Fix some bad hard coded responses that were failing to parse.
- Add extra context (extra messages recieved) when scenarios fail.
Change-Id: Idfdcde70776967c2d419e76a21a6c6d1c59902f7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/472700
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
Auto-Submit: Jake Macdonald <jakemac@google.com>
- Adds support for normalizing package dependency roots using the
package config
- Updates the project generator so it can handle multiple project roots
and analysis contexts, as well as package configs.
- Handle sending watcher messages to the analyzer.
Removes the basic "initialization" scenario that didn't have any real
value other than being an example github project scenario.
Change-Id: I5b49effe524d325360a754296f4cc3578ad49527
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/471360
Commit-Queue: Jake Macdonald <jakemac@google.com>
Auto-Submit: Jake Macdonald <jakemac@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Kallen Tu