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

Added reflection based discovery of analyzers and fixes #698

Merged

Conversation

JoeRobich
Copy link
Member

No description provided.

@JoeRobich
Copy link
Member Author

From the failed Formatting_Check leg:

  The dotnet CLI version is '3.1.300'.
  Using MSBuild.exe located in 'C:\Program Files\dotnet\sdk\3.1.300\'.
  Formatting code files in workspace 'D:\a\1\s\format.sln'.
  Loading workspace.
  Complete in 4651ms.
  Determining formattable files.
  Complete in 725ms.
  Running formatters.
  src\Analyzers\AnalyzerFinderHelpers.cs(1,1): Fix file encoding.
  src\Analyzers\AnalyzerReferenceAnalyzerFinder.cs(1,1): Fix file encoding.
  Running Code Style analysis.
  Determining diagnostics...
  src\Analyzers\Extensions.cs(173,21): use 'var' instead of explicit type
  Complete in 4075ms.
  Fixing diagnostics...
  Complete in 500ms.
  Analysis complete in 4575ms.
  Complete in 5433ms.
  Formatted code file 'D:\a\1\s\src\Analyzers\AnalyzerFinderHelpers.cs'.
  Formatted code file 'D:\a\1\s\src\Analyzers\AnalyzerReferenceAnalyzerFinder.cs'.
  Formatted code file 'D:\a\1\s\src\Analyzers\Extensions.cs'.
  Writing formatting report to: './artifacts/log/format-report.json'.
  Formatted 3 of 74 files.
  Format complete in 10885ms.
##[error]Cmd.exe exited with code '2'.

- script: dotnet run --project ./src/dotnet-format.csproj -- @validate.rsp
- script: dotnet publish ./src/dotnet-format.csproj -c Release
displayName: Publish dotnet-format
- script: dotnet ./artifacts/bin/dotnet-format/Release/netcoreapp2.1/publish/dotnet-format.dll @validate.rsp
Copy link
Member Author

Choose a reason for hiding this comment

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

In order to run codestyle analyzers, we need to publish before we run because not all dependencies are copied into the build folder.

{
var reportFilePath = GetReportFilePath(reportPath!); // IsNullOrEmpty is not annotated on .NET Core 2.1
var reportFilePath = GetReportFilePath(formatOptions.ReportPath!); // IsNullOrEmpty is not annotated on .NET Core 2.1
Copy link
Contributor

Choose a reason for hiding this comment

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

:(

@@ -29,27 +29,27 @@ internal static class CodeFormatter
new EndOfLineFormatter(),
new CharsetFormatter(),
new ImportsFormatter(),
new AnalyzerFormatter(new RoslynCodeStyleAnalyzerFinder(), new AnalyzerRunner(), new SolutionCodeFixApplier()),
new AnalyzerFormatter("Code Style", new RoslynCodeStyleAnalyzerFinder(), new AnalyzerRunner(), new SolutionCodeFixApplier()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add constants for these strings?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do in a followup pr. likely want to change how these formatters are instantiated regardless.

src/dotnet-format.csproj Outdated Show resolved Hide resolved
src/FormatCommand.cs Outdated Show resolved Hide resolved
@jmarolf
Copy link
Contributor

jmarolf commented Jun 9, 2020

internal class FormatOptions

So excited for this to be a record in C# 9

src/Program.cs Outdated Show resolved Hide resolved
internal static class AnalyzerFinderHelpers
{
public static ImmutableArray<(DiagnosticAnalyzer Analyzer, CodeFixProvider? Fixer)> LoadAnalyzersAndFixers(
IEnumerable<Assembly> assemblies,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add unit tests for this class?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will add in a follow up

@jmarolf
Copy link
Contributor

jmarolf commented Jun 9, 2020

@JoeRobich This all looks fantastic. All I would ask is that you add new perf tests for these scenarios so it will be easier to iterate on improving them. I still need to do the work to get the perf tests to run on CI :(

@JoeRobich JoeRobich mentioned this pull request Jun 9, 2020
7 tasks
@JoeRobich JoeRobich merged commit 528bf3b into dotnet:feature/analyzers Jun 9, 2020
@jmarolf jmarolf mentioned this pull request Jun 25, 2020
3 tasks
@JoeRobich JoeRobich deleted the add-reflection-discovery branch March 5, 2021 21:02
vdurante pushed a commit to vdurante/format that referenced this pull request Feb 29, 2024
Added reflection based discovery of analyzers and fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants