Completion does not include TopLevelVariableElement

import 'package:analyzer/dart/element/element.dart';

void f() {
  TLVE^;
}

Is this the same as #61134 ?

I hoped that presence of all letters would be enough to filter out all but TopLevelVariableElement, but it does not work as I expected. Surprisingly, the deprecated typedef is present.

I'm investigating.

Weirdly, if you remove the abstract keyword from the declaration of TopLevelVariableElement, you get the completion for the implicit unnamed constructor but still not the type...

And if you manually append letters to the name, it does show up but renaming it doesn't have the same effect.

Edit: Removing the typedef does make it show up again, this probably has some relation.

I'll keep digging.

@FMorschel

@FMorschel I added some logging, and it seems that the class TopLevelVariableElement is filtered out by this check for whether it is visible (not shadowed):

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

Lines 1754 to 1759 in 93725ad

    
             void _suggestClass(ClassElement element, ImportData? importData) { 
        
               if (!visibilityTracker.isVisible( 
        
                 element: element, 
        
                 importData: importData, 
        
               )) { 
        
                 return;

The reason this filters it out is because VisibilityTracker already has the class name in _declaredNames. The reason that happens is that _suggestConstructors was called earlier, and that first checks if the class is visible:

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

Lines 1878 to 1881 in 93725ad

    
           !visibilityTracker.isVisible( 
        
             element: constructors.first.enclosingElement, 
        
             importData: importData, 
        
           )) {

I think this was added as an optimization (if the class is not visible, don't show any constructors), but since isVisible adds the name to _declaredNames and it is called before the class is added, this results in subsequent calls always returning false.

Possibly the solution is for isVisible to have a flag for whether it records the name as used when it returns true that is set to false when being used in this way. Although it the code was ever re-ordered to suggest classes first, then it would fail to add the constructors, so maybe a better fix would be for _declaredNames to actually record the elements, and still return true if it is asked about the same element again? (@bwilkerson since you're most familiar with this code, do you have opinions/preferences?)

I think your analysis is correct. The method isVisible has a side-effect of making the name appear to be non-visible after it's called. That doesn't create a problem if we only checked the visibility immediately before creating a candidate suggestion, but on line 1878 we're using it to optimize the code, and the side-effect creates the bug.

There are two solutions I can think of:

add a parameter indicating whether the name should become non-visible as a result of the call (as you suggested), or
have two methods whose names more clearly indicate the difference in behavior.

I don't have a strong preference, but if we go with (1) then I'd want the parameter to be required so that it's clear at every call site which behavior we're actually getting. Option (1) is attractive because then we don't have to think of names for the two methods, and I don't have any good suggestions at the moment, but that doesn't necessarily mean that it's the better option.

On a related note, the analysis above was made harder because of a poor choice of names. I overloaded the meaning of "visible" so that it's used both in the sense of the visibility of an identifier in a given library ('public' vs. 'private') and in the sense of something being visible 'in scope'. I think the former is the better definition of the term because I think that's the more common meaning of the term.

The fix (which isn't a required part of fixing the bug) is probably to rename the VisibilityTracker to something like ScopeTracker and to rename VisibilityTracker.isVisible to something like ScopeTracker.isUnshadowed.

If someone does this, then we need to look carefully at every use of VisibilityTracker.isVisible to see which semantic we ought to be using. I hope that we're using the right semantics everywhere, but it should still be checked.

@FMorschel

I too think a flag would be easier to name. I also agree that using visible here is a little confusing, so renaming it would be good.

@FMorschel are you still planning on working on this? I'm going to be out the tail end of this week and all of next week, but I can take a look when I'm back if nobody else did.

Yes, I am.

I'll see if I can make some progress, but with your guidance above, it should be easy enough. If I hit any other walls, I'll let you know.

Thanks!

I've come back to this, and it seems that the problem happens at DeclarationHelper._addConstructorsForAliasedElement, where the checkVisibility flag for the constructor suggestions wasn't being defined (default true). So when we get to suggesting the type, it is "not visible".

This suggests that when building this library namespace, we add TopLevelVariableElement2 first and TopLevelVariableElement after it, and this is the only reason this bug happens.

That is why my above findings make sense. I'll see if I can reproduce this ordering on a test file, but I was having some trouble with that previously on an empty project. I'll edit here if I get somewhere.

Edit: I could swear I'd tested like this previously, but it seems I didn't try adding the typedef before (lines above) the abstract class declaration on my tests. Or I did so on my own file and not an imported one, not sure. Either way, this reproduces it:

import.dart

typedef MyClass2 = MyClass;

abstract class MyClass {}

test.dart

import 'import.dart';

void foo() {
  MyClas;
}

Here is the CL with the fix for the completion. I'm not sure the suggested parameter is needed because the call to that method shouldn't be happening there. But we can discuss this at the CL and I can update this thread later if necessary.

https://dart-review.googlesource.com/c/sdk/+/444701

[DAS] Fixes completion not showing abstract class with alias declared…

Navigation Menu

Repository navigation

Completion does not include TopLevelVariableElement #61244

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	void _suggestClass(ClassElement element, ImportData? importData) {
	if (!visibilityTracker.isVisible(
	element: element,
	importData: importData,
	)) {
	return;

	!visibilityTracker.isVisible(
	element: constructors.first.enclosingElement,
	importData: importData,
	)) {

Completion does not include TopLevelVariableElement #61244

Description

Activity

FMorschel commented on Aug 5, 2025

DanTup commented on Aug 8, 2025

bwilkerson commented on Aug 11, 2025

DanTup commented on Aug 11, 2025

FMorschel commented on Aug 11, 2025

FMorschel commented on Aug 11, 2025

FMorschel commented on Aug 11, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions