Analyzer: new Hint: NULL_CHECK_ALWAYS_FAILS

This Hint reports when an expression of type Null is null-checked with `!`.
This check will always fail at runtime.

Fixes https://github.com/dart-lang/sdk/issues/43982

Change-Id: I6e2eac4f7b7720a6b9a1c02066470eb039aa9010
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170122
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

pkg/analyzer/lib/error/error.dart

@@ -535,6 +535,7 @@ const List<ErrorCode> errorCodeValues = [
   HintCode.NULL_AWARE_BEFORE_OPERATOR,
   HintCode.NULL_AWARE_IN_CONDITION,
   HintCode.NULL_AWARE_IN_LOGICAL_OPERATOR,
+  HintCode.NULL_CHECK_ALWAYS_FAILS,
   HintCode.NULLABLE_TYPE_IN_CATCH_CLAUSE,
   HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE,
   HintCode.OVERRIDE_ON_NON_OVERRIDING_FIELD,

pkg/analyzer/lib/src/dart/error/hint_codes.dart

@@ -1434,6 +1434,15 @@ class HintCode extends AnalyzerErrorCode {
       "The value of the '?.' operator can be 'null', which isn't appropriate "
           "as an operand of a logical operator.");
 
+  /**
+   * This hint indicates that a null literal is null-checked with `!`, but null
+   * is never not null.
+   */
+  static const HintCode NULL_CHECK_ALWAYS_FAILS = HintCode(
+      'NULL_CHECK_ALWAYS_FAILS',
+      "This null-check will always throw an exception because the expression "
+          "will always evaluate to 'null'.");
+
   /**
    * No parameters.
    */

pkg/analyzer/lib/src/error/best_practices_verifier.dart

@@ -613,6 +613,9 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
   @override
   void visitPostfixExpression(PostfixExpression node) {
     _deprecatedVerifier.postfixExpression(node);
+    if (node.operand.staticType?.isDartCoreNull ?? false) {
+      _errorReporter.reportErrorForNode(HintCode.NULL_CHECK_ALWAYS_FAILS, node);
+    }
     super.visitPostfixExpression(node);
   }
 

pkg/analyzer/test/src/dart/resolution/postfix_expression_test.dart

@@ -614,11 +614,13 @@ void f(Map<String, int> a) {
   }
 
   test_nullCheck_null() async {
-    await assertNoErrorsInCode('''
+    await assertErrorsInCode('''
 void f(Null x) {
   x!;
 }
-''');
+''', [
+      error(HintCode.NULL_CHECK_ALWAYS_FAILS, 19, 2),
+    ]);
 
     assertType(findNode.postfix('x!'), 'Never');
   }

pkg/analyzer/test/src/diagnostics/null_check_always_fails_test.dart

@@ -0,0 +1,68 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analyzer/src/error/codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(NullNeverNotNullTest);
+  });
+}
+
+@reflectiveTest
+class NullNeverNotNullTest extends PubPackageResolutionTest
+    with WithNullSafetyMixin {
+  test_nullable() async {
+    await assertNoErrorsInCode(r'''
+void f(int? i) {
+  i!;
+}
+''');
+  }
+
+  test_nullLiteral() async {
+    await assertErrorsInCode(r'''
+void f() {
+  null!;
+}
+''', [
+      error(HintCode.NULL_CHECK_ALWAYS_FAILS, 13, 5),
+    ]);
+  }
+
+  test_nullLiteral_parenthesized() async {
+    await assertErrorsInCode(r'''
+void f() {
+  null!;
+}
+''', [
+      error(HintCode.NULL_CHECK_ALWAYS_FAILS, 13, 5),
+    ]);
+  }
+
+  test_nullType() async {
+    await assertErrorsInCode(r'''
+void f() {
+  g()!;
+}
+Null g() => null;
+''', [
+      error(HintCode.NULL_CHECK_ALWAYS_FAILS, 13, 4),
+    ]);
+  }
+
+  test_nullType_awaited() async {
+    await assertErrorsInCode(r'''
+void f() async {
+  (await g())!;
+}
+Future<Null> g() async => null;
+''', [
+      error(HintCode.NULL_CHECK_ALWAYS_FAILS, 19, 12),
+    ]);
+  }
+}

pkg/analyzer/test/src/diagnostics/test_all.dart

@@ -464,6 +464,7 @@ import 'null_aware_before_operator_test.dart' as null_aware_before_operator;
 import 'null_aware_in_condition_test.dart' as null_aware_in_condition;
 import 'null_aware_in_logical_operator_test.dart'
     as null_aware_in_logical_operator;
+import 'null_check_always_fails_test.dart' as null_check_always_fails;
 import 'null_safety_read_write_test.dart' as null_safety_read_write;
 import 'nullable_type_in_catch_clause_test.dart'
     as nullable_type_in_catch_clause;
@@ -963,6 +964,7 @@ main() {
     null_aware_before_operator.main();
     null_aware_in_condition.main();
     null_aware_in_logical_operator.main();
+    null_check_always_fails.main();
     null_safety_read_write.main();
     nullable_type_in_catch_clause.main();
     nullable_type_in_extends_clause.main();