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

Don't document anything SPI-public #1263

Closed
mludowise-stripe opened this issue Jul 10, 2021 · 4 comments
Closed

Don't document anything SPI-public #1263

mludowise-stripe opened this issue Jul 10, 2021 · 4 comments

Comments

@mludowise-stripe
Copy link

mludowise-stripe commented Jul 10, 2021

We recently started Swift's new SPI (System Programming Interfaces) support to expose classes & methods to other modules within our ecosystem. To use this, we annotate our SPI-available classes with @_spi(STP) public class MyClass {...} (where STP is our own namespace). Here's an example in our repo.

Our intent is that no one should use anything SPI-exposed with the STP namespace in our API except for the modules that we own. So we don't want anything that is exposed using @_spi(STP) to be displayed in our docs.

Currently, we're able to avoid most of SPI-conforming classes, types, and methods from getting exposed in our docs by using the :nodoc:, however any public class that conforms to an SPI-public protocol still displays its protocol conformance like this:
image

Steps to reproduce:

  1. git clone git@github.com:stripe/stripe-ios.git && cd stripe-ios && checkout origin/mludowise/spi_jazzy_example
  2. ./ci_scripts/build_documentation.sh
  3. open docs/docs/Classes/STPApplePayContext.html

This is the git branch containing my example: https://github.com/stripe/stripe-ios/tree/mludowise/spi_jazzy_example

Proposal

Omit anything annotated with @_spi(...) from generated docs by default.

You could add an optional flag like --include-swift-spi namespace,namespace,... if the documenter wants the ability to explicitly include SPI documentation for specific namespaces. Although, typically the purpose of a System Programming Interfaces is for internal uses, so I'm doubtful that it's necessary or useful to include a flag like this.

I think the solution would involve skipping documentation for anything that has @_spi in the key.parsed_declaration entry from sourcekitten, but I'm not completely sure about that.

@johnfairh
Copy link
Collaborator

We should do this -- I was hoping that @_spi would be made non-underscored this year and get some proper tooling support to make the implementation easier but will have to do without.

I think it's safer to include the flag, users find all kinds of ways of using jazzy including internal documentation and not including a way to say 'include all public APIs including SPI' feels like pushing work down the line (eg. docs for someone who has to consume the SPI in question).


In the case you mention above of not wanting to see the STPAnalyticsProtocol conformance, you should be able to :nodoc: the extension that adds it, something like

/// :nodoc:
@_spi(foo) extension C : P {
...
}

It's probably a bug in :nodoc: if this doesn't work.

@mludowise-stripe
Copy link
Author

Good point about making it an option to include SPI documentation. I feel like there could be some wonky edge cases with trying to explicitly include some SPI name spaces but not others...

For example, what is the expected behavior if you pass --include-swift-spi FOO for the following:

@_spi(FOO) @_spi(BAR) public class MyClass {...}

Perhaps making it a simple boolean flag where either all or no SPI-public APIs are included in documentation would suffice.


Thanks for noting we should be able to use :nodoc:. I swear I tried this before and I couldn't get working, but I must have been doing something wrong because now it seems to be working 🤪. Thank you!

@johnfairh
Copy link
Collaborator

I've taken a closer look at @_spi and made a first attempt at Jazzy support.

I agree with you that getting into the details of SPI groups is way too much right now: the implementation is pretty shaky and I'd expect it to change on the way to becoming a real Swift feature. And the main Jazzy feature is as originally requested, to just exclude them: so we'll go with a single flag to opt back in any @_spi.

@johnfairh
Copy link
Collaborator

Fixed in master.

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

No branches or pull requests

2 participants