This will help identify issues like https://github.com/dart-lang/sdk/issues/55621 in future from the logs without needing to repro. Before, there was just a gap in the logs that wasn't obvious.
A sample log looks like:
```
1715080652096:Req:{"jsonrpc"::"2.0","id"::2,"result"::null,"clientRequestTime"::1715080652091}
1715080652253:Info:Running "pub upgrade" in "C::\Users\danny\AppData\Local\.dartServer\.plugin_manager\723cb7b2bec3011e09cd16421250ff7a\analyzer_plugin"
1715080653311:Info:Running "pub upgrade" took 0::00::01.057950
1715080653393:Res:{"id"::3,"jsonrpc"::"2.0","method"::"window/workDoneProgress/create","params"::{"token"::"ANALYZING"}}
```
(This was the only instance of `Process.runSync` in the server)
Change-Id: I2ccc5a7c538ae7a236a76df020c59014982a2e19
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/365602
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This has a few nice follow-on benefits:
* SnippetProducer no longer needs to rely on LinterContext.
* Many classes no longer need a PathContext passed around, or a
`String content`.
* BestPracticesVerifier has _3 fewer parameters_, and 1 fewer field.
* `_InvalidAccessVerifier._isInTestDirectory` no longer needs to be
`late`.
Change-Id: I0c2059feae22a75f7c706676cc228fa404dd0cf7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/365300
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Similar to `computeConstantValue`, an Expression or a
ConstructorDeclaration each have enough information on their fields
to calculate these values.
* This also lets us remove the now unused
`CorrectionProducer.getLinterContext`.
* Also two functions in prefer_const_constructors_in_immutibles can be
made static: `_hasConstConstructorInvocation` and
`_hasImmutableAnnotation` (so they get resorted in this change).
* Several rules no longer need to pass around a LinterContext.
Change-Id: I584c62e2673658c5004283842a6525eea89f805b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364964
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
We don't need any more information than what is found on an Expression,
in order to attempt to compute a constant value. This simplifies some
lint rules, a quick fix, and BestPracticesVerifier.
Change-Id: Id02b7988be28e7d8846642b1ab890290a4279743
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364821
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Basically, in `_visitInstanceCreation`, we make sure that, given an
expression like `(e) => C(e))`, that the function type of the outer
function expression is the same as the function type of `C(e)`.
And little cleanups:
* _extractElementsOfSimpleIdentifiers's returned Iterable is only used
to check whether it contains something, so change it to a Set.
* In `isFinalNode`, use `unParenthesized` to unwrap parens, rather
calling recursively.
* In `_Visitor`, we stored a `LinterContext`, but we only need the
TypeSystem object, so just store that for more terse code later.
* In `_visitInstanceCreation`, remove the local function, `matches`,
by checking whether all arguments are SimpleIdentifiers.
* Convert `isFinalElement` to an extension getter on `Expression`.
* Remove helper `isTearoffAssignable`. It is more concise and I think
easier to read to just use TypeSystem.isSubtypeOf on its own.
* When looking at a VariableDeclaration, get the type from the
declared element, which will include inference, rather than the
literal type annotation on the declaration.
Fixes https://github.com/dart-lang/linter/issues/4914
Fixes https://github.com/dart-lang/linter/issues/3516
Change-Id: I3e89ee4dc011473511d375b0ad324988232c8c96
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364628
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Just a lot of tidying in preparation for this code being public API
in the `analysis_server_plugin` package.
A big change:
* Convert `CorrectionProducer.assistArguments`,
`CorrectionProducer.fixArguments`, and
`CorrectionProducer.multiFixArguments` each from a `List<Object>?`
to a `List<String>?`. This should be a no-op.
Smaller changes:
* Make private: `AssistProcessor.generators`, `.multiGenerators`,
`.assistContext`, `.assists`,
* Make private: `CorrectionProducerContext.selectionOffset`,
`.selectionLength`, `.utils`, `.sessionHelper`, `.unitResult`,
`.applyingBulkFixes`, `.diagnostic`, `.node`, `.token`.
* Make private: `FixProcessor.fixContext`.
* Remove `CorrectionProducerContext.selectionEnd`, `.unit`, `.file`,
`.session`, `.workspace`, `.typeProvider`; these can be accessed via
getters.
* Remove unused `FixProcessor.computeFix()`.
* Make doc comments more idiomatic.
Change-Id: I100fe81aad612967191568fe207ff0b807f131b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364420
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
During FFI leaf calls, the Dart GC will not run. This means that we
can pass pointers into `TypedData` to FFI calls that take `Pointer`
arguments.
After this CL, we have three types of arguments that can flow into
`Pointer` argument in an FFI call:
* `Pointer`.
* `TypedData`: Any typed data including views.
* `_Compound`: A TypedData/Pointer and an offset in bytes.
The is only possible for `@Native external` functions, `asFunction`
does not support passing in `TypedData`. (See related GitHub issues
for discussion. TLDR: FFIgen should generate bindings without config.)
`.address` expressions on `TypedData` and `Array` elements do _not_
introduce bounds checks, even though `TypedData` and `Array` have
bounds information. E.g. `ffiNative(Uint8List(10)[20].address)` does
not throw.
Implementation details:
The CFE analyzes call-sites to `@Native external` functions. If the
arguments are `.address` expressions, it transforms the call site to
pass the compound or `TypedData`. If an additional offset needs to be
applied, the CFE constructs a new `_Compound` with the correct offset
in bytes.
The CFE then also creates a new `@Native external` function which have
`TypedData`s and `_Compound`s parameters. To avoid name clashes, these
functions are postfixed with `#` and `P`, `T`, or `C` for each Pointer
parameter.
TEST=pkg/vm/testcases/transformations/ffi/address_of_*
In the VM, `TypedData` arguments are passed as tagged values, and the
address is loaded inside the `FfiCallInstr`. `_Compound` arguments
turn into two IL definitions, one for the `TypedDataBase` (tagged),
and one for the offset in bytes (unboxed). The address is then loaded
inside the `FfiCallInstr` and the offset in bytes is applied.
Adding the offset in bytes required an extra temp register for ia32.
Also, it uncovered that the temp register in arm32 was conflicting
with the argument registers. However, TMP should suffice instead.
TEST=tests/ffi/address_of_array_generated_test.dart
TEST=tests/ffi/address_of_struct_generated_test.dart
TEST=tests/ffi/address_of_typeddata_generated_test.dart
Closes: https://github.com/dart-lang/sdk/issues/44589
Closes: https://github.com/dart-lang/sdk/issues/54771
CoreLibraryReviewExempt: VM only, unsupported in dart2wasm
Change-Id: I01fb428cfd6f9096a34689c2819c124a8003cb6b
Cq-Include-Trybots: dart/try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64c-try,vm-aot-win-release-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-debug-x64c-try,vm-win-release-ia32-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360882
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
It turns out the superclass of FixProcessor and AssistProcessor does
not buy us much:
* It has 1 method, which has one call site. The method is just two
statements, so that can be inlined.
* It has 10 public final fields:
* 5 of these were simply unused. Never referenced. 🤷
* The other 5 were referenced, but only directly in the direct
subclasses. These were set in the constructor, all coming from an
"assist context" or a "fix context", which is ultimately a more
direct way to reference them later in AssistProcessor and
FixProcessor.
Change-Id: I2b1d20f2f79206d744ad4edeedc3463d4523cf0f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364360
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
* Move the only 3 files from server_plugin to analysis_server_plugin.
* Copy some test infra into analysis_server_plugin. This is temporary,
as we need some shared test infra location.
Change-Id: If2b41d436c9d3051e590f60ae2eb7ab31e529321
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364161
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
In this CL, I do not change the signature of any EditBuilder code, as
it is public analyzer_plugin API. I only change doc comments and a few
impl nits here and there, to make more modern or idiomatic.
This CL is isolated this way so that when I do a copy to the
analysis_server_plugin package, the moves will be smaller, readable
diffs (almost zero diff, just imports and things like that).
Change-Id: Ibc8a5ddb9a679278dd7223a3e2d868fe21c4acef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363881
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Fixes https://github.com/dart-lang/sdk/issues/54247
The main piece of this CL is a rewrite of the `ignoredElements`
extension on CommentToken. This was a private extension getter, but
is made public now for two reasons: first is to reference it in tests,
and second is to make it available for a new lint rule in an upcoming
CL.
The new implementation drops all use of RegExp in order to more
precisely track when we may be at the end of a list of diagnostic
names and/or types. So the functional change here is that a diagnostic
name or type followed by whitespace instead of a comma marks the end
of the list. Any characters following that whitespace are not parsed,
as they are considered freeform comment text. While the new
implementation parses a comment code-unit-by-code-unit, it is still
a rather concise impl.
* Rename DiagnosticName and DiagnosticType to IgnoredDiagnosticName
and IgnoredDiagnosticType; these names make more sense to me,
specifically when they are used outside of the ignore_info.dart
file.
* I introduce a new test file, ignore_info_test, to capture these
various parsing particulars, that aren't specifically concerned with
matching an ignore comment with a diagnostic. A few test cases from
ErrorSuppressionTest are moved over here.
* A number of String (well, int, technically) helper functions are
moved to analyzer, along with their tests.
Fixes https://github.com/dart-lang/sdk/issues/54247
Change-Id: Ife08d448a4e03b8a55b183d88a3ea8d07812fbdc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363820
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This CL makes 3 different changes. I'm happy to split them out into
smaller CLs if you'd like me to.
The first is to add the class `NotImportedCompletionPass`, which is the
proposed replacement for the `NotImportedContributor`. I believe that
the pass is complete, but it currently doesn't contribute any
suggestions because the methods invoked by the two "operations" aren't
yet implemented.
That said, it does do real work in terms of searching through the
libraries that aren't imported. That work is currently duplicated by
the contributor, which would likely cause a performance degredation.
As a result, I've commented out the invocation of the pass (but have
verified that every test passes when it's not commented out).
The second is to start suggesting extension members from local
extensions. I'm not sure why, but prior to this CL they appear to have
only been suggested by the `NotImportedContributor`, which isn't
always run. I'm not sure where members from imported extensions are
currently being imported, but I'll figure that out before I remove
the `NotImportedContributor`.
The third is to change the element being compared against when
computing the inheritance distance. We were previously using the
container of the element being suggested, which caused the inheritance
distance to always be the same, no matter what the type hierarchy
looks like. We need better tests for relevance, but I didn't add any
in this CL.
Change-Id: Ic82a15cab5d022f89da234a8b31bc6bca34ccb5c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363760
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
VS Code uses the definition range to show a preview of the hovered item (when holding `ctrl`). We usually expand a variable declaration to the parent list so that this preview includes "var", "final" and/or any type.
However, if the list contains multiple variables, this resulted in us showing other variables in the preview too which is confusing/misleading.
This change keeps the original behaviour for VariableDeclarationLists with only a single variable, and otherwise keeps the specific variables range.
Fixes https://github.com/Dart-Code/Dart-Code/issues/4968
Change-Id: I9e3cfd672f51040b60f9791ba89a937d5733c628
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363280
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
There were some inconsistencies with insertion of "const" keywords when selecting colors from the color picker (eg. https://github.com/Dart-Code/Dart-Code/issues/5068).
Some of this was because some tests were testing different their own ranges that didn't match the ones the server would have provided (which meant the replacements were different).
This updates the tests to ensure all tests are using ranges that the server would have produced, changes to ensure any existing const keywords are included in the color expression (so if in future we allowed selecting named colours, existing const constructor keywords would be correctly removed) and fixes up some places where we didn't actually produce Colors (assigning top-level color variables in an initializer and constant patterns) but had tests assuming we did (which failed with the new checks).
Fixes https://github.com/Dart-Code/Dart-Code/issues/5068
Change-Id: Id8814a426df4cbc350febabd11c963447c4b0e73
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362801
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Srujan Gaddam