Skip to content

Commit

Permalink
Stop suggesting private static members in libraries where they are no…
Browse files Browse the repository at this point in the history
…t visible

I suspect that I broke this recently with my refactoring work.
Unfortunately there were no tests to tell me that I'd done so.

An alternative would be to move the visibility checks into the
SuggestionBuilder. I'd been kind of trying to keep that class focused on
_how_ to build suggestions rather than _whether_ to build suggestions,
but there might be value in having the logic for _whether_ to build them
also be centralized. If so, it could either be in SuggestionBuilder or in
a separate (wrapper) class. Thoughts welcomed.

Change-Id: I71198583496a685dd7865ef7955ef2dabaa3e1f4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148680
Reviewed-by: Jaime Wren <jwren@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
bwilkerson authored and commit-bot@chromium.org committed May 19, 2020
1 parent daa8d68 commit 3a764b7
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 8 deletions.
Expand Up @@ -19,48 +19,58 @@ class StaticMemberContributor extends DartCompletionContributor {
@override
Future<List<CompletionSuggestion>> computeSuggestions(
DartCompletionRequest request, SuggestionBuilder builder) async {
if (request.libraryElement == null) {
var library = request.libraryElement;
if (library == null) {
// Gracefully degrade if the library could not be determined, such as a
// detached part file or source change.
// TODO(brianwilkerson) Consider testing for this before invoking _any_ of
// the contributors.
return const <CompletionSuggestion>[];
}
bool isVisible(Element element) => element.isAccessibleIn(library);
var targetId = request.dotTarget;
if (targetId is Identifier && !request.target.isCascade) {
var element = targetId.staticElement;
if (element is ClassElement) {
for (var accessor in element.accessors) {
if (accessor.isStatic) {
if (accessor.isStatic &&
!accessor.isSynthetic &&
isVisible(accessor)) {
builder.suggestAccessor(accessor, inheritanceDistance: -1.0);
}
}
for (var constructor in element.constructors) {
builder.suggestConstructor(constructor, hasClassName: true);
if (isVisible(constructor)) {
builder.suggestConstructor(constructor, hasClassName: true);
}
}
for (var field in element.fields) {
if (field.isStatic && (!field.isSynthetic || element.isEnum)) {
if (field.isStatic &&
(!field.isSynthetic || element.isEnum) &&
isVisible(field)) {
builder.suggestField(field, inheritanceDistance: -1.0);
}
}
for (var method in element.methods) {
if (method.isStatic) {
if (method.isStatic && isVisible(method)) {
builder.suggestMethod(method, inheritanceDistance: -1.0);
}
}
} else if (element is ExtensionElement) {
for (var accessor in element.accessors) {
if (accessor.isStatic) {
if (accessor.isStatic &&
!accessor.isSynthetic &&
isVisible(accessor)) {
builder.suggestAccessor(accessor, inheritanceDistance: -1.0);
}
}
for (var field in element.fields) {
if (field.isStatic) {
if (field.isStatic && !field.isSynthetic && isVisible(field)) {
builder.suggestField(field, inheritanceDistance: -1.0);
}
}
for (var method in element.methods) {
if (method.isStatic) {
if (method.isStatic && isVisible(method)) {
builder.suggestMethod(method, inheritanceDistance: -1.0);
}
}
Expand Down
Expand Up @@ -22,6 +22,30 @@ class StaticMemberContributorTest extends DartCompletionContributorTest {
return StaticMemberContributor();
}

Future<void> test_class_static_notPrivate() async {
addSource('/home/test/lib/a.dart', '''
class A {
static int _f;
static String get _g => '';
static int _m() {}
static set _s(v) {}
A._();
}
''');
addTestSource('''
import 'a.dart';
void f() {
A.^;
}
''');
await computeSuggestions();
assertNotSuggested('_f');
assertNotSuggested('_g');
assertNotSuggested('_m');
assertNotSuggested('_s');
assertNotSuggested('A._');
}

Future<void> test_enumConst() async {
addTestSource('enum E { one, two } main() {E.^}');
await computeSuggestions();
Expand Down Expand Up @@ -108,6 +132,28 @@ main() {E.^}
assertSuggestField('s', 'String');
}

Future<void> test_extension_static_notPrivate() async {
addSource('/home/test/lib/a.dart', '''
extension E {
static int _f;
static String get _g => '';
static int _m() {}
static set _s(v) {}
}
''');
addTestSource('''
import 'a.dart';
void f() {
E.^;
}
''');
await computeSuggestions();
assertNotSuggested('_f');
assertNotSuggested('_g');
assertNotSuggested('_m');
assertNotSuggested('_s');
}

Future<void> test_implicitCreation() async {
addSource('/home/test/lib/a.dart', '''
class A {
Expand Down

0 comments on commit 3a764b7

Please sign in to comment.