Skip to content

Recommend static members for type parameters#53942

Merged
CyrusNajmabadi merged 5 commits intodotnet:mainfrom
Youssef1313:recommend-interface-statics-type-parameter
Jul 2, 2021
Merged

Recommend static members for type parameters#53942
CyrusNajmabadi merged 5 commits intodotnet:mainfrom
Youssef1313:recommend-interface-statics-type-parameter

Conversation

@Youssef1313
Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 commented Jun 8, 2021

@AlekseyTs What's the situation for consuming interfaces with statics in VB? i.e, Anything needed for VB completion?

Fixes #53930

@Youssef1313 Youssef1313 requested a review from a team as a code owner June 8, 2021 12:53
@ghost ghost added the Area-IDE label Jun 8, 2021
{
excludeInstance = true;
excludeStatic = false;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would roll this into the check above. and remove the (INamespaceOrTypeSymbol) cast when assigning to container symbol.

@AlekseyTs
Copy link
Copy Markdown
Contributor

Recommend static members for type parameters

I didn't check the implementation. Note, only static abstract members are valid in this context. So, non-abstract statics probably shouldn't be offered.

@AlekseyTs
Copy link
Copy Markdown
Contributor

What's the situation for consuming interfaces with statics in VB? i.e, Anything needed for VB completion?

We haven't done any work on VB side yet. It is not clear at this point if we are going to enable this scenario in VB as this would likely require a language spec change.

@Youssef1313
Copy link
Copy Markdown
Member Author

this cast will fail. note that containerSymbol is just ISymbol, so just remove this cast altogetehr.

It doesn't fail since ITypeParameterSymbol is under INamespaceOrTypeSymbol in the hierarchy. But it's redundant anyways, so I'll get rid of it.


Note, only static abstract members are valid in this context. So, non-abstract statics probably shouldn't be offered.

Indeed, this seems like a bug I have currently. Will fix it.

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

It doesn't fail since ITypeParameterSymbol is under INamespaceOrTypeSymbol in the hierarchy. But it's redundant anyways, so I'll get rid of it.

Touche :)

interface I1
{

static abstract void M1();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static abstract void M1();

Please add a non-abstract static method. It is not valid to offer it in this context.

@jinujoseph jinujoseph added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Jun 9, 2021
excludeInstance = true;
excludeStatic = false;
containerSymbol = (INamespaceOrTypeSymbol)symbol;
abstractStaticsOnly = symbol.Kind == SymbolKind.TypeParameter;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't this just be abstractsOnly?

var namedSymbols = excludeStatic
? symbols.WhereAsArray(s => !(s.IsStatic || s is ITypeSymbol))
: symbols;
: (abstractStaticsOnly ? symbols.WhereAsArray(s => s.IsAbstract && s.IsStatic) : symbols);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't the static check alreacy have happened because we passed excludeInstance to GetMemberSymbols. so we only need to check abstracts here, right?

@AlekseyTs
Copy link
Copy Markdown
Contributor

The feature branch has been merged into 'main', this PR should be retargeted to 'main'

@Youssef1313 Youssef1313 changed the base branch from features/StaticAbstractMembersInInterfaces to main June 21, 2021 19:24
@CyrusNajmabadi CyrusNajmabadi merged commit 1015d26 into dotnet:main Jul 2, 2021
@ghost ghost added this to the Next milestone Jul 2, 2021
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

Thanks!

@Youssef1313 Youssef1313 deleted the recommend-interface-statics-type-parameter branch July 2, 2021 18:41
@allisonchou allisonchou modified the milestones: Next, 17.0.P3 Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Static abstract members of interfaces are not included into completion list on a type parameter

5 participants