[Code style] Require explicitly specified types for public properties

[stmontgomery]

This amends the code style guidelines for the project to require an explicit type for all properties whose access level is public or greater, and adjusts the code accordingly.

Motivation:

See the explanation in the code style document for rationale. This topic recently came up in a PR discussion.

Checklist:

• Code and documentation should follow the style of the Style Guide.
• If public symbols are renamed or modified, DocC references should be updated.

[stmontgomery]

@swift-ci please test

[grynspan]

I'm not morally opposed to the change, but I do have two questions:

1. The inferred type of a value can't really change in the described way:

   This is meant to protect against future changes to the code called by the initialization expression causing the inferred type of its property to change unknowingly, which could break clients.

   0 will always be inferred as Int, for instance. Is there a specific breakage that occurred that you want to avoid again, or is this just proactive?

2. Why not require this at all layers? By only requiring it on public and up (do you really want @usableFromInline internal and up, actually?), we are effectively creating two coding styles in our repo depending on the presence or absence of the public keyword. We could surely apply this rule universally if we wanted? If it's about the size of the diff, we can also break it up into multiple PRs.

[stmontgomery]

I'm not morally opposed to the change, but I do have two questions:

Re: 1. This is meant to be proactive. I agree, the inferred types of literal values like true and 0 aren't going to change. What I am concerned about, based on past experiences, is the possibility that we might change our code to replace literals like those with a value derived from someplace else, such as

-public var maximumValueDepth = 10
+public var maximumValueDepth = _defaultContentLimit

If the type of this hypothetical _defaultContentLimit (not shown) is something other than Int, or (worse) its type differs on a per-platform basis, then that change might accidentally change the inferred type of one of our APIs. So this is about maintaining compatibility. The compiler requires explicitness in most other public aspects of a module, but property types are one which it doesn't currently enforce, so this is an attempt to mandate that in our own style guidelines.

Re: 2, I don't want to stop using type inference for all properties. I think it's very useful and valid to continue using it for less-than-public decls, I just want to protect external compatibility. I think considering this "two coding styles" is overstating the impact; we have a precedent for applying special rules to private decls, for example (requiring an underscore prefix) and I don't find that onerous.

Yes, I do mean @usableFromInline too, and I'll amend the PR to reflect that.

[stmontgomery]

@swift-ci please test

[grynspan]

What I am concerned about, based on past experiences, is the possibility that we might change our code to replace literals like those with a value derived from someplace else, such as

To bikeshed a bit, what if we made the rule "any exported property that is not initialized with a literal value must have an explicit type"? Then public let x = 0 is fine, but public let x = _y needs a type, which ought to resolve that sort of problem.

[stmontgomery]

@swift-ci please test

[stmontgomery]

To bikeshed a bit, what if we made the rule "any exported property that is not initialized with a literal value must have an explicit type"? Then public let x = 0 is fine, but public let x = _y needs a type, which ought to resolve that sort of problem.

This is all subjective, it's code style after all, but I worry that has too much nuance. The expression could be an array or dictionary literal, for example, and if so its complete type would be inferred from its elements, so it could still run into his problem if the types of those elements change. I think "if it's public, declare the type explicitly" is simpler to remember and apply consistently.

[stmontgomery]

Last commit was just documentation, and the previous commit passed CI. Merging.