Skip to content

Migrate runtime renderer to NNBD #2849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Nov 1, 2021
Merged

Migrate runtime renderer to NNBD #2849

merged 5 commits into from
Nov 1, 2021

Conversation

jcollins-g
Copy link
Contributor

Still can't quite switch on strong NNBD verification until the AOT renderers are done, but getting closer!

Now with correct base branch selected.

@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Oct 27, 2021
jcollins-g
jcollins-g commented Oct 27, 2021
View reviewed changes
Copy link
Contributor Author

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of questions I have about the port, hoping @srawlins can answer them.

lib/src/mustachio/renderer_base.dart
this.renderValue});
bool Function(T)? isNullValue,
this.renderValue})
: isNullValue = (isNullValue ?? (_) => false);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mostly food for NNBD, as we will pass this in wherever it is used? Maybe it should be required?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea. I think fine to leave a TODO and refactor later.

test/mustachio/foo.runtime_renderers.dart
@@ -547,6 +548,7 @@ class Renderer_Property3 extends RendererBase<Property3> {
}

const _invisibleGetters = {
'Object': {'hashCode', 'runtimeType'},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not expect the change in _invisibleGetters and the debugger is being particularly uncooperative at the moment in my attempts to figure out what is happening. Could this be indicative of a larger problem and so I should pursue this, or do you think this is OK?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its fine... Not sure why its happening but I don't think its bad.

@jcollins-g jcollins-g requested a review from srawlins October 27, 2021 21:23
srawlins
srawlins approved these changes Oct 27, 2021
View reviewed changes
test/mustachio/foo.runtime_renderers.dart
@@ -547,6 +548,7 @@ class Renderer_Property3 extends RendererBase<Property3> {
}

const _invisibleGetters = {
'Object': {'hashCode', 'runtimeType'},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its fine... Not sure why its happening but I don't think its bad.

tool/mustachio/codegen_runtime_renderer.dart
.map((tp) => tp.getDisplayString(withNullability: false)),
// withNullability will give star types, we actually want nullable or
// non-nullable only for generation.
.map(_renderSingleType),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fixup.

lib/src/mustachio/renderer_base.dart
this.renderValue});
bool Function(T)? isNullValue,
this.renderValue})
: isNullValue = (isNullValue ?? (_) => false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea. I think fine to leave a TODO and refactor later.

@jcollins-g jcollins-g merged commit 4fe6b1d into dart-lang:nnbd Nov 1, 2021
srawlins added a commit that referenced this pull request Jan 12, 2022
@srawlins @jcollins-g
@simolus3 @munificent @parlough @scheglov
Merge NNBD branch into master (#2886)
5cb56cd 
* Start the NNBD migration branch with additional github configuration and a basic "non-migration" of all libraries (#2739)

* Update analyzer version to 2.0.0

* grind build for analyzer 2

* First steps toward migrating

* Basic migration infrastruture for new NNBD branch

* use separate workflow file for nnbd branch, for clarity

* thinko

* Merge master into nnbd branch (#2747)

* Switch to using CompilationUnitElement.classes (#2743)

* Prepare dartdoc_options for migration (#2745)

* more specific imports in options

* do not allow autoinitialization

* dartfmt

* parameterize the valueWithContext type

* Use alternative constructors instead of externalizing, didn't realize the autodetect was so widely used

* more subclasses

* Empty commit to straighten out GitHub Actions

* Convert options/logging libraries to null safety (#2748)

* Update analyzer version to 2.0.0

* grind build for analyzer 2

* First steps toward migrating

* Start migrating some things around the edges and see what happens

* Basic migration infrastruture for new NNBD branch

* use separate workflow file for nnbd branch, for clarity

* thinko

* Switch to using CompilationUnitElement.classes (#2743)

* Ignore null safe import warnings for presubmits

* Prepare dartdoc_options for migration (#2745)

* more specific imports in options

* do not allow autoinitialization

* dartfmt

* parameterize the valueWithContext type

* Use alternative constructors instead of externalizing, didn't realize the autodetect was so widely used

* more subclasses

* Empty commit to straighten out GitHub Actions

* dartfmt

* update NNBD branch from head (#2758)

* Switch to using CompilationUnitElement.classes (#2743)

* Prepare dartdoc_options for migration (#2745)

* more specific imports in options

* do not allow autoinitialization

* dartfmt

* parameterize the valueWithContext type

* Use alternative constructors instead of externalizing, didn't realize the autodetect was so widely used

* more subclasses

* Disable the unstable template checks and prepare for 2.14 stable. (#2752)

* Disable one of the template checks?  and add beta branch for testing

* beta is the new stable

* empty commit - force check reset

* Add a blurb to the README about requiring analysis (#2753)

* Allow comment references on generic typedefs (#2756)

Co-authored-by: Janice Collins <[email protected]>

* Prepare for dartdoc 2.0.0 (#2754)

* Prepare for dartdoc 2.0.0

* Update changelog for additional bugfix

Co-authored-by: Simon Binder <[email protected]>

* Migrate dartdoc_options.dart (#2749)

* Switch to using CompilationUnitElement.classes (#2743)

* Prepare dartdoc_options for migration (#2745)

* more specific imports in options

* do not allow autoinitialization

* dartfmt

* parameterize the valueWithContext type

* Use alternative constructors instead of externalizing, didn't realize the autodetect was so widely used

* more subclasses

* flatten

* Disable the unstable template checks and prepare for 2.14 stable. (#2752)

* Disable one of the template checks?  and add beta branch for testing

* beta is the new stable

* empty commit - force check reset

* Add a blurb to the README about requiring analysis (#2753)

* Allow comment references on generic typedefs (#2756)

Co-authored-by: Janice Collins <[email protected]>

* Prepare for dartdoc 2.0.0 (#2754)

* Prepare for dartdoc 2.0.0

* Update changelog for additional bugfix

* review comments

Co-authored-by: Simon Binder <[email protected]>

* Migrate package_meta, io_utils, and utils. (#2760)

* flatten

* Review comments

* opt-out for inheriting container

* A few more libraries migrated

* Migrate warnings.dart (#2787)

* Migrate mustachio to null safety (#2801)

* Migrate various util and small libraries (#2805)

* Migrate tool_runner to NNBD (#2804)

* Migrate tool runner

* dartfmt

* getAs -> getValueAs

* comment

* Migrate the renderers to NNBD (#2816)

* Migrate renderers

* dartfmt

* Migrate most mustachio tests to null safety (#2810)

* Merge main branch to NNBD (#2826)

* Refactor more documentation comment generation into DocumentationComment (#2817)

* Straight move

* Partial

* delete experiment

* rebuild

* remove ??= from a cut and paste

* Continuing the crushing down of documentation handling (#2818)

* Straight move

* Partial

* delete experiment

* rebuild

* moved computeDocumentationComment and documentationComment

* Move more documentation comment handling

* remove ??= from a cut and paste

* Documentation comment move for combos

* better as a getter

* Remove all deprecated bits in preparation for next release (#2821)

* Remove all deprecated bits in preparation for next release

* make some things final

* documentationComment can now be non-nullable (#2819)

* Straight move

* Partial

* delete experiment

* rebuild

* moved computeDocumentationComment and documentationComment

* Move more documentation comment handling

* remove ??= from a cut and paste

* Documentation comment move for combos

* better as a getter

* Untangle synthetic combo documentation in preparing for non-null

* rebuild

* documentationComment can be made non-nullable

* Update test.yaml to delete sdk check from beta branch

* tab conversion error

* correct name

* Stop testing beta in the bots (#2824)

* disable beta

* try switching on stable

* stable doesn't work for a lot of things now either :-(

* try enabling the main bot

* Change "dartfmt" -> "dart format". (#2822)

Co-authored-by: Janice Collins <[email protected]>

Co-authored-by: Bob Nystrom <[email protected]>

* Migrate more libraries to NNBD (#2827)

* migrate category

* Migrate model_comment_reference and the parser

* Fix test failure

* Migrate comment_referable and model_object_builder (#2832)

* squash

* fix error in filter setup

* fix test

* Migrate element_type to nnbd (#2834)

* Migrate markdown_processor and matching_link_result to NNBD (#2835)

* Migrate element_type to nnbd

* Migrate markdown_processor and matching_link_result

* Migrate most of lib/src/generators to nnbd (#2837)

* squash

* empty commit - reset actions

* Migrate most of the non-ModelElement bits of lib (#2838)

* squash

* rebuild

* Fixup hints

* Migrate most of what's left in tools/ (#2840)

* squash

* rebuild

* migrate more things

* subprocess

* type adjustment in grinder

* comments

* Canonicalize default input dir

* Revert "Canonicalize default input dir"

This reverts commit 5d76a51.

* Just disable the test, something odd is going on with MemoryResourceProvider.

* Migrate ModelElement and subclasses to NNBD (#2843)

* squash

* rebuild

* migrate more things

* subprocess

* type adjustment in grinder

* partial

* partial 2

* no errors, ship it

* clean up a lot of warnings

* All warnings done so really ship it

* dartfmt

* close

* manual changes on renderers

* comments

* partial

* fix some errors post-migration

* holy cow it passes the tests now

* make mustache happier and clean up an interface

* forgot to remove a fixme

* Work around strange super-not-allowed error in stable

* review comments

* Migrate runtime renderer to NNBD (#2849)

* squash from chain merges

* Things seem to work now, if not completely cleanly.

* Get runtime_renderer_render_test ready for NNBD strong mode, fix a few more problems

* deconfuse github

* add todo

* Update nnbd branch for latest changes and new resources-dir option (#2867)

* Stop using JavaFile. (#2856)

* Add a resources-dir option (#2857)

* Skip sdk-analyzer job (#2863)

* Use 'index' to access the enum value index. (#2865)

Co-authored-by: Sam Rawlins <[email protected]>

* Re-enable sdk-analyzer job, but make it not failing (#2864)

* Update new resources-dir option for null safety

* Fix html generator test merge

Co-authored-by: Konstantin Scheglov <[email protected]>
Co-authored-by: Sam Rawlins <[email protected]>

* Utilize new Object.hash function instead of quiver code (#2868)

* Minor cleanup to lints and some top level files (#2872)

* Cleanup categorization to simplify template generation migration (#2875)

* Cleanup model_element migration and surrounding accesses (#2876)

* Migrate the generated AOT renderers to null safety (#2874)

* Cleanup package_graph migration (#2869)

* Migrate several test files to null safety (#2871)

* Migrate comment_referable tests (#2877)

* Migrate tool/grind.dart to null safety (#2870)

* Migrate more tests; correct other migration bits (#2880)

* Correct some bad mustachio migration; migrate mustachio tests (#2879)

* Migrate the testing/ packages (#2881)

* Migrate most end2end tests; null safety tweaks (#2882)

* Make a few more nullable fields late final (#2883)

* Migrate model_test, the last file, to null safety (#2884)

* Merge master branch into nnbd branch (#2885)

* Stop using JavaFile. (#2856)

* Add a resources-dir option (#2857)

* Skip sdk-analyzer job (#2863)

* Use 'index' to access the enum value index. (#2865)

Co-authored-by: Sam Rawlins <[email protected]>

* Re-enable sdk-analyzer job, but make it not failing (#2864)

Co-authored-by: Konstantin Scheglov <[email protected]>

Co-authored-by: Janice Collins <[email protected]>
Co-authored-by: Simon Binder <[email protected]>
Co-authored-by: Bob Nystrom <[email protected]>
Co-authored-by: Parker Lougheed <[email protected]>
Co-authored-by: Konstantin Scheglov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srawlins srawlins srawlins approved these changes

Assignees
No one assigned
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jcollins-g @srawlins