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

Adopt latest roslyn analyzers #40839

Merged
merged 2 commits into from Jan 10, 2020
Merged

Adopt latest roslyn analyzers #40839

merged 2 commits into from Jan 10, 2020

Conversation

@jcouv
Copy link
Member

jcouv commented Jan 9, 2020

I'm holding off using #nullable enable in any project because there is a bug in that analyzer. After adding nullable annotations to the public API files, the analyzer complains with RS0016 and RS0017...

@jcouv jcouv self-assigned this Jan 9, 2020
@jcouv jcouv force-pushed the jcouv:new-analyzers branch from 6a0ef02 to 10e0b8b Jan 9, 2020
@@ -27,7 +27,7 @@
</PropertyGroup>
<PropertyGroup>
<!-- Versions used by several individual references below -->
<RoslynDiagnosticsNugetPackageVersion>2.9.6</RoslynDiagnosticsNugetPackageVersion>
<RoslynDiagnosticsNugetPackageVersion>3.0.0-beta2.20059.3+77df2220</RoslynDiagnosticsNugetPackageVersion>

This comment has been minimized.

Copy link
@jcouv

jcouv Jan 9, 2020

Author Member

@dotnet/roslyn-infrastructure @jaredpar I've update the version number for roslyn analyzer package. I'm seeing the new analyzers kick in within VS, but it looks like build.cmd is still using some older version (2.9.6 as far as I can tell).
Is that expected? What's the process from here?

This comment has been minimized.

Copy link
@mavasani

mavasani Jan 9, 2020

Contributor

@jcouv - is command line build using older version of the analyzer package OR compiler toolset?

This comment has been minimized.

Copy link
@jcouv

jcouv Jan 9, 2020

Author Member

@mavasani Not sure how to tell the difference. Probably both.
Assuming that analyzer package is referenced by compiler toolset (correct me if that's wrong), there is no compiler toolset which references the new analyzer package that I need (since it's brand new).

@jcouv jcouv marked this pull request as ready for review Jan 9, 2020
@jcouv jcouv requested review from dotnet/roslyn-ide as code owners Jan 9, 2020
@jcouv jcouv requested a review from mavasani Jan 9, 2020
@jcouv jcouv added the Area-IDE label Jan 9, 2020
@@ -205,3 +205,7 @@ csharp_space_between_square_brackets = false
csharp_prefer_braces = true:silent
csharp_preserve_single_line_blocks = true
csharp_preserve_single_line_statements = true

# error RS0037: PublicAPI.txt is missing '#nullable enable'

This comment has been minimized.

Copy link
@mavasani

mavasani Jan 9, 2020

Contributor

Will this be done as a separate PR or with further commits in this PR?

This comment has been minimized.

Copy link
@jcouv

jcouv Jan 9, 2020

Author Member

Each project needs to decide whether they want to enable this. But they cannot enable this right now, because of the issue above (two versions of analyzers involved).

@@ -572,7 +572,9 @@ public override void Initialize(AnalysisContext context)
public sealed class AnalyzerWithCSharpCompilerDiagnosticId : DiagnosticAnalyzer
{
public static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(
#pragma warning disable RS1029 // Do not use reserved diagnostic IDs.

This comment has been minimized.

Copy link
@mavasani

mavasani Jan 9, 2020

Contributor

Might want to turn off RS1029 for all non-shipping projects: https://github.com/dotnet/roslyn/blob/master/eng/config/rulesets/NonShipping.ruleset

This comment has been minimized.

Copy link
@sharwell

sharwell Jan 9, 2020

Member

The targeted suppression seems fine for now, considering it was scope-limited

.editorconfig Outdated Show resolved Hide resolved
Co-Authored-By: Sam Harwell <sam@tunnelvisionlabs.com>
@jcouv jcouv merged commit e606428 into dotnet:master Jan 10, 2020
18 checks passed
18 checks passed
WIP Ready for review
Details
license/cla All CLA requirements met.
Details
roslyn-CI Build #20200109.23 succeeded
Details
roslyn-CI (Linux_Test coreclr) Linux_Test coreclr succeeded
Details
roslyn-CI (SourceBuild_Test) SourceBuild_Test succeeded
Details
roslyn-CI (Windows_CoreClr_Unit_Tests debug) Windows_CoreClr_Unit_Tests debug succeeded
Details
roslyn-CI (Windows_CoreClr_Unit_Tests release) Windows_CoreClr_Unit_Tests release succeeded
Details
roslyn-CI (Windows_Correctness_Test) Windows_Correctness_Test succeeded
Details
roslyn-CI (Windows_Desktop_Spanish_Unit_Tests) Windows_Desktop_Spanish_Unit_Tests succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests debug_32) Windows_Desktop_Unit_Tests debug_32 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests debug_64) Windows_Desktop_Unit_Tests debug_64 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests release_32) Windows_Desktop_Unit_Tests release_32 succeeded
Details
roslyn-CI (Windows_Desktop_Unit_Tests release_64) Windows_Desktop_Unit_Tests release_64 succeeded
Details
roslyn-CI (Windows_Determinism_Test) Windows_Determinism_Test succeeded
Details
roslyn-CI (macOS_Test) macOS_Test succeeded
Details
roslyn-integration-CI Build #20200109.23 succeeded
Details
roslyn-integration-CI (VS_Integration debug_async) VS_Integration debug_async succeeded
Details
roslyn-integration-CI (VS_Integration release_async) VS_Integration release_async succeeded
Details
@jcouv jcouv deleted the jcouv:new-analyzers branch Jan 10, 2020
@jcouv jcouv added this to the 16.5.P3 milestone Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.