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.

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

Allow double underscore as lambda parameter name #2758

Merged
merged 1 commit into from Aug 23, 2018

Conversation

Projects
None yet
2 participants
@jnm2
Copy link
Contributor

jnm2 commented Aug 23, 2018

Closes #1606 using 58e7be6 as prior art.

⚠Tests SA1100UnitTests.TestIndexerAsync and SA1100UnitTests.TestIndexerWithInvokationAsync were failing immediately after cloning using ReSharper's runner, before any changes were made.

@jnm2

This comment has been minimized.

Copy link
Contributor Author

jnm2 commented Aug 23, 2018

@sharwell CI has a couple dozen errors of the form:

db2Pdb.exe : File 'C:\projects\stylecopanalyzers\StyleCop.Analyzers\StyleCop.Analyzers.Test.CSharp7\bin\Debug\net46\System.Collections.Immutable.dll' doesn't contain an embedded PDB.
At C:\projects\stylecopanalyzers\build\opencover-report.ps1:82

@@ -199,6 +199,10 @@ This rule is disabled by default in StyleCop Analyzers, but can be enabled by us
:warning: StyleCop Analyzers does not report SA1305 for parameters in overriding methods and methods which implement an
interface. StyleCop Classic reported SA1305 for all methods.

### SA1313

StyleCop Analyzers treats allows the double underscore `__` as a lambda parameter name as well as the single underscore.

This comment has been minimized.

@sharwell

sharwell Aug 23, 2018

Member

📝 This document is for changes relative to StyleCop Classic. If I'm not mistaken, that library will ignore parameters with any number of underscores. The difference in StyleCop Analyzers is we only make the exception for one or two.

This comment has been minimized.

@jnm2

jnm2 Aug 23, 2018

Author Contributor

I wasn't sure so I left the wording ambiguous :D Good to know.

}

/// <summary>
/// Verifies this diagnostic does not check whether or not a parameter named <c>_</c> is being used.

This comment has been minimized.

@sharwell

sharwell Aug 23, 2018

Member

💡 __

@sharwell

This comment has been minimized.

Copy link
Member

sharwell commented Aug 23, 2018

db2Pdb.exe : File 'C:\projects\stylecopanalyzers\StyleCop.Analyzers\StyleCop.Analyzers.Test.CSharp7\bin\Debug\net46\System.Collections.Immutable.dll' doesn't contain an embedded PDB.

Yes, it's part of a workaround waiting for someone to merge OpenCover/opencover#814.

@jnm2 jnm2 force-pushed the jnm2:permit_double_underscore_lambda_param branch from 8162086 to 94cfa9b Aug 23, 2018

@codecov

This comment has been minimized.

Copy link

codecov bot commented Aug 23, 2018

Codecov Report

Merging #2758 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2758      +/-   ##
==========================================
+ Coverage   97.38%   97.38%   +<.01%     
==========================================
  Files         751      751              
  Lines       99236    99270      +34     
  Branches     3246     3246              
==========================================
+ Hits        96645    96679      +34     
  Misses       1732     1732              
  Partials      859      859
@@ -199,6 +199,10 @@ This rule is disabled by default in StyleCop Analyzers, but can be enabled by us
:warning: StyleCop Analyzers does not report SA1305 for parameters in overriding methods and methods which implement an
interface. StyleCop Classic reported SA1305 for all methods.

### SA1313

StyleCop Analyzers treats allows the single and double underscore (`_` and `__`) as lambda parameter names.

This comment has been minimized.

@sharwell

sharwell Aug 23, 2018

Member

💡 It would be good to file a follow-up issue for someone to test if StyleCop Classic allows arbitrary numbers of underscores, and if so, update this section to explicitly state the difference.

@jnm2

This comment has been minimized.

Copy link
Contributor Author

jnm2 commented Aug 23, 2018

@sharwell #2759 and force pushed again.

@sharwell sharwell added this to the 1.1.0 Beta 9 milestone Aug 23, 2018

@sharwell sharwell merged commit a81a66a into DotNetAnalyzers:master Aug 23, 2018

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.