drop an "Extended Symbol" page when its children are curated elsewhere

[QuietMisdreavus]

Bug/issue #, if applicable: rdar://107729630

Summary

When Swift-DocC gets symbol graphs with extension block symbols, it creates "Extended Module/Class/Structure/etc" symbols to automatically curate the extension symbols into. However, if the extension symbols are then curated in the rest of the documentation hierarchy, these extra symbols are left in the topic graph and create empty pages that don't need to be there. This PR adds a pass to DocumentationContext.register(_:) to drop these symbols if all their children have been curated elsewhere. (This pass is done recursively, so "Extended Module" pages will drop if all their child "Extended Class/Structure/etc" pages are also dropped.)

Dependencies

None

Testing

The following code was used to test this functionality:

/// a namespace for organization.
///
/// ## Topics
///
/// - ``Swift/Array/asdf``
public enum MyNamespace {}

public extension Array {
  var asdf: Int { count }
}

Steps:

1. Build a symbol graph for the above code that includes extension block symbols. (The code above was used to create the ModuleWithSingleExtension test fixture added in this PR, which can be used as a shortcut.)
2. Add the resulting symbol graphs to a documentation catalog and generate documentation.
3. Ensure that the resulting documentation doesn't contain the "Extended Module" page for Swift, nor the "Extended Structure" page for Array.

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
• [ n/a ] Updated documentation if necessary

[QuietMisdreavus]

@swift-ci Please test

[theMomax]

Thanks for the PR, this is definitely a useful addition!

One thing I'd like to discuss, though, is how this change affects references to extension pages that get dropped. What would happen if you added a (non-curating) reference to ``Swift/Array`` to your test example (on a page that is not removed, e.g. ``ModuleWithSingleExtension``, or on a page that is also removed, e.g. ``Swift``).

Could you please document the intended behavior for both scenarios in a test case? I'm also not sure what the ideal behavior is, really.

In the case where the referencing page is also removed (``Swift`` links to ``Swift/Array``), I think we should just make sure that we don't generate a warning. This might very well already be the case with your implementation.

In the case where the referencing page is not removed (``ModuleWithSingleExtension`` links to ``Swift/Array``), I see three options:

1. We do not remove the referenced page, because the reference suggests it is still relevant.
2. We remove the referenced page, and we emit a warning with a specialised diagnostic for the actually "valid" reference.
3. We remove the referenced page, and silently downgrade the reference to a monospaced text without link.

[QuietMisdreavus]

@theMomax Thanks for the review! Right now it looks like the behavior for when you actually curate or reference a page is a bit awkward:

• If you merely reference an extension page without curating it, the link is still considered "live" but the destination page doesn't exist.
• If you curate it into the top-level page (i.e. ModuleWithSingleExtension) the same thing happens: The link is still there, but the page is not.
• If you curate it into a second-level page that's not also an extension (e.g. the MyNamespace enum in the demo project i added) then that copy of the page still exists, alongside all its children! This is because my code doesn't recurse into non-extension pages, so it sees an enum and skips it, despite there being an extension curated in there.

I think what i would want to happen in the case of a non-curating reference is your option (2), with maybe some extra logic for if there are extension files for the extension pages that keep them "live". I'll take a look at adding these suggestions.

[QuietMisdreavus]

I just pushed a couple commits to handle @theMomax's review comments. They've added the following functionality:

1. Links to "Extended Symbol" pages which have been removed will generate a warning: "The topic 'TopicName' is an empty extension page and cannot be linked to." I'm not 100% confident in the diagnostic replacement system to be able to recommend dropping the symbol link to an inline code span (or some other kind of recommendation), so i left that blank for now.
2. "Extended Symbol" pages which have a documentation extension file of their own are not dropped, even if all their children have been canonically curated elsewhere. This also means that symbol links to them will not generate a warning since they will properly resolve.

[QuietMisdreavus]

@swift-ci Please test

[QuietMisdreavus]

@swift-ci Please test

[ethan-kusters]

I'm seeing an issue with symbols that are included in relationships sections. Here I built ArgumentParser with:

swift package --disable-sandbox preview-documentation --target ArgumentParser --include-extended-types --experimental-skip-synthesized-symbols

The resulting docs are much nicer now (they exclude all of the empty Bool, Int64, etc pages) but now I'm seeing broken links in the relationships sections for ExpressibleByArgument. When I click on one of the links I end up on a non-existent page.

Ideally I think these symbols would still appear but as monospace references instead:

### Conforming Types

`Swift.Bool`
`Swift.Double`
...

[QuietMisdreavus]

@ethan-kusters Oh right, i wanted to look into that! 😅 I don't know offhand where those are rendered, but i can look at that next.

[QuietMisdreavus]

@swift-ci Please test

[QuietMisdreavus]

@swift-ci Please test

[QuietMisdreavus]

@swift-ci Please test

[ethan-kusters]

I'm not sure if it should be resolved yet, but even with the latest changes I'm still able to reproduce the issue I was seeing with ArgumentParser.

[QuietMisdreavus]

@ethan-kusters Oh interesting! I fixed the broken links when you reference them from a symbol link, but not when they're automatically inserted like that. Let me take a look.

[QuietMisdreavus]

@ethan-kusters I just pushed up a solution. Can you try it now?

[QuietMisdreavus]

@swift-ci Please test

[ethan-kusters]

@ethan-kusters I just pushed up a solution. Can you try it now?

Woot! Looks great.

In other cases like this we include the module namespace (i.e. Swift.Foo):

Is it simple to do the same thing here? If not – we can definitely leave it for a future PR.

[QuietMisdreavus]

That seems doable - the title defers to the LinkTitleResolver, which itself seems to defer to the symbol information. However, i feel like this PR has ballooned a little bit and the current state of things is serviceable for the moment. I can file an issue to follow up if you'd like.

[ethan-kusters]

However, i feel like this PR has ballooned a little bit and the current state of things is serviceable for the moment. I can file an issue to follow up if you'd like.

That would be great. Thank you!

[QuietMisdreavus]

@ethan-kusters Just filed #554 to track it.

Review comments addressed