This reverts commit 43db92d9ed.
Revert "Tweaks for comments of classes around augmentations."
This reverts commit 6d0fb5094f.
Change-Id: I9508169158d536302579346defebf696edfe4bec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/315660
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
In following CLs I plan to add `final` and `sealed` modifiers.
Change-Id: Ia6b9091034ed9f8f6f8576dbe6029d7e72a9d6b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/315249
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Previously, flow analysis used three classes to keep track of
information about expressions that have been visited:
- ExpressionInfo, to keep track of information about expressions whose
truth or falsity has an effect on flow analysis (such as `== null`
checks, `is` checks, and combinations thereof those using `&&` and
`||`), as well as identifying the literal expression `Null`.
- ReferenceWithType, to keep track of information about expressions
that represent something that can be promoted (references to
variables and fields), as well as the static type of the expression.
- EqualityInfo, which wrapped ExpressionInfo and ReferenceWithType,
and also redundantly stored the static type of the expression.
These have now been combined into a single class hierarchy with a base
class called `ExpressionInfo`. This makes the code easier to reason
about, because it is no longer necessary to think about which of the
three above classes is needed in a given situation. Also, it helps
prepare for a follow-up CL in which I plan to refactor how flow
analysis gathers this information; the refactor will be easier with
just a single class hierarchy of information to be gathered.
This required a modest expansion of the API to flow analysis to
include more static types, since previously, static types weren't
needed by the ExpressionInfo class.
Change-Id: Id3de8b19049f8d920ebe85ab58c624ae3e55f226
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304211
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The actual removal will happen in (to be rebased on top of this):
https://dart-review.googlesource.com/c/sdk/+/303280
Fixing direct uses of the deprecated `Identifier get name` is apparently
only about a half of the work. I should have followed my own advice to
the clients and prepared better.
These changes are necessary because we will stop creating and
visiting `Identifier`, and so `NamedType` should be handled directly.
Change-Id: Ie07b75b15eab277ed6c9b29a838561a7eb71f588
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303425
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
The removal itself will happen in a separate CL, as a breaking change.
But we still had a few places that relied on `SimpleIdentifier` being visited.
Change-Id: I8f554cec77858cb8bc7deb29d98f0d6def8685ee
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303008
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
I am concerned about the tests marked with TODOs because I don't know
why the behavior changed. It's possible that the contributor is still
producing the suggestions but that they're being filtered out later.
I checked using the debugger, and they're not in the list of results
that we're getting back, so it seems likely that they represent real
bugs. If we just weren't detecting the bug before then we're good.
But I can't think how rewriting the tests could effect what the user
is seeing, so I think it's safe to make these changes.
Change-Id: If72042b7ec24b97994a12449c811213b8f708f72
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303000
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
I'm actually not sure we are doing the right thing for general
aliases (that can now be nested arbitrarily deep), but at least
this commit avoids throwing an exception.
Change-Id: Ide3f7035b476f79fff5c542e2ddbb342045a874e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301983
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Ilya Yanok <yanok@google.com>
Null-aware calls, like `v?.extMethod()` don't require `v` to be
non-nullable, even if `extMethod` was declared on a non-nullable type.
Tested: added a new test.
Change-Id: I113ecde21e480bcd467367d0954b721c2ad9f7e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301982
Reviewed-by: Paul Berry <paulberry@google.com>
It looks like it's failing as a result of my previous changes
to make public function/method arguments nullable by default.
I probably missed it, since it doesn't run with assertions
enabled.
Change-Id: Ifcf045d778302a0be771c3a75c94cec97cbf8060
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301981
Reviewed-by: Paul Berry <paulberry@google.com>
Using Identifier expression inside NamedType does not make sense.
Change-Id: I4a61d2b472fd66fb7c5e6e92a80cccb391b06d49
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/294920
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This will help with any references to deprecated Hints in other
codebases, like nnbd_migraion or linter...
Change-Id: Iccd43d8e3113e6e3e0e458bf959c7f4d48185f35
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/294182
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This fixes a minor bug in flow analysis which was preventing it from
recognizing when a switch statement was trivially exhaustive, meaning
one of its reachable cases was guaranteed to always match.
This mostly addresses
https://github.com/dart-lang/language/issues/2980, but flow analysis
still fails to recognize that:
- A list pattern containing a just a single rest pattern always
matches (unless the rest pattern has a subpattern that may fail to
match).
- A null check pattern always matches if its subpattern always matches
and the matched value type is non-nullable.
- The relational pattern `!= null` always matches if its subpattern
always matches and the matched value type is non-nullable.
Fortunately, these drawbacks are small and don't lead to unsoundness.
I'll try to address them in follow up CLs.
Bug: https://github.com/dart-lang/language/issues/2980
Change-Id: Ie9f8564cde66a5a2c41114033ca3ff0e1a0f139a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293860
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
These were flagged as unused by `unreachable_from_main` but they are
used by `dart:mirrors`.
Change-Id: I3637b4b234f0bf7fe795977cb7ec6a1ca5449810
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293006
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This reverts commit 0ef7ed2ebd.
Reason for revert: Tests are failing in google3 - linter needs to be bumped
Original change's description:
> [analyzer] Move 4 more HintCodes to be WarningCodes, UNUSED_*
>
> Bug: https://github.com/dart-lang/sdk/issues/50796
> Change-Id: Ib5b153bc6e64bc433df1f05c53d82f71b470bbec
> TEST=presubmit bots
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290703
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
> Reviewed-by: Alexander Aprelev <aam@google.com>
> Commit-Queue: Samuel Rawlins <srawlins@google.com>
> Reviewed-by: Nate Bosch <nbosch@google.com>
Bug: https://github.com/dart-lang/sdk/issues/50796
Change-Id: If1c460bdcf422033648417da5ba2f5fbc1b459c2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291460
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Oleh Prypin <oprypin@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Commit-Queue: Oleh Prypin <oprypin@google.com>
Recent changes that made migration tool more conservative affected
functions used in Angular's DI: functions annotated with `@Injectable`
must have non-nullable non-annotated arguments. Previously that
worked just because there was no reason to make them nullable.
Make such functions' arguments explicitly non-nullable.
Bug: b/250862403
Change-Id: I27207994b058ba845aafd7776d14380e8104e8f8
Tested: added a test
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/289505
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Ilya Yanok <yanok@google.com>
With the last change to make public methods' arguments
nullable by default, people started complaining about arguments'
type being widened in overrides.
Stop the migration tool from doing this.
Change-Id: I4ada6e15d21e91c0e17bd4452fba5b42b6133930
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288244
Commit-Queue: Ilya Yanok <yanok@google.com>
Reviewed-by: Ilya Yanok <yanok@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Maybe? With an explicit type check we don't try to make an impression
that anything more happens here, e.g. for type parameters.
Similarly, for https://github.com/dart-lang/sdk/issues/36697 we would need to introduce `InvalidType`, which is not `DynamicType`. And so, `DartType.isDynamic` should not return `true` for `InvalidType`, it is not a property that various types may have, and should be replaced with explicit `is DynamicType`.
We also have `UnknownInferredType`, but it should never leak to clients.
Change-Id: I302b06355143d97bb52922c805dbad585a522e0a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288340
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
That forces them to be nullable, unless there is a
reason to be non-nullable.
I excluded setters and generic arguments from this
nullification. Also we don't touch `operator ==`, where
changing argument type to `Object?` doesn't make sense.
This is a response to some teams asking for the migration
tool to be super conservative. Otherwise people migrate
some methods to have non-nullable arguments and they are
still called with `null`s at runtime in mixed mode.
That was the only way I found to implement the "nullable
arguments by default" request.
It broke a good deal of tests. I updated them mostly to use
private functions, so the old behavior is preserved, but
sometimes I couldn't figure out a way to trigger the desired
codepath with the new behavior... I also suspect that now
there is a non-zero amount of tests that are passing but
don't really exercise what they were supposed to exercise.
Change-Id: I2eb8020008c6cd5c7507e7782e977d70bceac58b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/285680
Commit-Queue: Ilya Yanok <yanok@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
This breaks a good deal of tests and overall feels like preventing
a whole bunch of "make it non-nullable" stuff from working.
On the other hand, in cases where not the whole program is migrated
at once, and pieces of it keep being legacy and the whole thing
running in mixed mode, we would like to keep it on the conservative
side, so treat comparisons to `null` as a nullability signal.
I deleted some tests while updating others, I think a good deal of
updated tests probably should be deleted instead, but I kept them
for review, as these diffs show what exactly this change breaks.
Change-Id: I3d29dd446aefb7cfeb6219cffd9060ca8f57e50d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283681
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Ilya Yanok <yanok@google.com>
Previously, if multiple switch cases shared a body, and at least one
of those cases promoted a match variable using a `when` clause, the
promotion would not be carried over to the merged variable seen in the
shared case body. This was happening because promotions performed in
the `when` clause were applied to the promotion key associated with
the client's representation of the variable, whereas flow analysis was
computing the state of the merged variable based on the promotion keys
used internally while visiting the pattern (which don't include
promotions from the `when` clause).
With this change, flow analysis now computes the merged variable state
based on the promotion keys associated with the client's
representation of the variable components, so promotions from `when`
clauses take effect.
In the process, I've moved a lot of the tracking of these promotion
keys from the `TypeAnalyzer` class to the `_FlowAnalysisImpl` class.
This avoids the need to expose more flow analysis internals through
its public API.
Fixes#51399.
Bug: https://github.com/dart-lang/sdk/issues/51399
Change-Id: I8f1bb6e49ceb8441bb743c63af61c119df9041f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283441
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
If we statically know the types are unrelated, there is like
no chance for the cast to succeed at runtime. This is likely
a sign that automatic migration went wrong somewhere and human
intervention is needed. The cast just delays the problem to
runtime. Skip the cast and let it fail at compile time instead.
Change-Id: Ib557650b368c8f2839029e8a2e54c6bf9474035b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280119
Commit-Queue: Ilya Yanok <yanok@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
This should allow for better error recovery in the case where a label
target can't be found.
Change-Id: I05ec107a4ecef3f73cfba5931b77b8250707d2ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280120
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
`() => throw bla` has type `Null Function()` in legacy mode.
That's get recorded by the migration tool, so, when it later
checks if the type is compatible to the expected one, it
inserts an unnecessary cast.
Change-Id: I80d86ff715e16b038b4cbceae2fccfa76948320d
Bug: b/173016016
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279380
Commit-Queue: Ilya Yanok <yanok@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Fixes#51095
TEST=ci
CoreLibraryReviewExempt: There are no API changes, just removal of superfluous words in the comments.
Change-Id: Ib1020c62fe6baed5ca68f0074323f025cc90e9f8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279500
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
