Use Microsoft.CodeAnalysis.PublicApiAnalyzers

[dougbu]

• Add tooling to ensure no API changes are made in patch updates #4259 1/2
• followup 2/3 for 5266918
• includes baselines for 16 MVC projects
  • will automated further additions in another PR
• suppress warnings that may cause back-compat problems if fixed

nit: sort @(LatestPackageReference) a bit better

[dougbu]

@ajaybhargavb @JamesNK I realize this and src/Mvc/Mvc.Core/src/Routing/PageLinkGeneratorExtensions.cs aren't particularly new. But, can this problem be addressed and not suppressed without breaking back-compat❔ If not, do you suggest we take the hit now❔

[JamesNK]

The only way to resolve the warning is changing method signatures which is a breaking change.

I don't know of any issues caused by the overloads. Suppressing is fine.

@javiercn

[sharwell]

I would recommend using attribute-style suppressions for methods, with the Justification set to "Required to maintain compatibility".

[dougbu]

That seems to work but VS tends to crash or ignore me when using attribute suppressions for this issue. I eventually got it to fix one instance and copied my brand new attribute everywhere else. Something may be busted in this area.

[JunTaoLuo]

LGTM

[Pilchie]

I suggest that once we fork for RC1, we move all of those items to the .Shipped files so that we can track any changes that happen.

[dougbu]

I suggest that once we fork for RC1, we move all of those items to the .Shipped files so that we can track any changes that happen.

Agreed. Should I disable any other warnings from https://github.com/dotnet/roslyn-analyzers/blob/master/src/PublicApiAnalyzers/Microsoft.CodeAnalysis.PublicApiAnalyzers.md in the meantime❔

In my next PR, I need to

1. add a small document covering planned use of these files and link to the documentation of the analyzer package
   • I'll include a note about RC1 plans here
2. add lots o' PublicApi.*.txt files
3. automate this infrastructure more e.g. provide a way to automatically add missing PublicApi.*.txt files and update them (if possible) from the command line

[Pilchie]

I don't think we want to disable any of the other warnings.

[dougbu]

Issue about the second item on my list is #24347. Please help if you have time.

[dougbu]

A new version of Microsoft.CodeAnalysis.PublicApiAnalyzers containing @mavasani's dotnet/roslyn-analyzers#3916 improvement will address the last item on my list. We'll pick that up when it's ready.

Note: The new package will make missing PublicApi.*.txt files fail our builds because we treat warnings as errors. Anything left in #24347 will need to be done in one fell swoop unless we make use of the package conditional on the files existing.

Good news is we can probably delete

aspnetcore/eng/targets/CSharp.Common.targets

Lines 23 to 30 in 0437117

	<AdditionalFiles Include="PublicAPI.Shipped.txt"
	Condition=" Exists('$(MSBuildProjectDirectory)\PublicAPI.Shipped.txt') " />
	<AdditionalFiles Include="PublicAPI.Unshipped.txt"
	Condition=" Exists('$(MSBuildProjectDirectory)\PublicAPI.Unshipped.txt') " />
	<AdditionalFiles Include="PublicAPI.Shipped.txt"
	Condition=" Exists('$(MSBuildProjectDirectory)\$(_TFMDirectory)\PublicAPI.Shipped.txt') " />
	<AdditionalFiles Include="PublicAPI.Unshipped.txt"
	Condition=" Exists('$(MSBuildProjectDirectory)\$(_TFMDirectory)\PublicAPI.Unshipped.txt') " />

when using the new package.

[dougbu]

Hmm, probably need to keep the $(_TFMDirectory) items for projects with slightly different public APIs per TFM.

[mavasani]

Hmm, probably need to keep the $(_TFMDirectory) items for projects with slightly different public APIs per TFM.

Yes, that sounds correct.