[analysis_server] Exclude some potential side effects from Inline Values

When the property experiment is enabled, we may evaluate getters in inline values. For some types, the chance of these having side effects are much higher (for example accessing `length`, `last`, `first` on `Iterable`s or `Stream`s.

This suppresses inline values on these types (both the variables themselves to avoid any `toString()`s, and any getters).

Fixes #60402

Change-Id: Ie9b524a679df5e39856ecd900d94f2fb41b779bf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420703
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>

Author: DanTup

pkg/analysis_server/lib/src/lsp/handlers/handler_inline_value.dart

@@ -14,6 +14,7 @@ import 'package:analysis_server/src/services/correction/dart/convert_null_check_
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element2.dart';
+import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/source/line_info.dart';
 import 'package:analyzer/src/dart/element/extensions.dart';
 
@@ -210,9 +211,36 @@ class _InlineValueCollector {
     );
   }
 
+  /// Returns whether [element] is something that should never be eagerly
+  /// evaluated because of potential side-effects (such as `iterable.length`).
+  bool _isExcludedElement(Element2 element) {
+    return switch (element) {
+      VariableElement2() => _isExcludedType(element.type),
+      GetterElement() => _isExcludedType(element.returnType),
+      _ => false,
+    };
+  }
+
+  /// Returns whether [type] is something that should never be eagerly
+  /// evaluated because of potential side-effects (such as `iterable.length`).
+  bool _isExcludedType(DartType? type) {
+    if (type == null) {
+      return false;
+    }
+    return type.isDartCoreIterable ||
+        type.isDartAsyncFuture ||
+        type.isDartAsyncFutureOr ||
+        type.isDartAsyncStream;
+  }
+
   /// Records an inline value [value] for [element] if it is within range and is
   /// the latest one in the source for that element.
   void _record(InlineValue value, Element2 element) {
+    // Don't create values for any elements that are excluded types.
+    if (_isExcludedElement(element)) {
+      return;
+    }
+
     var range = _getRange(value);
 
     // We only want to show each variable once, so keep only the one furthest
@@ -266,6 +294,11 @@ class _InlineValueVisitor extends GeneralizingAstVisitor<void> {
   @override
   void visitPrefixedIdentifier(PrefixedIdentifier node) {
     if (experimentalInlineValuesProperties) {
+      // Don't create values for excluded types or access of their properties.
+      if (collector._isExcludedType(node.prefix.staticType)) {
+        return;
+      }
+
       var parent = node.parent;
 
       // Never produce values for the left side of a property access.
@@ -286,7 +319,13 @@ class _InlineValueVisitor extends GeneralizingAstVisitor<void> {
 
   @override
   void visitPropertyAccess(PropertyAccess node) {
-    if (experimentalInlineValuesProperties && node.target is Identifier) {
+    var target = node.target;
+    if (experimentalInlineValuesProperties && target is Identifier) {
+      // Don't create values for excluded types or access of their properties.
+      if (collector._isExcludedType(target.staticType)) {
+        return;
+      }
+
       collector.recordExpression(
         node.canonicalElement,
         node.offset,

pkg/analysis_server/test/lsp/inline_value_test.dart

@@ -25,6 +25,26 @@ class InlineValueTest extends AbstractLspAnalysisServerTest {
   /// client configuration passed during initialization.
   bool experimentalInlineValuesProperties = false;
 
+  Future<void> test_iterables() async {
+    experimentalInlineValuesProperties = true;
+
+    // There are no marked ranges, because none of these should produce values.
+    code = TestCode.parse(r'''
+import 'dart:async';
+
+void f(
+  Iterable<int> p1,
+  Future<int> p2,
+  FutureOr<int> p3,
+  Stream<int> p4,
+) {
+  ^
+}
+''');
+
+    await verify_values(code);
+  }
+
   Future<void> test_parameter_declaration() async {
     code = TestCode.parse(r'''
 void f(int /*[0*/aaa/*0]*/, int /*[1*/bbb/*1]*/) {
@@ -36,6 +56,21 @@ void f(int /*[0*/aaa/*0]*/, int /*[1*/bbb/*1]*/) {
     await verify_values(code, ofType: InlineValueVariableLookup);
   }
 
+  /// Lists are included, iterables are not.
+  Future<void> test_parameter_iterables() async {
+    experimentalInlineValuesProperties = true;
+
+    code = TestCode.parse(r'''
+void f(List list1, List<int> /*[0*/list2/*0]*/, Iterable iterable1, Iterable iterable2) {
+  print(/*[1*/list1/*1]*/);
+  print(iterable1);
+  ^
+}
+''');
+
+    await verify_values(code, ofType: InlineValueVariableLookup);
+  }
+
   Future<void> test_parameter_read() async {
     code = TestCode.parse(r'''
 void f(int aaa, int bbb) {
@@ -156,6 +191,27 @@ void f() {
     await verify_values(code, ofType: InlineValueEvaluatableExpression);
   }
 
+  /// Lists are included, iterables are not.
+  Future<void> test_property_iterables() async {
+    experimentalInlineValuesProperties = true;
+
+    code = TestCode.parse(r'''
+void f(List<int> /*[0*/list/*0]*/, Iterable<int> iterable) {
+  print(/*[1*/list.length/*1]*/);
+  print(iterable.length);
+  ^
+}
+''');
+
+    await verify_values(
+      code,
+      ofTypes: {
+        0: InlineValueVariableLookup,
+        1: InlineValueEvaluatableExpression,
+      },
+    );
+  }
+
   Future<void> test_property_method() async {
     experimentalInlineValuesProperties = true;
 
@@ -300,6 +356,24 @@ void f() {
     await verify_values(code, ofType: InlineValueVariableLookup);
   }
 
+  /// Lists are included, iterables are not.
+  Future<void> test_variable_iterables() async {
+    experimentalInlineValuesProperties = true;
+
+    code = TestCode.parse(r'''
+void f() {
+  var list = [1,];
+  var iterable = list as Iterable<int>;
+
+  print(/*[0*/list/*0]*/);
+  print(iterable);
+  ^
+}
+''');
+
+    await verify_values(code, ofType: InlineValueVariableLookup);
+  }
+
   Future<void> test_variable_propertyAccess() async {
     code = TestCode.parse(r'''
 void f(int /*[0*/aaa/*0]*/) {