Access control for enum cases

+1 but make the introduction on access control modifiers be explicit the moment it is added to an enum case. Otherwise this is going to be too confusing. In Swift the absence of a control modifier usually means internal but that is not the case for Enums because the cases are public by default when the enum is public so if we introduce the ability of lowering this default for enums then all the cases should be explicit even internal cases.

3 Likes

Could you clarify what you mean with an example or two? Are you saying that we should have a breaking change where:

public enum E { case e }

would mean that e is internal (for consistency with other declarations) by default instead of public (current default)? Is the suggestion that we should warn on this, and suggest that people write public case instead? Or something else?

This pitch doesn't propose changing the behavior for existing enums and their cases, those would be treated based on the access control rules we have today.

I think the suggestion is for diagnostics like the following

public enum E {
  case e // OK, existing behavior
}

public enum F {
  case f1 // error: enum with internal case must specify visibility of all cases explicitly
         // fix-it: insert 'public '
  internal case f2
}

public enum G {
  public case g1 // OK
  internal case g2
}

I think I agree that the deviation from the usual "default access level is internal for members of public types" rule may justify forcing authors to be a bit more explicit. We could also have a rule that if any enum cases have access specifiers, then the default access level for all other cases becomes internal. I.e.:

public enum H {
  public case h1
  case h2 // this case is internal
}

Then the only exception to the "normal" access control behavior is "for an enum with no case-level access specifiers, all cases are as visible as the enum declaration itself."

3 Likes

This is precisely what I meant. Thank you.

@mayoff, but it will force you to always write default in top-level switches, at that case you probably may create an equatable struct for concrete Actions, and a static factory for this struct. Anyway this feature will be useful for unidirectional architectures in general

In TCA:

  • we usually don't switch at all over a component's Action type except inside that component;
  • when we do switch over some other component's Action type, we usually only care about a small number of its cases and have a wildcard case to ignore the others.

For example, in this DailyChallengeFeature module, the enum DailyChallengeAction has cases wrapping enum DailyChallengeResultsAction and enum NotificationsAuthAlertAction from other modules. The dailyChallengeReducer

  • use a wildcard for all wrapped DailyChallengeResultsAction values;
  • matches two cases of NotificationsAuthAlertAction and uses a wildcard for any others.

Hidden cases will not increase (or decrease) the need for wildcard matches in a TCA app.

There's something off to me about this example, and it's related to the @frozen @unknown default attribute. That was also added for a similar reason as this pitch (inaccessible enum cases that are linked to the consumer's binary without their knowledge), and it feels like if we are still adding stuff to solve that class of problem, we didn't do a good job solving it the first time around.

One of the alternative approaches to the @frozen @unknown default problem was making these objective c enums into structs in the overlay layer, and it seems to me that that would solve this problem too. As long as ~= is defined and the init is private, it would act exactly like an "unsealed" or nonexhaustive enum, giving us all the desired behavior with very little client code changes. It does seem like we're moving towards two general groups of enums — exhaustive and non-exhaustive.

What currently happens if a user is handed an unexposed UIKeyboardType case and the user tries exhaustively switch over it? Trap? It hits the @frozen @unknown default case?

I think this should be a first class part of this proposal — I would be surprised and upset if I saw some enum cases in a generated header, tried to switch over it, and then got hit with an error that I hadn't handled all of the cases.

7 Likes

I'm not sure that's the case. @frozen means the opposite—there are no cases hidden from public API, and the library promises never to add more, so clients can safely exhaustively match the cases.

4 Likes

Right, duh. I confused myself, I meant @unknown default.

I agree with other posters in this thread that it's natural to use @unknown default to refer to private cases, in addition to its current use for referring to yet-to-exist cases. The proposal at hand fills in the gap that Swift does not allow for private cases today (despite ObjC being able to emulate the pattern, and Apple's frameworks often making use of this ability).

9 Likes

I'm not sure it makes sense to hide private cases completely, as it annoyingly requires the use of an @unknown default with little context, and I'm not sure hiding the name of a case has much value anyway. We get all of the same functional benefits by just hiding the initializer, just as you would a private static func. Since Swift has already established an enum case :: static func equivalency in protocol conformances, why not just limit the proposal to that for private case foo?

