Migrate ModelElement and subclasses to NNBD #2843

jcollins-g · 2021-10-21T19:48:40Z

This is a first pass and definitely not quite optimal, but it passes tests locally. The general process was use the migration tool, accept what it did, then try to fix everything that was broken as a result. There are some FIXMEs regarding the library/package getters that should be corrected once we get the rest of dartdoc migrated and we can switch strong checks on and merge with head but otherwise it is pretty clean.

The major issue is that a lot of old idioms still need to be ported over; doing so will involve some restructuring and be a pretty drawn out process, but I think it might be best done after we merge NNBD to head.

…us+tools+modelelement

srawlins

I checked half the files. Happy to do more review later, but I think this is great to land as is.

srawlins · 2021-10-22T03:14:02Z

lib/src/element_type.dart

@@ -260,7 +260,7 @@ class AliasedElementType extends ParameterizedElementType with Aliased {
  ParameterizedType get type;

  /// Parameters, if available, for the underlying typedef.
-  List<Parameter> get aliasedParameters =>
+  late final List<Parameter> aliasedParameters =


Nice optimization

srawlins · 2021-10-22T03:18:19Z

lib/src/logging.dart

@@ -23,23 +23,23 @@ const Level progressLevel = Level('PROGRESS', 501);
 /// Has a value of `1201` – one more than [Level.SHOUT].
 const Level printLevel = Level('PRINT', 1201);

-void logWarning(Object message) {
+void logWarning(String? message) {


srawlins · 2021-10-22T03:18:42Z

lib/src/model/accessor.dart

@@ -17,57 +16,61 @@ import 'package:dartdoc/src/warnings.dart';

 /// Getters and setters.
 class Accessor extends ModelElement implements EnclosedElement {
-  GetterSetterCombo enclosingCombo;
+  /// The combo ([Field] or [TopLevelVariable] containing this accessor.


Missing )?

srawlins · 2021-10-22T03:21:04Z

lib/src/model/accessor.dart

-  Callable get modelType => _modelType ??=
-      modelBuilder.typeFrom((originalMember ?? element).type, library);
+  Callable? _modelType;
+  Callable get modelType => (_modelType ??= modelBuilder.typeFrom(


This is one heck of a statement. Does it get better if you do the late final thing? And I think as Callable?)! can be replaced with just as Callable).

Heh, yeah the automigration does not like this construct sometimes :-). Patched this up.

srawlins · 2021-10-22T03:23:58Z

lib/src/model/accessor.dart

      // TODO(jcollins-g): switch to [element.nonSynthetic] after analyzer 1.8
      return enclosingCombo.characterLocation;
    }
    return super.characterLocation;
  }

  @override
-  PropertyAccessorElement get element => super.element;
+  PropertyAccessorElement? get element =>


I might be wrong, but it looks like practically every access of this field immediately null-checks it. Can it be made non-nullable?

not wrong: this is another case of the FIXME where element has to be nullable in ModelElement but nowhere else because of dynamic

srawlins · 2021-10-22T03:26:08Z

lib/src/model/accessor.dart

  // The [enclosingCombo] where this element was defined.
-  GetterSetterCombo get definingCombo {
+  GetterSetterCombo? get definingCombo {


This could benefit from the late final pattern, as every access of definingCombo seems to immediately null-check it.

srawlins · 2021-10-22T03:33:15Z

lib/src/model/extension.dart


  Extension(
      ExtensionElement element, Library library, PackageGraph packageGraph)
      : super(element, library, packageGraph) {
-    extendedType = modelBuilder.typeFrom(_extension.extendedType, library);
+    extendedType = modelBuilder.typeFrom(element.extendedType, library);


extendedType might be made non-nullable and final by just moving this statement into a constructor initializer.

We can not reference modelBuilder in a constructor initializer as currently built. I could refactor that, and probably will eventually, by not passing packageGraph around and instead passing around modelBuilder. There are several more heads to chop from that hydra before doing that though. A late final works fine here for now?

Oops, I see. Yeah late final.

* 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]>

jcollins-g added 27 commits October 12, 2021 15:52

squash

a6ffd67

rebuild

e58770b

migrate more things

42b4084

subprocess

cd518ea

type adjustment in grinder

574f9e0

partial

a058b82

partial 2

922c7d8

no errors, ship it

983e42c

clean up a lot of warnings

5672bde

All warnings done so really ship it

3f064c3

dartfmt

3e6812b

close

dbf8721

manual changes on renderers

4f973d7

Merge branch 'nnbd' into nnbd-lib-miscellaneous

32366cf

Merge branch 'nnbd-lib-miscellaneous' into nnbd-lib-miscellaneous+tools

89641fb

Merge branch 'nnbd-lib-miscellaneous+tools' into nnbd-lib-miscellaneo…

1abc062

…us+tools+modelelement

comments

5193190

Merge branch 'nnbd-lib-miscellaneous+tools' into nnbd-lib-miscellaneo…

4dd55a3

…us+tools+modelelement

partial

1d0da25

fix some errors post-migration

c66cc0b

holy cow it passes the tests now

a752169

Merge branch 'nnbd' into nnbd-lib-miscellaneous+tools

b5a85e6

Merge branch 'nnbd-lib-miscellaneous+tools' into nnbd-lib-miscellaneo…

4883978

…us+tools+modelelement

Merge branch 'nnbd' into nnbd-lib-miscellaneous+tools

735873a

Merge branch 'nnbd-lib-miscellaneous+tools' into nnbd-lib-miscellaneo…

17f38f1

…us+tools+modelelement

make mustache happier and clean up an interface

c53da36

forgot to remove a fixme

00ff790

jcollins-g added the nnbd label Oct 21, 2021

google-cla bot added the cla: yes Google CLA check succeeded. label Oct 21, 2021

Work around strange super-not-allowed error in stable

420e1fd

jcollins-g marked this pull request as ready for review October 21, 2021 23:05

jcollins-g requested a review from srawlins October 21, 2021 23:05

srawlins approved these changes Oct 22, 2021

View reviewed changes

jcollins-g added 2 commits October 26, 2021 11:49

review comments

6e1360f

Merge branch 'nnbd' into nnbd-modelelement

6bdeea8

jcollins-g merged commit b81bcce into dart-lang:nnbd Oct 27, 2021

jcollins-g deleted the nnbd-modelelement branch October 27, 2021 02:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate ModelElement and subclasses to NNBD #2843

Migrate ModelElement and subclasses to NNBD #2843

jcollins-g commented Oct 21, 2021 •

edited

Loading

srawlins left a comment

srawlins Oct 22, 2021

srawlins Oct 22, 2021

srawlins Oct 22, 2021

srawlins Oct 22, 2021

jcollins-g Oct 26, 2021

srawlins Oct 22, 2021

jcollins-g Oct 25, 2021

srawlins Oct 22, 2021

jcollins-g Oct 26, 2021

srawlins Oct 22, 2021

jcollins-g Oct 26, 2021

srawlins Oct 26, 2021

Migrate ModelElement and subclasses to NNBD #2843

Migrate ModelElement and subclasses to NNBD #2843

Conversation

jcollins-g commented Oct 21, 2021 • edited Loading

srawlins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcollins-g commented Oct 21, 2021 •

edited

Loading