[analysis_server] Support properties in line values behind a flag
+ fix them not showing up inside methods
Change-Id: I885afe68d56ab60c20b816373fbd071592cfd463
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/412644
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/client_configuration.dart b/pkg/analysis_server/lib/src/lsp/client_configuration.dart
index 3cf71d2..99004a2 100644
--- a/pkg/analysis_server/lib/src/lsp/client_configuration.dart
+++ b/pkg/analysis_server/lib/src/lsp/client_configuration.dart
@@ -191,6 +191,10 @@
bool get completeFunctionCalls =>
_settings['completeFunctionCalls'] as bool? ?? false;
+ /// A flag for including property access in Inline Values.
+ bool get experimentalInlineValuesProperties =>
+ _settings['experimentalInlineValuesProperties'] as bool? ?? false;
+
/// A flag for enabling interactive refactors flagged as experimental.
///
/// This flag is likely to be used by both analysis server developers (working
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 5331f51..4900b76 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
@@ -4,11 +4,13 @@
import 'package:analysis_server/lsp_protocol/protocol.dart' hide MessageType;
import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/lsp/client_configuration.dart';
import 'package:analysis_server/src/lsp/error_or.dart';
import 'package:analysis_server/src/lsp/extensions/positions.dart';
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
import 'package:analysis_server/src/lsp/mapping.dart';
import 'package:analysis_server/src/lsp/registration/feature_registration.dart';
+import 'package:analysis_server/src/services/correction/dart/convert_null_check_to_null_aware_element_or_entry.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element2.dart';
@@ -66,13 +68,19 @@
// Find the function that is executing. We will only show values for
// this single function expression.
var node = await server.getNodeAtOffset(filePath, stoppedOffset);
- var function = node?.thisOrAncestorOfType<FunctionExpression>();
+ var function = node?.thisOrAncestorMatching(
+ (node) => node is FunctionExpression || node is MethodDeclaration,
+ );
if (function == null) {
return success(null);
}
var collector = _InlineValueCollector(lineInfo, applicableRange);
- var visitor = _InlineValueVisitor(collector, function);
+ var visitor = _InlineValueVisitor(
+ server.lspClientConfiguration,
+ collector,
+ function,
+ );
function.accept(visitor);
return success(collector.values.values.toList());
@@ -117,19 +125,64 @@
_InlineValueCollector(this.lineInfo, this.applicableRange);
- /// Records a variable inline value for [element] with [offset]/[length].
+ /// Records an expression inline value for [element] with [offset]/[length].
///
- /// Variable inline values are sent to the client without expressions/names
- /// because the client can infer the name from the range and look it up from
- /// the debuggers Scopes/Variables.
- void recordVariableLookup(Element2? element, int offset, int length) {
- if (element == null || element.isWildcardVariable) return;
-
+ /// Expression values are sent to the client without expressions because the
+ /// client can use the range from the source to get the expression.
+ void recordExpression(Element2? element, int offset, int length) {
assert(offset >= 0);
assert(length > 0);
+ if (element == null) return;
var range = toRange(lineInfo, offset, length);
+ var value = InlineValue.t1(
+ InlineValueEvaluatableExpression(
+ range: range,
+ // We don't provide expression, because it always matches the source
+ // code and can be inferred.
+ ),
+ );
+ _record(value, element);
+ }
+
+ /// Records a variable inline value for [element] with [offset]/[length].
+ ///
+ /// Variable inline values are sent to the client without names because the
+ /// client can infer the name from the range and look it up from the debuggers
+ /// Scopes/Variables.
+ void recordVariableLookup(Element2? element, int offset, int length) {
+ assert(offset >= 0);
+ assert(length > 0);
+ if (element == null || element.isWildcardVariable) return;
+
+ var range = toRange(lineInfo, offset, length);
+
+ var value = InlineValue.t3(
+ InlineValueVariableLookup(
+ caseSensitiveLookup: true,
+ range: range,
+ // We don't provide name, because it always matches the source code
+ // for a variable and can be inferred.
+ ),
+ );
+ _record(value, element);
+ }
+
+ /// Extracts the range from an [InlineValue].
+ Range _getRange(InlineValue value) {
+ return value.map(
+ (expression) => expression.range,
+ (text) => text.range,
+ (variable) => variable.range,
+ );
+ }
+
+ /// 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) {
+ var range = _getRange(value);
+
// Never record anything outside of the visible range.
if (!range.intersects(applicableRange)) {
return;
@@ -137,34 +190,28 @@
// We only want to show each variable once, so keep only the one furthest
// into the source (closest to the execution pointer).
- var existingPosition = values[element]?.map(
- (expression) => expression.range.start,
- (text) => text.range.start,
- (variable) => variable.range.start,
- );
- if (existingPosition != null &&
- existingPosition.isAfterOrEqual(range.start)) {
- return;
+ if (values[element] case var existingValue?) {
+ var existingPosition = _getRange(existingValue).start;
+ if (existingPosition.isAfterOrEqual(range.start)) {
+ return;
+ }
}
- values[element] = InlineValue.t3(
- InlineValueVariableLookup(
- caseSensitiveLookup: true,
- range: range,
- // We don't provide name, because it always matches the source code
- // for a variable and can be inferred.
- ),
- );
+ values[element] = value;
}
}
/// Visits a function expression and reports nodes that should have inline
/// values to [collector].
class _InlineValueVisitor extends GeneralizingAstVisitor<void> {
+ final LspClientConfiguration clientConfiguration;
final _InlineValueCollector collector;
- final FunctionExpression rootFunction;
+ final AstNode rootNode;
- _InlineValueVisitor(this.collector, this.rootFunction);
+ _InlineValueVisitor(this.clientConfiguration, this.collector, this.rootNode);
+
+ bool get experimentalInlineValuesProperties =>
+ clientConfiguration.global.experimentalInlineValuesProperties;
@override
void visitFormalParameter(FormalParameter node) {
@@ -182,7 +229,7 @@
@override
void visitFunctionExpression(FunctionExpression node) {
// Don't descend into nested functions.
- if (node != rootFunction) {
+ if (node != rootNode) {
return;
}
@@ -190,12 +237,53 @@
}
@override
- void visitSimpleIdentifier(SimpleIdentifier node) {
- switch (node.element) {
- case LocalVariableElement2(name3: _?):
- case FormalParameterElement():
- collector.recordVariableLookup(node.element, node.offset, node.length);
+ void visitPrefixedIdentifier(PrefixedIdentifier node) {
+ if (experimentalInlineValuesProperties) {
+ var parent = node.parent;
+
+ // Never produce values for the left side of a property access.
+ var isTarget = parent is PropertyAccess && node == parent.realTarget;
+ if (!isTarget) {
+ collector.recordExpression(node.element, node.offset, node.length);
+ }
}
+
+ super.visitPrefixedIdentifier(node);
+ }
+
+ @override
+ void visitPropertyAccess(PropertyAccess node) {
+ if (experimentalInlineValuesProperties && node.target is Identifier) {
+ collector.recordExpression(
+ node.canonicalElement,
+ node.offset,
+ node.length,
+ );
+ }
+
+ super.visitPropertyAccess(node);
+ }
+
+ @override
+ void visitSimpleIdentifier(SimpleIdentifier node) {
+ var parent = node.parent;
+
+ // Never produce values for the left side of a prefixed identifier.
+ // Or parts of an invocation.
+ var isTarget = parent is PrefixedIdentifier && node == parent.prefix;
+ var isInvocation = parent is InvocationExpression;
+ if (!isTarget && !isInvocation) {
+ switch (node.element) {
+ case LocalVariableElement2(name3: _?):
+ case FormalParameterElement():
+ collector.recordVariableLookup(
+ node.element,
+ node.offset,
+ node.length,
+ );
+ }
+ }
+
super.visitSimpleIdentifier(node);
}
diff --git a/pkg/analysis_server/test/lsp/inline_value_test.dart b/pkg/analysis_server/test/lsp/inline_value_test.dart
index 6d386fb..d61f50f 100644
--- a/pkg/analysis_server/test/lsp/inline_value_test.dart
+++ b/pkg/analysis_server/test/lsp/inline_value_test.dart
@@ -4,6 +4,7 @@
import 'package:analysis_server/lsp_protocol/protocol.dart';
import 'package:analyzer/src/test_utilities/test_code_format.dart';
+import 'package:collection/collection.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -20,6 +21,10 @@
class InlineValueTest extends AbstractLspAnalysisServerTest {
late TestCode code;
+ /// Whether to enable the inlineValuesProperties experiment flag in the
+ /// client configuration passed during initialization.
+ bool experimentalInlineValuesProperties = false;
+
Future<void> test_parameter_declaration() async {
code = TestCode.parse(r'''
void f(int /*[0*/aaa/*0]*/, int /*[1*/bbb/*1]*/) {
@@ -28,7 +33,7 @@
}
''');
- await verify_variables(code);
+ await verify_values(code, ofType: InlineValueVariableLookup);
}
Future<void> test_parameter_read() async {
@@ -38,7 +43,7 @@
}
''');
- await verify_variables(code);
+ await verify_values(code, ofType: InlineValueVariableLookup);
}
Future<void> test_parameter_write() async {
@@ -51,7 +56,7 @@
}
''');
- await verify_variables(code);
+ await verify_values(code, ofType: InlineValueVariableLookup);
}
Future<void> test_parameters_scope() async {
@@ -70,7 +75,156 @@
}
''');
- await verify_variables(code);
+ await verify_values(code, ofType: InlineValueVariableLookup);
+ }
+
+ Future<void> test_property_experimentDisabled() async {
+ code = TestCode.parse(r'''
+void f(String /*[0*/s/*0]*/) {
+ print(s.length); // No inline value, experiment not enabled
+ ^
+}
+''');
+
+ await verify_values(
+ code,
+ // The parameter is a variable even though this test is about properties.
+ ofType: InlineValueVariableLookup,
+ );
+ }
+
+ Future<void> test_property_getter() async {
+ experimentalInlineValuesProperties = true;
+
+ code = TestCode.parse(r'''
+void f(String /*[0*/s/*0]*/) {
+ print(/*[1*/s.length/*1]*/);
+ print(/*[2*/s.length.isEven/*2]*/);
+ ^
+}
+''');
+
+ await verify_values(
+ code,
+ ofTypes: {
+ 0: InlineValueVariableLookup,
+ 1: InlineValueEvaluatableExpression,
+ 2: InlineValueEvaluatableExpression,
+ },
+ );
+ }
+
+ Future<void> test_property_method() async {
+ experimentalInlineValuesProperties = true;
+
+ code = TestCode.parse(r'''
+class A {
+ void x() {}
+}
+void f(A /*[0*/a/*0]*/) {
+ a.x(); // No value for methods.
+ ^
+}
+''');
+
+ await verify_values(code, ofType: InlineValueVariableLookup);
+ }
+
+ Future<void> test_property_method_targets() async {
+ experimentalInlineValuesProperties = true;
+
+ code = TestCode.parse(r'''
+class A {
+ String x(int a) => a.toString();
+}
+void f(A /*[0*/a/*0]*/, int b) {
+ // No value for length because the expression contains a method call.
+ a.x(b).length;
+
+ // No value for either length of isEven because the expression contains a
+ // method call.
+ a.x(/*[1*/b/*1]*/).length.isEven;
+ ^
+}
+''');
+
+ await verify_values(code, ofType: InlineValueVariableLookup);
+ }
+
+ Future<void> test_property_setter() async {
+ experimentalInlineValuesProperties = true;
+
+ code = TestCode.parse(r'''
+class A {
+ int? x;
+}
+void f(A /*[0*/a/*0]*/) {
+ a.x = 1; // No value for setters.
+ ^
+}
+''');
+
+ await verify_values(code, ofType: InlineValueVariableLookup);
+ }
+
+ Future<void> test_scope_method_inNestedFunction() async {
+ code = TestCode.parse(r'''
+class A {
+ void method() {
+ void inner() {
+ var [!valueVar!] = 1;
+ ^
+ }
+ var noValueVar = 1;
+ }
+}
+''');
+
+ await verify_values(code, ofType: InlineValueVariableLookup);
+ }
+
+ Future<void> test_scope_method_notInNestedFunction() async {
+ code = TestCode.parse(r'''
+class A {
+ void method() {
+ void inner() {
+ var noValueVar = 1;
+ }
+ var [!valueVar!] = 1;
+ ^
+ }
+}
+''');
+
+ await verify_values(code, ofType: InlineValueVariableLookup);
+ }
+
+ Future<void> test_scope_topLevelFunction_inNestedFunction() async {
+ code = TestCode.parse(r'''
+void top() {
+ void inner() {
+ var [!valueVar!] = 1;
+ ^
+ }
+ var noValueVar = 1;
+}
+''');
+
+ await verify_values(code, ofType: InlineValueVariableLookup);
+ }
+
+ Future<void> test_scope_topLevelFunction_notInNestedFunction() async {
+ code = TestCode.parse(r'''
+void top() {
+ void inner() {
+ var noValueVar = 1;
+ }
+ var [!valueVar!] = 1;
+ ^
+}
+''');
+
+ await verify_values(code, ofType: InlineValueVariableLookup);
}
Future<void> test_variable_declaration() async {
@@ -84,7 +238,18 @@
}
''');
- await verify_variables(code);
+ await verify_values(code, ofType: InlineValueVariableLookup);
+ }
+
+ Future<void> test_variable_propertyAccess() async {
+ code = TestCode.parse(r'''
+void f(int /*[0*/aaa/*0]*/) {
+ aaa.isEven; // No inline value for var because it's in a property access.
+ ^
+}
+''');
+
+ await verify_values(code, ofType: InlineValueVariableLookup);
}
Future<void> test_variable_read() async {
@@ -96,7 +261,7 @@
}
''');
- await verify_variables(code);
+ await verify_values(code, ofType: InlineValueVariableLookup);
}
Future<void> test_variable_write() async {
@@ -110,7 +275,7 @@
}
''');
- await verify_variables(code);
+ await verify_values(code, ofType: InlineValueVariableLookup);
}
Future<void> test_variables_scope() async {
@@ -131,13 +296,23 @@
}
''');
- await verify_variables(code);
+ await verify_values(code, ofType: InlineValueVariableLookup);
}
- /// Verifies [code] produces the [expected] variables (and only those
- /// variables).
- Future<void> verify_variables(TestCode code) async {
- await initialize();
+ /// Verifies [code] produces values at the marked ranges.
+ ///
+ /// The [ofTypes] contains the kind of value to be expected for the range with
+ /// the same index. If a range is not included in [ofTypes] then [ofType] is
+ /// used instead.
+ Future<void> verify_values(
+ TestCode code, {
+ Type? ofType,
+ Map<int, Type>? ofTypes,
+ }) async {
+ await provideConfig(initialize, {
+ if (experimentalInlineValuesProperties)
+ 'experimentalInlineValuesProperties': true,
+ });
await openFile(mainFileUri, code.code);
await initialAnalysis;
@@ -147,11 +322,17 @@
stoppedAt: code.position.position,
);
- var expectedValues = code.ranges.ranges.map(
- (range) => InlineValue.t3(
- InlineValueVariableLookup(caseSensitiveLookup: true, range: range),
- ),
- );
+ var expectedValues = code.ranges.ranges.mapIndexed((index, range) {
+ return switch (ofTypes?[index] ?? ofType) {
+ const (InlineValueVariableLookup) => InlineValue.t3(
+ InlineValueVariableLookup(caseSensitiveLookup: true, range: range),
+ ),
+ const (InlineValueEvaluatableExpression) => InlineValue.t1(
+ InlineValueEvaluatableExpression(range: range),
+ ),
+ _ => throw 'No type provided for range $index',
+ };
+ });
expect(actualValues, unorderedEquals(expectedValues));
}
