Skip to content

Add global AnalyzerConfig with default configuration #13835

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
Merged
iSazonov merged 3 commits into from
Oct 26, 2020
Merged

Add global AnalyzerConfig with default configuration #13835

iSazonov merged 3 commits into from
Oct 26, 2020

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Oct 22, 2020 •
edited
Loading

PR Summary

  • Copy microsoft.codeanalysis.netanalyzers/5.0.0-rtm.20502.2/editorconfig/AllRulesDefault/.editorconfig to .globalconfig
  • Add Add IDE diagnostics to .globalconfig with most rules hidden
    • copied from dotnet/runtime and converted using RulesetToEditorconfigConverter.exe
  • Disable diagnostics for Microsoft.Management.UI.Internal

PR Context

Alternative to #12892

If Visual Studio 2019 is installed, it should be updated to 16.8 Preview5+ because OmniSharp appears to prefer the Visual Studio MSBuild instance to the StandAlone instance. Otherwise, the global AnalyzerConfig may be ignored.

PR Checklist

@iSazonov
Copy link
Collaborator

iSazonov commented Oct 22, 2020 •
edited
Loading

Why not follow https://github.com/dotnet/runtime/blob/master/eng/Analyzers.props?
(Also I'd think about xUnit tests folder too.)

GitHub
.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps. - dotnet/runtime

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Oct 22, 2020 •
edited by unfurl-links bot
Loading

Why not follow https://github.com/dotnet/runtime/blob/master/eng/Analyzers.props?
(Also I'd think about xUnit tests folder too.)
Previously, you had wanted to use EditorConfig instead of ruleset configuration.

GitHub
.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps. - dotnet/runtime

@iSazonov
Copy link
Collaborator

iSazonov commented Oct 22, 2020

Previously, you had wanted to use EditorConfig instead of ruleset configuration.

My thought was and is that we shouldn't have two configs - in EditorConfig and in ruleset. If VS Code supports(?) ruleset it is better to use ruleset and follow .Net team's pattern.

@xtqqczze xtqqczze mentioned this pull request Oct 22, 2020
Closed
8 tasks
@xtqqczze

This comment has been minimized.

Sign in to view
@xtqqczze xtqqczze closed this Oct 22, 2020
@xtqqczze xtqqczze reopened this Oct 23, 2020
@xtqqczze xtqqczze force-pushed the add-analyzers-props2 branch from 2a3256e to d90a6f9 Compare October 23, 2020 16:35
@xtqqczze
Add default global AnalyzerConfig
13d0258 
Add `microsoft.codeanalysis.netanalyzers\5.0.0-rtm.20502.2\editorconfig\AllRulesDefault\.editorconfig`
@xtqqczze xtqqczze force-pushed the add-analyzers-props2 branch from d90a6f9 to 13d0258 Compare October 23, 2020 17:03
@xtqqczze xtqqczze changed the title Add default global AnalyzerConfig Add global AnalyzerConfig with default configuration Oct 23, 2020
@xtqqczze xtqqczze marked this pull request as ready for review October 23, 2020 17:05
@xtqqczze
Copy link
Contributor Author

xtqqczze commented Oct 23, 2020

@iSazonov This PR uses .globalconfig file. It is the easiest solution and it just works, including TreatWarningsAsErrors.

This was referenced Oct 25, 2020
Merged
Merged
Merged
Merged
Closed
Merged
Merged
@iSazonov iSazonov added the CL-Tools Indicates that a PR should be marked as a tools change in the Change Log label Oct 26, 2020
@xtqqczze xtqqczze mentioned this pull request Oct 26, 2020
Merged
@iSazonov
Copy link
Collaborator

iSazonov commented Oct 26, 2020

I will merge because:

  • we should benefit from Roslyn analyzers that is best practice.
  • it is more convenient than alternative analyzers.
  • we will be able to force rules in PRs without requests from maintainers.
  • After we moved to .Net 5.0 RC1 Roslyn analyzers are already enabled by default.
  • VS Code OmniSharp works with EditorConfig and GlobalConfig (need latest VS 2019 16.8 Preview5+).

As a side notice, we could disable Analyzers in all CIs to speed up compiling and separate check (compile only) with enabled analyzers.

@iSazonov iSazonov merged commit ab378f6 into PowerShell:master Oct 26, 2020
@iSazonov iSazonov added this to the 7.2.0-preview.1 milestone Oct 26, 2020
@xtqqczze xtqqczze mentioned this pull request Oct 26, 2020
Merged
@xtqqczze xtqqczze deleted the add-analyzers-props2 branch October 26, 2020 16:22
@xtqqczze xtqqczze mentioned this pull request Nov 2, 2020
Merged
@ghost
Copy link

ghost commented Nov 17, 2020

🎉v7.2.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov iSazonov iSazonov approved these changes

Assignees

@iSazonov iSazonov

Labels

CL-Tools Indicates that a PR should be marked as a tools change in the Change Log

Projects

None yet

Milestone

7.2.0-preview.1

Development

Successfully merging this pull request may close these issues.

3 participants

@xtqqczze @iSazonov @adityapatwardhan