Code Analysis issues

[macote]

I want to let you know that had some issues with SEP and code analysis in our project. I committed some changes that might be helpful to some.

[m0sa]

Re generated code analysis, I think this is an issue with the StyleCop analyzer. Roslyn analyzers can be configured how to treat generated code with the GeneratedCodeAnalysisFlags via the DiagnosticStartAnalysisScope method. A more appropriate solution for this would be to set SEPrecompilerIncludeRazor to false, when DefineConstants contains CODE_ANALYSIS in your .csproj file, so the view code doesn't get generated in the first place, e.g.:

<PropertyGroup>
  <SEPrecompilerIncludeRazor Condition="$(DefineContants.Contains('CODE_ANALYSIS'))">false</SEPrecompilerIncludeRazor>
</PropertyGroup>

[m0sa]

Re the CODE_ANALYSIS check

The roslyn csc.exe compiler doesn't behave like that. In fact, there's not mention of CODE_ANALYSIS in the roslyn repo. According to my testing, it always runs all analyzers. The goal of this project is to be as similar as possible to the original, this check would be a breaking change in that regard. You can test your project against the original csc.exe's analyzer handling by running msbuild /p:SEPrecompilationSkip=true.

Again, the proper workaround for your specific need would be to clear the Analyzer items if CODE_ANALYSIS is not defined via .csproj / msbuild, no code changes needed, e.g.:

<ItemGroup>
  <Analyzer Condition="! $(DefineContants.Contains('CODE_ANALYSIS'))" Remove="@(Analyzer)" />
</ItemGroup>

[macote]

Thank you for the feedback. I'm learning a lot about all this. I agree that the csproj / msbuild approach is preferable. I tried what you suggested and I was able to make it work with the following analyzer condition:

<ItemGroup>
  ...
  <Analyzer Condition="'$(RunCodeAnalysis)' != 'true'" Remove="@(Analyzer)" />
</ItemGroup>

I also created a specific build configuration for razor analysis in which I set the SEPrecompilerIncludeRazor to true.

Now the only thing I have left in my modified version is the suppression of SA1633 for the generated 'CompiledFromDirectory'. Do you know if this can be included in the next SEP release?

[m0sa]

Looks like this could be solved by adding a [CompilerGeneratedAttribute] to the compiled views, and disabling it in FxCop. I need to check if that's what aspnet_compiler.exe does, in order to make it part of SE.Precompilation.

If you want to add that behavior yourself, you can just implement an ICompileModule and modify the SourceTrees generated from the .cshtml views before compilation.

[m0sa]

@macote any update on this?

[macote]

I've been very busy lately. I'll try to find some time by the end of the week to try your suggestion.

Thanks.

[macote]

I just completed some tests with the newest SEP code and it looks like I longer have to worry about that SA1633 warning I was getting since it is no longer reported in my analysis result. I tested individually a few commits and it seems that this change is the one that makes it go away. Should I be worried about anything else or you think this is expected?

[m0sa]

No, analyzers are more correct after the mentioned commit, since they're meant to analyze the source code, not transformed / generated code.

Maybe we should consider more granular control for compilation modules, add a BeforeAnalysis method, so a compilation module author could control whether a modification should be analyzed or not. But that's for the future...

On Thu, Jul 13, 2017, 21:16 Marc-André Côté notifications@github.com wrote:

I just completed some tests with the newest SEP code and it looks like I longer have to worry about that SA1633 warning I was getting since it is no longer reported in my analysis result. I tested individually a few commits and it seems that this change https://github.com/StackExchange/StackExchange.Precompilation/commit/565da3facf1cd7556c91487784f18ce1b75bcce5 is the one that makes it go away. Should I be worry about anything else or you think this is expected?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/StackExchange/StackExchange.Precompilation/issues/26#issuecomment-315175557, or mute the thread https://github.com/notifications/unsubscribe-auth/AAevlsNsa8BF8TNN9F57cuslUpa4741nks5sNm0VgaJpZM4NpiNf .

