Postfix completion

This defines some postfix completion templates for use in the IntelliJ editor.

R=brianwilkerson@google.com

To use it, you'll need a version of the Dart plugin with this PR patched in:
https://github.com/JetBrains/intellij-plugins/pull/534
Committed: https://github.com/dart-lang/sdk/commit/c26c2774ca4c8c41357ede2ca5dcb12542d94761

[messick, 2017-06-21 23:42:10]

Description was changed from

==========
Checkpoint: postfix completion
==========

to

==========
This defines some postfix completion templates for use in the IntelliJ editor.

To use it, you'll need a version of the Dart plugin with this PR patched in:
https://github.com/JetBrains/intellij-plugins/pull/534
==========

[messick, 2017-06-21 23:42:10]

messick@google.com changed reviewers:
+ brianwilkerson@google.com

[Brian Wilkerson, 2017-06-22 14:54:40]

lgtm

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/protocol/protocol_generated.dart (right):

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/protocol/protocol_generated.dart:7217: //  factory
EditListPostfixCompletionTemplatesResult.fromJson(JsonDecoder jsonDecoder,
String jsonPath, Object json) {
I'm surprised that our generator is generating commented out code.

If you commented it out manually, we'll need to find a different solution
because there's a test that will fail if the code in the generated files doesn't
match the generator output.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/src/edit/edit_domain.dart (right):

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/edit/edit_domain.dart:297: change = new
SourceChange("", edits: []);
If we are unable to generate a response we should return an error indicating the
reason for it (such as invalid file).

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/edit/edit_domain.dart:380: bool value;
Shouldn't this be initialized to a non-null value?

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
File
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart
(right):

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart:25:
final RangeFactory range = new RangeFactory();
This is already provided by
'package:analyzer_plugin/utilities/range_factory.dart', so can be removed from
here.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart:37:
"fori",
I initially assumed the "i" was for "in", but it appears not. Is this for
consistency with IntelliJ's Java support?

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart:74:
"tryon",
Should we add a try-finally kind (in a later CL)? Or other combinations of
try/catch/finally?

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart:254:
class PostfixCompletionKind {
Comment?

[messick, 2017-06-22 16:12:33]

Thanks for the feedback. I've already addressed most of the issues in my local
code base, but two of them will take more time, and one requires changes to the
Java code in the Dart plugin. I'll let you know when that's ready for review.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/protocol/protocol_generated.dart (right):

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/protocol/protocol_generated.dart:7217: //  factory
EditListPostfixCompletionTemplatesResult.fromJson(JsonDecoder jsonDecoder,
String jsonPath, Object json) {
On 2017/06/22 14:54:40, Brian Wilkerson wrote:
> I'm surprised that our generator is generating commented out code.
> 
> If you commented it out manually, we'll need to find a different solution
> because there's a test that will fail if the code in the generated files
doesn't
> match the generator output.

Has that test been run lately? I did not know there was a generator. I've been
writing these classes by hand, for each API extension I've added. I would not
expect my hand-written code to match the generator.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/src/edit/edit_domain.dart (right):

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/edit/edit_domain.dart:297: change = new
SourceChange("", edits: []);
On 2017/06/22 14:54:40, Brian Wilkerson wrote:
> If we are unable to generate a response we should return an error indicating
the
> reason for it (such as invalid file).

Acknowledged.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/edit/edit_domain.dart:380: bool value;
On 2017/06/22 14:54:40, Brian Wilkerson wrote:
> Shouldn't this be initialized to a non-null value?

Done.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
File
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart
(right):

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart:25:
final RangeFactory range = new RangeFactory();
On 2017/06/22 14:54:40, Brian Wilkerson wrote:
> This is already provided by
> 'package:analyzer_plugin/utilities/range_factory.dart', so can be removed from
> here.

Done.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart:37:
"fori",
On 2017/06/22 14:54:40, Brian Wilkerson wrote:
> I initially assumed the "i" was for "in", but it appears not. Is this for
> consistency with IntelliJ's Java support?

Yes, the Java version uses 'for' and 'fori' the same way. Kotlin has 'for'
meaning for-each. Java also has a 'forr' to count down instead of up, which I'm
considering adding to Dart.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart:74:
"tryon",
On 2017/06/22 14:54:40, Brian Wilkerson wrote:
> Should we add a try-finally kind (in a later CL)? Or other combinations of
> try/catch/finally?

Possibly; I'm undecided. Java has no generators for 'finally'.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/services/completion/postfix/postfix_completion.dart:254:
class PostfixCompletionKind {
On 2017/06/22 14:54:40, Brian Wilkerson wrote:
> Comment?

Done.

[Brian Wilkerson, 2017-06-22 16:22:46]

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/protocol/protocol_generated.dart (right):

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/protocol/protocol_generated.dart:7217: //  factory
EditListPostfixCompletionTemplatesResult.fromJson(JsonDecoder jsonDecoder,
String jsonPath, Object json) {
> Has that test been run lately? I did not know there was a generator. I've been
> writing these classes by hand, for each API extension I've added. I would not
> expect my hand-written code to match the generator.

I wouldn't either. I'm guessing the test was disabled somehow. The test is in
"pkg/analysis_server/tool/spec/check_all_test.dart". I wonder if we started
ignoring tests outside of the "test" directory.

Anyway, the generator is "pkg/analysis_server/tool/spec/generate_files". It will
generate both the Dart and Java classes needed to support the spec (although
there's more code that could be generated for each case).

[messick, 2017-06-22 23:36:50]

PTAL

I updated the spec, generated all the proper files, revised the Java code to
work with the new protocol, and uploaded a new branch to the PR for the Dart
plugin.

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
File pkg/analysis_server/lib/src/edit/edit_domain.dart (right):

https://codereview.chromium.org/2917943002/diff/80001/pkg/analysis_server/lib...
pkg/analysis_server/lib/src/edit/edit_domain.dart:297: change = new
SourceChange("", edits: []);
On 2017/06/22 14:54:40, Brian Wilkerson wrote:
> If we are unable to generate a response we should return an error indicating
the
> reason for it (such as invalid file).

I'm not sure what you're asking for here. All the edit domain requests follow
this pattern. None return errors.

[Brian Wilkerson, 2017-06-26 15:26:14]

lgtm

https://codereview.chromium.org/2917943002/diff/100001/pkg/analysis_server/to...
File pkg/analysis_server/tool/spec/spec_input.html (right):

https://codereview.chromium.org/2917943002/diff/100001/pkg/analysis_server/to...
pkg/analysis_server/tool/spec/spec_input.html:2020: The list of available
template.
"template" --> "templates"

[messick, 2017-06-26 16:52:34]

https://codereview.chromium.org/2917943002/diff/100001/pkg/analysis_server/to...
File pkg/analysis_server/tool/spec/spec_input.html (right):

https://codereview.chromium.org/2917943002/diff/100001/pkg/analysis_server/to...
pkg/analysis_server/tool/spec/spec_input.html:2020: The list of available
template.
On 2017/06/26 15:26:14, Brian Wilkerson wrote:
> "template" --> "templates"

Done.

[messick, 2017-06-26 16:53:07]

Description was changed from

==========
This defines some postfix completion templates for use in the IntelliJ editor.

To use it, you'll need a version of the Dart plugin with this PR patched in:
https://github.com/JetBrains/intellij-plugins/pull/534
==========

to

==========
This defines some postfix completion templates for use in the IntelliJ editor.

R=brianwilkerson@google.com

To use it, you'll need a version of the Dart plugin with this PR patched in:
https://github.com/JetBrains/intellij-plugins/pull/534
Committed:
https://github.com/dart-lang/sdk/commit/c26c2774ca4c8c41357ede2ca5dcb12542d94761
==========

[messick, 2017-06-26 16:53:08]

Committed patchset #7 (id:120001) manually as
c26c2774ca4c8c41357ede2ca5dcb12542d94761 (presubmit successful).

[Vyacheslav Egorov (Google), 2017-06-26 18:01:03]

vegorov@google.com changed reviewers:
+ vegorov@google.com

[Vyacheslav Egorov (Google), 2017-06-26 18:01:04]

This made analyzer bots red. 

Please either revert or fix.

[messick, 2017-06-26 18:24:50]

On 2017/06/26 18:01:04, Vyacheslav Egorov (Google) wrote:
> This made analyzer bots red. 
> 
> Please either revert or fix.

https://codereview.chromium.org/2960683002/