From 02651515eb1f8cb38ad8b8deb5d01bb7c256f17b Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Sun, 21 Oct 2018 21:12:13 +0000 Subject: [PATCH] Fix identifyWidgetExpression() for Flutter. Probably a bit pessimistic approach, allow only cases that we know to be valid and useful. R=brianwilkerson@google.com Bug: https://github.com/flutter/flutter-intellij/issues/2714 Change-Id: I8835c03d919c1f00a986ff3ef677c7f69995682f Reviewed-on: https://dart-review.googlesource.com/c/80985 Reviewed-by: Brian Wilkerson Commit-Queue: Konstantin Shcheglov --- .../lib/src/utilities/flutter.dart | 10 +- .../test/services/correction/assist_test.dart | 54 ++++++++ .../test/src/utilities/flutter_test.dart | 131 +++++++++++++++--- 3 files changed, 175 insertions(+), 20 deletions(-) diff --git a/pkg/analysis_server/lib/src/utilities/flutter.dart b/pkg/analysis_server/lib/src/utilities/flutter.dart index 06c3d3ce8bda5..f116530a504a6 100644 --- a/pkg/analysis_server/lib/src/utilities/flutter.dart +++ b/pkg/analysis_server/lib/src/utilities/flutter.dart @@ -229,12 +229,18 @@ InstanceCreationExpression identifyNewExpression(AstNode node) { /** * Attempt to find and return the closest expression that encloses the [node] - * and is a Flutter `Widget`. Return `null` if nothing found. + * and is an independent Flutter `Widget`. Return `null` if nothing found. */ Expression identifyWidgetExpression(AstNode node) { for (; node != null; node = node.parent) { if (isWidgetExpression(node)) { - return node; + var parent = node.parent; + if (parent is ArgumentList || + parent is ListLiteral || + parent is NamedExpression && parent.expression == node || + parent is Statement) { + return node; + } } if (node is ArgumentList || node is Statement || node is FunctionBody) { return null; diff --git a/pkg/analysis_server/test/services/correction/assist_test.dart b/pkg/analysis_server/test/services/correction/assist_test.dart index 2241cfd6fbee4..a4430d1dc533d 100644 --- a/pkg/analysis_server/test/services/correction/assist_test.dart +++ b/pkg/analysis_server/test/services/correction/assist_test.dart @@ -5014,6 +5014,60 @@ class FakeFlutter {\r } } + test_flutterWrapWidget_OK_prefixedIdentifier_identifier() async { + addFlutterPackage(); + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +abstract class Foo extends Widget { + Widget bar; +} + +main(Foo foo) { + return foo./*caret*/bar; +} +'''); + _setCaretLocation(); + await assertHasAssist(DartAssistKind.FLUTTER_WRAP_GENERIC, ''' +import 'package:flutter/widgets.dart'; + +abstract class Foo extends Widget { + Widget bar; +} + +main(Foo foo) { + return widget(child: foo./*caret*/bar); +} +'''); + } + + test_flutterWrapWidget_OK_prefixedIdentifier_prefix() async { + addFlutterPackage(); + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +abstract class Foo extends Widget { + Widget bar; +} + +main(Foo foo) { + return /*caret*/foo.bar; +} +'''); + _setCaretLocation(); + await assertHasAssist(DartAssistKind.FLUTTER_WRAP_GENERIC, ''' +import 'package:flutter/widgets.dart'; + +abstract class Foo extends Widget { + Widget bar; +} + +main(Foo foo) { + return /*caret*/widget(child: foo.bar); +} +'''); + } + test_flutterWrapWidget_OK_singleLine1() async { addFlutterPackage(); await resolveTestUnit(''' diff --git a/pkg/analysis_server/test/src/utilities/flutter_test.dart b/pkg/analysis_server/test/src/utilities/flutter_test.dart index b4a0ef0506084..c148ef059ccc9 100644 --- a/pkg/analysis_server/test/src/utilities/flutter_test.dart +++ b/pkg/analysis_server/test/src/utilities/flutter_test.dart @@ -90,22 +90,7 @@ var w = new Foo(); expect(getWidgetPresentationText(w), isNull); } - test_identifyWidgetExpression_identifier() async { - await resolveTestUnit(''' -import 'package:flutter/widgets.dart'; - -main() { - var text = new Text('abc'); - text; -} -'''); - { - Expression expression = findNodeAtString("text;"); - expect(identifyWidgetExpression(expression), expression); - } - } - - test_identifyWidgetExpression_instanceCreation() async { + test_identifyWidgetExpression_node_instanceCreation() async { await resolveTestUnit(''' import 'package:flutter/widgets.dart'; @@ -155,7 +140,7 @@ class MyWidget extends StatelessWidget { } } - test_identifyWidgetExpression_invocation() async { + test_identifyWidgetExpression_node_invocation() async { await resolveTestUnit(''' import 'package:flutter/widgets.dart'; @@ -185,7 +170,7 @@ Text createText(String txt) => new Text(txt); } } - test_identifyWidgetExpression_namedExpression() async { + test_identifyWidgetExpression_node_namedExpression() async { await resolveTestUnit(''' import 'package:flutter/widgets.dart'; @@ -199,6 +184,50 @@ Text createEmptyText() => new Text(''); expect(identifyWidgetExpression(childExpression), isNull); } + test_identifyWidgetExpression_node_prefixedIdentifier_identifier() async { + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +abstract class Foo extends Widget { + Widget bar; +} + +main(Foo foo) { + foo.bar; // ref +} +'''); + SimpleIdentifier bar = findNodeAtString('bar; // ref'); + expect(identifyWidgetExpression(bar), bar.parent); + } + + test_identifyWidgetExpression_node_prefixedIdentifier_prefix() async { + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +abstract class Foo extends Widget { + Widget bar; +} + +main(Foo foo) { + foo.bar; // ref +} +'''); + SimpleIdentifier foo = findNodeAtString('foo.bar'); + expect(identifyWidgetExpression(foo), foo.parent); + } + + test_identifyWidgetExpression_node_simpleIdentifier() async { + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +main(Widget widget) { + widget; // ref +} +'''); + Expression expression = findNodeAtString('widget; // ref'); + expect(identifyWidgetExpression(expression), expression); + } + test_identifyWidgetExpression_null() async { await resolveTestUnit(''' import 'package:flutter/widgets.dart'; @@ -222,6 +251,72 @@ Text createEmptyText() => new Text(''); } } + test_identifyWidgetExpression_parent_argumentList() async { + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +main() { + var text = new Text('abc'); + useWidget(text); // ref +} + +void useWidget(Widget w) {} +'''); + Expression expression = findNodeAtString("text); // ref"); + expect(identifyWidgetExpression(expression), expression); + } + + test_identifyWidgetExpression_parent_expressionStatement() async { + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +main(Widget widget) { + widget; // ref +} +'''); + Expression expression = findNodeAtString("widget; // ref"); + expect(identifyWidgetExpression(expression), expression); + } + + test_identifyWidgetExpression_parent_listLiteral() async { + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +main(Widget widget) { + return [widget]; // ref +} +'''); + Expression expression = findNodeAtString("widget]; // ref"); + expect(identifyWidgetExpression(expression), expression); + } + + test_identifyWidgetExpression_parent_namedExpression() async { + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +main() { + var text = new Text('abc'); + useWidget(child: text); // ref +} + +void useWidget({Widget child}) {} +'''); + Expression expression = findNodeAtString("text); // ref"); + expect(identifyWidgetExpression(expression), expression); + } + + test_identifyWidgetExpression_parent_returnStatement() async { + await resolveTestUnit(''' +import 'package:flutter/widgets.dart'; + +main(Widget widget) { + return widget; // ref +} +'''); + Expression expression = findNodeAtString("widget; // ref"); + expect(identifyWidgetExpression(expression), expression); + } + test_isWidget() async { await resolveTestUnit(''' import 'package:flutter/widgets.dart';