Home: Track public API changes better

[zivkan]

The Roslyn team have a public API analyzer. See: https://github.com/dotnet/roslyn-analyzers/blob/master/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md

For NuGet projects that are packed, we should track public API changes, to minimise breaking existing APIs, and be more intentional about adding new APIs.

In short, the way it works is the analyzer expects two files. PublicAPI.Shipped.txt and PublicAPI.Unshipped.txt. These files list all the public APIs in a project, and when the analyzer runs, it generates its own list of all public APIs in the project and compares to the union of all the APIs listed in the two text files. If there are any differences (public API in code not listed in files, or API listed in file not found in the code), it generates a warning, which we elevate to an error.

The analyzer does not support having APIs listed in the file that only appear in some TargetFrameworks and not others, so instead a seperate set of files are used for each TFM. This means the files are mostly duplicates of each other, with only small differences where the public API surface is different. It also means when there are changes to the public API surface, for example a new API that is added to all project TFMs, multiple PublicApi.*.txt files needs to be modified.

How we use the Shipped verses Unshipped files is to our discression. The analyzer has a fix which will add new APIs to the Unshipped file. It appears the intention is to manually move the contents of the file to the Shipped file when the assembly ships, so it's easier to track new APIs that have been added, but not yet shipped, in case the API should be changed before customers start using it.

Therefore, I propose that we use this feature with the following workflow:

• All projects that set PackProject to true use the analyzer
  
  
  
  • NuGet.CommandLine will be excempt, as one of its distribution methods is as a nupkg, but it is not intended to be a library.
  
  
  • Currently, some of our test utility projects are packed as nupkgs. I don't know the reason, but I do not believe we should support backwards compatibility for these projects
  
  
• When a new API is added, use the code fix to add the new API to the appropriate PublicAPI.Unshipped.txt file(s).
• When doing a code revew, a very strong justification is needed to remove any public APIs that need to be removed from PublicAPI.Shipped.txt.
• When doing a code review, when a PublicAPI.Unshipped.txt file is in the diff, question why the public API is needed.
• When NuGet's packages are released with release branding (not previews), part of the publishing steps that HotSeat need to do is to merge all contents of PublicAPI.Unshipped.txt files with their PublicAPI.Shipped.txt counterparts, to signal that all new APIs (since the previous GA release) have now been shipped and there is a higher bar to change them now.

[ViktorHofer]

@ericstj maybe also interesting for dotnet/runtime if we decide to collapse ref and src projects together.

[nkolev92]

I'm all for this change.

Think the process needs documented in the PR + client engineering issues repo.

One question.

How do we enforce:

• When a new API is added, use the code fix to add the new API to the appropriate PublicAPI.Unshipped.txt file(s).

Honor system or?

[zivkan]

If you add a public API, but don't modify either PublicAPI.Shipped.txt or PublicAPI.Unshipped.txt, then the analyzer will cause the build to fail.

Making sure that the Unshipped file is modified, rather than modifying the Shipped file, is up to the honor system and code reviewers. If this turns out to be a problem, we can investigate something, but for now I trust the team :)

[ericstj]

@ericstj maybe also interesting for dotnet/runtime if we decide to collapse ref and src projects together.

IMHO C# syntax is more readable and expressive than API txt files. I wonder if this analyzer could be made to take C# files as it's input.

[ericstj]

Opened https://github.com/dotnet/roslyn/issues/46257 with that suggestion.