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
Adds a preflight for the XML documentation cross-references that feed dotnet/sqlclient-api-docs, so malformed documentation IDs fail our own build instead of surfacing as xref-not-found warnings on an API Docs pull request. Motivated by the warnings reported against dotnet/sqlclient-api-docs#99.
Four commits: the first adds the validation and deliberately leaves its findings in place, the rest fix them and refine how findings are classified.
Verified by non-official run 177645, queued with failOnValidationError: true so the gates had to pass rather than merely report. All seven stages succeeded and all eleven validation steps came back green with zero warnings:
Validate XML documentation sources Microsoft.Data.SqlClient 0 warnings
Validate XML documentation sources Microsoft.SqlServer.Server 0 warnings
Validate generated XML documentation (six packages) 0 warnings
Validate packaged XML documentation whole drop 0 warnings
Validate localized resources 0 warnings
Validate NuGet packages 0 warnings
The packaged-documentation gate examined 29,510 cross-references across 15 files.
C# aliases in signatures, empty () on parameterless members, embedded whitespace, array T: UIDs
unknown-namespace-root
error
Misspelled namespaces, e.g. Microssoft.Data.SqlClient
unresolved-cref
error
Cross-references the compiler could not bind, emitted as !:
malformed-xml
error
Files that are not well-formed
stale-allowlist-entry
error
Allowlisted cross-references that no longer appear
missing-documentation
error
A project generates documentation but produced none, or a package ships an assembly without it
missing-public-uid
error
A public member references something that resolves nowhere
lib-documentation-trimmed
error
A package's lib/ XML has no remarks or examples
ref-documentation-untrimmed
error
A package's ref/ XML still has them
lib-ref-documentation-identical
error
lib/ and ref/ are byte-identical
unexpected-documentation
warning
Documentation found for a project that does not generate it
mismatched-docid-prefix
warning
Real member, wrong kind prefix (M: on a property, etc.)
missing-local-uid
info
As missing-public-uid, but from a non-public member, or where the public surface is unknown
documentation-not-expected
info
Nothing was validated, with the reason stated
unprefixed-cref
info
No T:/M:/… prefix; legal, the compiler binds it
No network access is required: these defects are decidable from documentation-ID syntax and the local member index, so the ~338 MB Learn xref map is not needed. It remains available via -ExternalXrefMapPath for local investigation.
Severity follows the member holding the reference
An unresolved reference from a public member becomes a visible xref-not-found on a published page. The same reference from an internal member reaches no reader, because internal members are never published.
A reference assembly contains the public API and nothing else, so the documentation in a package's ref/ folder is exactly that surface — no reflection needed, just a set lookup. Where no reference documentation is available the surface is unknown and nothing is escalated, which is the case for the per-assembly gates.
This separated 19 findings from 60 on the net8.0 package, and in the whole-drop gate reduced 187 undifferentiated warnings to 8 errors and 175 informational. The 8 were all one defect (below), now fixed; run 177645 reports zero.
Expectations come from the project, not the pipeline
GenerateDocumentationFile in the csproj is the single declaration; nothing is restated in YAML, so the two cannot drift:
generates documentation — it must exist; absence is an error
does not — absence is reported as information naming the reason, presence as a warning
Snippet validation follows the same principle: only the snippets a project's sources actually <include> are validated. Project files are parsed as XML, not searched as text — a comment naming GenerateDocumentationFile would otherwise read as setting it.
Where it runs
Snippet sources, before the build — defects reported in seconds against the file and line a developer edits.
Generated documentation, after the build — additionally sees !: cross-references and resolves against the members actually emitted.
Assembled packages, during package validation — the only point showing what a consumer receives.
All three run on Windows agents: the two jobs that host them, build-buildproj-job.yml and validate-packages-job.yml, both declare pool: type: windows. The pipeline's Linux jobs, version computation and symbol publishing, run no validation. Windows is therefore the only platform these gates cover, and net462 exists only there.
Validation gating
One failOnValidationError parameter governs the localization, XML documentation and package validation steps. When false, findings become warnings and the build continues; malformed inputs and a validator that fails to run still fail the step. sqlclient-official.yml sets it true; sqlclient-non-official.yml exposes it at queue time. No intermediate template declares a default.
Steps reporting warnings or errors emit ##vso[task.complete result=SucceededWithIssues;], because task.logissue alone leaves the task result untouched and a step carrying 307 warnings still rendered green. Informational findings are written as plain output and do not mark the step, so a run is flagged only when something needs attention. Run 177953 confirms this: 175 informational findings in the packaged gate produced zero warnings and a green step.
breakOnSdlError is renamed failOnSdlError to match.
Fixes
Documentation IDs — every cross-reference the gates reported: 5 array T: UIDs, 3 GetSchema DocIDs (empty parentheses, C# aliases), 1 embedded whitespace, and 29 wrong-kind prefixes across 16 snippet files.
Two defects only the public/internal split could surface.SqlDataReader.xml referenced T:Microsoft.Data.SqlClient — a namespace written with a type prefix. SqlMetaData.xml referenced T:Microsoft.Data.SqlClient.Server.SqlUserDefinedTypeAttribute, a namespace that contains no such type; the attribute is Microsoft.SqlServer.Server.SqlUserDefinedTypeAttribute, which eight other snippets already referenced correctly. Both sit on public API members, so both would have produced unresolved references on published pages.
lib/<tfm> documentation regression.lib/ is the source the API docs pipeline consumes and needs the full text; ref/ 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 07a928049 (#4033), so published documentation lost every remark and example for those frameworks — 1,226 documented members with 0 remarks, where the implementation has 3,235 members with 635 remarks and 175 examples. Both now take the implementation artifact.
netstandard2.0 has no implementation build, so the reference project now keeps an untrimmed copy beside the trimmed one and lib/netstandard2.0 uses that. Every lib target carries full text with no exception list.
TrimDocsForIntelliSense ordering. It ran after the output copy, so a first build published untrimmed reference documentation (1,523,234 bytes, 507 remarks) and only a second build published the trimmed file (736,069 bytes, 0 remarks). Whether ref/ was trimmed depended on how many times the project had been built. Now runs between CoreCompile and CopyFilesToOutputDirectory.
Snippet edits did not trigger rebuild.<include> files are read by the compiler but were not MSBuild inputs, so editing a snippet left the generated documentation stale — making a fix appear to be ignored by the validation. Declared via CustomAdditionalCompileInputs. Verified with a control: without it an edit is silently dropped; with it the edit is picked up, and an unchanged rebuild still skips CoreCompile.
Microsoft.SqlServer.Server now generates documentation. Its public types already carried complete comments — 63 documented members, 0 build 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. Run 177953 confirms the package now ships its documentation.
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.
Rules added during review
Code review drove a second round of grammar and resolution rules, each reproduced against the
validator before being fixed and covered by a test confirmed to fail without it:
Rule
What passed before
Recursive alias detection
T:...List{string} — the outer name is an ordinary type, so the alias inside was never examined
Conversion operator return types
M:...op_Implicit(System.Int32)~string — scanning stopped at the parameter list
Balanced generic delimiters
T:...List{System.String — read from the first brace to the last, so a missing one silently changed the argument list
Trailing signature text
M:...Bar(System.Int32)garbage and a bare trailing ~
Namespace resolution
N:Microsoft.Data.SqlClinet — namespaces were exempt from local resolution, so a misspelling passed every gate
Wrong-kind prefix on a namespace
N: naming a type is now a prefix mismatch
Overload versus prefix mismatch
An overload that was never emitted was reported as a prefix mismatch, advising a prefix be replaced with itself
Reversed parameter list
M:...Bad)( threw, ending the run without writing the JSON report that report-only mode exists to produce
Multidimensional arrays
T:System.Int32[,] and the [0:,0:] documentation-ID spelling
Angle-bracket generics
T:...List<System.String>
Two build-correctness fixes came from the same review. The documentation compile inputs were
declared in src/Directory.Build.props, which every project under src/ inherits, so editing any
snippet invalidated CoreCompile for all 31 projects including the 23 test projects that consume
none. They now live in src/Directory.Build.targets behind a property the five consuming projects
set, verified by evaluating the item in every project: 5 of 31 carry it, and a snippet edit still
rebuilds those five. Separately, TrimDocs.ps1 was not among the trimming target's Inputs, so
editing the script left the trimmed output considered up to date until someone built clean.
#4673 removes the RID-specific DLLs and changes lib/net462's XML from the implementation to the trimmed reference copy. Combined with the mappings it leaves untouched, all four lib/<tfm> folders would carry trimmed documentation — the regression this PR fixes, extended to the one framework that still escaped it. The two PRs otherwise agree: #4673 puts real implementation DLLs into lib/, this PR puts the matching full XML there. A plain merge conflicts on lib/net462 and must be resolved toward lib = full.
Testing
214 tests pass across eng/pipelines/onebranch/scripts/tests/, including the pre-existing suites.
Run 177953
on 439575d0e is the end-to-end evidence, and it ran with failOnValidationError at its true
default, so any error-severity finding would have failed the build:
All 11 validation steps succeeded — 48,221 cross-references in total. Across the run's 678 timeline
records: 628 succeeded, 50 skipped, none failed and none succeededWithIssues. That last count
is the informative one, since SucceededWithIssues is exactly what a step emits when the validator
finds something; its absence means nothing was found rather than findings being tolerated.
This is also the first run to exercise the reference-project restructure on Windows across all four
target frameworks, net462 included, which no local run can reach.
Four defects were found only by running the pipeline, each now covered by a test verified to fail when the defect is reintroduced:
Informational findings marked steps as needing attention
Also verified against real artifacts rather than only synthetic ones: both projects built, packages assembled reproducing both the broken and corrected nuspec mappings, and a package with its XML deliberately dropped.
Tests added or updated
Public API changes documented — n/a, no public API change
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.
paulmedynski
added
Area\Engineering
Use this for issues that are targeted for changes in the 'eng' folder or build systems.
Hotfix 7.1.1
PRs targeting main that should be backported to release/7.1 for 7.1.1.
labels
Sep 22, 2026
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>
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>
Addressed the Validate local namespace cref resolution finding in cf79cfd. Reproduced it first — with N:Microsoft.Data.SqlClinet and T:Microsoft.Data.SqlClinet.SqlConnection in the same file, only the type was reported. The namespace passed every gate, exactly as described.
Namespaces are now resolved against a set derived from the members the build did emit, taking every leading portion of each identifier.
One design note worth calling out. A leading portion can also name a type containing a nested one, so the derived set is deliberately permissive — but that cannot weaken anything, because the member index is consulted before the namespace set. So N:Microsoft.Data.SqlClient.SqlConnection is reported as mismatched-docid-prefix (was emitted as 'T:') rather than being admitted as a namespace. Ordering the checks this way meant I did not have to distinguish a namespace from a containing type in a DocID, which is not reliably possible. That wrong-prefix case is a second gap this closes.
I also split the unresolved message by prefix — "no matching documented member was emitted" explains nothing about a cref that never named a member.
Verification, since the false-positive risk here is real (doc/snippets contains three N:Microsoft.Data.SqlClient crefs that must keep resolving):
Ran against the emitted Microsoft.Data.SqlClient.xml for net9.0: 4,579 crefs, missing-local-uid=70 both before and after — byte-identical outcome, zero N: findings.
doc/snippets still clean at 4,687 crefs across 99 files.
200 tests pass, up from 197. The three new cases cover a namespace that resolves, a misspelling that does not, and the N:-on-a-type prefix mismatch; I confirmed the two failure cases fail against the previous logic rather than passing vacuously.
A nonexistent overload can be misclassified as a prefix mismatch when a parameterless overload exists. For example, if M:Widget.Run is emitted and a cref names M:Widget.Run(System.String), the namePart fallback selects Widget.Run and reports that M: should be changed to M:. Only use the fallback when the emitted prefixes do not already contain the cref's current prefix; otherwise this should remain a missing-UID finding.
Identify Learn API pages as the consumer of trimmed lib XML
The collapsed mapping did not remove remarks from IntelliSense: IntelliSense intentionally consumes the trimmed ref/ XML. It removed narrative content from the Learn API pages, which consume lib/. Naming the affected consumer correctly avoids contradicting the package-layout contract documented below.
This issue also appears on line 126 of the same file.
…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>
Avoid misclassifying missing overloads as prefix mismatches — reproduced exactly as described. An emitted M:Widget.Run with a cref naming M:Widget.Run(System.String) produced:
uses prefix M:, but Microsoft.Data.SqlClient.Widget.Run was emitted as M:
Self-contradictory, and it hid the real problem — the build emitted no such overload. The signature-less lookup is what lets a cref written as M: for a property be named as a prefix mismatch, but it also matches an overload that does not exist, since that shares its identifier and therefore its prefix with the overload that does. The fallback is now taken only when the emitted prefixes differ from the one written. The missing overload is reported as missing-local-uid, while M: on a property and P: on a method are still reported as mismatches.
Identify Learn API pages as the consumer of trimmed lib XML — correct, and this is a mapping I have gotten backwards before, so thank you for catching the residue. IntelliSense reads the trimmed ref/ copy by design and was unaffected by the 7.1.0 collapse; lib/ feeds the pipeline that builds the Learn pages, and those are what lost every remark and example. Fixed in both the header and the category table.
I swept the rest of the file rather than only the two lines cited. That turned up a third: the missing-documentation message also said "no IntelliSense text", but it fires for an assembly shipping no XML in either folder, so it now speaks of documentation text generally. The two remaining IntelliSense mentions are correct — both describe ref/ being trimmed. The lib-documentation-trimmed finding message already named the API docs pipeline correctly, so the header and table were the only places contradicting the contract in documentation.instructions.md.
Verification:
202 tests pass, up from 200. I confirmed the new overload case fails against the previous logic rather than passing vacuously.
Real corpus unaffected: the emitted Microsoft.Data.SqlClient.xml for net9.0 reports 4,579 crefs and missing-local-uid=70 both before and after, so no existing cref was travelling the misclassification path.
doc/snippets still clean at 4,687 crefs across 99 files.
The generic-syntax check only rejects angle brackets; it still accepts unbalanced braces. For example, T:System.Collections.Generic.List{System.String exits successfully with zero findings, although this malformed DocID cannot resolve. Please validate balanced and correctly nested generic delimiters and add a regression case before accepting the cref.
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>
Addressed in f737219. Reproduced first — T:System.Collections.Generic.List{System.String exited with zero findings, as did the misnested T:System.Collections.Generic.List}System.String{.
The failure is quiet rather than loud, which is what makes it worth gating: the generic argument list is read from the first brace to the last, so a missing delimiter yields a different argument set than the text shows, or none at all, and the identifier reaches Learn unresolvable. It is also why the existing recursive alias check could not catch it — with no closing brace there is no inner text to recurse into.
The delimiters are now counted 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, which is what catches the misnested form rather than only the truncated one.
I covered the signature position as well as the type position, since the same body is scanned for both: M:...Use(System.Collections.Generic.List{System.String) was equally accepted and is now rejected.
Verification:
208 tests pass, up from 202 — three rejected forms and three accepted controls, the controls covering a simple generic, a nested generic (Dictionary{System.String,List{System.Int32}}), and a generic inside a signature. I confirmed all three rejection cases fail against the previous logic while the controls passed both before and after, so the rule adds detection without narrowing what it accepts.
Real corpus unaffected: emitted Microsoft.Data.SqlClient.xml for net9.0 still reports 4,579 crefs with missing-local-uid=70, and doc/snippets is still clean at 4,687 crefs across 99 files.
Alias validation stops at the parameter list and never examines a conversion operator's return suffix. As a result, a DocID such as M:System.Foo.op_Implicit(System.Int32)~string passes even though the documented rule requires CLR type names throughout the signature; because System is external and no xref map is used in CI, later gates do not catch it either. Parse the text after ~ and run the same recursive alias/type validation over it, with a regression test beside the existing conversion-operator control.
This issue also appears on line 503 of the same file.
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>
Copilot review 2026-09-24T14:07:57Z — Validate aliases in conversion operator return suffixes (eng/pipelines/onebranch/scripts/validate-xml-docs.ps1:465, also line 503) — Fixed in 439575d.
Reproduced first: M:System.Foo.op_Implicit(System.Int32)~string and ~System.Collections.Generic.List{int} both passed with zero findings. The two cited lines share one cause — alias scanning ends at the parameter list, and the name used by the later checks ends at the (, so the text after ~ was read by neither.
The return type is now scanned alongside the parameters, and the message says "in its signature" rather than "in its parameter list", since the alias may be in either. A signature carrying aliases in both is reported once, listing both.
While fixing it I found an adjacent hole that was not reported: nothing after a parameter list was examined at all, so M:System.Foo.Bar(System.Int32)garbage and a bare trailing ~ also passed. Only a conversion operator's return marker may follow a parameter list, so both are now rejected.
Verification: 214 tests pass, up from 208. Five of the six new cases fail against the previous logic rather than passing vacuously; the sixth is an accept-control. The real corpora are unaffected — doc/snippets is clean at 4,687 crefs across 99 files, and the emitted Microsoft.Data.SqlClient.xml for net9.0 still reports 4,579 crefs with missing-local-uid=70, including its three real op_Implicit members.
Other sources inspected
Review threads: 10 total, all resolved, 0 unresolved.
Copilot suppressed findings: none. This Copilot version reports through "Open" / "Previously missed" sections rather than a suppressed-comments block.
Discussion comments: 8 inspected — 1 coverage report set aside as operational, 7 are my own earlier replies. No feedback requiring action.
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
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
Adds a preflight for the XML documentation cross-references that feed
dotnet/sqlclient-api-docs, so malformed documentation IDs fail our own build instead of surfacing asxref-not-foundwarnings on an API Docs pull request. Motivated by the warnings reported againstdotnet/sqlclient-api-docs#99.Four commits: the first adds the validation and deliberately leaves its findings in place, the rest fix them and refine how findings are classified.
Verified by non-official run 177645, queued with
failOnValidationError: trueso the gates had to pass rather than merely report. All seven stages succeeded and all eleven validation steps came back green with zero warnings:The packaged-documentation gate examined 29,510 cross-references across 15 files.
What is validated
eng/pipelines/onebranch/scripts/validate-xml-docs.ps1reports:invalid-docid()on parameterless members, embedded whitespace, arrayT:UIDsunknown-namespace-rootMicrossoft.Data.SqlClientunresolved-cref!:malformed-xmlstale-allowlist-entrymissing-documentationmissing-public-uidlib-documentation-trimmedlib/XML has no remarks or examplesref-documentation-untrimmedref/XML still has themlib-ref-documentation-identicallib/andref/are byte-identicalunexpected-documentationmismatched-docid-prefixM:on a property, etc.)missing-local-uidmissing-public-uid, but from a non-public member, or where the public surface is unknowndocumentation-not-expectedunprefixed-crefT:/M:/… prefix; legal, the compiler binds itNo network access is required: these defects are decidable from documentation-ID syntax and the local member index, so the ~338 MB Learn xref map is not needed. It remains available via
-ExternalXrefMapPathfor local investigation.Severity follows the member holding the reference
An unresolved reference from a public member becomes a visible
xref-not-foundon a published page. The same reference from an internal member reaches no reader, because internal members are never published.A reference assembly contains the public API and nothing else, so the documentation in a package's
ref/folder is exactly that surface — no reflection needed, just a set lookup. Where no reference documentation is available the surface is unknown and nothing is escalated, which is the case for the per-assembly gates.This separated 19 findings from 60 on the
net8.0package, and in the whole-drop gate reduced 187 undifferentiated warnings to 8 errors and 175 informational. The 8 were all one defect (below), now fixed; run 177645 reports zero.Expectations come from the project, not the pipeline
GenerateDocumentationFilein the csproj is the single declaration; nothing is restated in YAML, so the two cannot drift:Snippet validation follows the same principle: only the snippets a project's sources actually
<include>are validated. Project files are parsed as XML, not searched as text — a comment namingGenerateDocumentationFilewould otherwise read as setting it.Where it runs
!:cross-references and resolves against the members actually emitted.All three run on Windows agents: the two jobs that host them,
build-buildproj-job.ymlandvalidate-packages-job.yml, both declarepool: type: windows. The pipeline's Linux jobs, version computation and symbol publishing, run no validation. Windows is therefore the only platform these gates cover, andnet462exists only there.Validation gating
One
failOnValidationErrorparameter governs the localization, XML documentation and package validation steps. When false, findings become warnings and the build continues; malformed inputs and a validator that fails to run still fail the step.sqlclient-official.ymlsets ittrue;sqlclient-non-official.ymlexposes it at queue time. No intermediate template declares a default.Steps reporting warnings or errors emit
##vso[task.complete result=SucceededWithIssues;], becausetask.logissuealone leaves the task result untouched and a step carrying 307 warnings still rendered green. Informational findings are written as plain output and do not mark the step, so a run is flagged only when something needs attention. Run 177953 confirms this: 175 informational findings in the packaged gate produced zero warnings and a green step.breakOnSdlErroris renamedfailOnSdlErrorto match.Fixes
Documentation IDs — every cross-reference the gates reported: 5 array
T:UIDs, 3GetSchemaDocIDs (empty parentheses, C# aliases), 1 embedded whitespace, and 29 wrong-kind prefixes across 16 snippet files.Two defects only the public/internal split could surface.
SqlDataReader.xmlreferencedT:Microsoft.Data.SqlClient— a namespace written with a type prefix.SqlMetaData.xmlreferencedT:Microsoft.Data.SqlClient.Server.SqlUserDefinedTypeAttribute, a namespace that contains no such type; the attribute isMicrosoft.SqlServer.Server.SqlUserDefinedTypeAttribute, which eight other snippets already referenced correctly. Both sit on public API members, so both would have produced unresolved references on published pages.lib/<tfm>documentation regression.lib/is the source the API docs pipeline consumes and needs the full text;ref/is trimmed because remarks and examples render badly in Visual Studio tooltips.lib/net8.0andlib/net9.0had been taking the trimmed reference copy since07a928049(#4033), so published documentation lost every remark and example for those frameworks — 1,226 documented members with 0 remarks, where the implementation has 3,235 members with 635 remarks and 175 examples. Both now take the implementation artifact.netstandard2.0has no implementation build, so the reference project now keeps an untrimmed copy beside the trimmed one andlib/netstandard2.0uses that. Everylibtarget carries full text with no exception list.TrimDocsForIntelliSenseordering. It ran after the output copy, so a first build published untrimmed reference documentation (1,523,234 bytes, 507 remarks) and only a second build published the trimmed file (736,069 bytes, 0 remarks). Whetherref/was trimmed depended on how many times the project had been built. Now runs betweenCoreCompileandCopyFilesToOutputDirectory.Snippet edits did not trigger rebuild.
<include>files are read by the compiler but were not MSBuild inputs, so editing a snippet left the generated documentation stale — making a fix appear to be ignored by the validation. Declared viaCustomAdditionalCompileInputs. Verified with a control: without it an edit is silently dropped; with it the edit is picked up, and an unchanged rebuild still skipsCoreCompile.Microsoft.SqlServer.Servernow generates documentation. Its public types already carried complete comments — 63 documented members, 0 build warnings — but they were compiled away, so consumers got no IntelliSense. Set unconditionally:net46is type-forwards only and gets an essentially empty file, but aTargetFrameworkcondition would leave the project without a single unambiguous answer about whether it produces documentation. Run 177953 confirms the package now ships its documentation.Also records in the nuspec why
net462packs the real implementation into bothlib/andruntimes/, which is otherwise easy to mistake for an accident.Rules added during review
Code review drove a second round of grammar and resolution rules, each reproduced against the
validator before being fixed and covered by a test confirmed to fail without it:
T:...List{string}— the outer name is an ordinary type, so the alias inside was never examinedM:...op_Implicit(System.Int32)~string— scanning stopped at the parameter listT:...List{System.String— read from the first brace to the last, so a missing one silently changed the argument listM:...Bar(System.Int32)garbageand a bare trailing~N:Microsoft.Data.SqlClinet— namespaces were exempt from local resolution, so a misspelling passed every gateN:naming a type is now a prefix mismatchM:...Bad)(threw, ending the run without writing the JSON report that report-only mode exists to produceT:System.Int32[,]and the[0:,0:]documentation-ID spellingT:...List<System.String>Two build-correctness fixes came from the same review. The documentation compile inputs were
declared in
src/Directory.Build.props, which every project undersrc/inherits, so editing anysnippet invalidated
CoreCompilefor all 31 projects including the 23 test projects that consumenone. They now live in
src/Directory.Build.targetsbehind a property the five consuming projectsset, verified by evaluating the item in every project: 5 of 31 carry it, and a snippet edit still
rebuilds those five. Separately,
TrimDocs.ps1was not among the trimming target'sInputs, soediting the script left the trimmed output considered up to date until someone built clean.
Conflicts with #4673
#4673 removes the RID-specific DLLs and changes
lib/net462's XML from the implementation to the trimmed reference copy. Combined with the mappings it leaves untouched, all fourlib/<tfm>folders would carry trimmed documentation — the regression this PR fixes, extended to the one framework that still escaped it. The two PRs otherwise agree: #4673 puts real implementation DLLs intolib/, this PR puts the matching full XML there. A plain merge conflicts onlib/net462and must be resolved towardlib= full.Testing
214 tests pass across
eng/pipelines/onebranch/scripts/tests/, including the pre-existing suites.Run 177953
on
439575d0eis the end-to-end evidence, and it ran withfailOnValidationErrorat itstruedefault, so any error-severity finding would have failed the build:
All 11 validation steps succeeded — 48,221 cross-references in total. Across the run's 678 timeline
records: 628 succeeded, 50 skipped, none failed and none
succeededWithIssues. That last countis the informative one, since
SucceededWithIssuesis exactly what a step emits when the validatorfinds something; its absence means nothing was found rather than findings being tolerated.
This is also the first run to exercise the reference-project restructure on Windows across all four
target frameworks,
net462included, which no local run can reach.Four defects were found only by running the pipeline, each now covered by a test verified to fail when the defect is reintroduced:
-Switch:Valuebinds as a string under the PowerShell task's-Commanddot-sourcingAlso verified against real artifacts rather than only synthetic ones: both projects built, packages assembled reproducing both the broken and corrected nuspec mappings, and a package with its XML deliberately dropped.