swift-api-declarations: Add declarations in public protocols as public declarations

[adellibovi]

Hi Mattt,

This PR add support for correctly printing methods or properties defined in public protocols.
A protocol could be defined such as:

public Protocol {
  func publicMethod()
  var publicProperty()
}

Currently the implementation was skipping the method and the property as they are not marked with accessibility modifiers, in this case those are implicitly public.

While doing the PR I notice two things:

• We do not currently have any tests. Do we want experimental commands to be covered with tests?
• Symbol are currently defined with an Array. Did we considered using Set instead? I was just wondering if the choice of having Array instead of Set was because we care about the initial "order" or not. My assumption is that, if the order is not important, we may benefits from hashing in different contexts.

Thanks!

Edit:
Actually three things:

func representation(of symbol: Symbol, in module: Module) -> String?

Do we want to refactor this method to somehow unify the representationDescription of a symbol? Maybe under SwiftSemantics?

[adellibovi]

Hi @mattt any chance you could take a look at this? :)

[mattt]

Hi @adellibovi. Thanks for your contribution! I apologize for not responding sooner.

Around the time that you submitted this PR, I was in the middle of code refactoring to formalize what were ad-hoc relationships between symbols, and make them easier to reason about (See the swift-5.2 branch).

The good news is that this work forms a solid foundation to build on top of, including a fix for the issue you highlighted here (7ee7196). Unfortunately, it changed the public API in such a way that are incompatible with your PR.

Responding to the rest of your comment:

We do not currently have any tests. Do we want experimental commands to be covered with tests?

I'd held off on tests until the API stabilized a bit (it's hard to test behavior when you don't know what behavior you want). I think we're in a good place with the latest refactoring, so I've stubbed in some initial testing with 801ef41.

Symbol are currently defined with an Array. Did we considered using Set instead? I was just wondering if the choice of having Array instead of Set was because we care about the initial "order" or not. My assumption is that, if the order is not important, we may benefits from hashing in different contexts.

Symbol was originally constrained to use in arrays for lack of autosynthesized Hashable conformance due to the type-erased declaration property. But even with that available post-refactoring, symbols are still typically provided as arrays (or keys in dictionaries) rather than sets. Although order is not important, arrays are more convenient to work with, and I don't see enough added benefit to justify the additional step to construct sets from existing arrays.

Do we want to refactor this method to somehow unify the representationDescription of a symbol? Maybe under SwiftSemantics?

While the existing code was good enough for a PoC, I think the next step — before any additional refactoring — is to solicit feedback about this representation on the Swift forums. Given its resemblance to the new .swiftinterface text module format, I want to be careful not to introduce something that is deemed unnecessary or potentially confusing. For now, I've extracted the current swift-api-interface CLI into a separate repository.