Skip to content
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

Allow for emission of swift.extension symbols for extensions to external types in SymbolGraphGen #59047

Merged

Conversation

theMomax
Copy link
Contributor

This PR introduces a new mode to the Symbol Graph generation that changes how extensions to external types are handled. Since we are explicitly only talking about extensions to external types, all symbols and relationships under consideration live in Symbol Graph Files with names of the form A@B.symbols.json.

Any symbols resulting from such an extension are no longer directly associated with the extended type (which does not exist in the same Symbol Graph File), but with a new kind of symbol, the swift.extension symbol. There is one such symbol for every extension block that extends an external type. This swift.extension symbol is also connected to the symbol of the type it extends via an extensionTo relationship.

These changes are directly related to this discussion about the Symbol Graph Format. This initial pitch motivates the feature and gives more context on why these changes are necessary.

List of changes:

  • introduction of the swift.extension symbol and extensionTo relationship
  • adding support for ExtensionDecl to the Symbol class
  • adding a typeKind field to the symbol's extension mixin which indicates what kind of
    symbol was extended
  • introduction of the -emit-extension-block-symbols flag, which enables the behavior
    outlined below
  • adaptions to SymbolGraphASTWalker that ensure a swift.extension symbol is emitted
    for each extension to a type that does not exist in the local symbol graph
  • adaptions to SymbolGraph and SymbolGraphASTWalker that ensure relationships are
    correctly associated with the swift.extension symbol instead of the original type
    declaration's (extended nominal's) symbol where applicable
  • adaptions to SymbolGraphASTWalker that ensure swift.extension symbols are connected
    to their respective extended nominal's symbol using an extensionTo relationship

Overall, these changes should ensure that:

  • documentation frameworks can access the documentation comments on top of extension blocks for extensions to external types
  • documentation frameworks can reconstruct which members/conformances are associated with a certain extension block (and its documentation)
  • documentation frameworks can know what kind of type was extended (struct/class/enum/protocol)
  • legacy documentation frameworks don't observe a breaking change (as all breaking changes are hidden behind the CLI flag)

This PR can be seen as a first step to fixing SR-15410, but more changes to other repositories further up the stack will be necessary.

theMomax added a commit to theMomax/swift-docc-symbolkit that referenced this pull request Jun 9, 2022
…nd make graph format extensible

 - add symbol and relationship kinds introduced in apple/swift #59047 (swiftlang/swift#59047)
 - change Symbol.KindIdentifier to use a struct-type with static properties for known values
 - change Codable conformance of Symbol and Relationship to allow for encoding and decoding unknown Mixins
@theMomax theMomax marked this pull request as ready for review June 17, 2022 13:40
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.

This looks really nice, thanks! I have a couple of minor comments. Was there anything you wanted to add to this implementation before building on it in SymbolKit and Swift-DocC?

Also, some of the test files in this PR are missing the end-of-file newline. GitHub labels this as a little ⛔ icon at the end of the file. This shouldn't affect the tests building and passing, but Git (and other tools that expect it, like Vim) will make a note of it when looking at them.

lib/SymbolGraphGen/SymbolGraphASTWalker.cpp Outdated Show resolved Hide resolved
lib/SymbolGraphGen/SymbolGraph.cpp Outdated Show resolved Hide resolved
@theMomax theMomax changed the title [WIP] Allow for emission of swift.extension symbols for extensions to external types in SymbolGraphGen Allow for emission of swift.extension symbols for extensions to external types in SymbolGraphGen Jul 1, 2022
@theMomax
Copy link
Contributor Author

theMomax commented Jul 1, 2022

@QuietMisdreavus thanks for your review! This PR is complete in terms of features. I will add/adapt a test in regards to the Clang modules and modify the conditions accordingly if necessary. Once you and @daniel-grumberg are happy with code style/quality this PR can be merged.

@QuietMisdreavus
Copy link
Contributor

@swift-ci Please smoke test

1 similar comment
@QuietMisdreavus
Copy link
Contributor

@swift-ci Please smoke test

@theMomax theMomax force-pushed the emit-extension-block-symbols branch 2 times, most recently from 4858847 to c0ec7b7 Compare July 6, 2022 10:25
theMomax added a commit to theMomax/swift-docc-symbolkit that referenced this pull request Jul 7, 2022
theMomax added a commit to theMomax/swift-docc-symbolkit that referenced this pull request Jul 7, 2022
theMomax added a commit to theMomax/swift-docc-symbolkit that referenced this pull request Jul 7, 2022
theMomax added a commit to theMomax/swift-docc-symbolkit that referenced this pull request Jul 7, 2022
…nd make graph format extensible

 - add symbol and relationship kinds introduced in apple/swift #59047 (swiftlang/swift#59047)
 - change Symbol.KindIdentifier to use a struct-type with static properties for known values
 - change Codable conformance of Symbol and Relationship to allow for encoding and decoding unknown Mixins
@theMomax theMomax force-pushed the emit-extension-block-symbols branch 2 times, most recently from 8990940 to 27003e3 Compare July 8, 2022 17:58
theMomax added a commit to theMomax/swift-docc-symbolkit that referenced this pull request Jul 13, 2022
theMomax added a commit to theMomax/swift-docc-symbolkit that referenced this pull request Jul 13, 2022
…nd make graph format extensible

 - add symbol and relationship kinds introduced in apple/swift #59047 (swiftlang/swift#59047)
 - change Symbol.KindIdentifier to use a struct-type with static properties for known values
 - change Codable conformance of Symbol and Relationship to allow for encoding and decoding unknown Mixins
@daniel-grumberg
Copy link
Contributor

@swift-ci Please smoke 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.

Thanks so much! This looks ready to go for me; all the comments i left are pretty minor.

...however, it does look like i introduced another merge conflict. 😭 It should be pretty straightforward to fix, and once that's done and tested, i think we can land this!

lib/SymbolGraphGen/SymbolGraphASTWalker.cpp Outdated Show resolved Hide resolved
lib/SymbolGraphGen/SymbolGraphASTWalker.cpp Outdated Show resolved Hide resolved
lib/SymbolGraphGen/Symbol.cpp Outdated Show resolved Hide resolved
test/SymbolGraph/Symbols/Unavailable.swift Outdated Show resolved Hide resolved
theMomax added a commit to theMomax/swift-docc that referenced this pull request Sep 14, 2022
…tlang/swift#59047

 - introduce transformation generating internal Extended Types Symbol Graph Format from
   the Extension Block Symbol Graph Format emmitted by the compiler
 - register symbols and relationships used by Extended Types Symbol Graph Format
 - adapt reference collision detection logic to deal with emitted path components that
   occur when extending external nested types
 - adapt reference resolution logic to first search for paths in the local module
   by default and introduce shorthand absolute syntax with "/" prefix to mark paths
   as absolute (to be searched for only in the global scope)
 - adapt SymbolGraphLoader to automatically detect if input Symbol Graph Files use the
   Extension Block Symbol Graph Format and apply the ExtendedTypesFormatTransformation
   in case
 - improve Swift title token parsing to correctly identify Symbol titles of nested types,
   which contain "." infixes
theMomax added a commit to theMomax/swift-docc that referenced this pull request Sep 14, 2022
…tlang/swift#59047

 - introduce transformation generating internal Extended Types Symbol Graph Format from
   the Extension Block Symbol Graph Format emmitted by the compiler
 - register symbols and relationships used by Extended Types Symbol Graph Format
 - adapt reference collision detection logic to deal with emitted path components that
   occur when extending external nested types
 - adapt reference resolution logic to first search for paths in the local module
   by default and introduce shorthand absolute syntax with "/" prefix to mark paths
   as absolute (to be searched for only in the global scope)
 - adapt SymbolGraphLoader to automatically detect if input Symbol Graph Files use the
   Extension Block Symbol Graph Format and apply the ExtendedTypesFormatTransformation
   in case
 - improve Swift title token parsing to correctly identify Symbol titles of nested types,
   which contain "." infixes
theMomax added a commit to theMomax/swift-docc that referenced this pull request Sep 14, 2022
…tlang/swift#59047

 - introduce transformation generating internal Extended Types Symbol Graph Format from
   the Extension Block Symbol Graph Format emmitted by the compiler
 - register symbols and relationships used by Extended Types Symbol Graph Format
 - adapt reference collision detection logic to deal with emitted path components that
   occur when extending external nested types
 - adapt reference resolution logic to first search for paths in the local module
   by default and introduce shorthand absolute syntax with "/" prefix to mark paths
   as absolute (to be searched for only in the global scope)
 - adapt SymbolGraphLoader to automatically detect if input Symbol Graph Files use the
   Extension Block Symbol Graph Format and apply the ExtendedTypesFormatTransformation
   in case
 - improve Swift title token parsing to correctly identify Symbol titles of nested types,
   which contain "." infixes
