[analyzer] Re-work handling of null shorting using shared code.

In https://dart-review.googlesource.com/c/sdk/+/399480, a mechanism
was introduced to `_fe_analyzer_shared` to support null-shorting.
Sometime later, in
https://dart-review.googlesource.com/c/sdk/+/427341, the CFE was
changed to use this shared mechanism.

This CL completes the arc of work by changing the analyzer to use the
new shared mechanism.

The public API class `NullShortableExpression`, which was part of the
analyzer's old implementation of null shorting, is no longer used. It
should never have been exposed through the analyzer public API in the
first place, so it's been deprecated; I will update the pending
changes for analyzer 9.0.0 to delete it.

Change-Id: Ic711df6c537c454aa8a99861135e63c8dd277485
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400021
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: stereotype441

pkg/_fe_analyzer_shared/lib/src/type_inference/shared_inference_log.dart

@@ -257,6 +257,13 @@ abstract interface class SharedInferenceLogWriter {
     required String methodName,
   });
 
+  /// Called when null shorting terminates, and so the type of an expression is
+  /// made nullable.
+  ///
+  /// [expression] is the expression whose type is changed, and [type] is the
+  /// type it is changed to.
+  void recordNullShortedType(Object expression, SharedType type);
+
   /// Records the preliminary types chosen during either a downwards or a
   /// horizontal inference step.
   void recordPreliminaryTypes(List<SharedType> types);
@@ -788,6 +795,17 @@ abstract class SharedInferenceLogWriterImpl
     addEvent(new Event(message: 'LOOKUP $query FINDS $type'));
   }
 
