Scripts that load logs and results produced by package test_runner
will print more error details when a results file does not contain
a valid JSON map on each line.
Change-Id: Ia2789dd5c0bf565eb74f6960ec8783201ff9f0ae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359220
Commit-Queue: William Hesse <whesse@google.com>
Reviewed-by: Jonas Termansen <sortie@google.com>
I renamed a few classes, then sorted them, so the diff is big.
Broadly, here is a summary of the code before: two "ignore diagnostic"
correction producers were in place, and the one that inserts a
"ignore_for_file" comment (IgnoreDiagnosticInFile) uses a
`CorrectionUtils.getInsertionLocationIgnoreForFile` function to get an
InsertionLocation object, then passes that to `_computeEdit()`, a
method defined in a parent class. `_computeEdit()` takes that
InsertionLocation object, and uses LineInfo, to determine whether we
need to _append_ to an existing ignore-comment (in which case all
other InsertionLocation information is discarded), or write a full
comment, and use the InsertionLocation prefix and suffix.
To me this was a pretty complicated set-up: IgnoreDiagnosticInFile
calls out to a function defined in a separate library to do "the first
half" of the location calculation. Then the `_computeEdit()` function
has to finish the job, which might involve ignoring computations done
in the first half.
I've changed it to instead be the following design: Each class,
IgnoreDiagnosticInFile and IgnoreDiagnosticOnLine, wholly implements
`compute()` (removing the parent `_computeEdit()` method). The former
class has the much bigger task of finding an appropriate position near
the top of the file in which it can insert a comment. Instead of
tracking a lot of variables along the way, this code quickly calls out
to `insertAt()` as soon as it knows where it is inserting, whether it is
appending, and whether we need a newline before the comment, or after.
So everything about the offset, possible prefix, suffix, and decision
to-append is done in one place. The latter class,
IgnoreDiagnosticOnLine, has a much simpler `compute()`, the most
complex part is determining whether to append to an existing comment.
There is not much duplicated. The `insertAt()` method is defined in
the parent class; it is nice to have a method that is agnostic to the
ignore comment being inserted; it just calculates an indent, and
writes the comment.
Summary of changes:
* AbstractIgnoreDiagnostic, the parent of the two mentioned classes,
and IgnoreDiagnosticInAnalysisOptionsFile, is actually a base class,
shouldn't be public, and it's `_computeEdit()` was not relevant for
the 3rd subclass. So I rename it to _BaseIgnoreDiagnostic, and add a
new small class in between: _DartIgnoreDiagnostic. The new class
defines `insertAt()` (only relevant for Dart files), and requires
that subclasses define their `ignorePrefix`.
* `_isCodeUnignorable` is changed from a method to a getter.
* `DartEditBuilderImpl._linePrefix` is moved to an extension on ResolvedUnitResult, along with some helpers. It is moved to `src/` so
not technically part of public API. It should be moved to
server_plugin soon.
* `getInsertionLocationIgnoreForFile` and `getLinePrefix` are removed.
Change-Id: I4757ea2d8a3b43eeec0c8895c5c1fb3edc3ca8a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361006
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This is a step towards restricting use of `.error` and `.result` in `ErrorOr` to avoid runtime errors (or at least, the wrong runtime errors when error occur).
The reason for adding `.result` was to reduce excessive nesting when functions required the "result" version of multiple things in a chain. This is now handled better using records - we can do:
```
(foo1, foo2).mapResults((foo1, foo2) => ...);
```
This will execute the function only if both values are results and otherwise return the first error. These extensions exist for 2, 3, and 4 field records.
Additionally, I added Sync() versions of several of the helper methods to avoid using `FutureOr` (something that has caused problems with error handline in the past) and needing to `await` in synchronous code that just wants to map results.
This CL does not migrate _all_ uses of `.result` as it would make the CL much larger. Assuming the approach seems good and we will land this, I will migrate the rest.
Change-Id: Ib3f4c390818a8a26467d27c2472f6435055a6bd7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361141
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
ErrorOr is not related to (or used by) the spec or generated spec classes. It's a utility class for the server implementations of LSP handlers.
I'd like to extend this class and add some extensions (that require access to private fields) and don't feel it belongs in `package:language_server_protocol`.
(note: `package:language_server_protocol` is not published so I believe removing it from there is not a breaking change)
Change-Id: I06799b18d61d123364711a549231134c792e70e2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361140
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
AstNode.setProperty is expensive, since it causes a map to be
allocated for each AST node it's used with. It's more performant (and
IMHO clearer) to simply store a pointer to the bodyContext in the
FunctionBodyImpl.
Change-Id: I3e8856144a6428d408335f8c9b9c5b689e44fc92
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361041
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Currently for `printInt` below:
void main() {
void printInt(int i) {
print(i);
}
[1, 2, 3, 4, 5].forEach(printInt);
}
We generate the name `$main closure at file:///...`.
With this change, when a closure is a function declaration, we use its
name in the generated name. For the example above we now generate:
`$main closure printInt at file:///...`.
Fixes#55279.
Change-Id: Ia5e8cb512c24ac33be4920c505b9ef33514b10c2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361062
Reviewed-by: Jackson Gardner <jacksongardner@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
constant.
As const-only parameters are not planned, see
https://github.com/dart-lang/language/issues/1684, an annotation with analyzer support could help a bit at least, see
https://github.com/dart-lang/sdk/issues/29381.
The motivation is to enforce const arguments for methods annotated with
`@ResourceIdentifier`, to be able to record the argument values at build time, see https://dart-review.googlesource.com/c/sdk/+/329961.
Tested: pkg/analyzer/test/src/diagnostics/const_argument_test.dart
Change-Id: I2b8d2dce0c899fc0caa4985d892a5d031c747521
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357701
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Moritz Sümmermann <mosum@google.com>
Reviewed-by: Marya Belanger <mbelanger@google.com>
- Assertions are enabled in the compiler itself. DDC already enables
assertions in the test code by default.
- Runs tests in d8.
Change-Id: Ibdf285d9ab182c3859f4724b4c0740579d6d2377
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349361
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: William Hesse <whesse@google.com>
Type constraint generation requires construction of the `Future` type
in order to handle `FutureOr` input types. This CL adds a shared
method for creating `Future` types with the given type argument and
uses the shared method in the constraint generation methods in both
the Analyzer and the CFE.
Part of https://github.com/dart-lang/sdk/issues/54902
Change-Id: I4efa20b39243e8985c3edb819fa6afcf627a645c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359820
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
This CL abstracts the notion of a type parameter that the compiler
should infer for. Since the notion of such a parameter is used
throughout the constraint generation, it is represented as another
type variable in the shared interfaces, similarly to Type, TypeSchema,
and other similar abstractions.
The affected regions of constraint generation are the central points
of that algorithm and don't require additional tests since any test
that requires non-trivial type inference exercises a pass through
those points.
Part of https://github.com/dart-lang/sdk/issues/54902
Change-Id: Idc7fe13952c40e748e761cc90ab74e7ccf45ee50
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357611
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
This CL also removes unified analytics code from DDS and DevTools server.
Bug: https://github.com/dart-lang/sdk/issues/55280
Change-Id: I6f1c56cb8dce6f611ee73de7081c0a9cd43187c4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360221
Commit-Queue: Kenzie Davisson <kenzieschmoll@google.com>
Reviewed-by: Elias Yishak <eliasyishak@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
- Add a runtime flag to enable checks on non-nullable APIs that
return values from non-static JavaScript interop.
- Call a new helper method at the call site when these APIs are
detected to perform the null check.
- Add test file for the cases we can detect and enforce.
NOTE: This does not make non-static JavaScript interop sound.
This only adds more checks to enforce soundness with respect
to nullability in some cases. There are still holes that will
never be closed due to the permissive nature of this form of
JavaScript interop.
Change-Id: I2f88d1543a683fdc84d764e2b0eaafeb0ca73107
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358581
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
`addMethodInsert` and `addCaseClauseAtEndInsert` are only used by code
which combines them with DartEditBuilder, so the change is pretty
simple. A few utilities are needed in DartEditBuilder, which are also
not complicated, such as a utility for getting the existing indentation
string on a line. Using this utility in the shared code should improve
someo of the existing fixes and assists, which didn't use it before.
The same goes for looking at whether left and right brackets are
synthetic. So this change comes with a few minor improvements.
Change-Id: I5b235021ac234b4a50b09dca6d14a28b3f163e5e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360980
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This is in preparation for replacing the NotImportedContributor.
Change-Id: Ic3445372698ceb49811523c35c62cd03490e8399
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360901
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
The original code here was accessing `.result` on the result of `requireResolvedUnit` without checking it was an error. This meant if `requireResolvedUnit` failed, we would throw a new exception ("Value is not a result") instead of returning the original one.
Change-Id: I952edaac555eb4fccfb885f3f3c5bffc26ba0e90
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360880
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Change-Id: I68a3f28c78c6241f471c985e1fa836019b1a7dc9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360881
Commit-Queue: Ömer Ağacan <omersa@google.com>
Reviewed-by: Kevin Moore <kevmoo@google.com>
`Source.text` is the cached `utf8.decode(source)`. Use it instead of
decoding `source` for each `assert` statement.
Change-Id: I118992d8e71d75f048064e12aed32ff396273425
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360820
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
Say what those offsets are for, instead of what they're not for.
Change-Id: Ia8d0ef119386188d6d880a480d609bb7a324df8e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360643
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
I believe that this is the right thing to do from a UX perspective, but
this change could potentially cause flakiness in tests. There's support
in the LSP path for setting a bigger budget, but not for legacy based
tests (at least not yet).
Change-Id: I76353fd3e4cb4cf8fb7a55c27a3353415143dc06
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360740
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Avoids the use of one method with multiple code paths and returns
to handle any situation because it becomes very hard to reason
about what original source code leads to each path.
Fixes: https://github.com/dart-lang/sdk/issues/54463
Change-Id: I8158ae2a79e0f627f0703e2e253b4406022fc84b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357208
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
