diff --git a/pkg/analysis_server/lib/src/lsp/client_configuration.dart b/pkg/analysis_server/lib/src/lsp/client_configuration.dart index 3cf71d28c0b..99004a22d3f 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 @@ class LspGlobalClientConfiguration extends LspResourceClientConfiguration { 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 5331f51d28a..4900b762b39 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 @@ class InlineValueHandler // 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(); + 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 @@ class _InlineValueCollector { _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 @@ class _InlineValueCollector { // 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 { + 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 @@ class _InlineValueVisitor extends GeneralizingAstVisitor { @override void visitFunctionExpression(FunctionExpression node) { // Don't descend into nested functions. - if (node != rootFunction) { + if (node != rootNode) { return; } @@ -190,12 +237,53 @@ class _InlineValueVisitor extends GeneralizingAstVisitor { } @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 6d386fb8570..d61f50fc5aa 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 @@ void main() { 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 test_parameter_declaration() async { code = TestCode.parse(r''' void f(int /*[0*/aaa/*0]*/, int /*[1*/bbb/*1]*/) { @@ -28,7 +33,7 @@ void f(int /*[0*/aaa/*0]*/, int /*[1*/bbb/*1]*/) { } '''); - await verify_variables(code); + await verify_values(code, ofType: InlineValueVariableLookup); } Future test_parameter_read() async { @@ -38,7 +43,7 @@ void f(int aaa, int bbb) { } '''); - await verify_variables(code); + await verify_values(code, ofType: InlineValueVariableLookup); } Future test_parameter_write() async { @@ -51,7 +56,7 @@ void f(int aaa, int bbb, int ccc) { } '''); - await verify_variables(code); + await verify_values(code, ofType: InlineValueVariableLookup); } Future test_parameters_scope() async { @@ -70,7 +75,156 @@ void f(int aaa, int bbb, int ccc) { } '''); - await verify_variables(code); + await verify_values(code, ofType: InlineValueVariableLookup); + } + + Future 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 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 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 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 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 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 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 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 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 test_variable_declaration() async { @@ -84,7 +238,18 @@ void f() { } '''); - await verify_variables(code); + await verify_values(code, ofType: InlineValueVariableLookup); + } + + Future 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 test_variable_read() async { @@ -96,7 +261,7 @@ void f() { } '''); - await verify_variables(code); + await verify_values(code, ofType: InlineValueVariableLookup); } Future test_variable_write() async { @@ -110,7 +275,7 @@ void f() { } '''); - await verify_variables(code); + await verify_values(code, ofType: InlineValueVariableLookup); } Future test_variables_scope() async { @@ -131,13 +296,23 @@ void f() { } '''); - await verify_variables(code); + await verify_values(code, ofType: InlineValueVariableLookup); } - /// Verifies [code] produces the [expected] variables (and only those - /// variables). - Future 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 verify_values( + TestCode code, { + Type? ofType, + Map? ofTypes, + }) async { + await provideConfig(initialize, { + if (experimentalInlineValuesProperties) + 'experimentalInlineValuesProperties': true, + }); await openFile(mainFileUri, code.code); await initialAnalysis; @@ -147,11 +322,17 @@ void f() { 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)); }