Sdk: Crash in dart migrate

[cbracken]

Ran dart migrate in the root of package:quiver at 35f6d18fe989f92314210c20e3a2636cb90ba4ad.

Dart SDK version: 2.11.0-260.0.dev (dev) (Mon Oct 26 03:52:00 2020 -0700) on "linux_x64"

Expected behaviour: runs migration and hands me a URL for the tool service.

Actual behaviour:

Analyzing project...
[----------------------------------------------------------------------------------------------------------------/]No analysis issues found.

Generating migration suggestions...
[-                                                                                                                 [---------------------------------------------------/                                                             ]Aborting migration due to an exception.  This most likely is due to a
bug in the migration tool.  Please consider filing a bug report at:

https://github.com/dart-lang/sdk/issues/new
To attempt to perform migration anyway, you may re-run with
--ignore-exceptions.

Exception details:

NoSuchMethodError: The getter 'node' was called on null.
Receiver: null
Tried calling: node

#0      Object.noSuchMethod (dart:core-patch/object_patch.dart:51:5)
#1      EdgeBuilder._handleAssignment (package:nnbd_migration/src/edge_builder.dart:2297:56)
#2      EdgeBuilder._handleInvocationArguments (package:nnbd_migration/src/edge_builder.dart:2842:7)
#3      EdgeBuilder.visitMethodInvocation (package:nnbd_migration/src/edge_builder.dart:1244:24)
#4      MethodInvocationImpl.accept (package:analyzer/src/dart/ast/ast.dart:7129:49)
#5      EdgeBuilder._dispatch (package:nnbd_migration/src/edge_builder.dart:2134:22)
#6      EdgeBuilder._checkExpressionNotNull (package:nnbd_migration/src/edge_builder.dart:1920:20)
#7      EdgeBuilder._handleTarget (package:nnbd_migration/src/edge_builder.dart:2949:14)
#8      EdgeBuilder.visitMethodInvocation (package:nnbd_migration/src/edge_builder.dart:1227:22)
#9      MethodInvocationImpl.accept (package:analyzer/src/dart/ast/ast.dart:7129:49)
#10     EdgeBuilder._dispatch (package:nnbd_migration/src/edge_builder.dart:2134:22)
#11     EdgeBuilder._handleAssignment (package:nnbd_migration/src/edge_builder.dart:2248:20)
#12     EdgeBuilder.visitReturnStatement (package:nnbd_migration/src/edge_builder.dart:1446:7)
#13     ReturnStatementImpl.accept (package:analyzer/src/dart/ast/ast.dart:8559:49)
#14     EdgeBuilder._dispatch (package:nnbd_migration/src/edge_builder.dart:2134:22)
#15     EdgeBuilder.visitNode (package:nnbd_migration/src/edge_builder.dart:1288:9)
#16     GeneralizingAstVisitor.visitStatement (package:analyzer/dart/ast/visitor.dart:517:39)
#17     GeneralizingAstVisitor.visitBlock (package:analyzer/dart/ast/visitor.dart:165:31)
#18     BlockImpl.accept (package:analyzer/src/dart/ast/ast.dart:1070:49)
#19     EdgeBuilder._dispatch (package:nnbd_migration/src/edge_builder.dart:2134:22)
#20     EdgeBuilder.visitNode (package:nnbd_migration/src/edge_builder.dart:1288:9)
#21     GeneralizingAstVisitor.visitFunctionBody (package:analyzer/dart/ast/visitor.dart:324:45)
#22     GeneralizingAstVisitor.visitBlockFunctionBody (package:analyzer/dart/ast/visitor.dart:168:55)
#23     BlockFunctionBodyImpl.accept (package:analyzer/src/dart/ast/ast.dart:1025:49)
#24     EdgeBuilder._dispatch (package:nnbd_migration/src/edge_builder.dart:2134:22)
#25     EdgeBuilder._handleExecutableDeclaration (package:nnbd_migration/src/edge_builder.dart:2398:7)
#26     EdgeBuilder.visitMethodDeclaration (package:nnbd_migration/src/edge_builder.dart:1202:5)
#27     MethodDeclarationImpl.accept (package:analyzer/src/dart/ast/ast.dart:6976:49)
#28     EdgeBuilder._dispatch (package:nnbd_migration/src/edge_builder.dart:2134:22)
#29     EdgeBuilder._dispatchList (package:nnbd_migration/src/edge_builder.dart:2148:7)
#30     EdgeBuilder.visitClassOrMixinOrExtensionDeclaration (package:nnbd_migration/src/edge_builder.dart:601:7)
#31     EdgeBuilder.visitClassDeclaration (package:nnbd_migration/src/edge_builder.dart:568:5)
#32     ClassDeclarationImpl.accept (package:analyzer/src/dart/ast/ast.dart:1522:49)
#33     EdgeBuilder._dispatch (package:nnbd_migration/src/edge_builder.dart:2134:22)
#34     EdgeBuilder.visitNode (package:nnbd_migration/src/edge_builder.dart:1288:9)
#35     GeneralizingAstVisitor.visitCompilationUnit (package:analyzer/dart/ast/visitor.dart:202:51)
#36     CompletenessTracker.visitCompilationUnit.<anonymous closure> (package:nnbd_migration/src/utilities/completeness_tracker.dart:52:24)
#37     PermissiveModeVisitor.reportExceptionsIfPermissive (package:nnbd_migration/src/utilities/permissive_mode.dart:26:24)
#38     CompletenessTracker.visitCompilationUnit (package:nnbd_migration/src/utilities/completeness_tracker.dart:43:5)
#39     CompilationUnitImpl.accept (package:analyzer/src/dart/ast/ast.dart:2113:49)
#40     NullabilityMigrationImpl.processInput (package:nnbd_migration/src/nullability_migration_impl.dart:222:12)
#41     NonNullableFix.processUnit (package:nnbd_migration/src/front_end/non_nullable_fix.dart:165:15)
#42     _FixCodeProcessor.runLaterPhases.<anonymous closure> (package:nnbd_migration/migration_cli.dart:1169:19)
#43     _FixCodeProcessor.processResources (package:nnbd_migration/migration_cli.dart:1111:30)
<asynchronous suspension>
#44     _FixCodeProcessor.runLaterPhases (package:nnbd_migration/migration_cli.dart:1167:11)
#45     MigrationCliRunner.run (package:nnbd_migration/migration_cli.dart:773:48)
<asynchronous suspension>
#46     MigrateCommand.run (package:nnbd_migration/migration_cli.dart:277:72)
#47     CommandRunner.runCommand (package:args/command_runner.dart:197:27)
#48     DartdevRunner.runCommand (package:dartdev/dartdev.dart:299:24)
#49     CommandRunner.run.<anonymous closure> (package:args/command_runner.dart:112:25)
#50     new Future.sync (dart:async/future.dart:223:31)
#51     CommandRunner.run (package:args/command_runner.dart:112:14)
#52     runDartdev (package:dartdev/dartdev.dart:121:27)
#53     main (file:///b/s/w/ir/cache/builder/src/third_party/dart/pkg/dartdev/bin/dartdev.dart:11:9)
#54     _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:297:32)
#55     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

[cbracken]

/cc @stereotype441

[devoncarew]

Marking this as a P2 - we can bump up in the future if we determine this is impacting more than a few users.

[franklinyow]

Can we repo this consistently?

[mit-mit]

@devoncarew how do we know it's just a few users? We have less than 100 users using the null safety experiment, so even with a single repro, that a 1% failure rate.

[devoncarew]

We haven't seen this before, with non-trivial internal usage.

@stereotype441 - do you mind looking at this, to determine if this is likely to be something user will hit (i.e., not a less likely coding construct)?

[stereotype441]

I'm able to reproduce this using Quiver commit https://github.com/google/quiver-dart/commit/6d9643fc3228280befd949a25cd049332be5974f and the latest Dart SDK as of 332ce760558233294898ab34916483ee523402ec.

I'll investigate further to try to figure out the root cause, and when I do, I'll update this bug with information about how large I think the impact is.

Side note: users who encounter this problem are not dead in the water. Following the tool's suggestion of re-running with --ignore-exceptions does successfully work around the bug.

[stereotype441]

Ok, I've tracked the bug down far enough to estimate its impact. I believe it's fairly common.

The bug will occur with any call to Iterable.firstWhere, Iterable.lastWhere, or Iterable.singleWhere where the value supplied to the orElse argument is () => null and the target of the method call is a sufficently complex expression (e.g. a method call like allMatches(str) as opposed to a simple variable reference like x). In Quiver, the bug is triggered by this code:

  @override
  Match matchAsPrefix(String str, [int start = 0]) {
    return allMatches(str)
        .firstWhere((match) => match.start == start, orElse: () => null);
  }

I think this is sufficiently bad to consider a P1. Fortunately the fix should be fairly easy, and I have time available to work on it now.
I should have a fix out for review within an hour or two.

[stereotype441]

Fix is out for review: https://dart-review.googlesource.com/c/sdk/+/170162