refactor duplicate code into single visitor

refactor duplicate code into single visitor

BUG=
R=paulberry@google.com

Committed: https://code.google.com/p/dart/source/detail?r=42398

[danrubel, 2014-12-16 08:04:36]

danrubel@google.com changed reviewers:
+ paulberry@google.com

[Paul Berry, 2014-12-16 16:21:57]

Lgtm.  I've made some suggestions for possible areas of future improvement, but
none of them are blocking issues.

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

https://codereview.chromium.org/804323002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/imported_computer.dart:33:
OpTypeAstVisitor opTypeVisitor = new OpTypeAstVisitor(request.offset);
It looks like many of the computers now invoke OpTypeAstVisitor as their first
action.  As a future improvement, consider moving this invocation into
DartCompletionRequest in order to avoid duplicate computation.

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

https://codereview.chromium.org/804323002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/optype_ast_visitor.dart:238:
includeVoidReturnSuggestions = true;
Suggestion for future improvement: make the type of suggestions depend on where
we are inside the for statement.  For example, void return suggestions only
belong after the 2nd semicolon.  Return value suggestions only belong after the
1st or second semicolon.

https://codereview.chromium.org/804323002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/optype_ast_visitor.dart:385:
includeTypeNameSuggestions = true;
Possible future improvement: on the RHS of an "is" or "as" expression, don't
suggest types that are guaranteed to pass or guaranteed to fail the cast.  (See
dartbug.com/18860)

[danrubel, 2014-12-16 16:51:07]

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

https://codereview.chromium.org/804323002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/imported_computer.dart:33:
OpTypeAstVisitor opTypeVisitor = new OpTypeAstVisitor(request.offset);
On 2014/12/16 16:21:56, Paul Berry wrote:
> It looks like many of the computers now invoke OpTypeAstVisitor as their first
> action.  As a future improvement, consider moving this invocation into
> DartCompletionRequest in order to avoid duplicate computation.

Agreed. This CL got too involved so I pushed that off to a subsequent CL.

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

https://codereview.chromium.org/804323002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/optype_ast_visitor.dart:238:
includeVoidReturnSuggestions = true;
On 2014/12/16 16:21:56, Paul Berry wrote:
> Suggestion for future improvement: make the type of suggestions depend on
where
> we are inside the for statement.  For example, void return suggestions only
> belong after the 2nd semicolon.  Return value suggestions only belong after
the
> 1st or second semicolon.

Great point. Added TODO for future work.

https://codereview.chromium.org/804323002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/optype_ast_visitor.dart:385:
includeTypeNameSuggestions = true;
On 2014/12/16 16:21:56, Paul Berry wrote:
> Possible future improvement: on the RHS of an "is" or "as" expression, don't
> suggest types that are guaranteed to pass or guaranteed to fail the cast. 
(See
> dartbug.com/18860)

Good suggestion. Added TODO for future improvement.

[danrubel, 2014-12-16 16:53:39]

Committed patchset #2 (id:20001) manually as 42398 (presubmit successful).

[danrubel, 2014-12-16 21:23:18]

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

https://codereview.chromium.org/804323002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/imported_computer.dart:33:
OpTypeAstVisitor opTypeVisitor = new OpTypeAstVisitor(request.offset);
On 2014/12/16 16:51:07, danrubel wrote:
> On 2014/12/16 16:21:56, Paul Berry wrote:
> > It looks like many of the computers now invoke OpTypeAstVisitor as their
first
> > action.  As a future improvement, consider moving this invocation into
> > DartCompletionRequest in order to avoid duplicate computation.
> 
> Agreed. This CL got too involved so I pushed that off to a subsequent CL.

https://codereview.chromium.org/807743003/