update code completion suggestion relevance based on common usage

update code completion suggestion relevance based on common usage

BUG=
R=lukechurch@google.com, scheglov@google.com

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

[lukechurch, 2015-02-13 11:23:01]

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

[lukechurch, 2015-02-13 11:23:01]

lgtm after the removal of the ordering for Future.

- once this lands I will add in the type and retarget the getter.

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

https://codereview.chromium.org/925723002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:26:
'dart.async.Future': ['value', 'wait'],
Lets not land a change that affects something as important as 'Future' until the
actual signal is in. Otherwise it will make doing the analysis to determine the
impact un-necessarily complicated.

If you wanted to add a second partial sorting rule, consider:
dart.core.Set : [firstWhere, takeWhile, skipWhile]

https://codereview.chromium.org/925723002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:103:
class _BestTypeVisitor extends GeneralizingAstVisitor {
This seems more generally useful than only for doing completion ordering?

[danrubel, 2015-02-15 04:53:41]

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

https://codereview.chromium.org/925723002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:26:
'dart.async.Future': ['value', 'wait'],
On 2015/02/13 11:23:01, lukechurch wrote:
> Lets not land a change that affects something as important as 'Future' until
the
> actual signal is in. Otherwise it will make doing the analysis to determine
the
> impact un-necessarily complicated.
> 
> If you wanted to add a second partial sorting rule, consider:
> dart.core.Set : [firstWhere, takeWhile, skipWhile]

Good point. To better address this I've added a constructor to
CommonUsageComputer with which I can inject a specific map for testing. This
default selector map will not have any entries, and I'll have you fill it in
with the actual values when you have them.

https://codereview.chromium.org/925723002/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:103:
class _BestTypeVisitor extends GeneralizingAstVisitor {
On 2015/02/13 11:23:01, lukechurch wrote:
> This seems more generally useful than only for doing completion ordering?

Agreed, but I'm not planning major refactoring until after 1.9.

[danrubel, 2015-02-15 04:54:46]

New patchsets have been uploaded after l-g-t-m from lukechurch@google.com

[danrubel, 2015-02-15 04:56:27]

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

[scheglov, 2015-02-15 19:06:29]

LGTM

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

https://codereview.chromium.org/925723002/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:77:
typeElem.getAncestor((p) => p is LibraryElement);
typeElem.library

https://codereview.chromium.org/925723002/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:99:
suggestion.kind == CompletionSuggestionKind.INVOCATION &&
Field accessors?

https://codereview.chromium.org/925723002/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:112:
* An [AstVisitor] used to determine the best [DartType] of a node.
From implementation it does not look that it return the type of the given
"node".
It's more like its defining type.

https://codereview.chromium.org/925723002/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:115:
final entity;
type?

[danrubel, 2015-02-15 21:16:29]

New patchsets have been uploaded after l-g-t-m from scheglov@google.com

[danrubel, 2015-02-15 21:17:16]

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

https://codereview.chromium.org/925723002/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:77:
typeElem.getAncestor((p) => p is LibraryElement);
On 2015/02/15 19:06:29, scheglov wrote:
> typeElem.library

Done.

https://codereview.chromium.org/925723002/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:99:
suggestion.kind == CompletionSuggestionKind.INVOCATION &&
On 2015/02/15 19:06:28, scheglov wrote:
> Field accessors?

Great point. I have added support and tests for field, getter, and setter.

https://codereview.chromium.org/925723002/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:112:
* An [AstVisitor] used to determine the best [DartType] of a node.
On 2015/02/15 19:06:29, scheglov wrote:
> From implementation it does not look that it return the type of the given
> "node".
> It's more like its defining type.

Correct. I updated the comment per your suggestion.

https://codereview.chromium.org/925723002/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/services/completion/common_usage_computer.dart:115:
final entity;
On 2015/02/15 19:06:28, scheglov wrote:
> type?

Done.

[danrubel, 2015-02-15 21:17:58]

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