-
Notifications
You must be signed in to change notification settings - Fork 125
Migrate warnings.dart #2787
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.
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
Migrate warnings.dart #2787
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember if you land this to create a merge commit from the pull down rather than the usual squash and merge. I haven't found a way to prevent squash and merge on this branch in GitHub settings yet.
@@ -450,7 +450,7 @@ abstract class DartdocOption<T> { | |||
|
|||
/// To avoid accessing early, call [add] on the option's parent before | |||
/// looking up unless this is a [DartdocRootOption]. | |||
late final DartdocOption parent; | |||
late final DartdocOption<dynamic> parent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary? I'm trying to avoid having explicit dynamics dropped in to solve nullability problems...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was not necessary; it is a no-op. I was just trying to avoid implicit dynamic. Should I revert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. I actually don't know what is better here, I suppose I'm against putting dynamic in just in case some magic in the type system allows for something more specific later. That isn't really on solid ground though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, type args in a declaration are never filled in by inference. The strict-raw-types
analysis mode would catch each such case, so that the human doesn't have to guess at where inference happens and where it doesn't.
I can revert for now and add that analysis mode in a different CL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds fine
PackageWarningOptions get packageWarningOptions => | ||
optionSet['packageWarningOptions'].valueAt(context); | ||
|
||
bool get verboseWarnings => optionSet['verboseWarnings'].valueAt(context); | ||
} | ||
|
||
Future<List<DartdocOption<Object>>> createPackageWarningOptions( | ||
Future<List<DartdocOption<Object?>>> createPackageWarningOptions( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be able to drop the Object parameter here, that has worked in other cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the DartdocOption objects inside are DartdocOption<some nullable type> so it needed to be nullable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh. I guess I was suggesting to use an implicit dynamic here. Is there a best practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. I avoid dynamic at all costs 😁 . The leading recommendation in that vein is probably Effective Dart's https://dart.dev/guides/language/effective-dart/design#avoid-using-dynamic-unless-you-want-to-disable-static-checking
Oh, and squash and merge can work on the branch here since this isn't actually a merge from master in any way, I misunderstood from the PR description. |
* 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]>
This PR mostly just merges warnings.dart, a straightforward migration.
Additionally:
getAs
method to DartdocOption for easy casting._namePlaceholder
in help text.