…nal types in swiftSymbolGraphGen

This includes:
 - bumping the SWIFT_SYMBOLGRAPH_FORMAT_MINOR version
 - introduction of the "swift.extension" symbol and "extensionTo" relationship
 - adding support for ExtensionDecl to the Symbol class
 - adding a "typeKind" field to the symbol's extension mixin which indicates what kind
   of symbol was extended
 - intoduction of the -emit-extension-block-symbols flag, which enables the behavior
   outlined below
 - adaptions to SymbolGraphASTWalker that ensure a swift.extension symbol is emitted
   for each extension to a type that does not exist in the local symbol graph
 - adaptions to SymbolGraph and SymbolGraphASTWalker that ensure member and conformance
   relationships are correctly associated with the swift.extension symbol instead of
   the original type declaration's (extended nominal's) symbol where applicable
 - adaptions to SymbolGraphASTWalker that ensure swift.extension symbols are connected
   to their respective extended nominal's symbol using an extensionTo relationship

Testing:
- adds SymbolGraph tests that test behavior only relevant in
  -emit-extension-block-symbols mode
- adapts some SymbolGraph tests to additionally test similar behavior for
  extensions to external types in -emit-extension-block-symbols mode
- adapts some SymbolGraph tests to (additionally or exclusively) test the
  behavior with -emit-extension-block-symbols mode enabled

Bugfixes:
- fixes a bug where some conformsTo relationships implicated by the conformances
  declared on an extension to an external type were not emitted
  (see test/SymbolGraph/Relationships/ConformsTo/Indirect.swift)

Further changes:
- documents the strategy for naming and associating children declared in extensions
  to typealiases (see test/SymbolGraph/Relationships/MemberOf/Typealias.swift,
  test/SymbolGraph/Symbols/Names.swift)
@theMomax
Copy link
Contributor Author

Rebased, fixed conflicts, addressed your remaining comments, and squashed again. Thanks for the final review @QuietMisdreavus ! Can you please kick off the CI again?

@QuietMisdreavus
Copy link
Contributor

@swift-ci Please smoke test

@QuietMisdreavus
Copy link
Contributor

@theMomax Thanks so much! As soon as CI is green, i'll merge it.

QuietMisdreavus pushed a commit to swiftlang/swift-docc-symbolkit that referenced this pull request Sep 14, 2022
* introduce "extension" symbol kind + "extensionTo" relationship kind and make graph format extensible

 - add symbol and relationship kinds introduced in apple/swift #59047 (swiftlang/swift#59047)
 - change Symbol.KindIdentifier to use a struct-type with static properties for known values
 - change Codable conformance of Symbol and Relationship to allow for encoding and decoding unknown Mixins

* use Coder's userInfo instead of static property for storing and registering unknown Mixin types

* add identifier storage to ensure language prefixes are treated as usual

* add tests for custom Mixin coding + Relationship's Hashable conformance

* allow registering custom Symbol kinds to Decoder + add documentation

* add test checking Symbol.KindIdentifier.allCases

* remove CustomizableCoder protocol

* refactor handling of mixin coding errors

* move decoding/encoding closure from CodingKeys to MixinCodingInformation + unify error handling closures and coding closures

* mention that KindIdenifier/register(_:) should not be used concurrently in docs

* refactor Mixin Hashable conformance + improve code-docs

* add relationship hashing/equality test cases

* fix CodingUserInfoKey values to use org.swift prefix
@d-ronnqvist
Copy link
Contributor

Apologies for the late comment. Can we also add a new element in lib/Option/features.json? That help build systems know when they have a version of the Swift compiler that supports this new flag so that they don't need to hard code what version numbers support what flags.

It doesn't have to be done in this PR if you prefer to land this now and open a new PR that update features.json.

@theMomax
Copy link
Contributor Author

theMomax commented Sep 15, 2022

