Contains quite a bit of refactoring. One big change is that `PubspecValidator.validate()` now also finds lints. Not sure we want this, but it seemed desirable to me. (Otherwise there was no central place to filter the lints through the ignores).
Change-Id: Ic8a463a9718fb29bec0d4f7e849b6da7799aebc6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311980
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Make this change broke some tests because the Fix All code which uses temporary overlays would revert the overlays but not immediately apply those changes (so the file state was out of sync) to compute edits.
To fix this, the step to revert overlays also immediately applies those changes (rather than leaving them until the whole operation has completed).
+ improve a variable name
Change-Id: Ic76b1c25d808c07e1d9d7e304babf6c79556dc36
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330100
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Previously the cancellation of Legacy and LSP requests were distinct. Sending a cancellation for a (legacy) "lsp.handle" request would not signal to the LSP handler to cancel.
This passes down the legacy requests cancellation token to be used by the LSP handler.
An alternative option would've been to support sending LSP cancellation methods through `lsp.handle` but so far our assumption has been that only _requests_ will be sent through `lsp.handle` and LSP's cancellation is a notification.
Change-Id: I74dc472011b86578d1b40b3034ac6a252d317f59
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330300
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
We already supported moving a single folder, which we expanded to each file inside it. This adds support for moving multiple distinct items at the same time (eg. dragging multiple files).
To support this, we need to track a full mapping of source/destination paths (because files might be moved from different levels into the same level - we can't compute destination paths from a single `newFile` variable), and also build a unique set of references that need updating (rather than building edits as we go - where we might try to edit the same reference from both ends).
Fixes https://github.com/Dart-Code/Dart-Code/issues/4533
This is currently only supported for LSP.
Change-Id: I3a8ab8b2dcb13451111d38d2ec0e2f69d6084e28
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329320
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This is a no-op refactor extracted from an upcoming change to enable the onWillRenameFiles request to work for LSP over the Legacy protocol.
It's mostly lifting some API from the LSP server to the base server and some into a new mixin (`LspVerifyEditHelpersMixin`) to provide the required methods for verifying edits without the usual LSP base test classes.
Change-Id: Id6084a80e3afb9c6af1a84d9e8a18d460b5f4cce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329980
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Previously the fuzzy score was only used for filtering, then we truncated using the relevance score. This change uses the fuzzy score for sorting prior to truncation (calling back to relevance if it's the same) too.
Currently, VS Code in non-web contexts does not truncate (it's set to 100,000 items) so this change will only impact web contexts (currently set to 1,000 items to reduce payload sizes). From my testing, the difference in the first 30 items that will be seen is negligible (there are some differences between the servers fuzzy score and VS Codes, but they tend to be when there are typos).
Change-Id: Ia10d1991085054cec9c0c1071a516c8aa286156c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329323
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Makes behaviour of changing leading indent or trailing newlines opt-in.
There are only slightly more uses of this method that don't want this (now opt-in) behaviour, although I think this is slightly less surprising (for example the trailing newline behaviour was to "add a trailing newline only if it didn't already exist" which I don't think was obvious).
Mostly this was just changing the defaults and all of the call sites. However, some call sites were getting the old behaviour (inc leading/trailing whitespace), but then calling `.trim()`, so I didn't add the opt-in there because it would then be removed (they won't show up in the diff because they didn't actually get any changes).
Change-Id: I2f05b3a6266aaad5ee510ce2a8bf269b26d42d5d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329540
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
This is a minor refactor that adds an explicit test for generating minimal edits without going through the formatter.
There's no change in behaviour - this is just to reduce the noise in a future CL for inserted/deleted commas to be supported in computing minimal diffs.
Change-Id: I3d254bd7a90ccc7b11700b54110ece040e837363
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329601
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Fixes some cases where the same keyword was being suggested twice
because we were completing at the beginning of a statement which can
also be the beginning of an expression.
Change-Id: I7c12da8caed14438f9f1a16fa7121d1cbd96b891
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/328769
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
The original goal of this change was to add the `switchExpression` test in `flutter_wrap_center_test.dart`, but it failed on Windows and exposed a bug that affected a number of assists/fixes.
The issue was that indentation was being rewritten using a regex:
src.replaceAll(RegExp('^$indentOld', multiLine: true), indentNew)
On Windows, when `indentOld` is an empty string, both `\r` and `\n` are matched individually, resulting in indentation being inserted between them:
foo\r \n bar
To fix this, I changed all places using a regex for replacement like this to call the existing `CorrectionUtils.replaceSourceIndent` method which did not have this problem. However, it made two different assumptions:
1. Indentation should always be updated on the first line (which if `indentOld` is an empty string, results in extra unwanted indentation being prepended to the string)
2. A new EOL should always be added at the end of the new code
To handle this, I added two new parameters and kept the original behaviour the same (although, I'm not sure if defaulting to adding a newline makes a lot of sense).
Change-Id: Ifa714da62c2541ba54a5376aa67149c90396dec2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/328220
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
We rely on the result of the `ConstantVisitor` to indicate whether we have an error or a valid constant value.
This CL changes `evaluationResult` to be a `Constant` and changes error reporting to occur at a POE for evaluating a constant.
Last few chunks of cleaning up the constant evaluator, woo!
Change-Id: Icd41a4fcbab0626df36c6a83cd60ecbb59c2dcf0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324573
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
In this CL, we shake things up a bit to separate the idea of a "doc
directive" and a "doc directive tag". The doc comments have been
updated to reflect the change. We have to recognize and support end-
tags, and recover when an end-tag is missing, a start-tag is missing,
or end-tags are out-of-order.
I also introduce a notion of doc directive nesting, in a way that
should not be computationally expensive, nor memory expensive.
Take this text as an example:
/// {@template foo}
/// Text.
/// {@inject-html}
/// <p>Some HTML.</p>
/// {@end-inject-html}
/// {@youtube ... }
/// {@endtemplate}
Notice the doc directives nested in the following way:
* template directive:
* text: "Text."
* inject-html directive:
* text: "<p>Some HTML.</p>"
* youtube directive
I want to avoid storing any blocks of text on the DocDirective nodes,
to avoid what could be very excessive memory usage. And if I want to
avoid storing the text, I think there is little benefit in storing the
data for these directives in a tree structure. In this CL, the data
is stored in one List, `docDirectives` on the CommentImpl:
* [0] - template directive, with data about its opening tag and
closing tag.
* [1] - inject-html directive, with data about its opening tag and
closing tag.
* [2] - youtube tag, with data about its singular tag.
For syntax highlighting purposes, there is no benefit to understanding
the nesting. And dartdoc currently gets all of the comment text from
the AST (or maybe from offsets in the actual text???) so I think there
is currently no downside to not capturing the nesting structure in
the CommentImpl instance.
But I can see in the future it might be better for dartdoc to consume
an API with the nesting structure, and that nesting structure does not
necessarily need to contain a copy of any text; it could contain some
sort of 'Text' data, with offsets of text contained in block doc
directives.
Change-Id: Ib58ab68fe80eea76ee7fa912d00fc69cc74f72d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/326883
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
TL;DR: This CL is shown to reduce memory usage by ~660MB when asking
for many completions.
Before this CL the analyzer cached `_ElementCompletionData` for every
element that have ever had it calculated. This itself contains new
strings, lists etc.
This was likely (although I'm guessing) done because it was asked for
it a lot: In my benchmarks, asking for at most 250 completions for each
of the first 200 files (twice) it was asked more than 113 million times.
Likely (extrapolating) it would take ~26 minutes creating these if not
cached.
In practise we don't need to ask for >113 million of them though.
Each request only wants at most, say, 100 results and likely fewer.
In the benchmark the `build` command was only run ~31% more times than
the number of caches. Building 100 results (again just using the
average) would take ~1.4 ms so hardly matters.
This CL removes the caching (including the field on many `Element`s),
and only calculates it when needed (i.e. on `build` as it's about to be
sent to the users IDE).
When asking for at most 250 completions in the first 1000 files in flutter/flutter (80+ contexts) we save ~669MB of heap usage, making
the heap usage decrease from ~8.34GB to ~7.69GB.
Details:
```
ExtensionElementImpl (package:analyzer/src/dart/element/element.dart) (bytes):
Difference at 95.0% confidence
-1185728.00 +/- 413.05
-8.36% +/- 0.00%
SimpleIdentifierImpl (package:analyzer/src/dart/ast/ast.dart) (bytes):
Difference at 95.0% confidence
-1187104.00 +/- 133935.98
-1.14% +/- 0.13%
_List (dart:core) (instances):
Difference at 95.0% confidence
-1204916.00 +/- 5571.96
-10.08% +/- 0.05%
ConstTopLevelVariableElementImpl (package:analyzer/src/dart/element/element.dart) (bytes):
Difference at 95.0% confidence
-1301800.00 +/- 12081.85
-7.93% +/- 0.07%
_OneByteString (dart:core) (instances):
Difference at 95.0% confidence
-1825992.50 +/- 186205.44
-28.49% +/- 2.91%
ConstructorElementImpl (package:analyzer/src/dart/element/element.dart) (bytes):
Difference at 95.0% confidence
-5518400.00 +/- 135361.21
-6.29% +/- 0.15%
ClassElementImpl (package:analyzer/src/dart/element/element.dart) (bytes):
Difference at 95.0% confidence
-6820704.00 +/- 21606.00
-6.34% +/- 0.02%
CompletionDefaultArgumentList (package:analysis_server/src/services/completion/dart/utilities.dart) (bytes):
Difference at 95.0% confidence
-9054304.00 +/- 6195.82
-100.00% +/- 0.07%
InterfaceTypeImpl (package:analyzer/src/dart/element/type.dart) (bytes):
Difference at 95.0% confidence
-9941600.00 +/- 776821.86
-2.32% +/- 0.18%
ElementLocationImpl (package:analyzer/src/dart/element/element.dart) (bytes):
Difference at 95.0% confidence
-10401472.00 +/- 111022.97
-46.19% +/- 0.49%
_TwoByteString (dart:core) (bytes):
Difference at 95.0% confidence
-11262480.00 +/- 227076.11
-11.77% +/- 0.24%
MethodElementImpl (package:analyzer/src/dart/element/element.dart) (bytes):
Difference at 95.0% confidence
-13877752.00 +/- 351565.94
-7.70% +/- 0.20%
_ElementDocumentation (package:analysis_server/src/services/completion/dart/suggestion_builder.dart) (bytes):
Difference at 95.0% confidence
-18954304.00 +/- 202259.10
-100.00% +/- 1.07%
_GrowableList (dart:core) (bytes):
Difference at 95.0% confidence
-23003968.00 +/- 3135.17
-25.12% +/- 0.00%
Location (package:analyzer_plugin/protocol/protocol_common.dart) (bytes):
Difference at 95.0% confidence
-63403200.00 +/- 444171.45
-99.68% +/- 0.70%
_List (dart:core) (bytes):
Difference at 95.0% confidence
-71587648.00 +/- 2062003.75
-5.54% +/- 0.16%
Element (package:analyzer_plugin/protocol/protocol_common.dart) (bytes):
Difference at 95.0% confidence
-79254080.00 +/- 555214.31
-100.00% +/- 0.70%
_ElementCompletionData (package:analysis_server/src/services/completion/dart/suggestion_builder.dart) (bytes):
Difference at 95.0% confidence
-110955712.00 +/- 777300.03
-100.00% +/- 0.70%
_OneByteString (dart:core) (bytes):
Difference at 95.0% confidence
-258932360.00 +/- 9586397.08
-35.00% +/- 1.30%
maxRSS:
Difference at 95.0% confidence
-327434240.00 +/- 121326188.82
-3.05% +/- 1.13%
currentRSS:
Difference at 95.0% confidence
-551153664.00 +/- 121286389.20
-5.34% +/- 1.18%
heapCapacity:
Difference at 95.0% confidence
-555710464.00 +/- 164879781.95
-5.75% +/- 1.71%
heapUsage:
Difference at 95.0% confidence
-701868280.00 +/- 8054728.47
-7.84% +/- 0.09%
```
Change-Id: I3b3631520d56ab36dabaee913e802aa1b7ebacfe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/326822
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Sigurd Meldgaard