[SourceKit] Add accessibility in extension structure (SR-4823)

[marcelofabri]

Adds the key.accessibility key in source.lang.swift.decl.extension structure.

Resolves SR-4823.

[xwu]

...I may be misunderstanding what you're going for here, but as discussed previously on Swift-Evolution, extensions don't have their own accessibility. Sometimes, it is allowed to write public extension, for example, but that is intended to be a shorthand for all members being public. The extension itself continues to have no accessibility.

[marcelofabri]

I know that formally extensions themselves doesn't have accessibility, but since public extension is allowed, I think that information should be available in the structure for consumers.

For example, SwiftLint has a rule to enforce that if all the members from an extension have the same accessibility, you should use the shorthand form. Currently, the implementation needs to look for all tokens with attributeBuiltin kind, check if it's an ACL keyword and try to match with an extension declaration.

If this information was available on the extension structure, that implementation would be much simpler and reliable.

[xwu]

Cool, I can see how that's useful. However, it is misleading to label extensions as internal when they lack a modifier; that's simply not what they are.

[marcelofabri]

it is misleading to label extensions as internal when they lack a modifier; that's simply not what they are.

What do you think it'd be better? Omit the key when there's no modifier?

[xwu]

IMO, that would be superior, yes. Would that be a problematic approach?

[marcelofabri]

Would that be a problematic approach?

From my point of view, no. I'd just like to hear from more people involved in SourceKit before implementing it. Maybe @nkcsgexi?

[nkcsgexi]

@marcelofabri could you elaborated why we need to handle the accessibility of extension explicitly here? do we have issue with syntax highlighting?

[marcelofabri]

No issues with syntax highlighting, it's more about providing more information for clients.

In particular, SwiftLint has some rules that would benefit from that information:

• No Extension Access Modifier: generates warnings if an accessibility is explicitly added to an extension.
• Extension Access Modifier: generates a warning if all members of an extension have the accessibility (it could be only in the extension).
• Private over fileprivate: generates warnings when fileprivate is used in a place where private would be enough.

[nkcsgexi]

Interesting, thanks for clarification! Judging from the requirements, we really don't need to infer accessibility when an extension has no explicit one, am I right? i.e. we need to only report the accessibility user specified.

[marcelofabri]

Right, I just wanted to check if that'd be the preferred approach before making changes.

Just to be clear:
key.accessibility will only be present in source.lang.swift.decl.extension iff the accessibility was added explicitly.

[nkcsgexi]

Right, this seems to me a better approach. To do this, could you refactor the accessibility collection part of inferAccessibility to a new static function getAccessibilityStrictly , and using getAccessibilityStrictly here for your improvement?

[marcelofabri]

@nkcsgexi I've made the suggested changes in 86d9e91. 🎉

[nkcsgexi]

Thanks! Minor nitpick: this above three lines can be combined to:

 if (auto StrictAccess = getAccessibilityStrictly(ED))
   return StrictAccess.getValue();

[nkcsgexi]

Same here : )

[nkcsgexi]

@swift-ci please smoke test

[marcelofabri]

🎉