Added reflection based discovery of analyzers and fixes #698
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoeRobich
force-pushed
the
add-reflection-discovery
branch
from
e4622af
to
e42fb0f
Compare
JoeRobich
force-pushed
the
add-reflection-discovery
branch
from
e42fb0f
to
577fbd1
Compare
|
From the failed Formatting_Check leg: |
JoeRobich
force-pushed
the
add-reflection-discovery
branch
from
ffe2fa6
to
b7c20e4
Compare
JoeRobich
commented
Jun 9, 2020
| - 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 |
There was a problem hiding this comment.
In order to run codestyle analyzers, we need to publish before we run because not all dependencies are copied into the build folder.
jmarolf
reviewed
Jun 9, 2020
| { | ||
| 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 |
jmarolf
reviewed
Jun 9, 2020
| @@ -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()), | |||
There was a problem hiding this comment.
Can we add constants for these strings?
There was a problem hiding this comment.
Will do in a followup pr. likely want to change how these formatters are instantiated regardless.
jmarolf
reviewed
Jun 9, 2020
jmarolf
reviewed
Jun 9, 2020
jmarolf
reviewed
Jun 9, 2020
So excited for this to be a record in C# 9 |
jmarolf
reviewed
Jun 9, 2020
jmarolf
reviewed
Jun 9, 2020
jmarolf
reviewed
Jun 9, 2020
jmarolf
reviewed
Jun 9, 2020
| internal static class AnalyzerFinderHelpers | ||
| { | ||
| public static ImmutableArray<(DiagnosticAnalyzer Analyzer, CodeFixProvider? Fixer)> LoadAnalyzersAndFixers( | ||
| IEnumerable<Assembly> assemblies, |
There was a problem hiding this comment.
Can we add unit tests for this class?
There was a problem hiding this comment.
Will add in a follow up
jmarolf
reviewed
Jun 9, 2020
jmarolf
reviewed
Jun 9, 2020
jmarolf
reviewed
Jun 9, 2020
jmarolf
reviewed
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 :( |
jmarolf
approved these changes
Jun 9, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.