PublicAPI Analyzer treats `abstract class` implicit constructors as public

[333fred]

https://github.com/dotnet/roslyn/pull/51330/files#diff-9a6b818860122e38724a80cb5258e64ed6a82c7e1c81887a3440ab19b12c9758R25-R29

This change should have been flagged by the public API analyzer, as abstract class implicit constructors are protected, and this commit changed the accessibility of the constructor to public. However, the analyzer treats these constructors as public apis, not protected: https://github.com/sharwell/roslyn/blob/2aa1c3fa73c343f0f6b0444a8d46850fffbb2b98/src/Workspaces/Core/Portable/PublicAPI.Shipped.txt#L884.

[Youssef1313]

@333fred Just to make sure I understand the issue, for a class like: abstract class C { }, you want the public api files to explicitly containg the "protected" keyword? e.g: "protected C.C() -> void"?

If you don't want the entry completely, I don't think this would be correct behavior. protected members are still accessible through the base class.

[333fred]

@Youssef1313 the main problem is that the public API changed in the commit I linked, but the public API analyzer did not consider it to be changed. Something needs to be tracking this type of change.

[Youssef1313]

@333fred I can't think of a way this would affect API consumers. Since the class is abstract, it doesn't matter whether the ctor is public or protected.

It should be fairly easy to update the analyzer to track that. But upgrading will require a ton of updates in PublicAPI.*.txt files. So does it worth doing that? I think an analyzer for any public constructor in abstract class would do a better job here. (it exists: CA1012 - I'll enable it for roslyn repo)

[Youssef1313]

Actually, I think the real problem here will be any protected API except abstract class ctor (but that would be nice to handle - it actually won't require any extra cost).

For example:

public class C
{
    protected void M() { } // this will be included in PublicAPI.Shipped.txt. Changing it to public (or from public to protected) won't require any change in PublicAPI.*.txt.
}

I haven't confirmed that though. Confirmed 😕