[dart2wasm] Fix dry run 'is' checks subtyping nullabilities.

Running on a Flutter app turned up this nullability inconsistency which produced a couple false positives.

Added tests that match the 2 Flutter examples.

Change-Id: Ieb75c67086c0cd8b4e20312abf4840b5a4cad628
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/438780
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Nate Biggs <[email protected]>

Author: natebiggs

pkg/dart2wasm/lib/dry_run.dart

@@ -161,7 +161,7 @@ class _AnalysisVisitor extends RecursiveVisitor {
       : _typeEnvironment = TypeEnvironment(coreTypes, hierarchy),
         _jsAnyType = ExtensionType(
             coreTypes.index.getExtensionType('dart:js_interop', 'JSAny'),
-            Nullability.nullable);
+            Nullability.nonNullable);
 
   @override
   void visitLibrary(Library node) {
@@ -181,15 +181,19 @@ class _AnalysisVisitor extends RecursiveVisitor {
   @override
   void visitIsExpression(IsExpression node) {
     final operandStaticType = node.operand.getStaticType(_context);
-    if (_typeEnvironment.isSubtypeOf(operandStaticType, _jsAnyType)) {
+    if (_typeEnvironment.isSubtypeOf(
+        operandStaticType.withDeclaredNullability(Nullability.nonNullable),
+        _jsAnyType)) {
       errors.add(_DryRunError(
           _DryRunErrorCode.isTestValueError,
           'Should not perform an `is` test on a JS value. Use `isA` with a JS '
           'value type instead.',
           errorSourceUri: _enclosingLibrary?.importUri,
           errorLocation: node.location));
     }
-    if (_typeEnvironment.isSubtypeOf(node.type, _jsAnyType)) {
+    if (_typeEnvironment.isSubtypeOf(
+        node.type.withDeclaredNullability(Nullability.nonNullable),
+        _jsAnyType)) {
       errors.add(_DryRunError(
           _DryRunErrorCode.isTestTypeError,
           'Should not perform an `is` test against a JS value type. '
@@ -211,13 +215,15 @@ class _AnalysisVisitor extends RecursiveVisitor {
     // Check InterfaceType and ExtensionType
     if (type is TypeDeclarationType) {
       final arguments = type.typeArguments;
-      if (arguments.any((e) => _typeEnvironment.isSubtypeOf(e, _jsAnyType))) {
+      if (arguments.any((e) => _typeEnvironment.isSubtypeOf(
+          e.withDeclaredNullability(Nullability.nonNullable), _jsAnyType))) {
         return true;
       }
       return arguments.any(_hasJsTypeArguments);
     } else if (type is RecordType) {
       final fields = type.positional.followedBy(type.named.map((t) => t.type));
-      if (fields.any((e) => _typeEnvironment.isSubtypeOf(e, _jsAnyType))) {
+      if (fields.any((e) => _typeEnvironment.isSubtypeOf(
+          e.withDeclaredNullability(Nullability.nonNullable), _jsAnyType))) {
         return true;
       }
       return fields.any(_hasJsTypeArguments);

pkg/dart2wasm/test/dry_run/dry_run_test.dart

@@ -38,14 +38,15 @@ class TestExpectation {
 }
 
 class TestFinding {
+  final String rawString;
   final int errorCode;
   final String problemMessage;
   final Uri? errorSourceUri;
   final int? errorLine;
   final int? errorColumn;
 
-  TestFinding(this.errorCode, this.problemMessage, this.errorSourceUri,
-      this.errorLine, this.errorColumn);
+  TestFinding(this.rawString, this.errorCode, this.problemMessage,
+      this.errorSourceUri, this.errorLine, this.errorColumn);
 
   factory TestFinding.fromLine(String line) {
     final parts = line.split(' - ');
@@ -60,6 +61,7 @@ class TestFinding {
     final errorCode =
         int.parse(problemMessage.substring(errorCodeStart + 1, errorCodeEnd));
     return TestFinding(
+        line,
         errorCode,
         problemMessage.substring(0, errorCodeStart).trim(),
         Uri.parse(uri),
@@ -140,7 +142,11 @@ Future<TestResults> runTest(TestCase testCase) async {
   timer.stop();
   try {
     final exitCode = result.exitCode;
-    Expect.equals(testCase.expectations.isEmpty ? 0 : 254, exitCode);
+    if (testCase.expectations.isEmpty) {
+      Expect.equals(0, exitCode, 'Unexpected findings:\n${result.stdout}');
+    } else {
+      Expect.equals(254, exitCode, 'Expected findings but found none.');
+    }
     final findings = _parseTestFindings(result.stdout);
     _checkFindings(findings, testCase.expectations);
   } catch (e, s) {
@@ -236,7 +242,8 @@ void _checkFindings(
       findings.length,
       expectations.length,
       'Incorrect number of findings. '
-      'Expected: ${expectations.length}, Actual: ${findings.length}');
+      'Expected: ${expectations.length}, Actual: ${findings.length}\n'
+      'Findings:\n${findings.map((e) => e.rawString).join('\n')}}');
   for (final expectation in expectations) {
     final lineNumber = expectation.lineNumber;
     final lineFindings =

pkg/dart2wasm/test/dry_run/testcases/valid_interop_import.dart

@@ -4,11 +4,24 @@
 
 import 'dart:js_interop';
 
+class A<B> {
+  void foo() {
+    if (null is! B) {
+      print('');
+    }
+  }
+}
+
 void main() {
   JSAny? jsValue;
   if (jsValue.isA<JSString>()) {
     print(jsValue);
   } else if (jsValue.isA<JSArray>()) {
     print(jsValue);
   }
+
+  Object? dartObject;
+  if (dartObject case {'foo': final String foo}) {
+    print(foo);
+  }
 }