Swift: When Unused Code Is a Bug

Wow, Heisen-Swift, where the Heisenbugs are in the language specification!

What an object is, which is roughly equivalent to its observed behavior, should never depend on how it is declared.

   let greeter = LazyGreeter()
   let greeter1: Greeter = greeter
   print(greeter)
   print(greeter1)
   greeter.greet()
   greeter1.greet()

UPDATE: just in case it's not clear, this prints the following:

   greeter.LazyGreeter
   greeter.LazyGreeter
   sup
   Hello, World!

So the same object responds differently to the same message, depending on how it is declared. Yikes!

I'm reading the comments here about how the languages fails, or what it should have done better, but at the same time I'm feeling the same as I do when building up something with Kubernetes and the problem isn't really the language, it's the shortcuts we take with it to stay concise.

e.g. in Kubernetes you can give a Pod, Deployment, Service, Ingress, Volume, etc. etc. the exact same name and in fact many examples encourage that (because they are namespaced on the type). However this is practically just an artefact of a previous choice and when you're looking at your YAML files you may not immediately know that distinction unless you have some familiarity with the tech.

What it says is that you can call everything the exact same thing. And it will work, and it will seem nice or elegant.

So it is with this Swift example and having a protocol, an extension, and a base class all declaring the exact same thing. Even if the compiler figures it out, how would you without reading the spec or knowing where those files live (because in the real world they would be spread across the filesystem)? It's just poor consideration for comprehension.

Default implementations on protocols in Swift definitely can be a little dangerous. We've shied our team away from doing this unless the protocol is explicitly used as a mix-in type.

There's been some proposals around fixing these, one that comes to mind is: https://forums.swift.org/t/introducing-role-keywords-to-redu...

Coming from the .NET world, where extension methods are just syntactic sugar for static methods, the "bug" described in the article wasn't surprising to me. However, it did make me stop and think about how the example would look in C#. This was what I came up with:

    public interface IGreeter {
    }
    
    public static class GreeterExtensions {
        public void Greet(this IGreeter greeter) {
            // If you have CA turned on,
            // you'll get a warning for not
            // using the parameter "greeter"
            Console.WriteLine("Hello, World!");
        }
    }
    
    public class BaseGreeter : IGreeter {
        public abstract void Greet();
    }
    
    public class LazyGreeter : BaseGreeter {
        public override void Greet() {
            Console.WriteLine("sup");
        }
    }
    
    IGreeter greeter = new LazyGreeter();
    
    // This has to be GreeterExtensions.Greet(greeter),
    // because IGreeter is an empty interface
    greeter.Greet();

I think the syntax in C# makes it clearer that you're doing something funky mixing inheritance with extension methods. If you wanted to do it the "correct" way, I think it would look something like:

    public interface IGreeter {
        void Greet();
    }
    
    public class BaseGreeter : IGreeter {
        // Edit: abstract method wouldn't be equivalent to example
        // public abstract void Greet();
        public virtual void Greet() {
            Console.WriteLine("Hello, World!");
        }
    }
    
    public class LazyGreeter : BaseGreeter {
        public override void Greet() {
            Console.WriteLine("sup");
        }
    }
    
    IGreeter greeter = new LazyGreeter();
    greeter.Greet();

Which is more complicated than the idiomatic way of providing default behavior with inheritance:

    public class Greeter {
        public virtual void Greet() {
            Console.WriteLine("Hello, World!");
        }
    }
    
    public class LazyGreeter : Greeter {
        public override void Greet() {
            Console.WriteLine("sup");
        }
    }


    Greeter greeter = new LazyGreeter();
    greeter.Greet();

I'm not a huge fan of the Swift syntax; specifically, IMO, the declaration `class BaseGreeter: Greeter {}` hides where the implementation of `greet` is coming from. Without the method declaration it looks like it's being inherited from the protocol extension, when it's really not.

If I saw this in a code review, I would flag it for re-design. Mixing composition and inheritance especially in this way is definitely going to confuse anyone who doesn't read every line to figure out what's going on.

In general I’ve found protocol extensions to be an incredibly powerful concept. There’s a couple of WWDC talks about ‘protocol oriented programming’ that highlight some great, practical use cases.

I’ve run into the author’s problem a few times, and I’d like to think that this is just a bug or behaviour detail that a future Swift version could correct. Swift hasn’t quite matured fully yet, and this is a great example of that (these examples are becoming fewer every year though).

This is my favorite Swift gotcha :D

This bug bit me one time back before remote debugging came out when I was developing an app for a device that plugged into the lightning port. The only way to debug was to send log messages over the network to a log server running on my laptop, and the connected device sent messages to satellites where there was often a 10-minute gap in connectivity. And I couldn’t reproduce it in tests. That took some time to diagnose!

I like to bring this issue up if an interview candidate seems extremely bought in to protocol-driven development. I wouldn’t be surprised if any level of dev wasn’t aware of it, but would be impressed if anybody identified it.

Can anyone explain to me what the rationale for this is?

I can understand the advantage of using static dispatch, i.e. Greeter#greet(object), but I assume there is some mechanism that avoids calling Greeter#greet if BaseGreeter would implement greet and call BaseGreeter#greet(object) instead.

Why does extending a class which extends a protocol not make the extending class implement that protocol in Swift?

I wonder if this behavior is working as intended. It does look like a bug to the language itself, isn't?

It's good that Periphery catches the unused code.

But if that's the only reason it catches the problem, I think it could have room for improvement. For example in one place the error could exist, but at a different place, the real function could be called because the variable is declared as a LazyGreeter. Then Periphery wouldn't catch the problem.

One way to fully catch it would be to give a warning if a base class implements a protocol but a derived class doesn't. I'm not sure how many false positives this would have. Possibly you might only do this if the derived class tries to override a function from the base class's protocol.

