Improve handling of snippet symbol graph files #1302

d-ronnqvist · 2025-09-26T13:33:41Z

Bug/issue #, if applicable: rdar://147926589&161164434 #1280

Summary

This improves the handling of snippet symbol graph files by no longer relying on main module / bystander logic and instead detecting snippets based on their symbol kind and handling them in a dedicated code path.

By avoiding the main bystander symbol graph logic, this avoid issues with DocC miscategorizing the snippets symbol graph file as a separate module, resulting in their being two different "roots" of the documentation hierarchy (which is undefined behavior, with known nondeterminism, in DocC). The effect of this is that:

Snippets are no longer added to the PathHierarchy
Snippets no longer have ResolvedTopicReference identifiers
Snippets no longer have DocumentationNode or TopicGraph/Node values that need to be skipped over.

This doesn't result in any loss of functionality. Because snippet paths aren't resolved relative to the linked page—but relative to the package's root—and because snippet paths don't have overloads/disambiguation there wasn't any benefit from storing them in the PathHierarchy. Because snippets aren't pages, they also didn't benefit from having nodes in the context.

Now, all the snippet logic is "custom" but it's so simple that it's ~150 LOC including comments.

With custom logic for the snippets, it was easier to refine their path "resolution" and diagnostics.

I've long felt that it was unnecessary for snippets to all have the "my-package/Snippets" prefix, now with the new implementation it was easy to make that optional (while still supported for backwards compatibility). It was also easier to customize the diagnosis message and add near-miss suggestions for both paths and slices. The new tests cover both the optional path prefixes, rendering, and snippet diagnostics.

Lastly, I updated the user-facing documentation to cover these improvements.

Because #1280 is a regression in 6.2, the plan is to cherry-pick this into 6.2 (that's why the new documentation says that this works in 6.2)

Dependencies

None.

Testing

