Fix nullable annotations of SymbolDisplay.FormatPrimitive
#79869
Conversation
jjonescz
added
the
breaking-change
dotnet-policy-service
bot
added
Needs API Review
| static Microsoft.CodeAnalysis.CSharp.LanguageVersionFacts.TryParse(string? version, out Microsoft.CodeAnalysis.CSharp.LanguageVersion result) -> bool | ||
| static Microsoft.CodeAnalysis.CSharp.SymbolDisplay.FormatLiteral(char c, bool quote) -> string! | ||
| static Microsoft.CodeAnalysis.CSharp.SymbolDisplay.FormatLiteral(string! value, bool quote) -> string! | ||
| static Microsoft.CodeAnalysis.CSharp.SymbolDisplay.FormatPrimitive(object! obj, bool quoteStrings, bool useHexadecimalNumbers) -> string! |
There was a problem hiding this comment.
Is this what the fixer suggested? I thought that when shipped APIs change, we use some kind of REMOVED annotation or similar in the unshipped file. For example, if someone ran the tool to "mark APIs as shipped", we wouldn't want them to get the impression that FormatPrimitive was only just added in this release.
It's possible I am remembering things wrong and this is not a real problem, though.
There was a problem hiding this comment.
No, the fixer in VS didn't suggest anything (I'm not sure it can since this is a non-C# file), so I removed it manually. But then the analyzer didn't complain so I thought it's correct 🤷 But you're right, I also remember seeing some **REMOVED** prefix; will investigate more.
There was a problem hiding this comment.
The analyzer diagnostics which complain about the PublicAPI files, not matching the API surface in source code, are supposed to have fixers that update the PublicAPI files. However due to them being plain text and not C# getting the fixers to actually run on them I think has broken in the editor at various points.
There was a problem hiding this comment.
I haven't found any fixer for "removed APIs" - https://github.com/dotnet/roslyn/tree/main/src/RoslynAnalyzers/PublicApiAnalyzers/Core/CodeFixes
But I figured out how to mark as *REMOVED* manually.
| class C : I; | ||
| ``` | ||
|
|
||
| ## `SymbolDisplay.FormatPrimitive` API is now properly nullable annotated |
There was a problem hiding this comment.
This document is generally used for changes in the compiler's actual output, not API changes. In general, we do not consider nullability fixes to be "breaking", I'm not sure this actually needs this level of documentation.
There was a problem hiding this comment.
Hm, should I move this to https://github.com/dotnet/roslyn/blob/main/docs/Breaking%20API%20Changes.md instead?
There was a problem hiding this comment.
we do not consider nullability fixes to be "breaking"
I guess this means the answer is no. And I can remove the "breaking change" label as well. Thanks.
jjonescz
removed
breaking-change
3 participants
SymbolDisplay.FormatPrimitivereturnsnullfor unsupported types and its documentation always indicated that.However, the return type was incorrectly marked as non-nullable and this PR fixes that.