Catching unwanted uses of ~= on OptionSet

I recently spent a couple of hours trying to figure out why code like this never did what I wanted it to do:

    var opts: NSString.CompareOptions = [.caseInsensitive, .diacriticInsensitive]
    ....
    if opts ~= .diacriticInsensitive {
        print("Not using diacritics") <<<<< Never called
    }

It should have been:

    if opts.contains(.diacriticInsensitive) {
        print("Not using diacritics")
    }

Normal Set is prevented from compiling such a thing:

var opts = Set([1, 2])
if opts ~= 2 { // Fails to compile - Cannot convert value of type 'Int' to expected argument type 'Set<Int>'
}

I think OptionSet should also fail (with maybe a FixIt for "Use .contains instead").

~= is the pattern matching operator, not a "contains" operator, so I don't think the fix-it is sound.

The fact that OptionSet uses both an array-like, as well as a bare-element syntax, makes it perhaps a bit confusing that you're actually pattern matching two option sets against each other, and not a set and an element.

You'd get the same behaviour if you'd pattern match two Sets:

var opts = Set([1, 2])
if opts ~= [2] { 
   // never called
}

I think your solution is to stop treating ~= as a contains operator. It is not.

2 Likes

Please give me a practical real-world use case for actually wanting to use ~= on an OptionSet

I think it is often a code smell to use the ~= operator directly, not just for option sets, but for any type.

When switching over a variable, Swift uses ~= under the hood. Or when doing guard case, if case etc. If the type you're switching over is Equatable, the default switching behaviour is to jump to the branch that matches the value exactly. This is very useful in lots of cases.

OptionSet is equatable, and thus inherits this behaviour.

If we were to, say, offer an overload for OptionSet that uses .contains, this would very counter intuitive to most, methinks:

var opts: NSString.CompareOptions = [.caseInsensitive, .diacriticInsensitive]
....
switch opts {
case .diacriticInsensitive:
   print("Not using diacritics") // called, since it contains
case .caseInsensitive
   print("Not using case") // never called, since switch matched branch above ^
}
2 Likes

I’m still not sure I think it’s a good idea, but I’d imagine a contains-based implementation would work the other way around, where the sets are in the cases and the element being passed in. This is closer to the behavior of a switch with range-based cases. However, I can’t actually think of a use for such behavior, and the fact that that wasn’t someone else’s first thought on how it should work means that code using either pattern probably wouldn’t be clear.

4 Likes