Add a snippet to a package by following the steps in the "Adding Code Snippets to Your Content" documentation.
Link to those snippets from a module in the package, with and without slices.
Build documentation using swift package generate-documentation (so that snippets are included in the build)
- The snippets should resolve (or have actionable warnings if the snippets aren't found)
Preview the built documentation
- The relevant snippet content and slices should display inline on the pages that used @Snippet directives.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

Added tests
Ran the ./bin/test script and it succeeded
Updated documentation if necessary

rdar://147926589 rdar://161164434 swiftlang#1280

Also, update the behavior when a snippet slice is misspelled to match what's described in the warning.

d-ronnqvist · 2025-09-26T13:35:33Z

@swift-ci please test

d-ronnqvist · 2025-09-26T13:35:47Z

@swift-ci please test

QuietMisdreavus

Not quite done reading the tests yet, but this looks good so far. I want to come back this afternoon (US time) to finish reading up before i give a green tick. I do have one comment i wanted to make sure to post first, though.

QuietMisdreavus · 2025-09-26T14:48:05Z

Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift

        switch blockDirective.name {
        case Snippet.directiveName:
-            var problems = [Problem]()
+            var problems = [Problem]() // ???: DAVID IS IGNORED?


Left a debug comment here.

Thanks. I found that the warnings below weren't being printed because they were added to this local shadow of problems rather than self.problems. This comment was means to remind me to check if we wanted to emit the problems from parsing the Snippet or of this should be renamed ignoredProblems to clearly indicate that those problems have already been reported elsewhere.

Confirmed. Those argument parsing problems have already been reported elsewhere. If we report them here they're repeated in the console output. ~~I'll clarify~~ I clarified this in the code in afd6871

d-ronnqvist · 2025-09-26T16:03:32Z

@swift-ci please test

heckj

Looks great, thank you!!

Sources/SwiftDocC/Infrastructure/Link Resolution/SnippetResolver.swift

d-ronnqvist · 2025-09-26T16:13:39Z

Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift

-        // Test relative links from another article (which doesn't overlap with the snippet's path)
-        let sliceArticleID = try tree.find(path: "/Snippets/SliceIndentation", onlyFindSymbols: false)
-        XCTAssertThrowsError(try tree.findSymbol(path: "MySnippet", parent: sliceArticleID))
-        XCTAssertEqual(try tree.findSymbol(path: "Snippets/MySnippet", parent: sliceArticleID).identifier.precise, "$snippet__Test.Snippets.MySnippet")
-        XCTAssertEqual(try tree.findSymbol(path: "Snippets/Snippets/MySnippet", parent: sliceArticleID).identifier.precise, "$snippet__Test.Snippets.MySnippet")
-        XCTAssertEqual(try tree.findSymbol(path: "/Snippets/Snippets/MySnippet", parent: sliceArticleID).identifier.precise, "$snippet__Test.Snippets.MySnippet")


FYI: DocC never actually worked like this because of how snippet links were resolved. It's valid within the path hierarchy to do this, but misleading because it doesn't reflect what the caller does.

d-ronnqvist · 2025-09-26T16:16:45Z

Tests/SwiftDocCTests/Infrastructure/SnippetResolverTests.swift

+
+            let prefixLength = pathPrefix.count
+            XCTAssertEqual(logOutput, """
+            \u{001B}[1;33mwarning: Snippet named 'Frst' couldn't be found\u{001B}[0;0m


FYI: I'm not too happy about verifying escape code output like this because it makes the test really hard to read and debug if something goes wrong. Without highlights in the diagnostic configuration, there's no visual indication of the line range that the warning applies to (I'm not counting the character range after the file name). I feel that it's important for this test to verify that the correct portions of the line are highlighted for each warning and I didn't feel like it was within score of this PR to update the general diagnostic output. Considering that, I feel like verifying the output with escape codes is better than the alternative for now.

Sources/SwiftDocC/Infrastructure/Link Resolution/SnippetResolver.swift

d-ronnqvist · 2025-09-26T16:24:49Z

@swift-ci please test

d-ronnqvist · 2025-09-26T16:25:51Z

I'll wait for @QuietMisdreavus to also look at this and if everything looks good I'll merge it on Monday and open release/6.2 cherry-pick then.

d-ronnqvist · 2025-09-26T16:29:25Z

Sources/SwiftDocC/Infrastructure/Link Resolution/SnippetResolver.swift

+        self.snippets = snippets
+    }
+
+    func resolveSnippet(path authoredPath: String) -> SnippetResolutionResult {


FYI: there are some nice "Good first issue" improvements to be had here. For example, if the snippet file is nested in a deeper subdirectory but there's only one snippet with that base name, we could have a custom solution an/or note that says that a snippet with that name exist in a deeper path. After this is merged I can open an issue suggesting that.

d-ronnqvist · 2025-09-26T16:31:19Z

Tests/SwiftDocCTests/Infrastructure/SnippetResolverTests.swift

+        XCTAssertEqual(context.knownIdentifiers.count, 1, "The snippets don't have their own identifiers", file: file, line: line)
+
+        let reference = try XCTUnwrap(context.soleRootModuleReference, file: file, line: line)


In case you're curious: if you bring this test over to the main branch, these two assertions fail:

The first because there will be 4 symbols (the "ModuleName" module, the "Snippets" module, and the two snippet symbols)

The second because there will be two modules competing to be the documentation root, (hence there's no sole root).

QuietMisdreavus

This looks great!

d-ronnqvist · 2025-09-28T11:56:48Z

@swift-ci please test

* Separate snippets from other symbols rdar://147926589 rdar://161164434 swiftlang#1280 * Rename tests to clarify what they're verifying * Improve tests around optional snippet prefix components * Add additional test about resolving and rendering snippets * Make it easier to verify the diagnostic log output in tests * Add additional test about snippet warnings Also, update the behavior when a snippet slice is misspelled to match what's described in the warning. * Move snippet diagnostic creation to resolver type * Add near-miss suggestions for snippet paths and slices * Only highlight the misspelled portion of snippet paths * Update user-facing documentation about optional snippet paths prefixes * Clarify that Snippet argument parsing problems are reported elsewhere * Fix typo in code comment * Update docs about earliest version with an optional snippet path prefix

* Improve handling of snippet symbol graph files (#1302) * Separate snippets from other symbols rdar://147926589 rdar://161164434 #1280 * Rename tests to clarify what they're verifying * Improve tests around optional snippet prefix components * Add additional test about resolving and rendering snippets * Make it easier to verify the diagnostic log output in tests * Add additional test about snippet warnings Also, update the behavior when a snippet slice is misspelled to match what's described in the warning. * Move snippet diagnostic creation to resolver type * Add near-miss suggestions for snippet paths and slices * Only highlight the misspelled portion of snippet paths * Update user-facing documentation about optional snippet paths prefixes * Clarify that Snippet argument parsing problems are reported elsewhere * Fix typo in code comment * Update docs about earliest version with an optional snippet path prefix * Fix test that became flaky after recent snippets change (#1308) Depending on which symbol was _first_ in the dictionary of symbols, the entire symbol graph was categorized as either a snippet symbol graph or a regular symbol graph. However, real symbol graph files don't mix snippets and framework symbols like that. This fixes the test to don't mix snippets with real symbols. It also stops adding module nodes inside the symbol graph, because that also doesn't match real symbol graph files.

d-ronnqvist added 10 commits September 25, 2025 20:03

Separate snippets from other symbols

677f727

rdar://147926589 rdar://161164434 swiftlang#1280

Rename tests to clarify what they're verifying

64fac3c

Improve tests around optional snippet prefix components

552328a

Add additional test about resolving and rendering snippets

0ee9544

Make it easier to verify the diagnostic log output in tests

08d22c5

Add additional test about snippet warnings

757e3ef

Also, update the behavior when a snippet slice is misspelled to match what's described in the warning.

Move snippet diagnostic creation to resolver type

077b175

Add near-miss suggestions for snippet paths and slices

f70b35f

Only highlight the misspelled portion of snippet paths

82f0dfc

Update user-facing documentation about optional snippet paths prefixes

ceaab1c

d-ronnqvist requested a review from heckj September 26, 2025 13:34

This was linked to issues Sep 26, 2025

Snippet isn't consistently being recognized #1280

Closed

race condition with docc preview that appears to be acerbated by including snippets in DocC content #1084

Closed

Merge branch 'main' into separate-snippets-resolver

a50433d

QuietMisdreavus reviewed Sep 26, 2025

View reviewed changes

d-ronnqvist added 3 commits September 26, 2025 18:01

Clarify that Snippet argument parsing problems are reported elsewhere

afd6871

Merge branch 'main' into separate-snippets-resolver

474d5c0

Merge branch 'main' into separate-snippets-resolver

6fbe109

heckj approved these changes Sep 26, 2025

View reviewed changes

Sources/SwiftDocC/Infrastructure/Link Resolution/SnippetResolver.swift Outdated Show resolved Hide resolved

d-ronnqvist commented Sep 26, 2025

View reviewed changes

Fix typo in code comment

937f651

d-ronnqvist commented Sep 26, 2025

View reviewed changes

QuietMisdreavus approved these changes Sep 26, 2025

View reviewed changes

Update docs about earliest version with an optional snippet path prefix

45c0dd5

d-ronnqvist merged commit 527558e into swiftlang:main Sep 29, 2025
2 checks passed

d-ronnqvist deleted the separate-snippets-resolver branch September 29, 2025 08:15

This was referenced Sep 29, 2025

[6.2] Improve handling of snippet symbol graph files (#1302) #1306

Merged

[6.2.1] Improve handling of snippet symbol graph files (#1302) #1307

Merged

Fix test that became flaky after recent snippets change #1308

Merged

		XCTAssertEqual(context.knownIdentifiers.count, 1, "The snippets don't have their own identifiers", file: file, line: line)

		let reference = try XCTUnwrap(context.soleRootModuleReference, file: file, line: line)

Improve handling of snippet symbol graph files #1302

Improve handling of snippet symbol graph files #1302

Uh oh!

Conversation

d-ronnqvist commented Sep 26, 2025

Summary

Dependencies

Testing

Checklist

Uh oh!

d-ronnqvist commented Sep 26, 2025

Uh oh!

d-ronnqvist commented Sep 26, 2025

Uh oh!

QuietMisdreavus left a comment

Choose a reason for hiding this comment

Uh oh!

QuietMisdreavus Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

d-ronnqvist Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d-ronnqvist Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d-ronnqvist commented Sep 26, 2025

Uh oh!

heckj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

d-ronnqvist Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

d-ronnqvist Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

d-ronnqvist commented Sep 26, 2025

Uh oh!

d-ronnqvist commented Sep 26, 2025

Uh oh!

d-ronnqvist Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

d-ronnqvist Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

QuietMisdreavus left a comment

Choose a reason for hiding this comment

Uh oh!

d-ronnqvist commented Sep 28, 2025

Uh oh!

Uh oh!

Uh oh!

d-ronnqvist Sep 26, 2025 •

edited

Loading

d-ronnqvist Sep 26, 2025 •

edited

Loading