Skip to content

Commit a6c951d

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Fix private extensions showing up in other libraries
Fixes #61159 Change-Id: I13dfc3b0dc7ada94a6dddb72254d537c121d4fc4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444320 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]>
1 parent 587cbed commit a6c951d

File tree

2 files changed

+229
-51
lines changed

2 files changed

+229
-51
lines changed

pkg/analysis_server/lib/src/services/completion/dart/declaration_helper.dart

Lines changed: 55 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -464,41 +464,43 @@ class DeclarationHelper {
464464
(extendedType is InterfaceType) ? extendedType.element : null;
465465
if (includeMethods) {
466466
for (var method in extension.methods) {
467-
if (!method.isStatic) {
468-
if (method.isOperator) {
469-
continue;
470-
}
471-
_suggestMethod(
472-
method: method,
473-
importData: importData,
474-
referencingInterface: referencingInterface,
475-
);
467+
if (method.isStatic ||
468+
method.isOperator ||
469+
!method.isVisibleIn(request.libraryElement)) {
470+
continue;
476471
}
472+
_suggestMethod(
473+
method: method,
474+
importData: importData,
475+
referencingInterface: referencingInterface,
476+
);
477477
}
478478
}
479479
for (var accessor in extension.getters) {
480-
if (excludedGetters.contains(accessor.name)) {
480+
if (excludedGetters.contains(accessor.name) ||
481+
accessor.isStatic ||
482+
!accessor.isVisibleIn(request.libraryElement)) {
481483
continue;
482484
}
483-
if (!accessor.isStatic) {
485+
_suggestProperty(
486+
accessor: accessor,
487+
referencingInterface: referencingInterface,
488+
importData: importData,
489+
);
490+
}
491+
if (includeSetters) {
492+
for (var accessor in extension.setters) {
493+
if (accessor.isStatic ||
494+
!accessor.isVisibleIn(request.libraryElement)) {
495+
continue;
496+
}
484497
_suggestProperty(
485498
accessor: accessor,
486499
referencingInterface: referencingInterface,
487500
importData: importData,
488501
);
489502
}
490503
}
491-
if (includeSetters) {
492-
for (var accessor in extension.setters) {
493-
if (!accessor.isStatic) {
494-
_suggestProperty(
495-
accessor: accessor,
496-
referencingInterface: referencingInterface,
497-
importData: importData,
498-
);
499-
}
500-
}
501-
}
502504
}
503505

504506
/// Adds suggestions for any constructors that are visible within the not yet
@@ -805,49 +807,53 @@ class DeclarationHelper {
805807
var extension = instantiatedExtension.extension;
806808
if (includeMethods) {
807809
for (var method in extension.methods) {
808-
if (!method.isStatic) {
809-
if (method.isOperator) {
810-
continue;
811-
}
812-
_suggestMethod(
813-
method: method,
814-
isKeywordNeeded: isKeywordNeeded,
815-
isTypeNeeded: isTypeNeeded,
816-
);
810+
if (method.isStatic ||
811+
method.isOperator ||
812+
!method.isVisibleIn(libraryElement)) {
813+
continue;
817814
}
815+
_suggestMethod(
816+
method: method,
817+
isKeywordNeeded: isKeywordNeeded,
818+
isTypeNeeded: isTypeNeeded,
819+
);
818820
}
819821
}
820822
for (var getter in extension.getters) {
821823
if (excludedGetters.contains(getter.name)) {
822824
continue;
823825
}
824826
if (!getter.isSynthetic) {
825-
_suggestProperty(
826-
accessor: getter,
827-
isKeywordNeeded: isKeywordNeeded,
828-
isTypeNeeded: isTypeNeeded,
829-
);
827+
if (getter.isVisibleIn(libraryElement)) {
828+
_suggestProperty(
829+
accessor: getter,
830+
isKeywordNeeded: isKeywordNeeded,
831+
isTypeNeeded: isTypeNeeded,
832+
);
833+
}
830834
} else {
831835
// All fields induce a getter.
832836
var variable = getter.variable;
833837
if (variable is FieldElement) {
834-
_suggestField(
835-
field: variable,
836-
isKeywordNeeded: isKeywordNeeded,
837-
isTypeNeeded: isTypeNeeded,
838-
);
838+
if (variable.isVisibleIn(libraryElement)) {
839+
_suggestField(
840+
field: variable,
841+
isKeywordNeeded: isKeywordNeeded,
842+
isTypeNeeded: isTypeNeeded,
843+
);
844+
}
839845
}
840846
}
841847
}
842-
for (var setter in extension.setters) {
843-
if (!setter.isSynthetic) {
844-
if (includeSetters) {
845-
_suggestProperty(accessor: setter);
846-
}
847-
} else {
848+
if (includeSetters) {
849+
for (var setter in extension.setters) {
848850
// Avoid visiting a field twice. All fields induce a getter, but only
849-
// non-final fields induce a setter, so we don't add a suggestion for a
850-
// synthetic setter.
851+
// non-final fields induce a setter, so we don't add a suggestion for
852+
// a synthetic setter.
853+
if (setter.isSynthetic || !setter.isVisibleIn(libraryElement)) {
854+
continue;
855+
}
856+
_suggestProperty(accessor: setter);
851857
}
852858
}
853859
}

pkg/analysis_server/test/src/services/completion/dart/completion_test.dart

Lines changed: 174 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,178 @@ extension E on String {
331331
''');
332332
assertHasCompletion('m');
333333
}
334+
335+
Future<void> test_privateGetter_accessible() async {
336+
await getTestCodeSuggestions('''
337+
extension E on String {
338+
String get _privateGetter => '';
339+
}
340+
341+
void f(String s) {
342+
s.^;
343+
}
344+
''');
345+
assertHasCompletion('_privateGetter');
346+
}
347+
348+
Future<void> test_privateGetter_notAccessible_imported() async {
349+
newFile(convertPath('$testPackageLibPath/extensions.dart'), '''
350+
extension E on String {
351+
String get _privateGetter => '';
352+
}
353+
''');
354+
await getTestCodeSuggestions('''
355+
import 'extensions.dart';
356+
357+
void f(String s) {
358+
s.^;
359+
}
360+
''');
361+
assertHasNoCompletion('_privateGetter');
362+
}
363+
364+
Future<void> test_privateGetter_notAccessible_notImported() async {
365+
newFile(convertPath('$testPackageLibPath/extensions.dart'), '''
366+
extension E on String {
367+
String get _privateGetter => '';
368+
}
369+
''');
370+
await getTestCodeSuggestions('''
371+
void f(String s) {
372+
s.^;
373+
}
374+
''');
375+
assertHasNoCompletion('_privateGetter');
376+
}
377+
378+
Future<void> test_privateMethod_accessible() async {
379+
await getTestCodeSuggestions('''
380+
extension E on String {
381+
void _privateMethod() {}
382+
}
383+
384+
void f(String s) {
385+
s.^;
386+
}
387+
''');
388+
assertHasCompletion('_privateMethod');
389+
}
390+
391+
Future<void> test_privateMethod_notAccessible_mported() async {
392+
newFile(convertPath('$testPackageLibPath/extensions.dart'), '''
393+
extension E on String {
394+
void _privateMethod() {}
395+
}
396+
''');
397+
await getTestCodeSuggestions('''
398+
import 'extensions.dart';
399+
400+
void f(String s) {
401+
s.^;
402+
}
403+
''');
404+
assertHasNoCompletion('_privateMethod');
405+
}
406+
407+
Future<void> test_privateMethod_notAccessible_notImported() async {
408+
newFile(convertPath('$testPackageLibPath/extensions.dart'), '''
409+
extension E on String {
410+
void _privateMethod() {}
411+
}
412+
''');
413+
await getTestCodeSuggestions('''
414+
void f(String s) {
415+
s.^;
416+
}
417+
''');
418+
assertHasNoCompletion('_privateMethod');
419+
}
420+
421+
Future<void> test_privateSetter_accessible() async {
422+
await getTestCodeSuggestions('''
423+
extension E on String {
424+
set _privateSetter(String _) {}
425+
}
426+
427+
void f(String s) {
428+
s.^;
429+
}
430+
''');
431+
assertHasCompletion('_privateSetter');
432+
}
433+
434+
Future<void> test_privateSetter_notAccessible_imported() async {
435+
newFile(convertPath('$testPackageLibPath/extensions.dart'), '''
436+
extension E on String {
437+
set _privateSetter(String _) {}
438+
}
439+
''');
440+
await getTestCodeSuggestions('''
441+
import 'extensions.dart';
442+
443+
void f(String s) {
444+
s.^;
445+
}
446+
''');
447+
assertHasNoCompletion('_privateSetter');
448+
}
449+
450+
Future<void> test_privateSetter_notAccessible_notImported() async {
451+
newFile(convertPath('$testPackageLibPath/extensions.dart'), '''
452+
extension E on String {
453+
set _privateSetter(String _) {}
454+
}
455+
''');
456+
await getTestCodeSuggestions('''
457+
void f(String s) {
458+
s.^;
459+
}
460+
''');
461+
assertHasNoCompletion('_privateSetter');
462+
}
463+
464+
Future<void> test_privateVariable_accessible() async {
465+
await getTestCodeSuggestions('''
466+
extension E on String {
467+
String _privateVariable = '';
468+
}
469+
470+
void f(String s) {
471+
s.^;
472+
}
473+
''');
474+
assertHasCompletion('_privateVariable');
475+
}
476+
477+
Future<void> test_privateVariable_notAccessible_imported() async {
478+
newFile(convertPath('$testPackageLibPath/extensions.dart'), '''
479+
extension E on String {
480+
String _privateVariable = '';
481+
}
482+
''');
483+
await getTestCodeSuggestions('''
484+
import 'extensions.dart';
485+
486+
void f(String s) {
487+
s.^;
488+
}
489+
''');
490+
assertHasNoCompletion('_privateVariable');
491+
}
492+
493+
Future<void> test_privateVariable_notAccessible_notImported() async {
494+
newFile(convertPath('$testPackageLibPath/extensions.dart'), '''
495+
extension E on String {
496+
String _privateVariable = '';
497+
}
498+
''');
499+
await getTestCodeSuggestions('''
500+
void f(String s) {
501+
s.^;
502+
}
503+
''');
504+
assertHasNoCompletion('_privateVariable');
505+
}
334506
}
335507

336508
@reflectiveTest
@@ -560,7 +732,7 @@ class B implements A {
560732

561733
@failingTest
562734
Future<void> test_unnamedConstructor_inDifferentLibrary() async {
563-
newFile('/project/bin/b.dart', '''
735+
newFile('$testPackageLibPath/b.dart', '''
564736
class B implements A {
565737
B();
566738
}
@@ -671,7 +843,7 @@ void g() {}
671843
@reflectiveTest
672844
class SuperConstructorInvocationCompletionTest extends CompletionTestCase {
673845
Future<void> test_namedConstructor_notVisible() async {
674-
newFile('/project/bin/a.dart', '''
846+
newFile('$testPackageLibPath/a.dart', '''
675847
class A {
676848
A._() {}
677849
}

0 commit comments

Comments
 (0)