I'm so excited about this pitch, thanks @Varun_Gandhi!

This has been a source of constant and frequent pain for my work, as I very frequently had to resort to:

    public enum Message {
        case something
        case somethingElse
        case _testing(SomeOtherEnum)
        // or
        case _localOnly(SomeOtherEnum)
    }

and similar patterns, and it's mostly been "fine" by telling users to "please ignore those _ ones". but allowing to have these be internal would have been the real solution.

As such, I'm very supportive of the proposal and the featureset and it definitely addresses a real world pain we're facing in day to day development in enum heavy projects, esp modeling "messages" with them where some are never intended for outsiders to ever see or use.

5 Likes

One follow up question that I don't think was touched upon:

In today's swift:

internal struct X {}

public enum E {
    case a
    case x(X)
}

results in


-> % swiftc /tmp/test.swift
/tmp/test.swift:5:10: error: enum case in a public enum uses an internal type
    case x(X)
         ^ ~
/tmp/test.swift:1:17: note: type declared here
internal struct X {}
                ^

would it make sense for an internal case x(X) to be allowed to use internal types there?

Yes, that would be allowed, except if the enum is @frozen, in which case X needs to be @usableFromInline or public. More generally, the access rules for associated values should be similar to those for stored properties; after all, in some sense, associated values for enum cases are "conditionally stored properties."

4 Likes

Excellent, thanks for confirming. Will be good to call these out explicitly in the final proposal

1 Like

A quick thought on this: how should copying behave under this rule? That is:

// An initialiser of some struct/class:
init(_ val: SomeEnumWithPrivateCases) {
    // Technically, this is initialisation, but we can't know if we're copying a private enum case...?
    self.field = val
}

Copying would work as usual, similar to how copying works for structs with private stored properties or non-frozen enums under library evolution.

2 Likes

I'm just a bit concerned about the generality of this rule: imagine an enum as in your example:

public enum B {
  case v
  internal(init) case w
}

— that is, it has no associated values, so only the discriminator (v vs. w matters). If the module P ever passes me a value of B.w, then I can match it, copy it into some global memory perhaps, and then initialise it however I want as if this internal(init) restriction never existed:

var globalB: P.B!

func someFunc(_ b: B) {
    switch b {
    case w:
        // Aha, I've got that sweet B.w, let's store it
        globalB = b
    }
}

func someOtherFunc() {
    // Now I can use B.w surpassing the initalisation constraint
    let bw = globalB
    ...
}

— the important part, that is, is the lack of associated values (and thus any entropy): the fact that B has (in general) just two possible values makes it very easy for me to circumvent the restriction, collecting all possible values of B and using them as if I could write let _ = B.w all along.

In other words, the only way for me, the vendor of B, to ensure that B.w never gets constructed by a client is to never pass this case at all – but that defeats the purpose of allowing to match against the case. So either one shouldn't be able to match B.w at all or private(init) doesn't protect the value from being initialised in an indirect way.

Has exhaustivity of nested patterns been discussed? It would be a shame to have to nest switch statements just to get public exhaustivity. For example, if I wanted to switch on an instance of this:

enum Parent {
  case child(Child)

  enum Child {
    case tap
    internal case response
  }
}

I must nest switches to ensure a warning when new public cases are added:

switch parent {
case let .child(child):
  switch child {
  case .tap:
    break
  }
}

I'd like to flatten that nesting, but I imagine the warning will be lost in the following:

switch parent {
case .child(.tap):
  break
case .child:
  break
}

It'd be nice if @unknown could be used as a sub-pattern:

switch parent {
case .child(.tap):
  break
case .child(@unknown):
  break
}
5 Likes

This is true for any value of a type with publicly-viewable, privately-settable API, though, no? Like, if I have:

public struct S {
  public internal(set) var x: Int
  public init() { self.x = 0 }

  init(x: Int) { self.x = x }
  static var one = S(x: 1)
}

Then a client of S can inspect x, see if x == 1, and then save that "private" value for later use, effectively circumventing the internal restriction on S.one. I don't see that private(init) enum cases actually create a problem that's different in kind to the way access control already works in Swift.

3 Likes
Terms of Service

Privacy Policy

Cookie Policy