Proposing to expand `available` to introduce `discouraged`

What follows is a proposal draft for initial feedback. This is a link to a gist version. The gist will be updated as feedback from community members is processed. The cut-and-paste into this post will not.

We would greatly appreciate your thoughtful feedback on this pitch. Thank you.

Expanding available to introduce discouraged

Introduction

Swift's available attribute documents characteristics of a declaration's lifecycle. The attribute specifies when a declaration became available on a given platform, and if it's been deprecated, obsoleted, or renamed. We feel there's room to further nuance available . This proposal expands available to introduce discouraged , making declarations harder to accidentally use.

Swift-evolution thread: Proposing to expand available to introduce discouraged

Motivation

Public declarations exist for many reasons. Most typically, you want to create nice things. You expand your API surface, granting callers access to your declarations. That's not the only reason, though. Sometimes you expose implementation details when a declaration must be public. At other times, declarations are needed for compatibility, even when there's a newer more preferred approach.

As an API designer, you may want to steer adopters away from certain declarations. We believe you should be able to communicate when declarations are visible but should not be used. Your development environment should be able to pick up these cues and help coders to avoid using those calls.

For example, initializer requirements in the LiteralConvertible protocols are redundant with the initializers on the type conforming to the protocols. They are meant to be implemented, but not called. Marking them discouraged documents this reality. It also helps the development environment hide the discouraged declarations from cues like code completion.

You may also want to discourage a declaration that has been modernized or redesigned. Even when the original form is required for compatibility and cannot be deprecated, discouraging older forms helps move coders to newer declarations.

Discouraging visible-but-not-meant-for-use declarations provides tangible benefits:

  • It increases coding safety.
  • It supports clarity, as the current uses of available do, by co-locating availability status information with code documentation.
  • It offers a way to connect a declaration with development environment policy. A discouraged declaration may be warned or hidden from code completion depending on the implementation provided in an IDE.

Proposed solution

We propose to extend available , introducing discouraged . A discouraged declaration is publicly accessible but not meant for general use.

Detailed design

The available attribute currently supports the following arguments:

Argument Description
introduced The first platform or language version that supported the declaration.
renamed A name that entirely replaces the old declaration, emitting a compile-time error.
message A message to display when emitting a warning or error for deprecated and obsoleted declarations.
deprecated The first platform or language version where the declaration was deprecated.
obsoleted The first platform or language version where the declaration was removed and can no longer be used.
unavailable Indicates a declaration is not available for a given platform.

For example:

@available(swift, introduced: 4.0, message: "Please use failable String.init?(_:UTF8View) when in Swift 3.2 mode")

@available(swift, deprecated: 5.0, renamed: "firstIndex(of:)")

@available(*, deprecated, message: "all index distances are now of type Int")

@available(iOS 14.0, watchOS 7.0, tvOS 14.0, *)
@available(OSX, unavailable)

Our proposed design expands available .

Argument Description
discouraged A message to display that describes why this declaration is discouraged

Here's what this might look like. For implementation details, the message explains the circumstances leading to discouraging use. For updated and refactored APIs, the message drives "what to use" over "why this is discouraged".

@available(*, discouraged: "`_ShapeView` not for public use")

@available(*, discouraged: "`IntersectionSolver` is O(N^2). Use `ConfluenceSolver` instead.")

@available(discouraged: "Use newer Combine-based publisher instead")

IDE integration

Discouraged declarations can be enforced by the IDE experience in several ways:

  • They could be suppressed from code completion or only show up in code completion once they are the only unambiguous option.
  • They could given a strikethrough treatment.

Further, the new discouraged argument can be picked up and integrated into Quick Help.

Source compatibility

This change is purely additive and will not affect source compatibility.

Effect on ABI stability

No effect.

Effect on API resilience

No effect.

Alternatives considered

Not accepting the proposal and leaving availability as currently designed.

20 Likes

I'm not at all opposed to this. That said, I think that an eventual proposal needs to expand considerably on what, exactly, the distinction between "deprecated" and "discouraged" is.

20 Likes