+  @override
+  void recordNullShortedType(Object expression, SharedType type) {
+    addEvent(
+      new Event(
+        message:
+            'STATIC TYPE OF ${describe(expression)} REFINED BY NULL SHORTING '
+            'TO $type',
+      ),
+    );
+  }
+
   @override
   void recordPreliminaryTypes(List<SharedType> types) {
     checkCall(

pkg/analyzer/CHANGELOG.md

@@ -14,6 +14,8 @@
 * Deprecate `InterfaceFragment.supertype`, use `InterfaceElement.supertype` instead.
 * Deprecate `ExtensionTypeFragment.primaryConstructor`, use `ExtensionTypeElement.primaryConstructor`.
 * Deprecate `ExtensionTypeFragment.representation`, use `ExtensionTypeElement.representation`.
+* Deprecate `NullShortableExpression`. This interface should not be needed by
+  analyzer clients, and is no longer supported.
 
 ## 8.1.1
 * Fix for `EnumSet` usage when compiled to JavaScript.

pkg/analyzer/api.txt

@@ -1483,8 +1483,8 @@ package:analyzer/dart/ast/ast.dart:
     pattern (getter: DartPattern)
   NullLiteral (class extends Object implements Literal):
     literal (getter: Token)
-  NullShortableExpression (class extends Object implements Expression):
-    nullShortingTermination (getter: Expression)
+  NullShortableExpression (class extends Object implements Expression, deprecated):
+    nullShortingTermination (getter: Expression, deprecated)
   ObjectPattern (class extends Object implements DartPattern):
     fields (getter: NodeList<PatternField>)
     leftParenthesis (getter: Token)

pkg/analyzer/lib/dart/ast/ast.dart

@@ -183,6 +183,7 @@ export 'package:analyzer/src/dart/ast/ast.dart'
         NullAwareElement,
         NullCheckPattern,
         NullLiteral,
+        // ignore: deprecated_member_use_from_same_package
         NullShortableExpression,
         ObjectPattern,
         ParenthesizedExpression,

pkg/analyzer/lib/src/dart/ast/ast.dart

@@ -1119,6 +1119,7 @@ final class AssignedVariablePatternImpl extends VariablePatternImpl
 @AnalyzerPublicApi(message: 'exported by lib/dart/ast/ast.dart')
 abstract final class AssignmentExpression
     implements
+        // ignore: deprecated_member_use_from_same_package
         NullShortableExpression,
         MethodReferenceExpression,
         CompoundAssignmentExpression {
@@ -2159,7 +2160,10 @@ final class BreakStatementImpl extends StatementImpl implements BreakStatement {
 ///      | identifier
 @AnalyzerPublicApi(message: 'exported by lib/dart/ast/ast.dart')
 abstract final class CascadeExpression
-    implements Expression, NullShortableExpression {
+    implements
+        Expression,
+        // ignore: deprecated_member_use_from_same_package
+        NullShortableExpression {
   /// The cascade sections sharing the common target.
   NodeList<Expression> get cascadeSections;
 
@@ -7977,6 +7981,15 @@ sealed class ExpressionImpl extends CollectionElementImpl
     }
   }
 
+  /// Called when null shorting terminates, and so the type of an expression
+  /// needs to be made nullable.
+  ///
+  /// [type] is the new static type of the expression.
+  void recordNullShortedType(TypeImpl type) {
+    _staticType = type;
+    inferenceLogWriter?.recordNullShortedType(this, type);
+  }
+
   /// Record that the static type of the given node is the given type.
   ///
   /// @param expression the node whose type is to be recorded
@@ -11198,7 +11211,10 @@ final class FunctionExpressionImpl extends ExpressionImpl
 ///        [Expression] [TypeArgumentList]? [ArgumentList]
 @AnalyzerPublicApi(message: 'exported by lib/dart/ast/ast.dart')
 abstract final class FunctionExpressionInvocation
-    implements NullShortableExpression, InvocationExpression {
+    implements
+        // ignore: deprecated_member_use_from_same_package
+        NullShortableExpression,
+        InvocationExpression {
   /// The element associated with the function being invoked based on static
   /// type information.
   ///
@@ -13365,7 +13381,10 @@ final class ImportPrefixReferenceImpl extends AstNodeImpl
 ///        [Expression] '[' [Expression] ']'
 @AnalyzerPublicApi(message: 'exported by lib/dart/ast/ast.dart')
 abstract final class IndexExpression
-    implements NullShortableExpression, MethodReferenceExpression {
+    implements
+        // ignore: deprecated_member_use_from_same_package
+        NullShortableExpression,
+        MethodReferenceExpression {
   /// The expression used to compute the index.
   Expression get index;
 
@@ -16097,7 +16116,10 @@ final class MethodDeclarationImpl extends ClassMemberImpl
 ///        [ArgumentList]
 @AnalyzerPublicApi(message: 'exported by lib/dart/ast/ast.dart')
 abstract final class MethodInvocation
-    implements NullShortableExpression, InvocationExpression {
+    implements
+        // ignore: deprecated_member_use_from_same_package
+        NullShortableExpression,
+        InvocationExpression {
   /// Whether this expression is cascaded.
   ///
   /// If it is, then the target of this expression isn't stored locally but is
@@ -17866,7 +17888,12 @@ final class NullLiteralImpl extends LiteralImpl implements NullLiteral {
 }
 
 /// Abstract interface for expressions that may participate in null-shorting.
+///
+/// This is an analyzer-internal interface that was exposed through the public
+/// API by mistake. It is deprecated and will be removed in analyzer version
+/// 9.0.0.
 @AnalyzerPublicApi(message: 'exported by lib/dart/ast/ast.dart')
+@Deprecated('No longer supported.')
 abstract final class NullShortableExpression implements Expression {
   /// The expression that terminates any null shorting that might occur in this
   /// expression.
@@ -17883,10 +17910,14 @@ abstract final class NullShortableExpression implements Expression {
   /// Calling [nullShortingTermination] on any of these subexpressions yields
   /// the expression `a?.b[c] = d`, indicating that the null-shorting induced by
   /// the `?.` causes the rest of the subexpression `a?.b[c] = d` to be skipped.
+  @Deprecated('No longer supported.')
   Expression get nullShortingTermination;
 }
 
-base mixin NullShortableExpressionImpl implements NullShortableExpression {
+base mixin NullShortableExpressionImpl
+    implements
+        // ignore: deprecated_member_use_from_same_package
+        NullShortableExpression {
   @override
   Expression get nullShortingTermination {
     var result = this;
@@ -19121,6 +19152,7 @@ final class PatternVariableDeclarationStatementImpl extends StatementImpl
 abstract final class PostfixExpression
     implements
         Expression,
+        // ignore: deprecated_member_use_from_same_package
         NullShortableExpression,
         MethodReferenceExpression,
         CompoundAssignmentExpression {
@@ -19398,6 +19430,7 @@ final class PrefixedIdentifierImpl extends IdentifierImpl
 abstract final class PrefixExpression
     implements
         Expression,
+        // ignore: deprecated_member_use_from_same_package
         NullShortableExpression,
         MethodReferenceExpression,
         CompoundAssignmentExpression {
@@ -19531,7 +19564,10 @@ final class PrefixExpressionImpl extends ExpressionImpl
 ///        [Expression] '.' [SimpleIdentifier]
 @AnalyzerPublicApi(message: 'exported by lib/dart/ast/ast.dart')
 abstract final class PropertyAccess
-    implements NullShortableExpression, CommentReferableExpression {
+    implements
+        // ignore: deprecated_member_use_from_same_package
+        NullShortableExpression,
+        CommentReferableExpression {
   /// Whether this expression is cascaded.
   ///
   /// If it is, then the target of this expression isn't stored locally but is

pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart

@@ -117,8 +117,6 @@ class AssignmentExpressionResolver {
         flow.ifNullExpression_end();
       }
     }
-
-    _resolver.nullShortingTermination(node);
   }
 
   void _checkForInvalidAssignment(

pkg/analyzer/lib/src/dart/resolver/postfix_expression_resolver.dart

@@ -204,8 +204,6 @@ class PostfixExpressionResolver {
       }
       node.recordStaticType(receiverType, resolver: _resolver);
     }
-
-    _resolver.nullShortingTermination(node);
   }
 
   void _resolveNullCheck(
@@ -227,6 +225,7 @@ class PostfixExpressionResolver {
     _resolver.analyzeExpression(
       operand,
       SharedTypeSchemaView(_typeSystem.makeNullable(contextType)),
+      continueNullShorting: true,
     );
     operand = _resolver.popRewrite()!;
 
@@ -236,6 +235,5 @@ class PostfixExpressionResolver {
     node.recordStaticType(type, resolver: _resolver);
 
     _resolver.flowAnalysis.flow?.nonNullAssert_end(operand);
-    _resolver.nullShortingTermination(node);
   }
 }

pkg/analyzer/lib/src/dart/resolver/prefix_expression_resolver.dart

@@ -249,7 +249,6 @@ class PrefixExpressionResolver {
       }
       node.recordStaticType(staticType, resolver: _resolver);
     }
-    _resolver.nullShortingTermination(node);
   }
 
   void _resolveNegation(PrefixExpressionImpl node) {

pkg/analyzer/lib/src/generated/inference_log.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:_fe_analyzer_shared/src/type_inference/shared_inference_log.dart';
+import 'package:_fe_analyzer_shared/src/types/shared_type.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/element/type.dart';
@@ -223,6 +224,27 @@ final class _InferenceLogWriterImpl extends SharedInferenceLogWriterImpl
     _inBodyOrInitializer = false;
   }
 
+  @override
+  void recordExpressionRewrite({
+    Object? oldExpression,
+    required Object newExpression,
+  }) {
+    if (oldExpression != null) {
+      assertExpressionWasRecorded(oldExpression);
+    }
+    _recordedExpressions[newExpression] = true;
+    super.recordExpressionRewrite(
+      oldExpression: oldExpression,
+      newExpression: newExpression,
+    );
+  }
+
+  @override
+  void recordNullShortedType(Object expression, SharedType type) {
+    assertExpressionWasRecorded(expression);
+    super.recordNullShortedType(expression, type);
+  }
+
   @override
   void setExpressionVisitCodePath(
     Expression node,