Skip to content

Commit

Permalink
[CP] Suggest static fields/getters, setState().
Browse files Browse the repository at this point in the history 
Cherry picks for:
b253c4a
3428b44
1b9554d
4b08b7d

Or
https://dart-review.googlesource.com/c/sdk/+/229324
https://dart-review.googlesource.com/c/sdk/+/242281
https://dart-review.googlesource.com/c/sdk/+/247933
https://dart-review.googlesource.com/c/sdk/+/245623

Bug: #49054 (comment)
Bug: #49233
Change-Id: Ia91e46926e86dff63f718d71a2881dcd07125ce8
  • Loading branch information
@scheglov @whesse
scheglov authored and whesse committed Jul 12, 2022
1 parent 9ad08f1 commit 68cca0c
Show file tree
Hide file tree
Showing 22 changed files with 781 additions and 177 deletions.
4 changes: 1 addition & 3 deletions pkg/analysis_server/lib/src/domain_completion.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import 'package:analyzer/src/util/performance/operation_performance.dart';
import 'package:analyzer_plugin/protocol/protocol.dart' as plugin;
import 'package:analyzer_plugin/protocol/protocol_common.dart';
import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin;
import 'package:collection/collection.dart';

/// Instances of the class [CompletionDomainHandler] implement a
/// [RequestHandler] that handles requests in the completion domain.
Expand Down Expand Up @@ -208,8 +207,7 @@ class CompletionDomainHandler extends AbstractRequestHandler {
performanceList.add(completionPerformance);

var analysisSession = resolvedUnit.analysisSession;
var enclosingNode =
resolvedUnit.resolvedNodes.lastOrNull ?? resolvedUnit.parsedUnit;
var enclosingNode = resolvedUnit.parsedUnit;

var completionRequest = DartCompletionRequest(
analysisSession: analysisSession,
Expand Down
13 changes: 13 additions & 0 deletions pkg/analysis_server/lib/src/services/completion/dart/completion_manager.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,19 @@ class DartCompletionRequest {
}
}

if (entity is Token &&
entity.type == TokenType.STRING &&
entity.offset < offset &&
offset < entity.end) {
final uriNode = target.containingNode;
if (uriNode is SimpleStringLiteral && uriNode.literal == entity) {
final directive = uriNode.parent;
if (directive is UriBasedDirective && directive.uri == uriNode) {
return uriNode.value.substring(0, offset - uriNode.contentsOffset);
}
}
}

while (entity is AstNode) {
if (entity is SimpleIdentifier) {
var identifier = entity.name;
Expand Down
8 changes: 5 additions & 3 deletions pkg/analysis_server/lib/src/services/completion/dart/feature_computer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,10 +540,12 @@ class _ContextTypeVisitor extends SimpleAstVisitor<DartType> {
if (offset <= argument.offset) {
return typeOfIndexPositionalParameter();
}
if (argument.contains(offset) && offset >= argument.name.end) {
return argument.staticParameterElement?.type;
if (argument.contains(offset)) {
if (offset >= argument.name.end) {
return argument.staticParameterElement?.type;
}
return null;
}
return null;
} else {
if (previousArgument == null || previousArgument.end < offset) {
if (offset <= argument.end) {
Expand Down
7 changes: 6 additions & 1 deletion pkg/analysis_server/lib/src/services/completion/dart/fuzzy_filter_sort.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'package:analysis_server/src/protocol_server.dart';
import 'package:analysis_server/src/services/completion/dart/suggestion_builder.dart';
import 'package:analysis_server/src/services/completion/filtering/fuzzy_matcher.dart';
import 'package:collection/collection.dart';

final _identifierPattern = RegExp(r'([_a-zA-Z][_a-zA-Z0-9]*)');

Expand All @@ -14,7 +15,11 @@ List<CompletionSuggestionBuilder> fuzzyFilterSort({
required String pattern,
required List<CompletionSuggestionBuilder> suggestions,
}) {
var matcher = FuzzyMatcher(pattern, matchStyle: MatchStyle.SYMBOL);
final matchStyle =
suggestions.firstOrNull?.kind == CompletionSuggestionKind.IMPORT
? MatchStyle.FILENAME
: MatchStyle.SYMBOL;
final matcher = FuzzyMatcher(pattern, matchStyle: matchStyle);

double score(CompletionSuggestionBuilder suggestion) {
var textToMatch = suggestion.textToMatch;
Expand Down
13 changes: 9 additions & 4 deletions pkg/analysis_server/lib/src/services/completion/dart/local_library_contributor.dart
View file
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
66 changes: 34 additions & 32 deletions pkg/analysis_server/lib/src/services/completion/dart/local_reference_contributor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,24 +210,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 @@ -245,20 +228,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 @@ -434,6 +404,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
70 changes: 66 additions & 4 deletions pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,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 @@ -378,6 +385,7 @@ class SuggestionBuilder {
_addBuilder(
_createCompletionSuggestionBuilder(
accessor,
completion: enclosingPrefix + accessor.displayName,
kind: CompletionSuggestionKind.IDENTIFIER,
relevance: relevance,
isNotImported: isNotImportedLibrary,
Expand Down Expand Up @@ -825,6 +833,7 @@ class SuggestionBuilder {
// Let the user know that we are going to insert a complete statement.
displayText: 'setState(() {});',
),
textToMatchOverride: 'setState',
);
return;
}
Expand Down Expand Up @@ -1025,6 +1034,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 @@ -1392,6 +1438,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
22 changes: 22 additions & 0 deletions pkg/analysis_server/test/analysis_server_base.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import 'package:meta/meta.dart';
import 'package:test/test.dart';

import 'mocks.dart';
import 'src/utilities/mock_packages.dart';

/// TODO(scheglov) this is duplicate
class AnalysisOptionsFileConfig {
Expand Down Expand Up @@ -91,6 +92,9 @@ class PubPackageAnalysisServerTest with ResourceProviderMixin {
EnableString.super_parameters,
];

/// The path that is not in [workspaceRootPath], contains external packages.
String get packagesRootPath => '/packages';

Folder get sdkRoot => newFolder('/sdk');

File get testFile => getFile(testFilePath);
Expand Down Expand Up @@ -280,6 +284,8 @@ class PubPackageAnalysisServerTest with ResourceProviderMixin {
void writeTestPackageConfig({
PackageConfigFileBuilder? config,
String? languageVersion,
bool flutter = false,
bool meta = false,
}) {
if (config == null) {
config = PackageConfigFileBuilder();
Expand All @@ -293,6 +299,22 @@ class PubPackageAnalysisServerTest with ResourceProviderMixin {
languageVersion: languageVersion,
);

if (meta || flutter) {
var libFolder = MockPackages.instance.addMeta(resourceProvider);
config.add(name: 'meta', rootPath: libFolder.parent.path);
}

if (flutter) {
{
var libFolder = MockPackages.instance.addUI(resourceProvider);
config.add(name: 'ui', rootPath: libFolder.parent.path);
}
{
var libFolder = MockPackages.instance.addFlutter(resourceProvider);
config.add(name: 'flutter', rootPath: libFolder.parent.path);
}
}

writePackageConfig(testPackageRoot, config);
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/analysis_server/test/client/completion_driver_test.dart
View file
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 @@ -343,7 +343,7 @@ export 'a.dart';
await addTestFile('''
import 'a.dart';
void f() {
^
E v = ^
}
''');
assertSuggestion(
Expand Down Expand Up @@ -665,7 +665,7 @@ void f() {
Future<void> test_project_lib_setters_static() async {
newFile2('$testPackageLibPath/a.dart', r'''
class A {
static set g(int g) {}
static set foo(int _) {}
}
''');

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

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

/// See: https://github.com/dart-lang/sdk/issues/40626
Expand Down
2 changes: 2 additions & 0 deletions pkg/analysis_server/test/client/impl/completion_driver.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ class CompletionDriver with ExpectMixin {
} else if (notification.event == ANALYSIS_NOTIFICATION_ERRORS) {
var decoded = AnalysisErrorsParams.fromNotification(notification);
filesErrors[decoded.file] = decoded.errors;
} else if (notification.event == ANALYSIS_NOTIFICATION_FLUSH_RESULTS) {
// Ignored.
} else if (notification.event == SERVER_NOTIFICATION_ERROR) {
throw Exception('server error: ${notification.toJson()}');
} else if (notification.event == SERVER_NOTIFICATION_CONNECTED) {
Expand Down
Loading

0 comments on commit 68cca0c

Please sign in to comment.