Skip to content

Conversation

@d-ronnqvist
Copy link
Contributor

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

Summary

This adds a warning when an article's output JSON file would override the output JSON file of a symbol. To get a deterministic behavior across builds, DocC favors the symbol over the article.

Other tests that verify behavior that happens earlier than the JSON rendering need to be modified to expect this behavior until rdar://79745455 / #593 is fixed.

Dependencies

None

Testing

In a project with some symbol documentation:

  • Add an article with the same file name (except the file extension) as one of the top-level symbols.
  • Build documentation
    • There should be a warning about the article colliding with the symbol, suggesting to rename the article
    • The rendered documentation should contain the symbol page but not the article page (until it's renamed)

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

Copy link
Contributor

@patshaughnessy patshaughnessy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

Should we make this warning an error instead? Dropping an article entirely is a fairly dramatic problem, and maybe the author would miss the warning otherwise? ...for example if a Symbol was later introduced that happened to have the same name as an existing article?

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist
Copy link
Contributor Author

Should we make this warning an error instead? Dropping an article entirely is a fairly dramatic problem, and maybe the author would miss the warning otherwise? ...for example if a Symbol was later introduced that happened to have the same name as an existing article?

I don't think this should be an error.

This is how we've defined the error and warning severities:

An error severity should be used if further progress can't be made in some process or workflow.

A warning severity should be used for issues that don't block progress but the user ought to address as soon as possible.

In other words; errors stop the entire build and cause DocC to produce no output but warnings lets the build continue and allow DocC to produce some output.


If we were to make this an error, then any project who was impacted by this issue but wasn't aware of it would completely fail to build from one version to the next. So, at the very least I'd think that we'd need to start as a warning to give developers time to address the issue before we break their documentation builds.


Before adding this warning we would still drop a page. It would just be silent (no warning) and it would be nondeterministic if it was the symbol or the article that was lost.


As far as I know; we don't have any actual error-severity diagnostics today. The only diagnostics that use an error severity as for unexpected errors in code that can't throw (during rendering and navigator indexing) but these represent errors like failing to write a file to disk, that don't really fit the purpose of a Diagnostic and that would still fail the build as a thrown Error.

A diagnostic explains a problem or issue that needs the end-user's attention.


We have other warning severity diagnostics today that drop entire files, for example:

  • if a documentation extension file doesn't match any known symbol, DocC drops that file
  • if multiple documentation extension files match the same symbol, DocC drops all but one of those files
  • if multiple articles have the same file name, DocC drops all but one of those articles
  • if multiple assets (images or videos) have the same file name, DocC drops all but one of those assets

Today, developers can pass the --warnings-as-errors flag if they want DocC's warnings to fail the build. In the future it would be good to offer developers more control over which diagnostics they want to treat as errors but until we have that, I think the right call is to make this a warning.

@d-ronnqvist d-ronnqvist merged commit 0b356e8 into swiftlang:main Nov 12, 2025
2 checks passed
@d-ronnqvist d-ronnqvist deleted the warn-when-article-overrides-symbol branch November 12, 2025 14:23
@heckj
Copy link
Member

heckj commented Nov 12, 2025

(for what it's worth after the fact, the "soundness" checks in the GitHub Actions for swiftlang projects explicitly run the documentation builds with --warning-as-error to identify problems such as this one, so a warning is perfect - allows the developer to iterate quickly, but there's a net to catch with the soundness checks)

@d-ronnqvist
Copy link
Contributor Author

I opened this draft PR to allow developers to control the severity of specific diagnostics. I'll start a Forum discussion about it shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants