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

Roslynator seems to run on code generated by source generator and does not read editorconfig settings while doing so #876

Closed
DavidZidar opened this issue Jan 22, 2022 · 5 comments

Comments

@DavidZidar
Copy link

DavidZidar commented Jan 22, 2022

I have a project that references another project that has source generators, this seems to cause a bug in Roslynator v4.0.0 that makes it print incorrect warnings about missing configuration options even though they are present in the editorconfig file.

I have created a sample project that reproduces the issue:
https://github.com/DavidZidar/roslynator-analyzer-bug

Example log output:

> dotnet build

Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  TestAnalyzers -> C:\code\test\roslynator-bug\src\TestAnalyzers\bin\Debug\net6.0\TestAnalyzers.dll
CSC : warning ROS0003: Analyzer RCS1018 requires config option to be specified: roslynator_accessibility_modifiers [C:\code\test\roslynator-bug\src\TestProject\TestProject.csproj]
CSC : warning ROS0003: Analyzer RCS1096 requires config option to be specified: roslynator_enum_has_flag_style [C:\code\test\roslynator-bug\src\TestProject\TestProject.csproj]
  TestProject -> C:\code\test\roslynator-bug\src\TestProject\bin\Debug\net6.0\TestProject.dll

Build succeeded.

CSC : warning ROS0003: Analyzer RCS1018 requires config option to be specified: roslynator_accessibility_modifiers [C:\code\test\roslynator-bug\src\TestProject\TestProject.csproj]
CSC : warning ROS0003: Analyzer RCS1096 requires config option to be specified: roslynator_enum_has_flag_style [C:\code\test\roslynator-bug\src\TestProject\TestProject.csproj]
    2 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.91

Product and Version Used: Roslynator.Analyzers v4.0.0

Steps to Reproduce:

  1. Configure required Roslynator settings in editorconfig-file.
  2. Reference a project containing source generators that are generating any code.
  3. Build.

Actual Behavior:
Incorrect warnings are printed.

CSC : warning ROS0003: Analyzer RCS1018 requires config option to be specified: roslynator_accessibility_modifiers [C:\code\test\roslynator-bug\src\TestProject\TestProject.csproj]

Expected Behavior:
No warnings should be printed because the configuration options are in fact present.

@josefpihrt
Copy link
Collaborator

Hi,

thanks for the report and code sample.

It seems that code analysis is executed on files generated with source generator. This behavior is not specific to Roslynator analyzers. If you use this code:

class C
{
}

in your sample project you will get following error:

warning CA1812: C is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it static (Shared in Visual Basic).

The other thing I discovered is that the file path of the generated file is relative which make is incompatible with editorconfig (which requires absolute path to determine proper configuration)

This can be fixed by verifying if the source file path is relative or absolute and skip these files.

Current solution to this issue is to mark generated source as generated. Either set hintName to TestSource.Generated or similar or add auto-generated marker at the beginning of the source:

// <auto-generated>

Then the generated source will be excluded from code analysis.

Another option is to disable ROS0003.

I would be interested if Roslyn team intended to execute code analysis on source generator's files and if there is some way how to configure this behavior.

@DavidZidar
Copy link
Author

DavidZidar commented Jan 22, 2022

Oh, interesting, thank you that information. I didn't know that .Generated in the file name or that comment did anything special. That takes care of source code I generate myself, but unfortunately the source generators in Microsoft.Toolkit.Mvvm which I also use doesn't do either of those things so I'm still getting the warning even if I fix my own code. I can certainly report an issue to that team and hope that they'll fix this on their end.

Regarding the relative path, would it be possible to resolve the path relative to the project and check if the file exists on disk? I'm thinking maybe it wouldn't make sense to look for an editorconfig file for a virtual file.

Whether the Roslyn team intended to execute code analysis on source generator's files or not I can only guess, but I can imagine it being useful occasionally, like checking for nullability issues and such rather than stylistic issues.

Edit: I renamed my generated files with .Generated and added the // <auto-generated> comment for good measure and added <NoWarn>ROS0003</NoWarn> to get rid of the warnings from Microsoft.Toolkit.Mvvm. With those changes I'm no longer blocked.

@PhilPJL
Copy link

PhilPJL commented Jan 23, 2022

Hmmm, how do I disable ROS0003?

This doesn't work
image

image

@josefpihrt
Copy link
Collaborator

Following entry

dotnet_diagnostic.ROS0003.severity = none

will disable it but there are some false positives. Until it is resolved my recommendation is to suppress it:

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Roslynator", "ROS0003")]

I will probably disable by default analyzers that require option to be set. Which means that by default you should not see any warnings.

Another option is to disable ROS0003 by default.

@PhilPJL
Copy link

PhilPJL commented Jan 24, 2022

ok thanks, that's sorted it

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

No branches or pull requests

3 participants