You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add XML documentation validation to OneBranch builds and fix docs issues - #4730
This only recognizes the [] array suffix. Multidimensional array forms such as [,] or the documentation-ID form [0:,0:] also represent constructed types but do not match, so an invalid T: array cref passes. Match any bracketed array suffix here.
Adds a preflight for the XML documentation cross-references that feed
dotnet/sqlclient-api-docs, so malformed documentation IDs fail our own build
rather than surfacing as xref-not-found warnings on an API Docs pull request.
This commit exposes the problems; it does not fix them. The findings it
reports are left in place so they can be reviewed and fixed separately.
Validation
----------
eng/pipelines/onebranch/scripts/validate-xml-docs.ps1 reports, as errors:
documentation-ID syntax defects (C# aliases in signatures, empty parentheses
on parameterless members, embedded whitespace, array T: UIDs), unknown
namespace roots, cross-references the compiler could not bind, malformed
files, stale allowlist entries, documentation that should exist but does not,
and lib/ versus ref/ documentation trimming defects in a package. Resolution
findings that a target-framework-conditional or cross-assembly member can
legitimately trigger are warnings.
No network access is required: these defects are decidable from
documentation-ID syntax and the local member index, so the published Learn
xref map is not needed. It stays available via -ExternalXrefMapPath.
It runs at three points: snippet sources before the build, generated
documentation after it, and the assembled packages during package validation.
Only the last shows what a consumer actually receives.
Expectations come from the project
----------------------------------
GenerateDocumentationFile in the csproj is the single declaration, so nothing
is restated in the pipeline and the two cannot drift. A project that generates
documentation must produce it; one that does not is reported as information
naming the reason, and documentation appearing there is reported as a warning.
Snippet validation follows the same principle: only the snippets a project's
sources reference are validated. Project files are parsed as XML rather than
searched as text, because a comment naming the property would otherwise read
as setting it.
Gating
------
One failOnValidationError parameter governs the localization, XML
documentation, and package validation steps. When false, findings are reported
as warnings and the build continues; malformed inputs and a validator that
fails to run still fail the step. The official pipeline sets it true and the
non-official pipeline exposes it at queue time, with no intermediate template
declaring a default. Steps reporting findings mark themselves
SucceededWithIssues, because task.logissue alone leaves the task result
untouched and a step carrying warnings would otherwise render as a clean
success.
breakOnSdlError is renamed failOnSdlError to match.
Microsoft.SqlServer.Server
--------------------------
Enables documentation generation. Its public types already carry complete
documentation comments, producing 63 documented members with no warnings, but
they were compiled away so consumers got no IntelliSense. Set unconditionally:
net46 is type-forwards only and gets an essentially empty file, but a
TargetFramework condition would leave the project without a single
unambiguous answer about whether it produces documentation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Companion to the commit that added the validation. That one deliberately left
its findings in place; this one fixes them.
Documentation IDs
-----------------
Corrects every cross-reference the snippet and generated-documentation gates
reported: array T: UIDs that name no documentation page, a parameterless
method written with empty parentheses, C# aliases where a documentation ID
must name CLR types, whitespace inside a documentation ID, and 29
cross-references carrying the wrong kind prefix.
Packaging
---------
lib/<tfm> is the source the API docs pipeline consumes and must carry the full
text; ref/<tfm> is trimmed because remarks and examples render badly in Visual
Studio tooltips. lib/net8.0 and lib/net9.0 had been taking the trimmed
reference copy since 07a9280, so the published documentation lost every
remark and example for those frameworks. Both now take the implementation
artifact.
netstandard2.0 has no implementation build to take full documentation from, so
the reference project now keeps an untrimmed copy of its documentation beside
the trimmed one, and lib/netstandard2.0 uses that. Every lib target therefore
carries the full text without needing an exception.
Also records in the nuspec why net462 packs the real implementation into both
lib/ and runtimes/, which is otherwise easy to mistake for an accident.
Build correctness
-----------------
TrimDocsForIntelliSense ran after the output copy, so a first build published
untrimmed reference documentation and only a second build published the
trimmed file. It now runs between CoreCompile and CopyFilesToOutputDirectory,
so one build is enough.
Documentation comments take their text from doc/snippets through <include>,
which the compiler reads but MSBuild did not treat as an input. Editing a
snippet left the generated documentation stale until the project was rebuilt
from scratch, which made the validation above appear to ignore a fix. The
snippets are now declared as compile inputs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Detect bare inline xrefs to parameterized methods before Open Publishing reports them, and correct the affected SqlBulkCopy documentation links.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The parameter list was read from the first '(' to the last ')', so any stray
delimiter between them was swallowed into the argument text instead of being
reported. M:System.String.IndexOf(System.Char)) yielded the argument
'System.Char)', which names no type, and the cref reached Open Publishing as an
xref-not-found. The nested and repeated forms passed for the same reason.
Parentheses are now scanned alongside the braces: a documentation ID carries one
matched pair that never nests, so a closer that opens nothing, an unterminated
list, nesting and a second list are each reported. The scan leaves exactly one
pair, which is what made the old negative-length guard on the argument
arithmetic unreachable, so that guard folds into it.
Addresses review feedback on #4730. Verified against doc/snippets: the same 15
findings over 4687 crefs before and after, so nothing new is flagged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@priyankatiwari08 — 2026-09-25 — three notes, each also raised as an inline thread and answered there. Reposting because my previous summary got two of the outcomes wrong: it said both fixes were on the 7.1 cherry-pick branch (#4752) and "still need porting to main". They were already on this branch when it was written — f0ece3f landed two minutes earlier.
Note
Corrected outcome
.OUTPUTS omits documentation-not-expected
Already on main — validate-xml-docs.ps1:203, from f0ece3f. No porting needed
Package-root prefix match has no separator guard
Already on main — both sites, from f0ece3f, with the Contoso.Widgetref regression test
Microsoft.SqlServer.Server.csproj release note
Still open, deliberately — see below
On the release note: main is in the same position release/7.1 is. SqlServerNextVersion is the unreleased 1.1.0-preview1 and release-notes/MSqlServerServer/ has only a 1.0 folder. Release-notes folders here list shipped releases, so there is no in-progress file to write into, and creating a 1.1 folder now would record a release that has not been cut. The note belongs in the 1.1.0 release notes when that release is prepared; the packaging change itself is real and unchanged.
This guidance contradicts the new netstandard2.0 mapping in Microsoft.Data.SqlClient.nuspec, which intentionally sources lib/netstandard2.0 from the reference project's untrimmed output because no implementation build exists. Document that exception here; otherwise a future maintainer following this rule could undo the packaging fix.
Copilot - Your most recent "Previously missed" will not be addressed. We are planning to remove reference and not-supported assemblies in the near future.
Addresses Copilot review feedback on PR #4730.
Test-Cref read the argument text from any prefix, so a well-formed
parameter list on a namespace, type, field or event parsed cleanly and
produced no finding at all. T:System.String(System.Int32) passed the
gate and would reach Open Publishing as an unresolved xref.
Only a member that can be overloaded carries a parameter list: a method
(M:) and an indexed property (P:). Reject one on any other prefix,
before the arguments are read.
The message offers both corrections rather than one, because the cref
alone cannot say which was meant. The single occurrence this rule found
in doc/snippets wrote E: on DbDataAdapter.Update, which is a method, so
suggesting only the prefix-preserving form would have pointed at a cref
that still resolves to nothing. That snippet is corrected here.
Pester: 144 passed, 0 failed. Re-running the validator over doc/snippets
reports 0 invalid-docid findings, down from 1.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Split-DocIdArguments can return empty entries, but this loop accepts them. Consequently malformed IDs such as M:System.String.IndexOf(,System.Char) or a trailing-comma form pass source validation with no finding. Reject empty arguments and add cases for leading, repeated, and trailing separators.
Validate square-bracket balance in source cref signatures
The grammar check never validates square-bracket balance. A source cref such as M:System.String.IndexOf(System.Char[) currently exits successfully with zero findings, so this gate still allows a malformed array signature to reach Open Publishing. Validate bracket pairing before processing the signature and add an unmatched-bracket regression case.
Addresses two Copilot review-body findings on PR #4730.
Both defects survived because nothing downstream objected to them.
Split-DocIdArguments returns an empty string for a leading, repeated or
trailing comma, and Get-DocIdCoreTypeName reduces that to an empty name
which matches no C# alias, so M:System.String.IndexOf(,System.Char) drew
no finding at all. Reject an empty argument where the signature is split.
The array brackets were never paired. The splitter tracks their depth
only to place commas, and Get-DocIdCoreTypeName trims them whether or
not they matched, so M:System.String.IndexOf(System.Char[) reduced to an
ordinary type name and passed. Pair them alongside the brace check,
which is the rule it mirrors.
Both forms named no overload and would have reached Open Publishing as
unresolved xrefs.
Pester: 156 passed, 0 failed, including the multidimensional controls
[], [,] and [0:,0:], whose commas sit inside the brackets rather than
separating parameters. doc/snippets still reports 0 invalid-docid.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot review body 2026-10-01 - "Previously missed" (2)
Both reproduced, both valid, both fixed in 1781a6b. Noting for the record that the review header read Findings: None while the collapsed section carried these two.
Reject empty arguments in source cref signatures - validate-xml-docs.ps1:609. Correct. Split-DocIdArguments returns an empty string for a leading, repeated or trailing comma, and Get-DocIdCoreTypeName reduces that to an empty name which matches no C# alias, so nothing downstream objected:
NO FINDING : M:System.String.IndexOf(,System.Char)
NO FINDING : M:System.String.IndexOf(System.Char,)
NO FINDING : M:System.String.IndexOf(System.Char,,System.Int32)
Empty arguments are now rejected where the signature is split, with the leading, repeated and trailing cases you asked for. A whitespace-only argument gets its own test: the whitespace rule reports first and then continues against the stripped form, so that cref draws both findings rather than being excused by the first.
Validate square-bracket balance in source cref signatures - validate-xml-docs.ps1:644. Correct, and for the same underlying reason: nothing paired them. The splitter tracks bracket depth only to place commas, and Get-DocIdCoreTypeName trims brackets whether or not they matched, so M:System.String.IndexOf(System.Char[) reduced to an ordinary type name and passed with zero findings.
Pairing is now checked alongside the brace rule it mirrors, which also covers the T:System.Byte[ form rather than only signatures. The controls matter more than the rejections here, so they are explicit: [], [,] and [0:,0:] all still pass, since their commas sit inside the brackets rather than separating parameters.
Evidence: Pester 156 passed, 0 failed. Re-running the validator over doc/snippets reports 0 invalid-docid findings.
The three open threads from @priyankatiwari08 were answered on 2026-09-30 and nothing has changed on them, so they are not repeated here.
The brace check only verifies balance, so empty generic argument lists such as T:System.Collections.Generic.List{} and lists with empty entries such as T:System.Collections.Generic.Dictionary{System.String,} pass source validation with no finding. These are malformed DocIDs and will still become xref-not-found downstream. Validate that every constructed generic has one or more non-empty arguments, recursively, and add controls for valid nested generics.
This issue also appears on line 682 of the same file.
Addresses Copilot review feedback on PR #4730.
Pairing a delimiter says nothing about what it encloses, and both
remaining gaps were the same shape: Get-DocIdCoreTypeName cuts the name
at the first brace and trims brackets away whatever they hold, so a
balanced but meaningless group reduced to an ordinary type name and drew
no finding.
A constructed generic names one or more type arguments, so reject an
empty list and an empty entry. T:System.Collections.Generic.List{} and
Dictionary{System.String,} both passed before.
An array suffix encloses a dimension list and nothing else, so reject
anything that is not one. Use(System.Char[x]), [1] and [[]] all passed
the balance check before.
Both checks walk every group rather than the outermost one, via a new
Get-DocIdGroupContents helper, so a defect nested inside a valid outer
group is caught. This was reported as the recursion requirement for
generics, and the array rule gets it for free.
The array rule was not in the review's findings list, only in its
headline sentence; it was verified against the code before being fixed.
Pester: 171 passed, 0 failed, with controls for nested generics and for
the [], [,] and [0:,0:] dimension forms. doc/snippets still reports 0
invalid-docid.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both fixed in 90f6d77. One was the stated finding; the other appears only in the review's headline sentence and not in the findings list, so it is called out separately below.
Reject empty generic argument lists - validate-xml-docs.ps1:449 and :682. Correct. The brace rule only paired delimiters, and Get-DocIdCoreTypeName cuts the name at the first brace, so an empty list reduced to the perfectly ordinary name List and nothing objected:
NO FINDING : T:System.Collections.Generic.List{}
NO FINDING : T:System.Collections.Generic.Dictionary{System.String,}
NO FINDING : T:System.Collections.Generic.Dictionary{,System.String}
NO FINDING : T:System.Collections.Generic.List{System.Collections.Generic.List{}}
Now rejected as two distinct findings, an empty list and an empty entry, because they are different authoring mistakes. The recursion you asked for is structural rather than special-cased: a new Get-DocIdGroupContents helper returns every brace group including nested ones, so the inner {} above is caught as readily as an outer one. Controls added for valid nested generics, including Dictionary{System.String,List{System.Int32}}.
Balanced but malformed array suffixes - this is the "malformed balanced array suffixes" in your overview line, which had no entry in the findings list. I verified it against the code rather than taking the headline on trust, and it is real:
NO FINDING : M:...Use(System.Char[x])
NO FINDING : M:...Use(System.Char[1])
NO FINDING : M:...Use(System.Char[[]])
Same root cause: the brackets were paired but never read, and Get-DocIdCoreTypeName trims them whatever they hold. An array suffix now has to be an actual dimension list. The valid forms all still pass: [], [,], [0:,0:] and [0:5].
Evidence: Pester 171 passed, 0 failed. Re-running the validator over doc/snippets reports 0 invalid-docid findings.
One note for the record: this review's header read Findings: None, as did the previous one, while the collapsed sections carried real findings both times.
The three open threads from @priyankatiwari08 are unchanged since they were answered on 2026-09-30 and are not repeated here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Area\EngineeringUse this for issues that are targeted for changes in the 'eng' folder or build systems.Hotfix 7.1.2PRs targeting main that should be backported to release/7.1 for 7.1.2.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
paulmedynski
added
Area\Engineering
Summary
See #4752 for the complete implementation details and review history.