@iSazonov What do you think about adding StyleCop.Analyzers to build and to live analysis?

We could set SA1518 severity to warning to prevent regressions from #13574.

I think we could do the same as we do for analyzers. Only my concern is increasing time of the build so we could consider to move this (Roslyn analyzers and StyleCop.Analyzers) to separate GitHub check.

I think we could do the same as we do for analyzers. Only my concern is increasing time of the build so we could consider to move this (Roslyn analyzers and StyleCop.Analyzers) to separate GitHub check.

We should monitor build times, but there is value in running analyzers as part of each build rather then a separate check. One reason is conditionals in the pre-processor, we would need to run the analyzers for each combination of symbols, UNIX, CORECLR etc if we did not run during build.

I think we could do the same as we do for analyzers. Only my concern is increasing time of the build so we could consider to move this (Roslyn analyzers and StyleCop.Analyzers) to separate GitHub check.

We should monitor build times, but there is value in running analyzers as part of each build rather then a separate check. One reason is conditionals in the pre-processor, we would need to run the analyzers for each combination of symbols, UNIX, CORECLR etc if we did not run during build.

Running analyzers during local build also allows contributors to fix issues before submitting a PR.

we would need to run the analyzers for each combination of symbols, UNIX, CORECLR etc if we did not run during build.

We use only UNIX and WINDOWS.

@iSazonov Once this PR is merged I can move forward with SAXXXX drafts PRs. If the changes cause issues with build time we can deal with this in the first instance.

@xtqqczze How much are we slowing down the build?

@iSazonov It seems build on PowerShell-CI-Windows currently takes around 6 minutes, I've opened test PR #14122 with all analyzers disabled to see whether there is a difference.

@iSazonov It appears build time is quite variable, and having analyzers enabled does not measurably slow the build.

In this PR (161ccea) build time was 9m 1s
In #14122 (b0f6e3d) build time was 13m 33s
In #14122 (ddc3a70) build time was 9m 22s

The measurement I have used is the total for PowerShell-CI-windows (Build for Windows Windows Build) (so as to include xUnit build too).

Three 3 SA1507 violations were fixed in this PR, but CodeFactor has only reported one issue fixed. If we ignore the fix in tests, it is clear CodeFactor is not checking inside of if UNIX directive.

So if we want to enforce style rules through the entire codebase, we need to keep the StyleCop.Analyzers package that was added in #13963.

Originally posted by @xtqqczze in #14136 (comment)

🎉v7.2.0-preview.2 has been released which incorporates this pull request.🎉

Handy links:

• Release Notes