We no longer need to keep pointers to the nullability nodes directly
in the ExpressionChecks data structure; we merely need to record which
edges, if unsatisfied, would require a check.
This allowed the tests using assertNullCheck to be clarified--they now
simply verify that the appropriate edge exists, and then pass that
edge to assertNullCheck to verify that the ExpressionChecks object
points to the appropriate edge.
Change-Id: Iaaee51d1f23ca6f86a2fbf0a15ded6a844b2811b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106383
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This paves the way for being able to explain to the user exactly why
the migration tool made any given nullability decision.
Change-Id: I553e4881fdb37b238a066fcfba7c21ae919d7d6e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106381
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This paves the way for a follow-up CL that will introduce a more
general notion of the "origin" of an edge (of which ExpressionChecks
will be just one kind of origin).
Change-Id: I76ee12679d881ef95a4ba17a224fe85ecf6b589f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106380
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
In follow-up CLs this should allow us to simplify the generation of
fixes (since each fix is associated with one or more unsatisfied
edges) and to streamline tests.
Change-Id: I997f704aac41f270756adcdf8d382fb2263273eb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106180
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This method was unnecessary--the same functionality can be provided
more efficiently by getUpstreamEdges.
Also fixed a doc comment error in getUpstreamEdges.
Change-Id: I44560eced353b57e00b06e57c8c628de79d689e8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106121
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
I'll start implementing them and adding tests in future CLs.
Change-Id: Ic8cb5ed9bd00f05232869d7f4c585daa5d5ab4c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106122
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Change-Id: Ia9fdca760c4c0838cdb6bb31c00f296fbf21cbac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106102
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Auto-Submit: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Change-Id: Ia1a803bd52e94e15696c60c77938fd2f5c08bdaf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105981
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Auto-Submit: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This will allow me to put stronger assertions in the DecoratedType
constructor in a follow-up CL.
Change-Id: I375ae2b33c2f14948d9fedbdfbc000b8a1057b00
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105963
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The return value of the GraphBuilder visit methods should always be
the type of the AST node assuming it's used as an expression. When a
type name is used as an expression, its type is always a non-null
reference to the built-in type `Type`.
(Note: this wasn't causing any failures, but fixing it will allow me
to add stronger assertions in a follow-up CL.)
Change-Id: I62c72dcfc6c0f12f8d8a1a7de34a1d3f94c5e4f4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105962
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Change-Id: I4d75bc286fe8d5dfe063b30525a99dacc96b4ec6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105961
Reviewed-by: Dan Rubel <danrubel@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
I'll build on this in future CLs to address more complex cases, where
the type is instantiated to a bound.
Fixes#37213.
Change-Id: I1e23c5599557ef17177483222f15b1a7985b9ca8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105726
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This will simplify some situations where nullability migration would
otherwise have to visit the source code in a very careful order. For
example, when analyzing top level fields undergoing type inference, we
can simply create a node for the nullability of each top level field,
and then later use the "union" operation to hook up those nodes to the
nodes resulting from analyzing their initializers. Without the union
operation, we would have to be careful to visit the top level fields
in dependency order.
Change-Id: I3b07e1abccc5b1c9f1c7c9fa7b13fb6af60c07d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105800
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Previously, they were static, which made them easier to access, but
made it difficult to track their edges (because we couldn't safely
mutate them, so we had to store their edges in special fields of
NullabilityGraph). Now that we're passing NullabilityGraph all over
the place anyhow, there's no benefit to making them static anymore.
Change-Id: Ia3e7d32ae479f40f505621e5d6df04060e39b1c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105723
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
So that we don't accidentally start to use them elsewhere in the
migration engine.
Change-Id: Iee290f9fc8761c952a2aadb1f4f6bb7bfe061a3f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105700
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Previously we created artifical edges from a LUB node's components to
the LUB node; now the nullability propagation algorithm understands
how to propagate nullability downstream through LUB and substitution
nodes, so no artificial edges are needed.
Change-Id: Ib2fef14c3dcd58a013ad8e057fd55597a1deb4fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105402
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This paves the way for removing the transitional API altogether.
Change-Id: I95181b04173d3439d278f898ae3b5ec574108b32
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105411
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
It wasn't detected by the "unused field" hint because there is another
field in the same file with the same name that *is* used
(NullabilityMigration._graph).
Change-Id: I955f30d010ddaa2d218f61daf32978d391182708
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105410
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This gives us more flexibility for how we want to publish and deploy
the tool. We now have the option, for example, of making a command
line app that invokes the tool and does not depend on analysis_server.
Note that some testing infrastructure had to be duplicated. I plan to
consolidate this infrastructure in follow-up CLs.
Change-Id: I046506bc2bb5c3e467e15885f198ee0632351ee9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105463
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>