From f12f19b59efcdc58cbd662404e06abeae6d8b527 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Mon, 7 Apr 2025 10:38:20 -0700 Subject: [PATCH] [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 https://github.com/dart-lang/sdk/issues/60402 Change-Id: Ie9b524a679df5e39856ecd900d94f2fb41b779bf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420703 Commit-Queue: Brian Wilkerson Reviewed-by: Brian Wilkerson Reviewed-by: Phil Quitslund --- .../lsp/handlers/handler_inline_value.dart | 41 +++++++++- .../test/lsp/inline_value_test.dart | 74 +++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_inline_value.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_inline_value.dart index 35b6292ef80..dc4fe5a0433 100644 --- a/pkg/analysis_server/lib/src/lsp/handlers/handler_inline_value.dart +++ b/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 { @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 { @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, diff --git a/pkg/analysis_server/test/lsp/inline_value_test.dart b/pkg/analysis_server/test/lsp/inline_value_test.dart index 7b7f83fe1b1..a960fcbe5e5 100644 --- a/pkg/analysis_server/test/lsp/inline_value_test.dart +++ b/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 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 p1, + Future p2, + FutureOr p3, + Stream p4, +) { + ^ +} +'''); + + await verify_values(code); + } + Future 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 test_parameter_iterables() async { + experimentalInlineValuesProperties = true; + + code = TestCode.parse(r''' +void f(List list1, List /*[0*/list2/*0]*/, Iterable iterable1, Iterable iterable2) { + print(/*[1*/list1/*1]*/); + print(iterable1); + ^ +} +'''); + + await verify_values(code, ofType: InlineValueVariableLookup); + } + Future 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 test_property_iterables() async { + experimentalInlineValuesProperties = true; + + code = TestCode.parse(r''' +void f(List /*[0*/list/*0]*/, Iterable iterable) { + print(/*[1*/list.length/*1]*/); + print(iterable.length); + ^ +} +'''); + + await verify_values( + code, + ofTypes: { + 0: InlineValueVariableLookup, + 1: InlineValueEvaluatableExpression, + }, + ); + } + Future 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 test_variable_iterables() async { + experimentalInlineValuesProperties = true; + + code = TestCode.parse(r''' +void f() { + var list = [1,]; + var iterable = list as Iterable; + + print(/*[0*/list/*0]*/); + print(iterable); + ^ +} +'''); + + await verify_values(code, ofType: InlineValueVariableLookup); + } + Future test_variable_propertyAccess() async { code = TestCode.parse(r''' void f(int /*[0*/aaa/*0]*/) {