[SR-4865] target self should be illegal in property constructor

[mattneub]

Previous ID	SR-4865
Radar	None
Original Reporter	@mattneub
Type	Bug
Status	Resolved
Resolution	Duplicate

Attachment: Download

Environment

Xcode 8.3.2

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Bug
Assignee	None
Priority	Medium

md5: d3c2dbfe3cffd99ed51448d61dd795be

duplicates:

• [SR-4559] Method called 'self' can be confused with regular 'self' #47136

Issue Description:

This is legal in an instance property declaration:

    let listen : UIBarButtonItem = {
        let b = UIBarButtonItem(barButtonSystemItem: .play, target: self, action: #selector(doListen))
        return b
    }()

It shouldn't be. There is no self at this point. The code compiles but it isn't going to work, so it shouldn't compile at all.

I see a lot of people on Stack Overflow get caught by this sort of thing, but this is the first time I've made the same mistake myself. The usual context is in configuring a Timer.

[belkadan]

self in this case refers to the class, not an instance. This is probably not what the user intended.

[mattneub]

@belkadan It's not so simple. The same code works fine if this is a UIButton instead of a UIBarButtonItem. And there's no class method here, so why don't we crash when the button is tapped?

Moreover, the same code works fine if we change let to lazy var.

These distinctions are very confusing to the beginner (and even the not-so-beginner). If the compiler knows the difference between them, it should at least issue a warning.

Here's a test example (assume this is the root view controller of a navigation controller):

    let bbi1 : UIBarButtonItem = {
        let b = UIBarButtonItem(barButtonSystemItem: .play, target: self, action: #selector(doButton))
        return b
    }()

    lazy var bbi2 : UIBarButtonItem = {
        let b = UIBarButtonItem(barButtonSystemItem: .play, target: self, action: #selector(doButton))
        return b
    }()
    
    let button : UIButton = {
        let b = UIButton(type: .system)
        b.addTarget(self, action: #selector(doButton), for: .touchUpInside)
        b.setTitle("Button", for: .normal)
        b.sizeToFit()
        return b
    }()

    override func viewDidLoad() {
        super.viewDidLoad()
        self.button.frame.origin = CGPoint(x: 100, y: 100)
        self.view.addSubview(self.button)
        self.navigationItem.rightBarButtonItems = [self.bbi1, self.bbi2]
    }

    func doButton ( _ sender : Any ) {
        print("button")
    }

The first of those buttons doesn't do anything when tapped. The other two work fine. I'm not sure I know why. If the compiler knows, it should help out here.

I enclose that code in a test project. TargetSelfTest.zip

[belkadan]

Yes, the whole area is messed up. var vs. lazy var and "using a closure" vs. "not using a closure" all affect this, when it should be consistent across everything (rejected for var and probably allowed for lazy var).

[mattneub]

@belkadan Yep. Granted all that, I still don't get why it doesn't work for UIBarButtonItem (tapping does nothing) and does work for a UIButton (tapping calls the selector). A selector is a selector, a target is a target, `self` is `self`, `let` is `let`, define-and-call initialization is define-and-call initialization, so what on earth is happening here? Don't answer that, I just wanted to make sure that's in the mix somewhere when we come to tackle this from the compiler's point of view.

[dennisweissmann]

I just ran into this issue as well and this is definitely unexpected. I didn't even use a closure, just

private let rightArrow = UIBarButtonItem(title: ">", style: .plain, target: self, action: #selector(rightArrowPressed(_🙂))

I would expect the code to not compile but it compiles just fine and doesn't work.

[mattneub]

Caught another one: https://stackoverflow.com/questions/50662302/adding-a-target-to-a-button-programmatically-throws-an-error-unrecognized-selec

Interestingly, he had the issue with a UIButton.

[swift-ci]

Comment by Yu (JIRA)

This will crash on iOS 8, but works on iOS 9+

[mattneub]

Here's another one: https://stackoverflow.com/questions/63892806/button-to-instanciate-viewcontroller-is-not-working-after-hiding-it I really wish something would be done about this. Except that then I would not get numerous points on Stack Overflow, answering this over and over, as it catches people day after day... By the way, this was a particularly interesting one; the button works for a while and then stops talking to the target. I have no idea why.

[mattneub]

Also I would be curious to know why this is marked "resolved", as it is far from resolved. It catches people every day.

[mattneub]

I enclose a project that elicits the problem nicely.

TroubleShootingProject.zip

To see the problem, you have to do something slightly elaborate: click in the rectangle, then click on the Tap Me button. A presented view controller appears with chevron button. That button is broken: tapping it does not call its target-action method.

That is because the target-action was set in the instance property initializer. If you change let backButton to lazy var backButton, the problem goes away.

What I am saying here is that the Swift compiler should warn the user that the reference to self in the line v.addTarget(self, action: #selector(backButtonTapped), for: .touchUpInside) is not necessarily going to behave as expected. You cannot refer successfully to self in a nonlazy instance property define-and-call method, and the compiler should discourage you from trying to do so.

[nolanw]

I shipped code a few months ago that inadvertently does this, and it seemed to work ok until recently. I learned a bit about what's going on, though I still don't fully understand, and I thought I'd share.

We had something like this modified example from upthread:

class ViewController: UIViewController {
  let button : UIButton = {
    let b = UIButton(type: .system)
    // self below is really self.self aka unbound method UIViewController.`self`
    b.addTarget(self, action: #selector(doButton), for: .touchUpInside)
    return b
  }()

  @objc func doButton() {}
}

The target ends up being the unbound instance method ViewController.`self` (inherited from NSObject). The unbound method gets bridged to a __SwiftValue and passed to UIControl (its debug description is (Function), in case that comes up while poking in the debugger). Since UIControl doesn't retain its targets, the __SwiftValue deallocates shortly, and UIControl's weak reference zeroes out. Now it has a target of nil, meaning "check the responder chain". Since the view controller is in the button's responder chain, everything works out when the button is tapped.

My understanding evaporates when going to iOS's Settings app > Accessibility > Keyboard > Full Keyboard Access and enabling "Full Keyboard Access". Some of our users have this setting enabled and rightfully complained that the button no longer responded to taps. We eventually figured out that setting as the key environmental difference, and it led me to this bug. (This is on iOS 14.2, and it is reproducible both on device and in the simulator.) The __SwiftValue is still deallocated in this scenario, and the button's allTargets contains only an instance of NSNull, so I'm not sure what's going on. [edit: While in the nonfunctional state, enabling VoiceOver and focusing the button (then turning VoiceOver back off) makes it responds to taps again.]

I suggest that it is rarely intentional to access an unbound instance method named self from within a let property initializer, and emitting a warning would be appreciated. If the access is intentional, spelling it with backticks like `self` seems like a reasonable way to quash the warning.

Also, changing the action to #selector(self.doButton) reveals the issue, as the compiler will emit an error "Instance member 'self' cannot be used on type 'ViewController'; did you mean to use a value of this type instead?".

[swift-ci]

Comment by Jack March (JIRA)

@nolanw in my testing – from manually setting a first responder using becomeFirstResponder() – if there is a first responder, then that responder chain is used, otherwise it uses the responder chain of the sender. I would suggest using lldb + a private API to find the first responder:

po [[[UIApplication sharedApplication] keyWindow] performSelector:@selector(firstResponder)];

And seeing if it's different when the user has full keyboard access enabled. I couldn't reproduce any differences with full keyboard access in my test project so not sure if this will help, let me know either way!

[keith]

Note that there is a swiftlint rule for many cases of this that you can enable now before the warning lands in swift 5.6 https://realm.github.io/SwiftLint/self_in_property_initialization.html