[DO NOT MERGE] Make a POC around in-memory compilation of .NET Framework views #9244
Conversation
| namespace SonarAnalyzer.Rules.CSharp; | ||
|
|
||
| [DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
| public sealed class FrameworkViewCompiler : SonarDiagnosticAnalyzer |
There was a problem hiding this comment.
This is the entrypoint.
The goal of this analyzer is to make one dummy compilation (or one dummy compilation per view file) and slap our analyzers on top of it. Then we can ask for diagnostics and filter accordingly.
| { | ||
| // TODO: Read this from context.ProjectConfiguration().ProjectPath. | ||
| // Not sure how to set it up in the UTs. | ||
| private const string ROOT_DIR = """C:\dev\personal\aspnet\Lol"""; |
There was a problem hiding this comment.
Check TODO.
If you want to debug locally, make a boilerplate ASP.NET Framework project and put the fullpath here.
Optimally, we should use additional files, just like we do from the ProjectReader.
| context.RegisterCompilationAction( | ||
| c => | ||
| { | ||
| var mightBeUseful = c.ProjectConfiguration(); |
There was a problem hiding this comment.
I am not sure how to mock this in UTs, but it would be useful.
| foreach (var diagnostic in diagnostics.Where(x => supportedDiagnostics.Keys.Contains(x.Id))) | ||
| { | ||
| // TODO: This breaks when raising on out-of-compilation files. Find a replacement, maybe writing directly to the SARIF. | ||
| c.ReportIssue(diagnostic); |
There was a problem hiding this comment.
See this thread.
The main problem is that:
- On .NET Core, the Views are part of the compilation...but we do not care, as they are compiled anyways and we get analysis (almost) for free.
- On .NET Framework the Views are NOT part of the compilation, so if we propagate the diagnostic with
ReportIssue, roslyn throws anAD0001.
|
|
||
| protected override void Initialize(SonarAnalysisContext context) => | ||
|
|
||
| context.RegisterCompilationAction( |
There was a problem hiding this comment.
We need to optionally register.
- Opt-in only for .NET FM projects that contain cshtml files.
- Opt-out on SonarLint.
...probably more filtering required.
| var razorCompiler = new RazorCompiler(ROOT_DIR, filesProvider); | ||
| var dummyCompilation = compilation; | ||
|
|
||
| foreach (var razorDocument in razorCompiler.CompileAll().Where(x => x.Source.FilePath.Contains("Contact"))) |
There was a problem hiding this comment.
The .Where(x => x.Source.FilePath.Contains("Contact")) is a hacky patch to check specific file locally, feel free to alter/remove it.
| { | ||
| if (razorDocument.GetCSharpDocument()?.GeneratedCode is { } csharpCode) | ||
| { | ||
| var fp = dummyCompilation.SyntaxTrees.FirstOrDefault(x => x.FilePath.Contains("Contact")).FilePath; |
There was a problem hiding this comment.
This is a hack so that the syntax tree we pass on AddSyntaxTrees is not name-less, to avoid AD0001s.
| } | ||
| #region RULE_FINDER | ||
| // Copied from the test framework. | ||
| internal static class RuleFinder2 |
There was a problem hiding this comment.
TLDR: This class uses reflection and fetches all of our rules from the same assembly.
| { | ||
| types = types.Concat(UtilityAnalyzerTypes.Where(x => TargetLanguage(x) == language)); | ||
| } | ||
| foreach (var type in types.Where(x => x == typeof(VariableUnused))) // TODO: Remove ".Where(x => x == typeof(VariableUnused))" and see error. Locally it breaks for me with StackOverflow, at a compiled Regex. |
There was a problem hiding this comment.
Check TODO.
We will probably need to exclude some of our rules.
For now, we can add one or two just to test. I chose S1481 because it's really braindead and easy to add issues for.
| /// Compiler for manual Razor Views cross-compilation to C# code for .NET Framework projects. | ||
| /// We don't need to manually compile .NET Core Razor projects. They are compiled during build time and analyzed directly with Roslyn analyzers. | ||
| /// </summary> | ||
| internal sealed class RazorCompiler |
There was a problem hiding this comment.
This is the magic/hack part.
This package cross-compiles cshtml -> c#, but only for .NET Core.
So we do a lot of dirty things to make it play a bit better with the .NET FM type system.
| .AddReferences(AspNet4xReferences("5.2.7")) | ||
| .AddSnippet(""" | ||
| @{ | ||
| var uselessInView = 42; |
There was a problem hiding this comment.
Ideally, we would like to raise here.
| @@ -1,11 +1,167 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk.Web"> | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Made this a .NET FM project, to test a bit more realistically.
I think a better way is by attaching a debugger, especially in the initial stages, where we do not want to find the UT framework.
| <ItemGroup> | ||
| <Compile Include="Contact.cshtml" /> | ||
| <!--OR--> | ||
| <Content Include="Contact.cshtml" /> |
There was a problem hiding this comment.
HEY HEY! This is an important bit.
Line 123: Uses Compile. What happens:
- The file is added to the compilation by MSBuild.
- Roslyn does not understand what is going on, so it just makes an AST where the root node is the entire file 🤣
- BECAUSE it is part of the compilation, we raise successfully issues.
Line 125: Uses Content. What happens:
- The file is added as an additional, content file.
- Roslyn does not have it in the compilation context.
- If we raise inside it, we get an AD0001.
...unfortunately, the default behavior for views is line 125.
I think this concludes the fact that we CANNOT propagate the issue by using .ReportIssue on the analyzer.
But we could write the issues on disk, probably.
There was a problem hiding this comment.
Update:
After looking more into this, Content is fine.
If we name the generated file cshtml.g.cs and the location mappings work, the cshtml file is considered an ExternalFile, and raising there is accepted, so we can use ReportIssue.
…leanup rules finder
…to find the analyzers
No description provided.