Code completion API refactoring: replace resolveExpression(Expression) with resolveContainingExpres…

Code completion API refactoring: replace resolveExpression(Expression) with resolveContainingExpression(AstNode) and resolveContainingStatement(AstNode)

R=danrubel@google.com

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

[jwren, 2016-04-20 19:34:58]

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

[jwren, 2016-04-20 19:34:59]

Modified API as discussed, and some cleanup in the callers.

[danrubel, 2016-04-20 19:41:30]

LGTM once comments are addressed.

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/imported_reference_contributor.dart
(left):

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/imported_reference_contributor.dart:67:
node = request.target.containingNode;
We need to keep this line and the preceding comment

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/local_constructor_contributor.dart
(left):

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/local_constructor_contributor.dart:181:
node = request.target.containingNode;
We need to keep this line and the preceding comment.

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart
(left):

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart:171:
node = request.target.containingNode;
We need to keep this line and preceding comment.

[jwren, 2016-04-20 19:54:50]

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/imported_reference_contributor.dart
(left):

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/imported_reference_contributor.dart:67:
node = request.target.containingNode;
On 2016/04/20 19:41:30, danrubel wrote:
> We need to keep this line and the preceding comment

Since the while statement is gone, this means we'll have:

AstNode node = request.target.containingNode;
...
node = request.target.containingNode;

Is it possible for the request to be changed during the request.resolve... call?

[danrubel, 2016-04-20 19:59:34]

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/imported_reference_contributor.dart
(left):

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/imported_reference_contributor.dart:67:
node = request.target.containingNode;
On 2016/04/20 19:54:50, jwren wrote:
> On 2016/04/20 19:41:30, danrubel wrote:
> > We need to keep this line and the preceding comment
> 
> Since the while statement is gone, this means we'll have:
> 
> AstNode node = request.target.containingNode;
> ...
> node = request.target.containingNode;
> 
> Is it possible for the request to be changed during the request.resolve...
call?

Yes, thus we need this code here and elsewhere. If the AST did not change during
resolution, then we would not need to discard the cached node and target. Having
the AST change during resolution was a design decision from long ago.

[jwren, 2016-04-20 20:12:21]

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/imported_reference_contributor.dart
(left):

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/imported_reference_contributor.dart:67:
node = request.target.containingNode;
On 2016/04/20 19:59:34, danrubel wrote:
> On 2016/04/20 19:54:50, jwren wrote:
> > On 2016/04/20 19:41:30, danrubel wrote:
> > > We need to keep this line and the preceding comment
> > 
> > Since the while statement is gone, this means we'll have:
> > 
> > AstNode node = request.target.containingNode;
> > ...
> > node = request.target.containingNode;
> > 
> > Is it possible for the request to be changed during the request.resolve...
> call?
> 
> Yes, thus we need this code here and elsewhere. If the AST did not change
during
> resolution, then we would not need to discard the cached node and target.
Having
> the AST change during resolution was a design decision from long ago.

Got it, done.

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/local_constructor_contributor.dart
(left):

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/local_constructor_contributor.dart:181:
node = request.target.containingNode;
On 2016/04/20 19:41:30, danrubel wrote:
> We need to keep this line and the preceding comment.

Done.

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
File
pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart
(left):

https://codereview.chromium.org/1898983004/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart:171:
node = request.target.containingNode;
On 2016/04/20 19:41:30, danrubel wrote:
> We need to keep this line and preceding comment.

Done.

[jwren, 2016-04-20 20:13:18]

Description was changed from

==========
Code completion API refactoring: replace resolveExpression(Expression) with
resolveContainingExpression(AstNode) and resolveContainingStatement(AstNode)
==========

to

==========
Code completion API refactoring: replace resolveExpression(Expression) with
resolveContainingExpression(AstNode) and resolveContainingStatement(AstNode)

R=danrubel@google.com

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

[jwren, 2016-04-20 20:13:19]

Committed patchset #3 (id:40001) manually as
b13f1fe3138fbbae13ba91d26913dc53bc7b1aed (presubmit successful).