Fix in code completion for named parameters in constructor, method and function invocations, https:…

Fix in code completion for named parameters in constructor, method and function invocations, https://github.com/dart-lang/sdk/issues/25975

BUG=https://github.com/dart-lang/sdk/issues/25975
R=brianwilkerson@google.com, danrubel@google.com

Committed: https://github.com/dart-lang/sdk/commit/2a3acf94ec335210db72fd26bd5d27ead07125c2

[jwren, 2016-03-16 20:44:12]

jwren@google.com changed reviewers:
+ brianwilkerson@google.com, danrubel@google.com

[jwren, 2016-03-16 20:44:13]

This fixes the issue reported by Seth,
https://github.com/dart-lang/sdk/issues/25975. I also added 2 failing tests with
a TODO showing some additional work we could go add next.

[Brian Wilkerson, 2016-03-16 20:59:09]

LGTM

https://codereview.chromium.org/1806113002/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart
(right):

https://codereview.chromium.org/1806113002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart:263:
List<String> namedArgs, String name, bool prependComma) {
I think you mean "append" rather than "prepend".

https://codereview.chromium.org/1806113002/diff/1/pkg/analysis_server/test/se...
File
pkg/analysis_server/test/services/completion/dart/arglist_contributor_test.dart
(right):

https://codereview.chromium.org/1806113002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/completion/dart/arglist_contributor_test.dart:191:
test_Annotation_local_constructor_named_param_11() async {
I'm not familiar enough with the test framework to know: do we need some
negative tests to ensure that we don't get named parameter suggestions in a case
like "@A(1, ^, 3)"?

[jwren, 2016-03-16 21:59:51]

https://codereview.chromium.org/1806113002/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart
(right):

https://codereview.chromium.org/1806113002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart:263:
List<String> namedArgs, String name, bool prependComma) {
On 2016/03/16 20:59:09, Brian Wilkerson wrote:
> I think you mean "append" rather than "prepend".

Done.

https://codereview.chromium.org/1806113002/diff/1/pkg/analysis_server/test/se...
File
pkg/analysis_server/test/services/completion/dart/arglist_contributor_test.dart
(right):

https://codereview.chromium.org/1806113002/diff/1/pkg/analysis_server/test/se...
pkg/analysis_server/test/services/completion/dart/arglist_contributor_test.dart:191:
test_Annotation_local_constructor_named_param_11() async {
Done, added test_Annotation_local_constructor_named_param_negative above, and it
actually caught a bug which has been fixed

[danrubel, 2016-03-17 11:11:13]

LGTM. Thanks for pushing on this!

https://codereview.chromium.org/1806113002/diff/20001/pkg/analysis_server/lib...
File
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart
(right):

https://codereview.chromium.org/1806113002/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart:272:
List<String> namedArgs, String name, bool prependComma) {
prepend --> append

https://codereview.chromium.org/1806113002/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart:282:
completion.length,
if appending a comma, then the selection offset should probably be immediately
before the comment... completion.length - 1

https://codereview.chromium.org/1806113002/diff/20001/pkg/analysis_server/tes...
File
pkg/analysis_server/test/services/completion/dart/arglist_contributor_test.dart
(right):

https://codereview.chromium.org/1806113002/diff/20001/pkg/analysis_server/tes...
pkg/analysis_server/test/services/completion/dart/arglist_contributor_test.dart:423:
assertSuggestions(['radix: ,', 'onError: ,']);
assert here that the selection offset is right before the comma

[jwren, 2016-03-18 17:24:09]

https://codereview.chromium.org/1806113002/diff/20001/pkg/analysis_server/lib...
File
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart
(right):

https://codereview.chromium.org/1806113002/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart:272:
List<String> namedArgs, String name, bool prependComma) {
On 2016/03/17 11:11:13, danrubel wrote:
> prepend --> append

Done.

https://codereview.chromium.org/1806113002/diff/20001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/dart/arglist_contributor.dart:282:
completion.length,
On 2016/03/17 11:11:13, danrubel wrote:
> if appending a comma, then the selection offset should probably be immediately
> before the comment... completion.length - 1

Half of the tests fail if I make this change, in
completion_contributor_util.dart, the following assertion is made that is broken
by this change: expect(cs.selectionOffset, equals(completion.length));

Committing as is, when I go back to address the TODO below, I will revisit this
to get the selection right.

[jwren, 2016-03-18 17:26:31]

Description was changed from

==========
Fix in code completion for named parameters in constructor, method and function
invocations, https://github.com/dart-lang/sdk/issues/25975

BUG=https://github.com/dart-lang/sdk/issues/25975
==========

to

==========
Fix in code completion for named parameters in constructor, method and function
invocations, https://github.com/dart-lang/sdk/issues/25975

BUG=https://github.com/dart-lang/sdk/issues/25975
R=brianwilkerson@google.com, danrubel@google.com

Committed:
https://github.com/dart-lang/sdk/commit/2a3acf94ec335210db72fd26bd5d27ead07125c2
==========

[jwren, 2016-03-18 17:26:32]

Committed patchset #4 (id:60001) manually as
2a3acf94ec335210db72fd26bd5d27ead07125c2 (presubmit successful).