From 3b198ed6ae1af640759da49f115fce3739e82712 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Sun, 18 Oct 2015 13:28:12 -0700 Subject: [PATCH] Disable completion of formal parameter names as *types*. R=brianwilkerson@google.com, danrubel@google.com BUG= Review URL: https://codereview.chromium.org/1414653002 . --- .../lib/src/services/completion/optype.dart | 37 +++++++++++-- .../completion/suggestion_builder.dart | 2 +- .../completion/completion_test_util.dart | 8 +++ .../test/services/completion/optype_test.dart | 54 +++++++++++++++++-- 4 files changed, 94 insertions(+), 7 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/completion/optype.dart b/pkg/analysis_server/lib/src/services/completion/optype.dart index dc389196d50..3408e963771 100644 --- a/pkg/analysis_server/lib/src/services/completion/optype.dart +++ b/pkg/analysis_server/lib/src/services/completion/optype.dart @@ -325,7 +325,36 @@ class _OpTypeAstVisitor extends GeneralizingAstVisitor { @override void visitFormalParameterList(FormalParameterList node) { - optype.includeTypeNameSuggestions = true; + dynamic entity = this.entity; + if (entity is Token && entity.previous != null) { + TokenType type = entity.previous.type; + if (type == TokenType.OPEN_PAREN || type == TokenType.COMMA) { + optype.includeTypeNameSuggestions = true; + } + } + // Handle default normal parameter just as a normal parameter. + if (entity is DefaultFormalParameter) { + entity = entity.parameter; + } + // "(^ this.field)" + if (entity is FieldFormalParameter) { + if (offset < entity.thisKeyword.offset) { + optype.includeTypeNameSuggestions = true; + } + } + // "(Type name)" + if (entity is SimpleFormalParameter) { + // "(Type^)" is parsed as a parameter with the _name_ "Type". + if (entity.type == null) { + optype.includeTypeNameSuggestions = true; + } + // If inside of "Type" in "(Type^ name)", then include types. + if (entity.type != null && + entity.type.offset <= offset && + offset <= entity.type.end) { + optype.includeTypeNameSuggestions = true; + } + } } @override @@ -460,8 +489,10 @@ class _OpTypeAstVisitor extends GeneralizingAstVisitor { @override void visitNormalFormalParameter(NormalFormalParameter node) { - optype.includeReturnValueSuggestions = true; - optype.includeTypeNameSuggestions = true; + if (node.identifier != entity) { + optype.includeReturnValueSuggestions = true; + optype.includeTypeNameSuggestions = true; + } } void visitParenthesizedExpression(ParenthesizedExpression node) { diff --git a/pkg/analysis_server/lib/src/services/completion/suggestion_builder.dart b/pkg/analysis_server/lib/src/services/completion/suggestion_builder.dart index b8a5c205c71..124fb32b859 100644 --- a/pkg/analysis_server/lib/src/services/completion/suggestion_builder.dart +++ b/pkg/analysis_server/lib/src/services/completion/suggestion_builder.dart @@ -140,7 +140,7 @@ visitInheritedTypeNames(ClassDeclaration node, void inherited(String name)) { } /** - * Starting with the given class node, traverse the inheritence hierarchy + * Starting with the given class node, traverse the inheritance hierarchy * calling the given functions with each non-null non-empty inherited class * declaration. For each locally defined declaration, call [localDeclaration]. * For each class identifier in the hierarchy that is not defined locally, diff --git a/pkg/analysis_server/test/services/completion/completion_test_util.dart b/pkg/analysis_server/test/services/completion/completion_test_util.dart index 8edd4869cfd..50c2b8fd8d8 100644 --- a/pkg/analysis_server/test/services/completion/completion_test_util.dart +++ b/pkg/analysis_server/test/services/completion/completion_test_util.dart @@ -3771,6 +3771,14 @@ abstract class AbstractSelectorSuggestionTest extends AbstractCompletionTest { }); } + test_parameterName_excludeTypes() { + addTestSource('m(int ^) {}'); + return computeFull((bool result) { + assertNotSuggested('int'); + assertNotSuggested('bool'); + }); + } + test_partFile_TypeName() { // SimpleIdentifier TypeName ConstructorName addSource( diff --git a/pkg/analysis_server/test/services/completion/optype_test.dart b/pkg/analysis_server/test/services/completion/optype_test.dart index a2bf5c82c05..7f203095bff 100644 --- a/pkg/analysis_server/test/services/completion/optype_test.dart +++ b/pkg/analysis_server/test/services/completion/optype_test.dart @@ -631,7 +631,7 @@ class OpTypeTest { test_FunctionDeclaration_inLineComment4() { // Comment CompilationUnit addTestSource(''' - // normal comment + // normal comment // normal comment 2^ zoo(z) { } String name;'''); assertOpType(); @@ -952,7 +952,7 @@ class OpTypeTest { // Comment ClassDeclaration CompilationUnit addTestSource(''' class C2 { - // normal comment + // normal comment // normal comment 2^ zoo(z) { } String name; }'''); assertOpType(); @@ -1142,12 +1142,60 @@ class C2 { assertOpType(returnValue: true, typeNames: true); } - test_SimpleFormalParameter() { + test_SimpleFormalParameter_closure() { // SimpleIdentifier SimpleFormalParameter FormalParameterList addTestSource('mth() { PNGS.sort((String a, Str^) => a.compareTo(b)); }'); assertOpType(typeNames: true); } + test_SimpleFormalParameter_name1() { + // SimpleIdentifier SimpleFormalParameter FormalParameterList + addTestSource('m(String na^) {}'); + assertOpType(typeNames: false); + } + + test_SimpleFormalParameter_name2() { + // SimpleIdentifier SimpleFormalParameter FormalParameterList + addTestSource('m(int first, String na^) {}'); + assertOpType(typeNames: false); + } + + test_SimpleFormalParameter_type_optionalPositional() { + // SimpleIdentifier DefaultFormalParameter FormalParameterList + addTestSource('m([Str^]) {}'); + assertOpType(typeNames: true); + } + + test_SimpleFormalParameter_type_optionalNamed() { + // SimpleIdentifier DefaultFormalParameter FormalParameterList + addTestSource('m({Str^}) {}'); + assertOpType(typeNames: true); + } + + test_SimpleFormalParameter_type_withName() { + // SimpleIdentifier SimpleFormalParameter FormalParameterList + addTestSource('m(Str^ name) {}'); + assertOpType(typeNames: true); + } + + test_SimpleFormalParameter_type_withoutName1() { + // SimpleIdentifier SimpleFormalParameter FormalParameterList + addTestSource('m(Str^) {}'); + assertOpType(typeNames: true); + } + + test_SimpleFormalParameter_type_withoutName2() { + // FormalParameterList + addTestSource('m(^) {}'); + assertOpType(typeNames: true); + } + + test_SimpleFormalParameter_type_withoutName3() { + // SimpleIdentifier SimpleFormalParameter FormalParameterList + addTestSource('m(int first, Str^) {}'); + assertOpType(typeNames: true); + } + test_SwitchCase_before() { // SwitchCase SwitchStatement Block addTestSource('main() {switch(k) {^case 1:}}');