This pitch skips over precisely what diagnostics would be provided to the user if a discouraged declaration were used, when that's really the key information needed to evaluate it. My reading of it seems to imply that it would only surface information for IDE usage but not emit compiler diagnostics—is that the intention?

Using either interpretation though, I worry about introducing a new level of nuance here:

  1. If usage of a discouraged declaration only surfaces information in an IDE, but does not cause compiler warnings to be emitted, then we lose the ability to detect (and possibly fail on) usage of discouraged decls in automated builds without using a semantics-aware (i.e., SourceKit-based) external tool. It would be easier for the compiler to just emit a diagnostic. (If the compiler can warn me about deprecated decls, why not discouraged ones?)
  2. If usage of a discouraged declaration also emits a compiler warning, then it is not meaningfully distinct from deprecated except in the text of its message.

In most cases, I don't think the distinction between discouraged and deprecated would be very clear, and where it is I would expect the latter to be preferred because, assuming interpretation #1 above, deprecated is a stronger guarantee. I think we'd get more mileage by fixing deprecated so that it works better than it does today, and then looking at alternatives to discouraged for the other use cases that avoid the introduction of nuance.

Fixing deprecated

These examples look precisely like deprecation to me. When you say that "the original form is required for compatibility and cannot be deprecated", what is blocking the deprecation? If it's the warnings emitted by a module's own uses of the deprecated declaration, or by other valid uses that cannot be migrated easily, then we should solve that problem—it's one that has come up a few times before on this forum (one such thread). Swift today doesn't give any control over where deprecation warnings are emitted, and possible remedies have included omitting deprecation warnings in the same file/module, attaching a "deprecation scope" to a decl that says where its use is permitted, or just letting users bracket their code with directives that enable/disable deprecation warnings for a region.

This is an issue that we've faced on swift-protobuf, for example, because we would like to generate @available(*, deprecated) annotations for properties that represent deprecated messages/fields in the original .proto file. However, the generated serialization code in the same type must necessarily refer to the deprecated property in order to read/write it, so I wouldn't even be able to compile the type's own implementation without warnings. I don't think a new availability level is needed to solve that problem—I just want better control over the existing deprecation.

Discouraging access to decls that aren't "deprecated"

If the intention is to indicate that these initializers should not be directly called (something that I agree with), then we should take a stronger stance than documenting that via an attribute. Given that these protocols/initializers are already given special treatment by the compiler, it could theoretically ban direct calls, and we should weigh that against source compatibility concerns.

Is anyone writing code today that depends on being able to call these, and if so, do we believe that to be harmful enough to outweigh the cost of forcing them to migrate? If we don't believe that, do we think that it's still a serious enough issue to introduce a new level of nuance in @available?

More generally, this is a case of a declaration being forced to be public for external reasons, such as making it accessible to tests or to a closely-related companion module, but otherwise it's not meant to be generally used by clients. In those situations, I prefer the direction taken by the recently added experimental SPI support, because it's enforced by the compiler instead of just being a suggestion.

16 Likes

"Deprecated" is well known to produce warnings during builds. "Discouraged" would not. The proposal explains that some things need to exist, but should not generally be called - e.g. the initializer for the LiteralConvertible protocols. They cannot be deprecated, but you don't want normal users to drive-by use them.

Yes. I agree that it would be good to make that more clear in the proposal.

-Chris

6 Likes

As long as no warnings are generated this would be a way to indicate the designer's intended use of methods. I guess it then becomes the linter's job to tell us about our mistaken uses.

Another case where this might be useful is for methods that are only intended to be used from unit tests. While considered by some to be bad practice I sometimes find the need to have certain setup methods be public so they can be accessed from unit tests. Having this annotation would be a way to indicate that they shouldn't be used generally.

The usage of “discouraged” seems to be too close to “deprecated“. Something like deprecated lite. I propose we instead move closer to “rename” but as a suggestion instead of an error.

“preferred”: A symbol name that should be preferred over old declaration. This informs code completion and optionally provide fix-its to use preferred name.

3 Likes

It's seems that discouraged is a weaker version of deprecated, how about rename it to prudent or cautious?

