Skip to content

Commit

Permalink
Suggest static fields/getters.
Browse files Browse the repository at this point in the history
Change-Id: Ibf422215f1f1a6ddb062302a6c3e7b831b176468
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/229324
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
  • Loading branch information
scheglov authored and Commit Bot committed May 26, 2022
1 parent 7fd98f5 commit b253c4a
Show file tree
Hide file tree
Showing 12 changed files with 475 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:analysis_server/src/services/completion/dart/completion_manager.
import 'package:analysis_server/src/services/completion/dart/suggestion_builder.dart'
show SuggestionBuilder;
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/dart/element/visitor.dart';
import 'package:analyzer_plugin/src/utilities/completion/optype.dart';

Expand Down Expand Up @@ -52,10 +53,14 @@ class LibraryElementSuggestionBuilder extends GeneralizingElementVisitor {
_addConstructorSuggestions(element);
}
if (opType.includeReturnValueSuggestions) {
if (element.isEnum) {
for (var field in element.fields) {
if (field.isEnumConstant) {
builder.suggestEnumConstant(field, prefix: prefix);
final typeSystem = request.libraryElement.typeSystem;
final contextType = request.contextType;
if (contextType is InterfaceType) {
// TODO(scheglov) This looks not ideal - we should suggest getters.
for (final field in element.fields) {
if (field.isStatic &&
typeSystem.isSubtypeOf(field.type, contextType)) {
builder.suggestStaticField(field, prefix: prefix);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,24 +213,7 @@ class _LocalVisitor extends LocalDeclarationVisitor {

@override
void declaredClass(ClassDeclaration declaration) {
var classElt = declaration.declaredElement;
if (classElt != null && visibilityTracker._isVisible(classElt)) {
if (opType.includeTypeNameSuggestions) {
builder.suggestClass(classElt);
}

// Generate the suggestions for the constructors. We are required to loop
// through elements here instead of using declaredConstructor() due to
// implicit constructors (i.e. there is no AstNode for an implicit
// constructor)
if (!opType.isPrefixed && opType.includeConstructorSuggestions) {
for (var constructor in classElt.constructors) {
if (!classElt.isAbstract || constructor.isFactory) {
builder.suggestConstructor(constructor);
}
}
}
}
_declaredClassElement(declaration.declaredElement);
}

@override
Expand All @@ -248,20 +231,7 @@ class _LocalVisitor extends LocalDeclarationVisitor {

@override
void declaredEnum(EnumDeclaration declaration) {
var declaredElement = declaration.declaredElement;
if (declaredElement != null &&
visibilityTracker._isVisible(declaredElement) &&
opType.includeTypeNameSuggestions) {
builder.suggestClass(declaredElement);
for (var enumConstant in declaration.constants) {
if (!enumConstant.isSynthetic) {
var constantElement = enumConstant.declaredElement;
if (constantElement is FieldElement) {
builder.suggestEnumConstant(constantElement);
}
}
}
}
_declaredClassElement(declaration.declaredElement);
}

@override
Expand Down Expand Up @@ -437,6 +407,38 @@ class _LocalVisitor extends LocalDeclarationVisitor {
super.visitExtendsClause(node);
}

void _declaredClassElement(ClassElement? class_) {
if (class_ != null && visibilityTracker._isVisible(class_)) {
if (opType.includeTypeNameSuggestions) {
builder.suggestClass(class_);
}

if (!opType.isPrefixed &&
opType.includeConstructorSuggestions &&
!class_.isEnum) {
for (final constructor in class_.constructors) {
if (!class_.isAbstract || constructor.isFactory) {
builder.suggestConstructor(constructor);
}
}
}

if (!opType.isPrefixed && opType.includeReturnValueSuggestions) {
final typeSystem = request.libraryElement.typeSystem;
final contextType = request.contextType;
if (contextType is InterfaceType) {
// TODO(scheglov) This looks not ideal - we should suggest getters.
for (final field in class_.fields) {
if (field.isStatic &&
typeSystem.isSubtypeOf(field.type, contextType)) {
builder.suggestStaticField(field);
}
}
}
}
}
}

/// Return `true` if the [identifier] is composed of one or more underscore
/// characters and nothing else.
bool _isUnused(String identifier) => RegExp(r'^_+$').hasMatch(identifier);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,17 @@ class SuggestionBuilder {
/// invocation. The [inheritanceDistance] is the value of the inheritance
/// distance feature computed for the accessor or `-1.0` if the accessor is a
/// static accessor.
void suggestAccessor(PropertyAccessorElement accessor,
{required double inheritanceDistance}) {
assert(accessor.enclosingElement is ClassElement ||
accessor.enclosingElement is ExtensionElement);
void suggestAccessor(
PropertyAccessorElement accessor, {
required double inheritanceDistance,
bool withEnclosingName = false,
}) {
var enclosingPrefix = '';
var enclosingName = _enclosingClassOrExtensionName(accessor);
if (withEnclosingName && enclosingName != null) {
enclosingPrefix = '$enclosingName.';
}

if (accessor.isSynthetic) {
// Avoid visiting a field twice. All fields induce a getter, but only
// non-final fields induce a setter, so we don't add a suggestion for a
Expand Down Expand Up @@ -280,6 +287,7 @@ class SuggestionBuilder {
_addBuilder(
_createCompletionSuggestionBuilder(
accessor,
completion: enclosingPrefix + accessor.displayName,
kind: CompletionSuggestionKind.IDENTIFIER,
relevance: relevance,
isNotImported: isNotImportedLibrary,
Expand Down Expand Up @@ -926,6 +934,43 @@ class SuggestionBuilder {
);
}

/// Add a suggestion for a static field declared within a class or extension.
/// If the field is synthetic, add the corresponding getter instead.
///
/// If the enclosing element can only be referenced using a prefix, then
/// the [prefix] should be provided.
void suggestStaticField(FieldElement element, {String? prefix}) {
assert(element.isStatic);
if (element.isSynthetic) {
var getter = element.getter;
if (getter != null) {
suggestAccessor(
getter,
inheritanceDistance: 0.0,
withEnclosingName: true,
);
}
} else {
var enclosingPrefix = '';
var enclosingName = _enclosingClassOrExtensionName(element);
if (enclosingName != null) {
enclosingPrefix = '$enclosingName.';
}
var relevance =
_computeTopLevelRelevance(element, elementType: element.type);
_addBuilder(
_createCompletionSuggestionBuilder(
element,
completion: enclosingPrefix + element.name,
kind: CompletionSuggestionKind.IDENTIFIER,
prefix: prefix,
relevance: relevance,
isNotImported: isNotImportedLibrary,
),
);
}
}

/// Add a suggestion to reference a [parameter] in a super formal parameter.
void suggestSuperFormalParameter(ParameterElement parameter) {
_addBuilder(
Expand Down Expand Up @@ -1289,6 +1334,22 @@ class SuggestionBuilder {
);
}

/// Return the name of the enclosing class or extension.
///
/// The enclosing element must be either a class, or extension; otherwise
/// we either fail with assertion, or return `null`.
String? _enclosingClassOrExtensionName(Element element) {
var enclosing = element.enclosingElement;
if (enclosing is ClassElement) {
return enclosing.name;
} else if (enclosing is ExtensionElement) {
return enclosing.name;
} else {
assert(false, 'Expected ClassElement or ExtensionElement');
return null;
}
}

/// If the [element] has a documentation comment, return it.
_ElementDocumentation? _getDocumentation(Element element) {
var documentationCache = request.documentationCache;
Expand Down
8 changes: 4 additions & 4 deletions pkg/analysis_server/test/client/completion_driver_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ class CompletionWithSuggestionsTest1 extends AbstractCompletionDriverTest
@override
TestingCompletionProtocol get protocol => TestingCompletionProtocol.version1;

@failingTest
@FailingTest(reason: 'This test fails with available suggestions')
@override
Future<void> test_project_lib_multipleExports() async {
return super.test_project_lib_multipleExports();
Expand Down Expand Up @@ -329,7 +329,7 @@ export 'a.dart';
await addTestFile('''
import 'a.dart';
void f() {
^
E v = ^
}
''');
assertSuggestion(
Expand Down Expand Up @@ -605,7 +605,7 @@ void f() {
Future<void> test_project_lib_setters_static() async {
newFile('$testPackageLibPath/a.dart', r'''
class A {
static set g(int g) {}
static set foo(int _) {}
}
''');

Expand All @@ -619,7 +619,7 @@ void f() {
}
''');

assertNoSuggestion(completion: 'A.g');
assertNoSuggestion(completion: 'A.foo');
}

/// See: https://github.com/dart-lang/sdk/issues/40626
Expand Down
38 changes: 0 additions & 38 deletions pkg/analysis_server/test/domain_completion_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -391,44 +391,6 @@ void f() {
..suggestions.withElementClass.isEmpty;
}

Future<void> test_notImported_lowerRelevance_enumConstant() async {
newFile('$testPackageLibPath/a.dart', '''
enum E1 {
foo01
}
''');

newFile('$testPackageLibPath/b.dart', '''
enum E2 {
foo02
}
''');

await _configureWithWorkspaceRoot();

var response = await _getTestCodeSuggestions('''
import 'b.dart';
void f() {
foo0^
}
''');

check(response)
..assertComplete()
..hasReplacement(left: 4);

// `foo01` relevance is decreased because it is not yet imported.
check(response).suggestions.matches([
(suggestion) => suggestion
..completion.isEqualTo('E2.foo02')
..libraryUriToImport.isNull,
(suggestion) => suggestion
..completion.isEqualTo('E1.foo01')
..libraryUriToImport.isEqualTo('package:test/a.dart'),
]);
}

Future<void> test_notImported_lowerRelevance_extension_getter() async {
await _configureWithWorkspaceRoot();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,20 @@ extension CompletionSuggestionExtension
element.isNotNull.kind.isSetter;
}

void get isStatic {
element.isNotNull.isStatic.isTrue;
}

void get isStaticField {
isStatic;
isField;
}

void get isStaticGetter {
isStatic;
isGetter;
}

void get isTopLevelVariable {
kind.isIdentifier;
element.isNotNull.kind.isTopLevelVariable;
Expand Down Expand Up @@ -404,6 +418,45 @@ extension CompletionSuggestionsExtension
);
}

@useResult
CheckTarget<Iterable<CompletionSuggestionForTesting>> get fields {
var result = value
.where((suggestion) =>
suggestion.suggestion.kind == CompletionSuggestionKind.IDENTIFIER &&
suggestion.suggestion.element?.kind == ElementKind.FIELD)
.toList();
return nest(
result,
(selected) => 'fields ${valueStr(selected)}',
);
}

@useResult
CheckTarget<Iterable<CompletionSuggestionForTesting>> get getters {
var result = value
.where((suggestion) =>
suggestion.suggestion.kind == CompletionSuggestionKind.IDENTIFIER &&
suggestion.suggestion.element?.kind == ElementKind.GETTER)
.toList();
return nest(
result,
(selected) => 'getters ${valueStr(selected)}',
);
}

@useResult
CheckTarget<Iterable<CompletionSuggestionForTesting>> get methods {
var result = value
.where((suggestion) =>
suggestion.suggestion.kind == CompletionSuggestionKind.IDENTIFIER &&
suggestion.suggestion.element?.kind == ElementKind.METHOD)
.toList();
return nest(
result,
(selected) => 'setters ${valueStr(selected)}',
);
}

@useResult
CheckTarget<Iterable<CompletionSuggestionForTesting>> get namedArguments {
var result = value
Expand All @@ -429,6 +482,19 @@ extension CompletionSuggestionsExtension
);
}

@useResult
CheckTarget<Iterable<CompletionSuggestionForTesting>> get setters {
var result = value
.where((suggestion) =>
suggestion.suggestion.kind == CompletionSuggestionKind.IDENTIFIER &&
suggestion.suggestion.element?.kind == ElementKind.SETTER)
.toList();
return nest(
result,
(selected) => 'setters ${valueStr(selected)}',
);
}

@useResult
CheckTarget<Iterable<CompletionSuggestionForTesting>> get withElementClass {
return nest(
Expand All @@ -453,6 +519,14 @@ extension CompletionSuggestionsExtension
}

extension ElementExtension on CheckTarget<Element> {
@useResult
CheckTarget<bool> get isStatic {
return nest(
value.isStatic,
(selected) => 'isStatic ${valueStr(selected)}',
);
}

@useResult
CheckTarget<ElementKind> get kind {
return nest(
Expand Down
Loading

0 comments on commit b253c4a

Please sign in to comment.