Add symbolGraphMinimumAccessLevel option support

[Kyle-Ye]

Bug/issue #, if applicable:

Close #11

Summary

Add symbolGraphMinimumAccessLevel option support.

The current behavior for it is for library target we assume it to be public and for executable target we assume it to be internal implemented in defaultSymbolGraphOptions.

I propose to add symbol-graph-minimum-access-level option in command line so that we can opt-out the default internal behavior for executable target.

The usage is we want to documentation the executable target via docc bundle but we do not want to expose the internal symbol graph into the documentation.

See apple/swift-mmio#104 (comment)

Dependencies

None

Testing

See ArgumentsSymbolGraphMinimumAccessLevelTests.swift

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

Other Link

• Forum proposal: https://forums.swift.org/t/swift-docc-plugin-pitch-add-symbolgraphminimumaccesslevel-option-support/72212

[Kyle-Ye]

@swift-ci please test

[heckj]

As the individual who opened #11, I'm hugely in favor of this. I don't know what the intended pattern is in calling a command line option "experimental", but since this has a stable part of the API of DocC for some time, and is actively used and adjusted with "OTHER DOCC FLAGS" in a number of places, I'd very much prefer to drop the "experimental" from the CLI option.

[Kyle-Ye]

I don't know what the intended pattern is in calling a command line option "experimental"

Oh, I think we'll eventually not need experimental too. It's the suggestion I got from @ethan-kusters on swift-docc Slack channel.

Reply from Slack channel:
There's not - but #11 is tracking the issue. A PR would be great! Maybe we should start with an --experimental prefixed flag? Ideally I think we'd have configuration in the Package.swift itself.

[sofiaromorales]

Thanks for opening this PR @Kyle-Ye! I'll be reviewing this in the upcoming days :)

[d-ronnqvist]

Would it make sense for this property to return SymbolGraphOptions.AccessLevel? instead? That way the two call sites wouldn't need to use .flatMap { .init(rawValue: $0) }

[Kyle-Ye]

I'd like to. But SymbolGraphOptions.AccessLevel is defined in a module we can't access here.

[d-ronnqvist]

I see. Should we move this into SharedPackagePluginExtensions then so that it can access the necessary types?

[Kyle-Ye]

1. This is an alignment with Arguments+outputPath.swift.
2. If we move this into SharedPackagePluginExtensions , we could not access CommandLineOption anymore unless we move CommandLineOption too or remove CommandLineOption.symbolGraphMinimumAccessLevel dependency.
3. But in general, I agree your idea here.

[d-ronnqvist]

nit: This is a new file

Suggested change

	// Copyright (c) 2022 Apple Inc. and the Swift project authors
	// Copyright (c) 2024 Apple Inc. and the Swift project authors

[Kyle-Ye]

Nice catch.

[d-ronnqvist]

One comment and a question:

• I don't think this needs to say "experimental".
• Should we also define this in a PluginFlag so that it is listed in the swift package generate-documentation --help output as a "plugin option"?

[Kyle-Ye]

I don't think this needs to say "experimental".

+1 for this.

A PR would be great! Maybe we should start with an --experimental prefixed flag?

What do you think about it? cc @ethan-kusters

Should we also define this in a PluginFlag so that it is listed in the swift package generate-documentation --help output as a "plugin option"?

I will give some check on this part.

[rauhul]

Any updates on this? I'd love to adopt it in swift-mmio

[Kyle-Ye]

Any updates on this? I'd love to adopt it in swift-mmio

I'll take some time later this week to resolve the feedback I receive from the reviewers.

[d-ronnqvist]

Any updates on this? I'd love to adopt it in swift-mmio

I'll take some time later this week to resolve the feedback I receive from the reviewers.

There isn't any blocking feedback. I'd be happy to merge this as is and look at surfacing the new flag in the help text in a later PR.

[d-ronnqvist]

LGTM

[rauhul]

Can we rebase and merge this?

[Kyle-Ye]

There isn't any blocking feedback. I'd be happy to merge this as is and look at surfacing the new flag in the help text in a later PR.

Not have time to address all the PR comments. Maybe we can address them later since they are implementation details and there are downstream usage waiting this PR.

Can we rebase and merge this?

Done. I'd like to request @d-ronnqvist for a final approval since I removed experimental prefix.

[Kyle-Ye]

@swift-ci please test