[macote]

All my analysis issues are resolved then!

Thanks again.

[tillig]

I think I'm hitting a similar thing where I have XML documentation enabled and all the Razor views are throwing CS1591 errors due to missing documentation. However, this is a C# compiler error rather than code analysis.

Is there a way to have a module change the csc command line arguments to remove the documentation path before compilation? I've tried modifying the BeforeCompileContext.Arguments.DocumentationPath value (through reflection, because it's not publicly settable) and that doesn't work. I haven't been able to figure out how to modify the inbound CSC arguments or I'd try capturing them at that time.

[m0sa]

One workaround would be to have the views in a separate dll.

Another one would be to inject #pragma warning disable 1591 into the source files generated from views.

On Sat, Jul 29, 2017, 00:58 Travis Illig notifications@github.com wrote:

I think I'm hitting a similar thing where I have XML documentation enabled and all the Razor views are throwing CS1591 errors due to missing documentation. However, this is a C# compiler error rather than code analysis.

Is there a way to have a module change the csc command line arguments to remove the documentation path before compilation? I've tried modifying the BeforeCompileContext.Arguments.DocumentationPath value (through reflection, because it's not publicly settable) and that doesn't work. I haven't been able to figure out how to modify the inbound CSC arguments or I'd try capturing them at that time.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/StackExchange/StackExchange.Precompilation/issues/26#issuecomment-318780563, or mute the thread https://github.com/notifications/unsubscribe-auth/AAevlqH0lzYyuC-Brscy6JOt40LHiKoNks5sSmeDgaJpZM4NpiNf .

[tillig]

I'll have to look into it. My Roslyn-fu is a bit rusty and the sample shows how to add new stuff to the system but not modify anything. Thanks for the tip!

[tillig]

For folks who may come looking, here's what I ended up with in my module.

public class PragmaDisableModule : ICompileModule
{
  public void AfterCompile(AfterCompileContext context)
  {
  }

  public void BeforeCompile(BeforeCompileContext context)
  {
    var originalTrees = context.Compilation.SyntaxTrees.ToArray();
    foreach (var originalTree in originalTrees.Where(s => s.FilePath.EndsWith(".cshtml", StringComparison.OrdinalIgnoreCase)))
    {
      foreach (var declaration in originalTree.GetRoot().DescendantNodes().OfType<NamespaceDeclarationSyntax>().ToArray())
      {
        var originalOptions = originalTree.Options;
        var pragma = CreatePragmaDisable("CS1591");
        var updatedTrivia = declaration.GetLeadingTrivia().Add(SyntaxFactory.Trivia(pragma)).Add(SyntaxFactory.ElasticCarriageReturnLineFeed);
        var newRoot = originalTree.GetRoot().ReplaceNode(declaration, declaration.WithLeadingTrivia(updatedTrivia));
        var newTree = originalTree.WithRootAndOptions(newRoot, originalOptions);
        context.Compilation = context.Compilation.ReplaceSyntaxTree(originalTree, newTree);
      }
    }
  }

  private static SeparatedSyntaxList<ExpressionSyntax> CreateCodeList(IEnumerable<string> toDisable)
  {
    var codes = new SeparatedSyntaxList<ExpressionSyntax>();
    foreach (var code in toDisable)
    {
      codes = codes.Add(SyntaxFactory.IdentifierName(code));
    }

    return codes;
  }

  private static PragmaWarningDirectiveTriviaSyntax CreatePragmaDisable(params string[] toDisable)
  {
    var codes = CreateCodeList(toDisable);
    var pragma = SyntaxFactory.PragmaWarningDirectiveTrivia(SyntaxFactory.Token(SyntaxKind.DisableKeyword), codes, true);
    return pragma;
  }
}

[m0sa]

Thanks for sharing!