I agree that I need to rework the examples and description to better explain the difference between deprecated/renamed and discouraged. I see deprecation as lifecycle-driven whereas I visualize discouraged as more an intentional design decision: making or leaving a declaration public but less visible.

6 Likes

Since the may goal seems to be to have this appear via IDE autocomplete, how would a developer know they have code that should be considered for an update? i.e. - Odds are any discouraged api didn't start that way, so there will be existing code that uses the api. So how does a developer get a signal they they have code that they should likely consider migrating at some point?

1 Like

Apple uses a special version number for "soft deprecated API" in Objective-C headers.

// <os/availability.h>
#define API_TO_BE_DEPRECATED 100000

For example:

// Objective-C
API_DEPRECATED(..., macos(10.0, API_TO_BE_DEPRECATED));
// Swift
@available(macOS, introduced: 10.0, deprecated: 100000)

Would this be improved by adding a special literal (e.g. #tbd instead of 100000) for "soft deprecation" in Swift?

I support this pitch. I have recently used deprecated in order to warn the user against a sub-efficient use of a method.

The context: a SinglerPublisher subprotocol of Combine's Publisher provides the guarantee that a publisher publishes exactly one element, or an error. SinglePublisher · GitHub

The protocol comes with operators that check, at runtime, if a publisher honors the contract:

// Completes with a `SingleError` if contract is not honored
let singlePublisher = publisher.checkSingle()

// Raises a fatal error if contract is not honored
let singlePublisher = publisher.assertSingle()

Those runtime checks are both useless and costly for publishers that are statically known to conform to SinglePublisher, such as Just, Result.Publisher, Publishers.Map when upstream is already a single publisher, etc.

  • I want the compiler to discourage the user from using the checking operators checkSingle() and assertSingle() on such publishers.

  • I want the compiler to let the user know a checking operator checkSingle() and assertSingle() in no longer needed once SinglePublisher conformance has been added to a publisher type.

Today, I have to use deprecated:

extension SinglePublisher {
    /// :nodoc:
    @available(*, deprecated, message: "Publisher is already a single publisher")
    func checkSingle() -> CheckSinglePublisher<Self> {
        CheckSinglePublisher(upstream: self)
    }
    
    /// :nodoc:
    @available(*, deprecated, message: "Publisher is already a single publisher")
    public func assertSingle() -> AssertSinglePublisher<Self> {
        checkSingle().assertNoSingleFailure()
    }
    
    /// :nodoc:
    @available(*, deprecated, message: "Publisher is already a single publisher: use publisher.eraseToAnySinglePublisher() instead.")
    public func uncheckedSingle() -> AnySinglePublisher<Output, Failure> {
        AnySinglePublisher(self)
    }
}

Unfortunately, those methods are not deprecated. They are indeed quite supported. I just want the compiler to emit a warning when they are misused.

4 Likes

I'm in favor of this proposal but I believe there should be some way to surface discouragements for code that's already written. Let's say my framework evolves and at some point I have a better, or more favorable implementation of a feature that would make the original one discouraged. How would a user of my framework be notified of this if they already use my discouraged method?

A warning might be too similar to a deprecation, but I do think it's important that developers can easily discover (and fix) usage of discouraged methods. Could there be a "discouragement as warning" a compiler option so developers can choose how they want to see discouraged code usage surfaced?

It seems like this maybe should warn, just for a different reason. Deprecated = “this is old and maybe unsupported and might go away soon” whereas Discouraged = “this is fine but consider something better”. This might need a way to suppress the warning though, in particular for those who compile with treat-warnings-as-errors.

1 Like

I have three nits to pick with this proposal:

  • “Proposed solution” really ought to describe, at least briefly, the practical effects of discouraging a declaration. I gather that you want it to affect source editor features like code completion, but don’t expect it to cause a warning, but that information is really buried.

  • Why is discouraged part of @available, as opposed to being its own attribute? Do you imagine that something might be discouraged on one platform or version but not another? Currently, everything @available does at least can be made conditional on platform and version; you can also use those features unconditionally via @available, but that’s mainly just so they don’t use radically different syntax.

  • What is discouraged’s message for? If we don’t generate diagnostics when you use it, when does that message get displayed?

But I do think “hide this from code completion” probably deserves a better solution than “add a leading underscore”, so I support the general thrust.

7 Likes

I have need of a “hide from code completion” attribute and would support this pitch if it goes to proposal. However, I feel that in its current state it conflates two different things that ought to be separate:

  • The “hide from code completion” behaviour, for cases where symbols need to be exposed but shouldn’t ever be used by client code (the _ShapeView example in the pitch)
  • A “like deprecation, but weaker” attribute (the IntersectionSolver and newer Combine-based publisher examples)

I think these should be distinct proposals, quite possibly with different syntax approaches (as Brent said, the first one should probably be a stand-alone attribute, while the second is clearly a variant of @available).

2 Likes

The other point that I think this proposal needs to address under "alternatives considered" is:

"We already have a spelling for things that are supposed to be ignored by the compiler; the default way of spelling such an annotation is a comment. What makes this use case special so that it requires an attribute?"

To be clear, I think you can give a reasonable answer to this question, it's just (to me) a glaring omission from the proposal.

1 Like

I think there's a stronger corollary here: If something is only used to provide informational signal to a developer via the IDE but otherwise is not used by the compiler to affect the output, then it effectively is documentation. That's a sign to me that this feature, based on clarifications of its behavior from comments above, should be a doc comment markup tag instead of an attribute.

In fact, for a very long time Swift has had Recommended and Recommendedover markup tags that sound very much like what is being described here:

"- recommended:" indicates other declarations are preferred to the one decorated; to the contrary, "- recommendedover:" indicates the decorated declaration is preferred to those declarations whose names are specified.

There are some unit tests for the raw code completion response, but I can't seem to get this to work in Xcode (11.4 or 12b1). I don't notice any differences in code completion results or ordering if I use these tags, and in fact, the presence of - recommended: or - recommendedover: in a doc comment breaks Quick Help completely for that decl—the doc doesn't show up at all :confused: @Xi_Ge Is this expected/a known issue?

So, Recommendedover is somewhat similar to the discouraged being proposed here, except that it takes (requires) the name of an alternate declaration instead of a free-form message. If this is something folks want to pursue, maybe it would be better then to build on the existing feature set instead by:

  • making Recommended and Recommendedover work better in Xcode—de-prioritize or hide decls that are tagged with Recommended or listed in a Recommendedover tag in code completions.
  • formalize the format of the Recommended and Recommendedover tags to be more like Parameter so that they contain the exact decl-name of the alternative declarations and a descriptive message that the IDE can present in code completions.
  • add a - Discouraged: variant that would be used when there's not an alternative declaration to list; it would just contain a descriptive message and would de-prioritize or hide its decl in code completions.
22 Likes

Agreed. I prefer something like recommended and recommendedover though not sure if doc comments is the way to go. Are doc comments even part of the language?

Couple of nit pics:

  • @available(*, discouraged, message: "") (or @discouraged if it was its own annotation) should not generate compiler warnings, at least not without a way of suppressing them. Many projects use "treat-warnings-as-errors", as some pointed out. Raising warnings would require not using any "discouraged" code, which is a bit strict considering is only "discouraged" and not "forbidden".

  • That said, if there were no warnings or diagnostics generated at compile time, then there would be no way for users to read the "message" (eg. using swift build or swiftc from the command line).

Given the two points above, it seems to me that the goal of "discouraged" fits better with inline documentation. For example:

/// Slow Method
/// - Discouraged: Very slow method. Use `fasterMethod()` instead.
/// - Complexity: O(n2)
/// - seeAlso: `fasterMethod()`
func slowMethod() {
  // do something very slowly
}

/// Faster Method
/// - Complexity: O(1)
func fasterMethod() {
  // do something much faster
}

The above code currently renders as this:
Screen Shot 2020-07-20 at 10.02.30 AM

IDEs could be updated to render this new attribute in a more visible way, maybe with a warning indicator of some sort.