This seems like a problem with using class inheritance more than anything else.

I've run afoul of this a couple of times in my Swift delegate protocols. A really nice hack to implement Objective-C's optional protocol methods in Swift is to use a protocol extension to create a default method that can come back to bite you when you subclass objects that conform to that protocol.

Interesting. Swift has been my primary language for a couple of years now, but I’ve never run into this, mainly because I use subclasses far, far less often than I did under Objective-C. If this bug had existed in Obj-C back when It was still my primary language it likely would have bitten me.

Providing a protocol with a default concrete implementation of the interface? That's not passing code review here.

Would I have caught the bug? Probably not. But it looks like bad form to me to provide that default implementation to Greeter, so please rewrite your code.

If the concept is that protocol requirements represent the "customisation points" of a protocol, shouldn't this apply equally to conforming classes and subclasses with inherited conformance? So I would be in favour of fixing this. Conversely, perhaps protocol extension methods etc not declared as protocol requirements could e.g. elicit a compiler warning when "overridden" by conforming classes/subclasses with inherited conformance (since they are not intended as customisation points)? I think these changes would aid reasoning about protocols and reduce bugs.

I don't know that you can really call this a bug? It just seems to be the design choice. Though I agree, it seems odd at first, and appears possibly accidental, it could have been on purpose.

My question to swifters: what if LazyGreeter had another method, say lazyGreet, and you typed it as Greeter? Then you called let greeter : Greeter = LazyGreeter(); greeter.lazyGreet();

Would you expect this to work or fail?

I'd expect it to fail.

In that sense, I assume its simply that extension methods override child overrides. And its just something you need to know. That can even have interesting utility in some scenarios.

How much different is this from slicing in C++?

IE:

  #include <iostream>
  using namespace std;

  struct Greeter {
    void greet() {
        cout << "Hello, World!" << endl;
    }
  };

  struct BaseGreeter: Greeter {};

  struct LazyGreeter: BaseGreeter {
    void greet() {
        cout << "sup" << endl;
    }
  };

  int main() {
    Greeter greeter = LazyGreeter();
    greeter.greet(); // Prints "Hello, World"

    return 0;
  }

The explanation is counterintuitive and it's one of those many Swift-ish things I've learned to be wary of. I find that Swift tries to be 'all things' and to often I get lost trying to cobble together a mental picture of what a class is supposed to be given all the various ways to annotate it.

Another "unused code bug" would be when a locally scoped variable which is not supposed to be there shadows the intended global variable.

It's just a matter of global scoping and precedence. But yeah, sharing notation between inheritance and extension is a tad bit reductionist.

I'm a little undecided on Swift extensions as a whole, and particularly protocol extensions. Maybe I'm just paranoid but it feels like it leads to a more complicated code layout.

This is an interesting demonstration of a hard edge of the language. It's not because Swift holds some sort of grudge against dynamic dispatch, but rather because it's trying to reconcile some subtly incompatible design goals:

1. Adding a default implementation for a protocol requirement shouldn't stop existing types that correctly conform to that protocol from compiling. This is a reasonable user expectation and makes protocol composition a more powerful feature.

2. "Retroactive conformance" should be possible: someone should be able to conform a type they didn't write to a protocol of their own creation. It's a nifty feature that makes Swift extensions very powerful but also sometimes difficult to reason about.

3. Classes should inherit protocol conformances from their superclasses. As some of the Swift core team members have explained, this is not something that absolutely had to be the case, but it seemed sensible initially.

Points (1) and (2) preclude the language from requiring the `override` keyword for the subclass `LazyGreeter` to override a default implementation of a protocol requirement not itself overridden in the superclass `BaseGreeter`, since that would mean that (1) and/or (2) would become impossible. Getting rid of (3) would remove the surprising behavior but it is rather late in the evolution of the language to make such a large change.

The way to reason about this behavior is to consider what are intended "customization points" in the language. It isn't really covered in The Swift Programming Language, but (although not intuitive) it's a fairly teachable concept:

A separate but similar issue exists with the distinction between default implementations of protocol requirements (methods declared in both a protocol and a protocol extension) versus extension methods (methods not declared in a protocol but found in a protocol extension). The former is dynamically dispatched and is considered a customization point for conforming types, whereas the latter can only be shadowed by conforming types but is not a customization point. As described here: https://nomothetis.svbtle.com/the-ghost-of-swift-bugs-future

(The Swift standard library actually uses this concept to its advantage. For instance, the inequality operator != is not a customization point for the Equatable protocol and is implemented in an extension method, guaranteeing that !(a == b) is always equivalent to (a != b) in the generic context.)

When implementing subclasses of a third-party open class, only open methods are customization points. In that case, Swift provides compile-time checking to prohibit overriding non-open public methods. This is possible because Swift does not support "retroactive inheritance" from a superclass as it does retroactive conformance to a protocol. Also, the language allows later versions of a third-party open class to stop your existing subclasses from compiling because of a conflict between a later-added superclass method and a pre-existing method in your subclass that now lacks the `override` keyword. In other words, the compiler-enforced prohibition is possible because points (1) and (2) above were not design goals for subclassing as they were for protocol conformance.

In the case illustrated by the present article, where protocols are used in class hierarchies, a third notion of customization points arises. As demonstrated here, a protocol requirement not overridden by a base class is not a customization point for a subclass. The protocol requirement can be shadowed by the subclass but not overridden. Consistent with the design goals enumerated above, this allows for the vendor of a library protocol to provide additional default implementations at a later point without breaking user code. (But if the vendor of a conforming superclass then implements an overriding implementation, your subclass would cease to compile just as it would for any other conflict between superclass methods and subclass methods.)