-
Notifications
You must be signed in to change notification settings - Fork 160
(#1257) Combined module link issue - public extension of dependent module causes resolution failure #1327
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this PR and thanks for being upfront about the use of AI assistance.
The new test passes without the changes in PathHierarchy+Find which makes me think that either the test isn't reproducing the issue or possibly that the change isn't doing anything. Can you update the test so that if fails like in #1257 (without the fix and passes with the fix).
| } | ||
|
|
||
|
|
||
| func testAbsoluteLinksToOtherModuleWithExtensions() async throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test passes even without the changes in PathHierarchy+Find
1e112bb to
bc86b59
Compare
Thank you for the feedback and for catching the test behavior! I dug deeper for better understanding. So the test now makes sure that in-module paths are found and that paths leading to "outside" module can't be found and throw an error, which is expected (to be resolved later when modules are merged). |
…endent module causes resolution failure
The basic setup is this:
```swift
// ImportedModule
struct ExtendedType {}
struct BaseType {}
```
```swift
// MainModule
import ImportedModule
extension ExtendedType {
func extensionMethod()
}
```
With the above we have the following issues:
- `/ImportedModule/ExtendedType` **always** resolves to the type extension link
- `/ImportModule/BaseType` links are broken
Prioritize absolute links resolution to point to the module referenced in the link, so that `/ImportedModule/BaseType` finds the base type because the path starts with `/`.
To refer to the extended type or method, the non-absolute paths work in Xcode and with docc: `ImportedModule/ExtendedType` resolves to the type extension page, `ImportedModule/ExtendedType/extensionMethod()` resolves to the extension method page.
> ‼️ 🤖 Disclaimer: this is an AI-assisted change.
While AI was utilized to identify and apply the initial fix and create initial version of the tests, the code has been reviewed and modified where needed.
More importantly, I have tested the changes both with the sample project attached to the original issue and with a real work project where I'm using `xcodebuild docbuild`.
In the real world project I'm dealing with similar setup.
`ParentFramework` imports `ChildFramework` and extends `ChildType` and then uses paths like `/ChildFramework/ChildType` in the documentation.
By passing `DOCC_EXEC` build setting to `xcodebuild docbuild` invocation along with `--enable-experimental-combined-documentation` and other required flags I was able to produce doc archives for the 2 modules, then `docc merge` them into one and in the resulting doc archive all cross-linking works as expected.
I can also ue `ChildFramework/ChildType/extensionMethod()` links to create reference to the extension method doc page.
bc86b59 to
4343282
Compare
Bug/issue #, if applicable: #1257
Summary
The basic setup is this:
With the above we have the following issues:
/ImportedModule/ExtendedTypealways resolves to the type extension link/ImportModule/BaseTypelinks are broken/This/Does/Not/WorkFix
Prioritize absolute links resolution to point to the module referenced in the link, so that
/ImportedModule/BaseTypefinds the base type because the path starts with/.To refer to the extended type or method, the non-absolute paths work in Xcode and with docc:
ImportedModule/ExtendedTyperesolves to the type extension page,ImportedModule/ExtendedType/extensionMethod()resolves to the extension method page.Dependencies
N/A
Testing
While AI was utilized to identify and apply the initial fix and create initial version of the tests, the code has been reviewed and modified where needed.
More importantly, I have tested the changes both with the sample project attached to the original issue and with a real work project where I'm using
xcodebuild docbuild.In the real world project I'm dealing with similar setup.
ParentFrameworkimportsChildFrameworkand extendsChildTypeand then uses paths like/ChildFramework/ChildTypein the documentation.By passing
DOCC_EXECbuild setting toxcodebuild docbuildinvocation along with--enable-experimental-combined-documentationand other required flags I was able to produce doc archives for the 2 modules, thendocc mergethem into one and in the resulting doc archive all cross-linking works as expected. I can also ueChildFramework/ChildType/extensionMethod()links to create reference to the extension method doc page.Steps for testing with the sample project:
swift build./gen-docs.sh.build/plugins/Swift-DocC/outputs/intermediates/SubOne.doccarchiveClassOnedoes not resolve the link and looks like this:./gen-docs.shby adding the following at the start:./gen-docs.shagain.build/plugins/Swift-DocC/outputs/intermediates/SubOne.doccarchiveagainClassOnehas the/Core/Modulelink rendered properlyChecklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded