Add authoring analyzers for [ApiContract] and [ContractVersion]#2407
Add authoring analyzers for [ApiContract] and [ContractVersion]#2407Sergio0694 wants to merge 17 commits intostaging/3.0from
Conversation
…m cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ractVersion]' usage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>
Replace AttributeTargets.All with an explicit list of allowed targets and reformat AttributeUsage for clarity. Swap the Type/string constructor signatures and add XML <remarks> to each constructor to clarify when each overload applies (API-contract vs non-API-contract types). Note: the constructor signature order changed, which can be a breaking change for callers and may require code updates.
Iterate and validate each [ContractVersion] attribute instance and report diagnostics per-attribute. Simplifies and corrects the condition for version-only constructors by combining checks (isVersionOnlyConstructor && !isApiContractType) so the diagnostic is raised for each attribute applied to non-API contract types. Also updates minor comment wording.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…'ContractVersionAttribute' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…actVersionAttribute' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nalyzers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactor PublicTypeRequiresContractVersionAnalyzer to combine the cast and accessibility/containing-type checks into a single pattern-matching expression on context.Symbol. Removes a separate INamedTypeSymbol assignment and simplifies the early-return check without changing behavior.
| return; | ||
| } | ||
|
|
||
| context.ReportDiagnostic(Diagnostic.Create( |
There was a problem hiding this comment.
They might be doing regular versioning rather than contract versioning which we should take into account.
There was a problem hiding this comment.
and I believe we default version attribute if not specified.
There was a problem hiding this comment.
Yeah this is why this diagnostic is just info, not a warning. Is that fine or should we still change it?
There was a problem hiding this comment.
Given regular versioning and contract versioning have different audiences, I think we basically have a property that someone can set to true if they want to use contract versioning and if that is set, all the diagnostics for it are enabled and enforced. If it isn't set and it will default to false, then regular versioning is done and the diagnostics are not enforced. Thoughts?
There was a problem hiding this comment.
Mh having an additional MSBuild property feels a bit overkill perhaps. What about:
- Having an analyzer (info level) that fires if a public type has neither a version nor a contract version
- I know we add a version by default, this is why the level would just be info, not a warning
- Having an analyzer that warns if there's a public type without a contract version, only if there's at least another public type with a contract version. As in: if you do use contract versioning, you should be consistent and not mix.
- All other analyzers are always on, since they're about general correctness things.
This way there's also no property to worry about, things just adapt automatically to your code.
Thoughts?
There was a problem hiding this comment.
That could work as long as the diagnostic message for the first one is clear enough to be like you are not using regular or contract versioning, so we will default version to 1 unless you manually specify or so.
Summary
Adds a set of new Roslyn diagnostic analyzers in the
WinRT.SourceGenerator2project that validate Windows Runtime component authoring usage of[ApiContract]and[ContractVersion]. Also introduces small reusable extension helpers and refines the public surface ofContractVersionAttribute.All new analyzers are scoped to component-authoring scenarios (
CsWinRTComponent = true) and are skipped otherwise.Motivation
Authoring Windows Runtime components in C# has a number of WinMD-level constraints that are easy to get wrong silently — invalid
[ApiContract]enum cases, mismatched[ContractVersion]constructor usage, missing contract version metadata, etc. These analyzers surface those issues at edit/build time with actionable diagnostics, rather than producing a malformed.winmdthat fails (or worse, succeeds with wrong metadata) downstream.Changes
New diagnostics (
CSWINRT2010–CSWINRT2015)CSWINRT2010—ValidApiContractEnumTypeAnalyzer(Warning): warns when an[ApiContract]enum defines enum cases. API contract types are represented as empty struct types in WinRT and any enum cases are ignored when generating the.winmd.CSWINRT2011/CSWINRT2012/CSWINRT2013—ValidContractVersionAttributeAnalyzer(Warning): validates[ContractVersion]applications:2011: the(uint)and(string, uint)constructors must target an API contract type.2012: the(Type, uint)constructor must not target an API contract type.2013: theTypeargument of the(Type, uint)constructor must itself be a valid API contract type.CSWINRT2014—ApiContractTypeRequiresContractVersionAnalyzer(Warning): every[ApiContract]enum must declare its contract version using a version-only constructor of[ContractVersion].CSWINRT2015—PublicTypeRequiresContractVersionAnalyzer(Info): public top-level types in a component should declare their associated API contract via[ContractVersion(typeof(SomeContract), version)]. Skips API contract enums (covered byCSWINRT2014) and types that already have any[ContractVersion]applied (validity is reported byCSWINRT2011–CSWINRT2013).Files
src/Authoring/WinRT.SourceGenerator2/Diagnostics/Analyzers/: new analyzersValidApiContractEnumTypeAnalyzer,ValidContractVersionAttributeAnalyzer,ApiContractTypeRequiresContractVersionAnalyzer,PublicTypeRequiresContractVersionAnalyzer.src/Authoring/WinRT.SourceGenerator2/Diagnostics/DiagnosticDescriptors.cs: descriptors forCSWINRT2010–CSWINRT2015.src/Authoring/WinRT.SourceGenerator2/AnalyzerReleases.Shipped.md: release tracking entries for the new diagnostics.src/Authoring/WinRT.SourceGenerator2/Extensions/AttributeDataExtensions.cs: newAttributeData.GetLocation(...)andAttributeData.GetArgumentLocation(int, ...)extensions, factored out of the analyzer code.src/Authoring/WinRT.SourceGenerator2/Extensions/ISymbolExtensions.cs: newISymbol.GetAttributes(INamedTypeSymbol)overload that yields only attributes of the specified type, eliminating the manualforeach+SymbolEqualityComparerfilter pattern at call sites.src/WinRT.Runtime2/Windows.Foundation.Metadata/ContractVersionAttribute.cs: small API/doc tweaks aligned with the new analyzer expectations.src/Tests/SourceGenerator2Test/:Test_ValidApiContractEnumTypeAnalyzer.cs,Test_ValidContractVersionAttributeAnalyzer.cs,Test_ApiContractTypeRequiresContractVersionAnalyzer.cs,Test_PublicTypeRequiresContractVersionAnalyzer.cs.Helpers/CSharpAnalyzerTest{TAnalyzer}.cs: extendedVerifyAnalyzerAsyncwith anisCsWinRTComponentparameter that injects a.globalconfigsettingbuild_property.CsWinRTComponent = trueso the gated analyzers actually run under test.