Skip to content

Lower severity of source-symbol-not-found diagnostic #966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

d-ronnqvist
Copy link
Contributor

Bug/issue #, if applicable:

Summary

This PR makes two changes to the diagnostic when the source symbol of a symbol graph relationship can't be found:

  • Elaborate in the summary and explanation to clarify that this is an issue with the tool that created the symbol graph and not something that the developer can resolve themselves.
  • Lower the severity of this diagnostic so that this issue doesn't fail the build.

Since the solution to this issue is for the tool that created the symbol graph file to not include this relationship (or move it to a different file), it's safe for DocC to proceed without processing this symbol graph relationship.

Dependencies

None.

Testing

  • Manually edit a symbol graph file to add a relationship where the "source" isn't a symbol ID for a symbol defined in that symbol graph file.
  • Build documentation for that symbol graph file.
  • There should be a warning, not an error, about the incorrect symbol graph relationship.

Checklist

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

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

…relationship is missing.

Since the solution is for the tool that created the symbol graph file to not emit that relationship, DocC can safely skip it and proceed with the build. This issue isn't severe enough to fail the build.
@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

source: nil, severity: .warning, range: nil,identifier: "org.swift.docc.SymbolNodeNotFound",
summary: "Source symbol \(relationship.source.singleQuoted) not found locally, from \(relationship.kind.rawValue.singleQuoted) relationship to \(relationship.target.singleQuoted)",
explanation: """
The "source" of a symbol graph relationship should always refer to a symbol in the same symbol graph file.
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing we might want to consider here is direct the user to file a bug report on Swift/Clang with information that can help diagnose the issue (in our case, the symbol graph file or at least a portion of it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was thinking of that but at the point where we're creating these diagnostics we no longer know which symbol graph file a given relationship came from.

We could include information about how to report an issue for both Swift and Clang and let the developer decide which to file a bug report on.

@d-ronnqvist
Copy link
Contributor Author

After an offline discussion about this PR I've further reduced the severity to a debug-only assertion in 2e64b4b.

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

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

Looks great! I'm glad we're taking out this error; it's confusing and unactionable, and creates a situation where an issue in other tools causes Swift-DocC to fail, when it could just as easily continue on.

@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit b54cb7a into swiftlang:main Jul 15, 2024
2 checks passed
@d-ronnqvist d-ronnqvist deleted the symbol-not-found-diagnostic-message branch July 15, 2024 08:30
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