I'm going to do some refactoring in future CLs that may make some of
these classes and/or methods unnecessary. Making them private will
help highlight when things become unnecessary (because it will allow
dead code hints to be triggered).
Change-Id: I201bf5be0131e69af00789308e17926d98db6f5b
Reviewed-on: https://dart-review.googlesource.com/c/93369
Auto-Submit: Paul Berry <paulberry@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Rather than have each PotentialModification keep track of its own
source, it's less wasteful to keep track of a map from each source to
a list of the potential modifications for that source.
Change-Id: Ib579f5159100195496a850fbf795b073fc927110
Reviewed-on: https://dart-review.googlesource.com/c/93466
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
If a method parameter is used in unconditional control flow in a way
that a `null` value would directly lead to an exception (i.e. by
dereferencing it, or by passing it to a method that requires a
non-nullable value), this is treated as implying that the method
parameter is intended to be non-nullable.
Change-Id: I4f55e4c95b3cfaee0a2ba9367b47d51083e0b7b1
Reviewed-on: https://dart-review.googlesource.com/c/93363
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
If a named parameter is annotated as `@required`, then this overrides
the NamedNoDefaultParameterHeuristic; the parameter is considered
required regardless of what happens at call sites. A duplicate
`@required` annotation is not inserted.
Change-Id: Ib1385d0a65dd9001bb7abede9de2a319f65a1f86
Reviewed-on: https://dart-review.googlesource.com/c/92844
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Currently assuming that the language team chooses option 2 for
https://github.com/dart-lang/language/issues/156#issuecomment-460525075.
To handle the case where the user does not specify a default value,
and all call sites supply the named parameter, there are two possible
heuristics:
- assumeNullable: mark the parameter as nullable, since the implicit
default is `null`, even though the implicit default is never used.
- assumeRequired: consider the parameter to be `@required`, ensuring
that the implicit default will never be used.
I've provisionally chosen `assumeRequired` as the default behavior.
Functionality not yet implemented:
- If `@required` is already present in the source code, this should
override the heuristic; also it should not be added again.
- If the parameter is already marked as nullable in the source code,
this should override the heuristic.
- Uses of `@required` should be translated to `required`.
- If `@required` is already present in the source code, and a default
value is supplied, the default value should be removed.
- If the parameter is used in a way that demonstrates it was intended
to be non-nullable (i.e. it appears in a place in unconditional
control flow that would cause an exception to be thrown if it was
null), this should override the heuristic and cause it to be marked
as `@required`.
Change-Id: I9f2ec63787fdb6215e6075ecfcc830ed9942b8b5
Reviewed-on: https://dart-review.googlesource.com/c/92532
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
At this point the migration tool is beginning to make assumptions
about decisions the language team hasn't made yet, so I've created a
class to document those assumptions and allow us to test variants.
Change-Id: I4638d7f534cfa2dbd8460277351045c370377ff8
Reviewed-on: https://dart-review.googlesource.com/c/92394
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This allows us to try it out on substantial codebases (e.g. the
analyzer itself) without crashing, even though a lot of the underlying
logic is not yet complete.
Change-Id: I24a45eea1889f7d9afa30c0398f359d32ffb52ad
Reviewed-on: https://dart-review.googlesource.com/c/92404
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This required modifying the implementation of nullability migration so
that it no longer holds onto AST nodes, but instead records sources
and offsets. This will allow the tool to scale to large projects
since the AST for one file can be garbage collected while the AST for
another file is analyzed.
Additional changes:
- provisional_api_test.dart is updated so that it tests the API in two
ways: once feeding identical ASTs to prepareInput/processInput, and
once feeding conceptually equal (but not identical) ASTs.
- The internal analyzer API is changed so that the finish() method
simply returns a list of modifications, rather than a map from path
to modification list.
- Files are tracked by Source rather than by path.
- Modification objects remember their Source (this should pave the way
for allowing multiple files to be migrated in a single run of the
tool).
- The Variables class is split in two, with the methods needed for
testing in a private class inside migration_test.dart. These test
methods can retain pointers to AST nodes without creating
scalability problems for the production code.
Change-Id: Idd3f3b6893a25bb4294b4b4c820e20e4bc47ffd8
Reviewed-on: https://dart-review.googlesource.com/c/92220
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Instead of returning a list of SourceFileEdit objects, we now return a
list of SingleNullabilityFix objects. Each SingleNullabilityFix can
be queried to determine (a) which source file it applies to, (b) the
exact set of source edits to apply, and (c) the kind of change.
Future CLs will add the ability to query (d) the location of the
change, and (e) the reason for the change.
Change-Id: I5a42958ac70d1e39802efd5915d0856109b4a7db
Reviewed-on: https://dart-review.googlesource.com/c/92151
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Limitations at this point:
- Only one file can be migrated at a time. If you try to migrate more
than one file, you will get an assertion failure.
- The ResolvedUnitResult passed to prepareInput must contain precisely
the same unit (in terms of `identical`) as the ResoledUnitResult
passed to processInput. Otherwise you will get an assertion
failure.
- There is not yet any way to query diagnostic information about the
migration (e.g. heuristic decisions that were made).
- Only a very small number of constructs can be successfully migrated
at this point.
- The implementation is not sensibly organized at this point. Most of
it is in the analyzer, and then wrapped in the analysis_server.
Really it all should go in the analysis_server.
I'll be working to rectify these limitations over the next few days,
which will probably involve a lot of rework under the hood. But
hopefully the API in provisional_api.dart should remain stable, so
that DartFix code can start being written against it.
Change-Id: Idc3ebab380448a812d5e83338789570e33c55af7
Reviewed-on: https://dart-review.googlesource.com/c/92020
Reviewed-by: Dan Rubel <danrubel@google.com>
Commit-Queue: Dan Rubel <danrubel@google.com>
Auto-Submit: Paul Berry <paulberry@google.com>
Paul Berry