nnbd migration #2452

pq · 2021-02-12T21:33:24Z

First cut at migration using the migration tool.

As discussed, I'd like to land this (if green) and iterate on improvements.

See: dart-lang/sdk#58322

…nter into nnbd_auto_migration

coveralls · 2021-02-12T22:47:53Z

Coverage decreased (-0.03%) to 96.599% when pulling cbeb372 on nnbd_auto_migration into 4f06a0e on master.

…nter into nnbd_auto_migration

bwilkerson

Sorry, I thought you were ready for a review, but I've had to refresh several times since I started, and now I have no idea what I have and haven't looked at.

It probably doesn't matter, though, because there's way too much to do a decent review, so I'll just have to trust that you'll clean it up after you commit it. I made a couple of specific requests near the beginning, but in general here's a list of the kinds of issues I saw that I think should be cleaned up:

Every use of the null check operator (!) should be removed. There are lots of different ways to do that depending on the context, every single one should be removed, or there should be a comment explaining why it isn't possible to get rid of it.
Every question mark that can be removed should be. There are lots of places where there are nullable types that I think we can get rid of. One common pattern before was to check for null argument values in every method, but the code is much cleaner in a null safe world if we check at every call site.
Every local variable that was created in order to hold a casted value should be removed.

bwilkerson · 2021-02-12T22:01:13Z

lib/src/ast.dart

@@ -268,7 +268,7 @@ bool _checkForSimpleSetter(MethodDeclaration setter, Expression expression) {
  if (expression is! AssignmentExpression) {
    return false;
  }
-  final assignment = expression as AssignmentExpression;
+  final assignment = expression;


This assignment is unlikely to be necessary now.

bwilkerson · 2021-02-12T22:04:11Z

lib/src/formatter.dart

@@ -14,15 +14,15 @@ import 'util/score_utils.dart';
 // Number of times to perform linting to get stable benchmarks.
 const benchmarkRuns = 10;

-String getLineContents(int lineNumber, AnalysisError error) {
+String getLineContents(int? lineNumber, AnalysisError error) {


lineNumber should not be nullable.

bwilkerson · 2021-02-12T22:05:35Z

lib/src/formatter.dart

@@ -14,15 +14,15 @@ import 'util/score_utils.dart';
 // Number of times to perform linting to get stable benchmarks.
 const benchmarkRuns = 10;

-String getLineContents(int lineNumber, AnalysisError error) {
+String getLineContents(int? lineNumber, AnalysisError error) {
  final path = error.source.fullName;
  final file = File(path);
  String failureDetails;
  if (!file.existsSync()) {


It isn't part of the migration, but there's no value in testing whether the file exists. Just read the content and catch the exception if the file doesn't exist.

bwilkerson · 2021-02-12T22:12:19Z

lib/src/formatter.dart

-  final bool machineOutput;
-  final bool quiet;
+  final bool? machineOutput;
+  final bool? quiet;


bool? is a strong signal that there's a bug in the code. Consider making these non-nullable and clean up the code.

bwilkerson · 2021-02-12T22:13:12Z

lib/src/formatter.dart

@@ -322,10 +322,10 @@ class SimpleFormatter implements ReportFormatter {

 class _Stat implements Comparable<_Stat> {
  final String name;
-  final int elapsed;
+  final int? elapsed;


This should be non-nullable.

bwilkerson · 2021-02-12T22:14:04Z

lib/src/rules/always_put_control_body_on_new_line.dart

@@ -93,14 +93,14 @@ class _Visitor extends SimpleAstVisitor<void> {
    _checkNodeOnNextLine(node.body, node.rightParenthesis.end);
  }

-  void _checkNodeOnNextLine(AstNode node, int controlEnd) {
+  void _checkNodeOnNextLine(AstNode? node, int controlEnd) {


node should be non-nullable.

bwilkerson · 2021-02-12T22:19:58Z

lib/src/rules/avoid_renaming_method_parameters.dart

@@ -82,7 +82,7 @@ class _Visitor extends SimpleAstVisitor<void> {
      return;
    }

-    final classElement = parentElement as ClassElement;
+    final classElement = parentElement;


This variable is unnecessary now.

bwilkerson · 2021-02-12T22:22:22Z

lib/src/rules/avoid_returning_null.dart

      _visitFunctionBody(node.body);
    }
  }

-  void _visitFunctionBody(FunctionBody node) {
+  void _visitFunctionBody(FunctionBody? node) {


node should be non-nullable. The analyzer APIs have been updated to disallow any function from having a null body. The compiler won't catch the change when you update to the next version of analyzer.

pubspec.yaml

pq · 2021-02-12T23:47:23Z

As described in dart-lang/sdk#58322, landing this as-is with tidying to follow.

nnbd (tool) migration

b1798f2

google-cla bot added the cla: yes label Feb 12, 2021

pq and others added 3 commits February 12, 2021 13:43

bump to dev SDK

8e249ca

sort

1672a8a

Merge branch 'nnbd_auto_migration' of https://github.com/dart-lang/li…

d555ab2

…nter into nnbd_auto_migration

pq marked this pull request as draft February 12, 2021 21:48

pq and others added 6 commits February 12, 2021 14:13

type fixes

2296aa6

run unsound

d1711dc

fmt

5b57e85

Merge branch 'nnbd_auto_migration' of https://github.com/dart-lang/li…

356e953

…nter into nnbd_auto_migration

null check

ac581da

warning cleanup

a8b2650

pq changed the title ~~nnbd (tool) migration~~ nnbd migration Feb 12, 2021

more unsound

00896a7

pq added 2 commits February 12, 2021 14:50

+ http

b85544d

Merge branch 'nnbd_auto_migration' of https://github.com/dart-lang/li…

22cbae0

…nter into nnbd_auto_migration

bwilkerson approved these changes Feb 12, 2021

View reviewed changes

null-check fixes

2614340

bwilkerson reviewed Feb 12, 2021

View reviewed changes

pubspec.yaml Outdated Show resolved Hide resolved

pq added 2 commits February 12, 2021 15:22

null fixes

1342e64

tidy

d897e72

pq marked this pull request as ready for review February 12, 2021 23:35

null-check fix

cbeb372

pq mentioned this pull request Feb 12, 2021

package:linter nnbd migration plan dart-lang/sdk#58322

Closed

4 tasks

pq merged commit fd0a11f into master Feb 12, 2021

pq deleted the nnbd_auto_migration branch February 12, 2021 23:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nnbd migration #2452

nnbd migration #2452

pq commented Feb 12, 2021 •

edited

Loading

coveralls commented Feb 12, 2021 •

edited

Loading

bwilkerson left a comment

bwilkerson Feb 12, 2021

bwilkerson Feb 12, 2021

bwilkerson Feb 12, 2021

bwilkerson Feb 12, 2021

bwilkerson Feb 12, 2021

bwilkerson Feb 12, 2021

bwilkerson Feb 12, 2021

bwilkerson Feb 12, 2021

pq commented Feb 12, 2021

nnbd migration #2452

nnbd migration #2452

Conversation

pq commented Feb 12, 2021 • edited Loading

coveralls commented Feb 12, 2021 • edited Loading

bwilkerson 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

pq commented Feb 12, 2021

pq commented Feb 12, 2021 •

edited

Loading

coveralls commented Feb 12, 2021 •

edited

Loading