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
Cherry-picks the XML documentation validation work from #4730 onto release/7.1, plus the defects it found here.
Open Publishing resolves every cref against the Learn xref map when our API docs are ingested into dotnet/sqlclient-api-docs. Until now the only report of a broken reference arrived as a warning on a pull request in that repository, days later. Nothing in our own build looked.
What was broken in the documentation
21 public members shipped with no documentation. A doc comment pulls its text in with <include>, and when the path matches nothing the compiler does not fail — it copies the unresolved element into the output and moves on. The member lands in the package with no summary, parameters or returns.
Verified by restoring the published Microsoft.Data.SqlClient 7.0.1 package and counting members whose documentation is still just an unresolved <include>:
XPath compares attributes case-sensitively, so each of these matched nothing. 15 include paths were wrong in total; all 2712 in the tree now resolve. Most have been broken since the PR that first added XML documentation, nearly seven years ago.
What reaches Learn depends on whether a member was ever ingested before its include broke, since mdoc merges rather than deletes:
The six SspiAuthenticationParameters members are undocumented on Learn today — the constructor and all five properties render with empty descriptions (live page), because the type was added in Feb 2026 with a path that never matched.
The rest retain text from an earlier ingestion, now decoupled from this repository. BeginExecuteXmlReader(AsyncCallback, object) is demonstrably stale: it still carries the Microssoft typo our snippet fixed in Fix | Addressing documentation issues along with spelling errors #2889 (Sep 2024). SqlBulkCopyColumnOrderHint's constructor happens to still match our snippet, so nothing is visibly wrong there — but no edit to those snippets could have reached Learn either.
41 cref defects corrected — 28 wrong documentation-ID prefixes (T: on a property, P: on a method), 5 array type references (T:System.Byte[], which has no page), 4 wrong namespaces or member names, 4 missing overload signatures. Oldest dates to #2869 (Sep 2024).
1 enum field with ignored remarks.RegisteredApplication.SqlPackage explained why sqlpackage reports its own identifier rather than the Data-Tier Application Framework's, in a <remarks> node that ECMA2Yaml discards on enum fields. Folded into the summary.
What was broken in the build
Every lib/ XML in 7.1.0 shipped trimmed. The driver ships two documentation files per target framework and they must differ: lib/ carries the full text for the Learn pages, ref/ has <remarks> and <example> stripped because they render badly in IntelliSense. The nuspec pointed both at the reference project's trimmed output, so the Learn pages built from 7.1.0 lost every remark and example. Introduced by #4257 (May 2026), shipped in 7.1.0 (Sep 2026).
lib/ now comes from the implementation project. netstandard2.0 has no implementation build, so the reference project emits an untrimmed copy alongside the trimmed one, and the package takes that.
Also fixed: references into a package not built in the same run (buildSqlServer: false) reported 24 false errors, because a .nupkg carries only its own documentation. The gate now resolves them against the published dependency's documentation, restored at the exact version depended upon.
When the validation runs
Per package build, over the documentation snippets that project's sources actually reference
Per package build, over the generated XML, after build and before pack
Once per run in Validate Packages, over the XML inside every .nupkg — the only point where the lib//ref/ mapping a consumer receives is visible
Gating on error; failOnValidationError: false downgrades findings to warnings for a shake-out run
No network access required, and no dependency on the 338 MB Learn xref map
Resolution — crefs into this repository must name a member the build emitted; namespaces resolve against the namespaces members occupy
Prefix correctness — T: on a property is separated from a member that was never emitted, so the message names the actual defect
Unresolved includes — an <include> surviving into generated documentation proves the member lost everything
Enum field remarks — discarded by the docs build, so they must be folded into the summary
Package layout — lib/ must not be trimmed, ref/ must be, the two must differ, and every assembly declaring GenerateDocumentationFile must ship its XML
Severity follows visibility: a defect on a public member is an error, the same defect on an internal one is informational, since only public members are published
xref-not-found for Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding. Our snippet was corrected by Fix | Addressing documentation issues along with spelling errors #2889, but BeginExecuteXmlReader's include had already stopped matching, so the member contributed nothing for the ingestion to write over, and the pre-Fix | Addressing documentation issues along with spelling errors #2889 text survived in the API docs repository. The two occurrences under BeginExecuteReader, whose includes still worked, were refreshed and corrected in the same run — which is why one of three remained. This is what led to the unresolved-include rule.
ECMA2Yaml_Enum_NoRemarks on RegisteredApplication, fixed as above.
The packaged gate was also run against the five .nupkg files produced by the failing pipeline run: it reproduced that run exactly (29330 crefs, 13 files, 24 errors) and passes with 0 once dependency documentation is supplied, every other count unchanged.
Validation
251 Pester tests pass. The reference assembly builds with no unresolved includes. The snippets gate passes over 4687 crefs in 99 files.
OneBranch non-official run 26268.2 succeeded with failOnValidationError: true, including source, generated implementation, generated reference, and packaged XML documentation validation.
Not picked from the source branch: the 8.0.0-preview1 version bump (main-only), and #4710/#4699, which are already present here in their 7.1 form.
One pre-existing issue is unrelated to this PR: release/7.1 packs stable 7.1.0 against prerelease Microsoft.SqlServer.Server 1.1.0-preview1, so NU5104 fails any non-official run with buildSqlServer: true.
Learn preview audit follow-up
After the initial API-docs verification above, the latest Learn Build previews from dotnet/sqlclient-api-docs#99 were crawled and compared with the source snippets, compiler-generated XML and API-docs PR XML.
The bot comment exposes only the first 25 changed files, so the audit enumerated the PR directly and checked all 39 changed API type pages plus 95 member-route candidates. It verified summaries, remarks, parameters, return/value text, exceptions, examples, included samples, enum field descriptions and xrefs.
That end-to-end comparison found additional silent publication gaps:
ActiveDirectoryAuthenticationProviderOptions lost its type summary and remarks because its class-level include selected every child below <members> rather than the type's documentation block.
SqlBulkCopyColumnOrderHintCollection.Clear and RemoveAt had documented declarations in the ref surface but no corresponding wrappers in the unified implementation. Their remarks and examples therefore never reached implementation XML or Learn.
A full generated-XML pass with the new rule found two more over-broad includes: OnChangeEventHandler selected its wrapper node, while SqlClientConnectionCloseBefore.Name recursively selected the complete type block.
SqlDbTypeExtensions.Json and Vector selected their wrapper elements instead of their summary/return elements.
Three SqlJson members placed standard <exception> elements under a nonstandard <throw> wrapper, which prevented those exceptions from being published normally.
All of these paths and wrappers are now corrected. Clear and RemoveAt are declared as documented new wrappers that delegate to CollectionBase, preserving the existing OnClear/OnRemove behavior and public ref surface.
The validator now reports unexpected-documentation-element when an unrecognized top-level container wraps standard documentation elements in compiler output. The rule deliberately permits extensible metadata elements such as <related>, <content> and <devnote> when they do not wrap standard docs. This catches the over-broad-include failure mode without treating the XML documentation format as a closed schema.
A reusable source-to-preview audit procedure is also recorded in .github/instructions/documentation.instructions.md, including full PR file enumeration, type/member traversal, expected Learn transformations, authenticated Edge/CDP access and credential-handling constraints.
Follow-up validation
XML validator Pester suite: 129 passed, 0 failed
SqlBulkCopyColumnOrderHintCollectionTest on net8.0: 22 passed, 0 failed
Validator over the rebuilt provider and Azure extension XML: passed with no gating findings
Generated XML spot checks confirm the repaired summaries, remarks, examples, parameters and exceptions are emitted as top-level publishable elements
git diff --check: passed
Inline xref validation follow-up
The next API-docs ingestion exposed five xref-not-found warnings in the newly published SqlBulkCopyColumnOrderHintCollection.Clear and RemoveAt documentation:
The target methods exist, but Markdown <xref:...> tokens are copied verbatim by the compiler. A parameterized method has no bare UID, so Open Publishing could not resolve those targets without either a full signature or the encoded overload wildcard %2A. The dormant snippet text predated this branch; adding the documented Clear and RemoveAt wrappers caused it to reach generated XML for the first time.
All five links now use %2A. The XML validator also scans inline Markdown xrefs in CDATA, indexes parameterized methods from generated documentation, and reports bare-parameterized-method-xref when a bare method target cannot resolve. Exact links to types, properties, fields, events and parameterless methods remain valid.
Validation after this follow-up:
XML validator Pester suite: 129 passed, 0 failed
Snippet validation: 4687 crefs and 635 inline xrefs across 99 files; passed
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>
(cherry picked from commit 752bc4a)
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>
(cherry picked from commit 01cbd08)
The generated-documentation and packaged-documentation gates reported 187
missing-local-uid warnings, none of which were actionable. They marked three
steps as succeeded-with-issues and buried a real defect.
Two populations were mixed together. A reference to an internal type that
carries no documentation of its own reaches no reader, because internal members
are never published. A reference from a public member does reach a reader, and
becomes an unresolved reference on the published page.
A reference assembly contains the public API and nothing else, so the
documentation in a package's ref/ folder is exactly that surface. Severity now
follows the member holding the reference: from a public member it is an error,
and otherwise it is informational. Where no reference documentation is
available the surface is unknown and nothing is escalated, which is the case
for the per-assembly gates.
On the net8.0 package this separates 19 findings from 60.
The separation exposed a namespace written with a type prefix in
SqlDataReader.xml, which would have produced an unresolved reference on a
published page. Corrected to N:.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit ed8aed6)
Demoting missing-local-uid to informational changed its severity but not how
it was reported: every non-gating finding was written as a warning, and any
finding at all marked the step as succeeded-with-issues. A step reporting
nothing but informational findings therefore still drew attention to itself.
task.logissue has no informational level, so informational findings are now
written as plain output and only warnings and errors mark the step.
Also corrects a cref naming a type that does not exist. The attribute is
Microsoft.SqlServer.Server.SqlUserDefinedTypeAttribute, which eight other
snippets already reference correctly; the ninth named a namespace that
contains no such type. It is referenced from the public SqlMetaData
constructors, so the published page would have carried an unresolved
reference. This was the only finding the packaged-documentation gate reported
against the public API surface.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 4761b5a)
Resolves the unresolved review threads on #4730, and the three items the
reviewer recorded in the review body rather than as threads.
Validator
---------
The extraction root is cleared wholesale before packages are expanded. Removing
only the destinations of the current packages left expansions from an earlier
invocation in place, and the whole root is scanned afterwards, so stale XML was
validated and its members entered the local UID index.
Alias detection recurses into generic arguments. The check reduced a parameter
to its core type, which discards the arguments, so an alias nested inside one
such as List{string} went unreported.
The local UID lookup compares the normalized identifier. The whitespace rule
rewrites the cref body, but the lookup still used the original, so a cref whose
only defect was whitespace also reported a prefix mismatch it did not have.
Package validation no longer propagates PackageValidator's exit code. The
script decides what that code means and reports accordingly, but the task
re-propagated the stale value and failed report-only runs.
Documentation build
-------------------
The trim no longer rewrites its own input. An isolated analyzer build redirects
bin while deliberately sharing obj, so rewriting the intermediate file left the
shared copy trimmed, and a later real build could publish trimmed text as the
full text or omit the untrimmed copy entirely. The compiler's output now stays
authoritative, the trimmed result is a separate intermediate that the output
copy is pointed at, and the untrimmed copy is keyed on the output path.
Verified across a clean build, two incremental rebuilds, and a real build
following an isolated one, for every target framework.
Documentation
-------------
Corrects the consumer table in documentation.instructions.md, which had lib and
ref the wrong way round: lib carries the full text for the API docs pipeline
and ref is trimmed for IntelliSense.
Corrects three crefs: an Add overload that does not exist, DataAccessKind where
the documented property takes SystemDataAccessKind, and prose calling the
SqlClientFactory.Instance field a property.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 5ccb6cd)
Two gaps in the documentation-ID grammar, both reported in review.
Angle brackets were never rejected. A documentation ID writes generic arguments
in braces, so a cref such as T:System.Collections.Generic.List<System.String>
is malformed, but it carries an allowed namespace root and resolves against no
local index in source mode, so it passed the preflight and would still produce
the unresolved published reference this validation exists to prevent. Angle
brackets are now rejected wherever they appear, with the braced form suggested.
The constructed-type rule recognized only the [] suffix. The multidimensional
forms [,] and the documentation-ID spelling [0:,0:] denote constructed types
just as [] does, so a T: reference to one passed. Any bracketed array suffix is
now matched. The same suffixes remain legal as parameter types, where they name
a real signature rather than a type page.
Adds regression cases for both, including controls for the valid braced generic
and for a multidimensional array used as a parameter type. Reverting either
rule fails three of them.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 22c2831)
Report-only downgrades findings so a run can surface everything at once, but it
was also downgrading malformed-xml. A file that could not be parsed was never
examined, so none of its cross-references were checked and suppressing it
reported an all-clear for content nobody read. A malformed packaged XML file
could therefore pass a non-official run.
This also contradicted the stated contract, which the pipeline guidance and the
sibling validation scripts already follow: report-only covers findings, while
unreadable or missing input fails outright because it produced no result to
downgrade.
Unreadable files are now reported and fail in either mode, with a message
naming how many could not be read. The report is still written first, so the
findings remain available. Ordinary findings continue to be downgraded.
Adds a case for the contract and one confirming report-only still works for
readable files, and updates the two existing cases that asserted the old
message.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 8b9d832)
… sibling doc directories
Two findings, both in code that predates the previous round.
A wrong-kind prefix was always a warning, even where the reference documentation
proves the containing member is public. Every pipeline invocation gates on
error, so a new M: reference to a public property would stay unresolved on the
published page yet pass validation. It is classified the same way as an
unresolvable reference now, and for the same reason: the wrong prefix produces
a UID that matches nothing, so from a public member it leaves an unresolved
reference. Non-public and unknown-surface cases keep their lower severity.
The classification lived inline in one of the two branches that report an
unresolvable reference, which is how the branches came to disagree. It is now a
single function both call.
The compile inputs covered only the repository-level doc/snippets tree. The
Abstractions and Azure projects include documentation from a doc directory
beside their own src directory, so editing one of those files left CoreCompile
up to date and published stale documentation. Both locations are now declared.
Verified on Abstractions with a control: without the addition an edit to its
documentation is silently dropped from the build output, and with it the edit
is picked up. Escalating wrong-kind prefixes introduces no new errors against
the current packages, the earlier prefix corrections having already cleared the
public ones.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 10a6c2e)
The CustomAdditionalCompileInputs wildcards were declared in src/Directory.Build.props,
which every project under src/ inherits. That gave all 31 projects, 23 of them test
projects that contain no <include> references, a dependency on the whole doc/snippets
tree, so touching any snippet invalidated CoreCompile solution-wide.
Move the item group to src/Directory.Build.targets, imported after the project body has
run, and guard it with an IncludesDocumentationSnippets property that the five projects
actually using <include> set for themselves. Those five continue to rebuild when a
snippet changes; the other 26 no longer see the inputs at all.
Also correct a cref in SqlUserDefinedTypeAttribute.xml. The sentence lists the
attribute's properties, alongside MaxByteSize which is already written as P:, so Format
there is the property rather than the enum type that shares its name. The other
references to the enum in that file and its siblings are genuinely the type and are
left alone.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit e7a9f5a)
Three fixes from review.
The alias check for type crefs compared only the outer type name, which a generic
argument never reaches: T:List{string} passed because List is a perfectly ordinary
type. Use Get-DocIdAlias, which already recurses, so an alias nested at any depth is
reported, and cover the form with tests. This also catches an alias in the declaring
type of a member cref, whose name part is examined by the same code.
The documentation trimming target listed only the generated XML as an input, so editing
TrimDocs.ps1 left the trimmed output considered up to date and kept text produced by the
old rules until someone built clean. Add the script to Inputs through a property, and
spell its path with forward slashes because the same property is passed to pwsh, where a
backslash is an escape character rather than a separator on non-Windows hosts.
The failOn documentation described a severity mapping that no longer held: it omitted
missing-documentation and the two public resolution categories from the error list, and
called missing-local-uid a warning when it is informational. Since the pipelines gate on
error, that understated the default gate. Restate it in terms of the public/non-public
split that decides severity, and pin the whole table with a test, as nothing else tied
the prose to the code.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 7aa2ddf)
A cref whose closing parenthesis precedes its opening one, such as
M:Microsoft.Data.SqlClient.Sample.Bad)(, satisfied the test for a ')' being present
anywhere in the body and then asked Substring for a negative length. The exception
escaped, so the run ended without writing the JSON report, including under -ReportOnly
where the report is the entire point of the mode.
Compare the two positions instead of testing for the character. LastIndexOf answers -1
when ')' is absent, which is below every valid opening position, so one comparison
covers both an unterminated list and a reversed one.
Neither form was covered, which is how the arithmetic beside them went unnoticed; both
are now tested, and the reversed case asserts the report exists rather than only the
finding.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 0be0c45)
A cref naming a namespace was exempt from local resolution, because the compiler emits
no <member> entry for a namespace and every N: cref would otherwise have been reported
as missing. The exemption also excused a misspelling: N:Microsoft.Data.SqlClinet
carries an allowed namespace root, and no pipeline invocation supplies an external xref
map, so it passed every gate and still became an unresolved reference on Learn.
Derive the set of namespaces from the members that were emitted, taking every leading
portion of each identifier, and resolve N: crefs against it. A leading portion may also
name a type containing a nested one, which is harmless here: the member index is
consulted first, so a namespace prefix on a type is reported as the prefix mismatch it
is rather than being admitted as a namespace.
The unresolved message is chosen by prefix, as saying that no matching member was
emitted explains nothing about a cref that never named a member.
Verified against the emitted documentation for Microsoft.Data.SqlClient: the three real
N: crefs still resolve and the finding count is unchanged at 70.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit cf79cfd)
…t consumer
Looking an identifier up without its signature is what lets a cref written as M: for a
property be reported as a prefix mismatch rather than a bare lookup failure. The same
fallback also matched a cref naming an overload that does not exist, because it shares
its identifier, and therefore its prefix, with the overload that does: a cref for
Run(System.String) against an emitted Run produced "uses prefix 'M:', but ... was
emitted as 'M:'", advising a replacement of the prefix with itself and hiding that the
build emitted no such member. Take the fallback only when the emitted prefixes differ
from the one written.
The header and the category table also named IntelliSense as what the 7.1.0 collapse
harmed. IntelliSense reads the trimmed ref/ copy by design and was unaffected; lib/
feeds the pipeline that builds the Learn pages, and those are what lost every remark and
example. Name that consumer, matching the finding messages and the package layout
contract in documentation.instructions.md. The missing-documentation message named
IntelliSense too, though it covers an assembly shipping no XML in either folder, so it
now speaks of documentation text generally.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 52a2ca5)
The generic-syntax rule rejected angle brackets but said nothing about braces that do
not pair up, so T:System.Collections.Generic.List{System.String passed with no findings
despite naming nothing. Such a cref is read from the first brace to the last, so a
missing delimiter quietly produces a different argument list than the text shows, or
none at all, and the identifier reaches Learn unresolvable.
Count the delimiters before the cref is taken apart, since the extraction that follows
assumes they pair up. A closing brace with nothing open ends the scan, as nothing later
can balance it.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit f737219)
Alias scanning covered the parameter list, and the name taken for the later checks
ends at the '(', so the return type of a conversion operator was read by neither:
M:System.Foo.op_Implicit(System.Int32)~string passed with no findings even though a
documentation ID names CLR types throughout. Scan the return type with the parameters,
and describe the finding as the signature rather than the parameter list, since the
alias may now be in either.
Nothing else may follow a parameter list, so text there that is not a return marker, or
a marker naming no type, is reported rather than dropped. Both forms reached none of
the surrounding checks, which read the name before the '(' and the arguments within it.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 439575d)
A NuGet package carries only its own XML documentation, so a cref from the
SqlClient package into Microsoft.SqlServer.Server resolves against the SqlServer
package in the drop. When SqlServer is not built there is no such package, and
the packaged documentation gate reported 24 missing-public-uid errors for
T:Microsoft.SqlServer.Server.SqlUserDefinedTypeAttribute -- a reference a
consumer resolves perfectly well against the published package the build depends
on.
Build output does not have the problem, because a dependency's documentation
file is copied next to the assembly that consumed it. Only the packaged gate
sees a package in isolation, which is why this surfaced there alone.
Add -DependencyDocumentationPath, whose documents are indexed for resolution but
never themselves validated: a defect in an already-published package cannot be
fixed by the run that reports it, and the crefs it contains point back into the
version of this repository it shipped against. They are also kept out of the
public API surface, so a member cannot count as public here on the strength of
where it sits in a package this run did not produce.
restore-package-documentation.ps1 collects that documentation from the exact
version depended upon, restored through the repository's NuGet.config so it
comes from the same governed feed as every other restore. The version is
required to be exact, since a range would leave undetermined which version
answered a reference.
The job supplies it only when SqlServer was not built; when it was, resolution
against a published copy would mask a reference the build itself broke.
Verified against the packages from the failing run: the gate reproduces
29330 crefs across 13 files with 24 errors, and passes with 0 once the
dependency documentation is supplied, every other count unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The API docs build for 7.1 reported ECMA2Yaml_Enum_NoRemarks against
RegisteredApplication: <remarks> on an enum field is ignored, so only the
summary is rendered. The note explaining why sqlpackage reports its own
identifier rather than the Data-Tier Application Framework's reached no page,
and nothing in our own build said so.
Fold that text into the field's summary, where it publishes.
Add an enum-field-remarks rule so the next one is caught here rather than in
another repository's validation run. Nothing in a snippet or in generated
documentation says which types are enums, so the enum declarations in source are
read for the answer. Their member names are read too, because a snippet
identifies a member only by its element name and may hold more than one block
for the same type: <SqlConnectionIPAddressPreferenceNetfx> documents the type
for another platform, and asking whether the name is a declared member is what
tells it apart from a field without guessing from the name.
In generated documentation the rule is confined to the public API surface. The
implementation assembly documents its own P/Invoke enums -- Interop.Windows
.NtDll.CreateOptions and the like -- whose remarks are discarded just the same,
but which reach no published page for anything to be lost from. Where no
reference documentation identifies the surface the rule stays silent, matching
how every other rule here treats an unknown surface.
Verified against the packages from the failing run: the rule reports the one
public field, once per target framework, and none of the internal ones.
The other finding on that run, an xref-not-found for a misspelled
'Microssoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding', needs no
change here. Our snippet was corrected by #2889 and the ingestion under review
is removing the stale text; the shipped 7.1.0 package contains no occurrence.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
paulmedynski
changed the title
[7.1 Cherry-pick] Add XML documentation validation to OneBranch builds
[7.1] Add XML documentation validation to OneBranch builds and fix docs issues
Sep 25, 2026
@mdaigle — 2026-09-24, DISMISSED — "Looks good. just waiting for confirmation from the non-official build and docs team"
No change required; posting the two confirmations you were waiting on.
Non-official build — run 26267.4 on e719f759c succeeded. All 7 stages green, 0 failures and 0 warnings, including the Validate Packages stage that failed on the previous run. The new dependency-documentation step ran and resolved the 24 false missing-public-uid errors to 0, with every other count unchanged.
Docs team — the latest ingestion on dotnet/sqlclient-api-docs#99 (bb59698) moved from warnings to suggestions. The xref-not-found for EnableOptimizedParameterBinding is gone, and the six SspiAuthenticationParameters members that rendered with empty descriptions on Learn now carry their documentation.
One suggestion remains on that PR — ECMA2Yaml_Enum_NoRemarks on RegisteredApplication — which is stale input rather than a live defect: that ingestion was built from a package predating the fix in 1d55820, and the enum-field-remarks rule reports 0 against the packages from run 26267.4.
Feedback from this PR's Copilot review thread is answered in the thread itself. The codecov comment is build status, not feedback, and was not replied to.
This condition skips source validation for the Abstractions and Azure packages, although both projects use local XML snippets (for example, Abstractions/src/SqlAuthenticationMethod.cs includes Abstractions/doc/SqlAuthenticationMethod.xml). That leaves source-only checks uncovered; notably, SqlAuthenticationMethod is a public enum, while generated/package validation only reports enum-field remarks when a ref/ document established the public surface, and these extension packages do not provide that surface. Run this gate for those two package jobs as well and point it at their local doc directories (or derive referenced snippet paths from projectPath).
Add a reusable Learn preview audit procedure, detect over-broad XML include paths, and repair documentation that Open Publishing silently discarded.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enable XML snippet source validation for the Abstractions and Azure
package jobs, and restrict the documented CDP browser origin.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot — “Enable source validation for Abstractions and Azure package jobs”
Fixed in 95bca33. The shared build job now accepts a per-project documentationSnippetsPath and runs source validation whenever that path is configured. The OneBranch stages configure the existing doc/snippets directory for SqlClient and SqlServer and the package-local doc directories for Abstractions and Azure.
The direct OneBranch compiler could not start because its integration assumes a remote named origin; this worktree uses the discovered github remote. Editor diagnostics reported no errors in the changed YAML.
Copilot — CDP wildcard origin review-body summary
Fixed in 95bca33 and answered in the linked review thread. The command now permits only the documented loopback origin and explicitly prohibits wildcard origins for authenticated profiles.
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>
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.
6 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
the
Area\Engineering
mdaigle
left a comment
Cherry-picks the XML documentation validation work from #4730 onto
release/7.1, plus the defects it found here.Open Publishing resolves every cref against the Learn xref map when our API docs are ingested into
dotnet/sqlclient-api-docs. Until now the only report of a broken reference arrived as a warning on a pull request in that repository, days later. Nothing in our own build looked.What was broken in the documentation
21 public members shipped with no documentation. A doc comment pulls its text in with
<include>, and when the path matches nothing the compiler does not fail — it copies the unresolved element into the output and moves on. The member lands in the package with no summary, parameters or returns.Verified by restoring the published Microsoft.Data.SqlClient 7.0.1 package and counting members whose documentation is still just an unresolved
<include>:ref/lib/SqlCommand.BeginExecuteReader×2,EndExecuteReader,BeginExecuteXmlReader(AsyncCallback, object)[@name="AsyncCallbackAndstateObject"]vs…AndStateObjectin the snippet;IAsyncResult2names no blockSqlBulkCopyColumnMappingctors ×3,…MappingCollection.Add×4,SqlClientPermission..ctor/.Add(net462)SqlBulkCopyColumnOrderHint..ctor,…Collection.Add(string, SortOrder)SspiAuthenticationParameters..ctor+Resource,ServerName,UserId,DatabaseName,PasswordXPath compares attributes case-sensitively, so each of these matched nothing. 15 include paths were wrong in total; all 2712 in the tree now resolve. Most have been broken since the PR that first added XML documentation, nearly seven years ago.
What reaches Learn depends on whether a member was ever ingested before its include broke, since mdoc merges rather than deletes:
SspiAuthenticationParametersmembers are undocumented on Learn today — the constructor and all five properties render with empty descriptions (live page), because the type was added in Feb 2026 with a path that never matched.BeginExecuteXmlReader(AsyncCallback, object)is demonstrably stale: it still carries theMicrossofttypo our snippet fixed in Fix | Addressing documentation issues along with spelling errors #2889 (Sep 2024).SqlBulkCopyColumnOrderHint's constructor happens to still match our snippet, so nothing is visibly wrong there — but no edit to those snippets could have reached Learn either.41 cref defects corrected — 28 wrong documentation-ID prefixes (
T:on a property,P:on a method), 5 array type references (T:System.Byte[], which has no page), 4 wrong namespaces or member names, 4 missing overload signatures. Oldest dates to #2869 (Sep 2024).1 enum field with ignored remarks.
RegisteredApplication.SqlPackageexplained whysqlpackagereports its own identifier rather than the Data-Tier Application Framework's, in a<remarks>node that ECMA2Yaml discards on enum fields. Folded into the summary.What was broken in the build
Every
lib/XML in 7.1.0 shipped trimmed. The driver ships two documentation files per target framework and they must differ:lib/carries the full text for the Learn pages,ref/has<remarks>and<example>stripped because they render badly in IntelliSense. The nuspec pointed both at the reference project's trimmed output, so the Learn pages built from 7.1.0 lost every remark and example. Introduced by #4257 (May 2026), shipped in 7.1.0 (Sep 2026).lib/now comes from the implementation project.netstandard2.0has no implementation build, so the reference project emits an untrimmed copy alongside the trimmed one, and the package takes that.Also fixed: references into a package not built in the same run (
buildSqlServer: false) reported 24 false errors, because a.nupkgcarries only its own documentation. The gate now resolves them against the published dependency's documentation, restored at the exact version depended upon.When the validation runs
.nupkg— the only point where thelib//ref/mapping a consumer receives is visibleerror;failOnValidationError: falsedowngrades findings to warnings for a shake-out runWhat it checks
T:on a property is separated from a member that was never emitted, so the message names the actual defect<include>surviving into generated documentation proves the member lost everythinglib/must not be trimmed,ref/must be, the two must differ, and every assembly declaringGenerateDocumentationFilemust ship its XMLVerified against the real API docs build
Checked against dotnet/sqlclient-api-docs#99, which reported two findings:
xref-not-foundforMicrossoft.Data.SqlClient.SqlCommand.EnableOptimizedParameterBinding. Our snippet was corrected by Fix | Addressing documentation issues along with spelling errors #2889, butBeginExecuteXmlReader's include had already stopped matching, so the member contributed nothing for the ingestion to write over, and the pre-Fix | Addressing documentation issues along with spelling errors #2889 text survived in the API docs repository. The two occurrences underBeginExecuteReader, whose includes still worked, were refreshed and corrected in the same run — which is why one of three remained. This is what led to the unresolved-include rule.ECMA2Yaml_Enum_NoRemarksonRegisteredApplication, fixed as above.The packaged gate was also run against the five
.nupkgfiles produced by the failing pipeline run: it reproduced that run exactly (29330 crefs, 13 files, 24 errors) and passes with 0 once dependency documentation is supplied, every other count unchanged.Validation
251 Pester tests pass. The reference assembly builds with no unresolved includes. The snippets gate passes over 4687 crefs in 99 files.
OneBranch non-official run 26268.2 succeeded with
failOnValidationError: true, including source, generated implementation, generated reference, and packaged XML documentation validation.Not picked from the source branch: the 8.0.0-preview1 version bump (
main-only), and #4710/#4699, which are already present here in their 7.1 form.One pre-existing issue is unrelated to this PR:
release/7.1packs stable 7.1.0 against prereleaseMicrosoft.SqlServer.Server 1.1.0-preview1, soNU5104fails any non-official run withbuildSqlServer: true.Learn preview audit follow-up
After the initial API-docs verification above, the latest Learn Build previews from dotnet/sqlclient-api-docs#99 were crawled and compared with the source snippets, compiler-generated XML and API-docs PR XML.
The bot comment exposes only the first 25 changed files, so the audit enumerated the PR directly and checked all 39 changed API type pages plus 95 member-route candidates. It verified summaries, remarks, parameters, return/value text, exceptions, examples, included samples, enum field descriptions and xrefs.
That end-to-end comparison found additional silent publication gaps:
ActiveDirectoryAuthenticationProviderOptionslost its type summary and remarks because its class-level include selected every child below<members>rather than the type's documentation block.SqlBulkCopyColumnOrderHintCollection.ClearandRemoveAthad documented declarations in the ref surface but no corresponding wrappers in the unified implementation. Their remarks and examples therefore never reached implementation XML or Learn.OnChangeEventHandlerselected its wrapper node, whileSqlClientConnectionCloseBefore.Namerecursively selected the complete type block.SqlDbTypeExtensions.JsonandVectorselected their wrapper elements instead of their summary/return elements.SqlJsonmembers placed standard<exception>elements under a nonstandard<throw>wrapper, which prevented those exceptions from being published normally.All of these paths and wrappers are now corrected.
ClearandRemoveAtare declared as documentednewwrappers that delegate toCollectionBase, preserving the existingOnClear/OnRemovebehavior and public ref surface.The validator now reports
unexpected-documentation-elementwhen an unrecognized top-level container wraps standard documentation elements in compiler output. The rule deliberately permits extensible metadata elements such as<related>,<content>and<devnote>when they do not wrap standard docs. This catches the over-broad-include failure mode without treating the XML documentation format as a closed schema.A reusable source-to-preview audit procedure is also recorded in
.github/instructions/documentation.instructions.md, including full PR file enumeration, type/member traversal, expected Learn transformations, authenticated Edge/CDP access and credential-handling constraints.Follow-up validation
SqlBulkCopyColumnOrderHintCollectionTestonnet8.0: 22 passed, 0 failednet8.0build: succeeded, 0 warningsnetstandard2.0build: succeeded, 0 warningsgit diff --check: passedInline xref validation follow-up
The next API-docs ingestion exposed five
xref-not-foundwarnings in the newly publishedSqlBulkCopyColumnOrderHintCollection.ClearandRemoveAtdocumentation:SqlBulkCopy.WriteToServertwiceSqlBulkCopyColumnOrderHintCollection.RemoveAttwiceSqlBulkCopyColumnOrderHintCollection.RemoveonceThe target methods exist, but Markdown
<xref:...>tokens are copied verbatim by the compiler. A parameterized method has no bare UID, so Open Publishing could not resolve those targets without either a full signature or the encoded overload wildcard%2A. The dormant snippet text predated this branch; adding the documentedClearandRemoveAtwrappers caused it to reach generated XML for the first time.All five links now use
%2A. The XML validator also scans inline Markdown xrefs in CDATA, indexes parameterized methods from generated documentation, and reportsbare-parameterized-method-xrefwhen a bare method target cannot resolve. Exact links to types, properties, fields, events and parameterless methods remain valid.Validation after this follow-up:
git diff --check: passedA fresh API-docs ingestion is still required after updated packages are available to verify the final rendered Learn pages.