Skip to content

Add classification API support for VB members, locals and parameters, and tweak C# implementation#25126

Merged
DustinCampbell merged 10 commits intodotnet:dev15.7.xfrom
DustinCampbell:vb-member-classification
Mar 5, 2018
Merged

Add classification API support for VB members, locals and parameters, and tweak C# implementation#25126
DustinCampbell merged 10 commits intodotnet:dev15.7.xfrom
DustinCampbell:vb-member-classification

Conversation

@DustinCampbell
Copy link
Member

@DustinCampbell DustinCampbell commented Feb 28, 2018

This is a follow up to #24931 that tweaks C# classification and adds support for Visual Basic. This only affects the classification APIs and does is not (yet) exposed in Visual Studio. This is related to #3976.

The design tweaks I've made to the original C# implementation (#24931) are:

  • Extension methods are only classified at declarations or when used as a reduced form. If an extension method is called as a static method invocation (i.e. Enumerable.Select(...)), it will be classified as an ordinary method.
  • Local constants are now classified as constants rather than locals.
  • EnumFieldName has been renamed to EnumMemberName to better match the rest of the Roslyn API (i.e. EnumMemberDeclarationSyntax).

Note that extension method declarations in VB are impossible to classify syntactically. So, they VB methods are classified as methods and the semantic classifier adds an additional classification for extension methods.

In addition, this PR unifies the classification unit test infrastructure quite a bit. It might be useful to review one commit at a time if it's too cumbersome.

cc @mkrueger

This change renames the recently added `ClassificationTypeNames.EnumFieldName` to
`ClassificationTypeNames.EnumMemberName` to be consistent with other Roslyn APIs (such as EnumMemberDeclarationSyntax).
In addition, the newly added `ClassificationTypeNames` constants for member classification were incorrectly added to
PublicAPI.Shipped.txt rather than PublicAPI.Unshipped.txt. This commit fixes that (since the APIs haven't shipped yet).
This change ensures that extension methods are only classified specially when declared or when
called in a reduced form. If an extension method is called as an ordinary static method invocation,
it is classified as an oridinary method.
@DustinCampbell
Copy link
Member Author

@dotnet-bot retest this please

{
return ClassificationBuilder.Inactive(value);
}
protected FormattedClassification Struct(string text) => ClassificationBuilder.Struct(text);
Copy link
Member

Choose a reason for hiding this comment

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

Would a using static of ClassificationBuilder avoid the need to have all of these?

Copy link
Member Author

Choose a reason for hiding this comment

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

heh-heh. Yes, I think you're right. I'll take a look at that.

@DustinCampbell DustinCampbell requested a review from a team March 1, 2018 12:28

public const string FieldName = "field name";
public const string EnumFieldName = "enum field name";
public const string EnumMemberName = "enum member name";
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to update EditorColors.xml in the VS repo for theming?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if the right thing happened there when the C# ones were introduced either.

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need to update EditorColors.xml yet. This is just an API-only change. In VS, these all map back to "identifier" for now. When we introduce this feature for VS we'll update it for theming.

@jinujoseph
Copy link
Contributor

@Pilchie for ask mode approval for 15.7.P2

@Pilchie
Copy link
Member

Pilchie commented Mar 2, 2018

Approved for 15.7 P2

@DustinCampbell
Copy link
Member Author

Thanks all!

@DustinCampbell DustinCampbell merged commit 45a8675 into dotnet:dev15.7.x Mar 5, 2018
@DustinCampbell DustinCampbell deleted the vb-member-classification branch August 24, 2021 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants