Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IDE1006: Support multiple underscores #68086

Merged
merged 2 commits into from
May 10, 2023
Merged

IDE1006: Support multiple underscores #68086

merged 2 commits into from
May 10, 2023

Conversation

Sjoerdsjoerd
Copy link
Contributor

@Sjoerdsjoerd Sjoerdsjoerd commented May 4, 2023 •
edited by sharwell
Loading

The issue describes the problem when multiple underscores are used, and the IDE still gives the IDE1006 message.

In a quick glance I could not find any unit tests for IDE1006, so I tested it locally and it seems to work.

Closes #40713

@Sjoerdsjoerd Sjoerdsjoerd requested a review from a team as a code owner May 4, 2023 14:08
@ghost ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label May 4, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels May 4, 2023
@Sjoerdsjoerd

This comment was marked as resolved.

Sign in to view
sharwell
sharwell requested changes May 4, 2023
View reviewed changes
Copy link
Member

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

Tests can be added to these files:

https://github.com/dotnet/roslyn/blob/main/src/Analyzers/CSharp/Tests/NamingStyles/NamingStylesTests.cs
https://github.com/dotnet/roslyn/blob/main/src/Analyzers/VisualBasic/Tests/NamingStyles/NamingStylesTests.vb

@sharwell sharwell added the Feature - IDE1006 Naming styles label May 4, 2023
Youssef1313
Youssef1313 suggested changes May 4, 2023
View reviewed changes
Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

Best to have another PR to dotnet/roslyn-analyzers as well:

https://github.com/dotnet/roslyn-analyzers/blob/main/src/Utilities/Compiler/Extensions/ISymbolExtensions.cs

@Sjoerdsjoerd
Copy link
Contributor Author

Tests can be added to these files:

https://github.com/dotnet/roslyn/blob/main/src/Analyzers/CSharp/Tests/NamingStyles/NamingStylesTests.cs https://github.com/dotnet/roslyn/blob/main/src/Analyzers/VisualBasic/Tests/NamingStyles/NamingStylesTests.vb

I have extended the discard tests with the extra underscores

@Sjoerdsjoerd
Copy link
Contributor Author

Best to have another PR to dotnet/roslyn-analyzers as well:

https://github.com/dotnet/roslyn-analyzers/blob/main/src/Utilities/Compiler/Extensions/ISymbolExtensions.cs

Sure, I will create an PR there as well.

@Sjoerdsjoerd Sjoerdsjoerd mentioned this pull request May 5, 2023
Merged
sharwell
sharwell approved these changes May 5, 2023
View reviewed changes
Copy link
Member

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

This appears to meet the behavior described in the original issue, but leaving the final review to @mavasani to confirm there haven't been any changes to the space over the past 3 years.

@mavasani
Copy link
Contributor

mavasani commented May 6, 2023

This appears to meet the behavior described in the original issue, but leaving the final review to @mavasani to confirm there haven't been any changes to the space over the past 3 years.

I can’t recall any subsequent design discussions on this, so seems fine to me.

@Sjoerdsjoerd
Copy link
Contributor Author

The pipeline is failing. Is it possible to restart it?

@CyrusNajmabadi CyrusNajmabadi merged commit 7bf85c9 into dotnet:main May 10, 2023
@CyrusNajmabadi
Copy link
Member

Thanks!

@ghost ghost added this to the Next milestone May 10, 2023
@Sjoerdsjoerd Sjoerdsjoerd deleted the feature/40713-MultipleUnderscoresAsDiscards branch May 10, 2023 20:30
@allisonchou allisonchou mentioned this pull request May 12, 2023
Closed
@Cosifne Cosifne modified the milestones: Next, 17.7 P2 May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Youssef1313 Youssef1313 Youssef1313 requested changes

@sharwell sharwell sharwell approved these changes

Assignees
No one assigned
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature - IDE1006 Naming styles untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Milestone
17.7 P2
Development

Successfully merging this pull request may close these issues.

IDE1006 should not warn when name is all underscores
6 participants
@Sjoerdsjoerd @mavasani @CyrusNajmabadi @sharwell @Youssef1313 @Cosifne