@d-ronnqvist if you don’t mind, I‘d prefer to update the features.json with my next PR, which will add support for default implementation relationships. Before that, the extension block symbol feature isn’t really „complete“ anyway. I cut that part out of this PR as the change set had become quite large already on this PR and the accompanying swift-docc PR. Don’t worry, though, the follow up PR won’t be large!

theMomax added a commit to theMomax/swift-docc that referenced this pull request Sep 15, 2022
…tlang/swift#59047

 - introduce transformation generating internal Extended Types Symbol Graph Format from
   the Extension Block Symbol Graph Format emmitted by the compiler
 - register symbols and relationships used by Extended Types Symbol Graph Format
 - adapt SymbolGraphLoader to automatically detect if input Symbol Graph Files use the
   Extension Block Symbol Graph Format and apply the ExtendedTypesFormatTransformation
   in case
 - improve Swift title token parsing to correctly identify Symbol titles of nested types,
   which contain "." infixes

added tests:
 - test detection of the Extension Block Symbol Graph Format and application of the
   ExtendedTypesFormatTransformation in SymbolGraphLoader
 - test the ExtendedTypesFormatTransformation
 - extend tests for Swift title token parsing with test-cases containing "." infixes
theMomax added a commit to theMomax/swift-docc that referenced this pull request Sep 16, 2022
…tlang/swift#59047

 - introduce transformation generating internal Extended Types Symbol Graph Format from
   the Extension Block Symbol Graph Format emmitted by the compiler
 - register symbols and relationships used by Extended Types Symbol Graph Format
 - adapt SymbolGraphLoader to automatically detect if input Symbol Graph Files use the
   Extension Block Symbol Graph Format and apply the ExtendedTypesFormatTransformation
   in case
 - improve Swift title token parsing to correctly identify Symbol titles of nested types,
   which contain "." infixes

added tests:
 - test detection of the Extension Block Symbol Graph Format and application of the
   ExtendedTypesFormatTransformation in SymbolGraphLoader
 - test the ExtendedTypesFormatTransformation
 - extend tests for Swift title token parsing with test-cases containing "." infixes
@QuietMisdreavus
Copy link
Contributor

I'll go ahead and merge this, since it's all ready to go and doesn't affect anything unless you add the new flag.

@QuietMisdreavus QuietMisdreavus merged commit 453fd22 into swiftlang:main Sep 16, 2022
theMomax added a commit to theMomax/swift-docc that referenced this pull request Sep 16, 2022
…tlang/swift#59047

 - introduce transformation generating internal Extended Types Symbol Graph Format from
   the Extension Block Symbol Graph Format emmitted by the compiler
 - register symbols and relationships used by Extended Types Symbol Graph Format
 - adapt SymbolGraphLoader to automatically detect if input Symbol Graph Files use the
   Extension Block Symbol Graph Format and apply the ExtendedTypesFormatTransformation
   in case
 - improve Swift title token parsing to correctly identify Symbol titles of nested types,
   which contain "." infixes

added tests:
 - test detection of the Extension Block Symbol Graph Format and application of the
   ExtendedTypesFormatTransformation in SymbolGraphLoader
 - test the ExtendedTypesFormatTransformation
 - extend tests for Swift title token parsing with test-cases containing "." infixes
d-ronnqvist pushed a commit to swiftlang/swift-docc that referenced this pull request Sep 16, 2022
…tlang/swift#59047 (#369)

- introduce transformation generating internal Extended Types Symbol Graph Format from
   the Extension Block Symbol Graph Format emmitted by the compiler
 - register symbols and relationships used by Extended Types Symbol Graph Format
 - adapt SymbolGraphLoader to automatically detect if input Symbol Graph Files use the
   Extension Block Symbol Graph Format and apply the ExtendedTypesFormatTransformation
   in case
 - improve Swift title token parsing to correctly identify Symbol titles of nested types,
   which contain "." infixes

added tests:
 - test detection of the Extension Block Symbol Graph Format and application of the
   ExtendedTypesFormatTransformation in SymbolGraphLoader
 - test the ExtendedTypesFormatTransformation
 - extend tests for Swift title token parsing with test-cases containing "." infixes
ahoppen added a commit to ahoppen/swift-source-compat-suite that referenced this pull request Sep 20, 2022
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.

4 participants