dotnet / roslyn-analyzers

MIT License
1.55k stars 460 forks source link

Missing or invalid release header #4484

Closed dominikjeske closed 3 years ago

dominikjeske commented 3 years ago

I have created my source generator with diagnostic info and some analyzer messages. Acording to instruction https://github.com/dotnet/roslyn-analyzers/blob/master/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md I created this file:

https://github.com/dominikjeske/HomeCenter/blob/MotionService/HomeCenter.SourceGenerators/AnalyzerReleases.Unshipped.md

but unfortunatelly I have an warning message:

Warning RS2007 Analyzer release file 'AnalyzerReleases.Unshipped.md' has a missing or invalid release header '## Release 1.0'.

Youssef1313 commented 3 years ago

@mavasani Should this be moved to roslyn-analyzers repo?

Youssef1313 commented 3 years ago

@dominikjeske Can you check whether the warning is fixed with the PR https://github.com/dominikjeske/HomeCenter/pull/1?

mavasani commented 3 years ago

@dominikjeske Releases are only allowed in shipped file for already shipped releases. Unshipped file only tracks analyzers in upcoming release, so cannot have a release header.

Youssef1313 commented 3 years ago

@mavasani While I missed this, I think there is still a problem with the BOM character:

image

https://github.com/dotnet/roslyn-analyzers/blob/6dea9b867e32607bc02f958efe46d4b2d1220e45/src/Microsoft.CodeAnalysis.Analyzers/Core/MetaAnalyzers/ReleaseTrackingHelper.cs#L52

If https://github.com/dotnet/roslyn-analyzers/pull/4485 build passed, then the above guess isn't correct. (Edit: Build passed). So there is no problem with bom character in any case.

dominikjeske commented 3 years ago

@Youssef1313 Thanks for help but after merge and rebuild I still have this message on VS 16.8.2

@mavasani I don't need release but when I ommit it I have other warning "Warning RS2007 Analyzer release file 'AnalyzerReleases.Unshipped.md' has a missing or invalid release header 'Rule ID | Category | Severity | Notes'. ".

I just want to get rid of warnings - without those files I have warning that I don't have those files and when I create them from template I have original error. For me checking for this files is fragile and rules are not clear on how to format those files.

mavasani commented 3 years ago

@dominikjeske You can please try with the following unshipped releases file?

### New Rules
Rule ID | Category | Severity | Notes
--------|----------|----------|--------------------
SG0001  |  SourceGenerators  |   Error  | Error in source generator with log
SG0002  |  SourceGenerators  |   Error  | Error in source generator
SG0003  |  SourceGenerators  |   Info   | Source code generated
mavasani commented 3 years ago

Also, this file should be automatically generated via the code fix. You shouldn't be manually changing the formatting of this file.

dominikjeske commented 3 years ago

The file is created by codefix but file is empty so there is no clear rules how to do it right - it is only text file with no editor support. When changing to your proposal I have those errors in each DiagnosticDescriptor (and there is no previous version)

Warning RS2001 Rule 'SG0001' has a changed 'Category' or 'Severity' from the last release. Either revert the update(s) in source or add a new up-to-date entry to unshipped release file.

mavasani commented 3 years ago

@dominikjeske The code fixes are actually offered in source files at the location you new up DiagnosticDescriptor. We have been using the release tracking analyzer in this repo for many months and haven't seen a need to make any manual edits to it.

dominikjeske commented 3 years ago

I’m not sure we are on same page so I will describe whole developer experience – for me it is far from ideal.

  1. I’m creating new DiagnosticDescriptor

private readonly DiagnosticDescriptor _errorRuleWithLog = new DiagnosticDescriptor("SG0001", "SG0001: Error in source generator", "Error in source generator<{0}>: '{1}'. Log file details: '{2}'", "SourceGenerator", DiagnosticSeverity.Error, isEnabledByDefault: true);

and have first warning

Warning RS2008 Enable analyzer release tracking for the analyzer project containing rule 'SG0001'

  1. I can fix this with fixer but fix is creating two empty files AnalyzerReleases.Unshipped.md and AnalyzerReleases.Shipped.md. The best part is that this is not enouuh and same error still exists. By googling I found that I have to add this section in csproj… only to see next warning
  <ItemGroup >
    <AdditionalFiles Include="AnalyzerReleases.Shipped.md" />
  </ItemGroup>
  1. Now I see this error:

Warning RS2000 Rule 'SG0001' is not part of any analyzer release.

  1. So I tried to fill those file like in ReleaseTrackingAnalyzers.Help and that gives me original warning

Warning RS2007 Analyzer release file 'AnalyzerReleases.Unshipped.md' has a missing or invalid release header '## Release 1.0'.

Now I’m don’t know how this file should be formed to give me no errors.

  1. Additional thing is that sometimes warnings are not changing even after clean and rebuild. I have to restart VS to be sure.
mavasani commented 3 years ago

I can fix this with fixer but fix is creating two empty files AnalyzerReleases.Unshipped.md and AnalyzerReleases.Shipped.md

This is by design. These files should automatically be added as additional files, so step 2 should not be necessary.

So I tried to fill those file like in ReleaseTrackingAnalyzers.Help and that gives me original warning

You don't need to do this manually. RS2000 has a code fix which should automatically add the required entry. Feel free to hand edit the Notes column as appropriate. Please do not make any other modifications by hand.

dominikjeske commented 3 years ago

@mavasani Thanks for assist - You partially have right :) I tried couple times but additional files are not created. Also sometimes analyzers are not working instatly so sometimes clean/rebuild will work but best option is to restart VS. After replay the process, add additional files manually I could use RS2000. Those things are simple when you know how they should work.