Swiftlint: weak_delegate False Positives

[racer1988]

New Issue Checklist

• [x] Updated SwiftLint to the latest version (SwiftLint 0.33.0)
• [x] I searched for existing GitHub issues
  similar to #2019

Describe the bug

private lazy var tableViewDelegate : MyTableViewDelegate = {
        let delegate = MyTableViewDelegate()
        delegate.actionDelegate = self
        return delegate
    }()

where

@objc class MyTableViewDelegate: NSObject, UITableViewDelegate

this is not a delegate that needs to be weak. it's a table view delegate.

tableView.delegate = tableViewDelegate

where the delegate is not retained by the tableView so it mustn't be weak

Complete output when running SwiftLint, including the stack trace and command used

$ swiftlint lint file

Linting 'file.swift' (1/1)

file.swift:29:18: warning: Weak Delegate Violation: Delegates should be weak to avoid reference cycles. (weak_delegate)

Environment

• SwiftLint version (run swiftlint version to be sure)?

``` ./swiftlint version
0.33.0

* Installation method used (Homebrew, CocoaPods, building from source, etc)?

`CocoaPods`

* Paste your configuration file:

disabled_rules:

• line_length
• variable_name
• force_cast
• control_statement
• file_length
• force_try
• empty_enum_arguments
• cyclomatic_complexity
• type_body_length
• function_body_length
• type_name
• vertical_whitespace
• unused_closure_parameter
• comma
• legacy_constructor
• opening_brace
• trailing_semicolon
• trailing_newline
• implicit_getter
• empty_parentheses_with_trailing_closure
• valid_ibinspectable
• todo
• large_tuple
• block_based_kvo
• redundant_string_enum_value
• syntactic_sugar
• statement_position
• empty_parameters
• function_parameter_count
• nesting
• closing_brace
• closure_parameter_position
• shorthand_operator
• trailing_whitespace
• colon
• mark
• unused_optional_binding
• redundant_discardable_let
• unneeded_break_in_switch
• multiple_closures_with_trailing_closure
• fallthrough
• for_where
• notification_center_detachment

excluded: # paths to ignore during linting. Takes precedence over included.

• Pods
  ```

• Are you using nested configurations?

no

If so, paste their relative paths and respective contents.

• Which Xcode version are you using (check xcode-select -p)?

Version 10.2.1 (10E1001)

• Do you have a sample that shows the issue? Run echo "[string here]" | swiftlint lint --no-cache --use-stdin --enable-all-rules
  to quickly test if your example is really demonstrating the issue. If your example is more
  complex, you can use swiftlint lint --path [file here] --no-cache --enable-all-rules.

see decribe the bug section

[NrmeenTomoum]

I got same Error
Weak Delegate Violation: Delegates should be weak to avoid reference cycles. (weak_delegate)
@racer1988 you reached for any solution?

[racer1988]

@NrmeenTomoum yes, this is a "bug" in SwiftLint and I wanted to report it so that the rule itself can be modified to address this.

why the rule is triggered? I even saw this happens even if the class or variable are not called delegate

[marcelofabri]

SwiftLint currently doesn't know if a delegate property is meant to be set externally (so it should be weak) or if it's there to act as a delegate for another object.

You can workaround this by naming the property something that doesn't end with delegate. You can also disable the rule locally.

That said, we probably could add an exception for private properties in the rule.

[racer1988]

@marcelofabri I agree, however this is triggered even if the variable is not called delegate!

[marcelofabri]

It triggers if the variable ends with delegate

[robertmryan]

That said, we probably could add an exception for private properties in the rule.

That’s a promising idea (though, that of course presumes that you haven’t accepted an external reference in the init method), e.g.

class Foo {
    private var delegate: FooDelegate?

    init(delegate: FooDelegate) {
        self.delegate = delegate
    }
}

---

While we’re thinking about alternatives, another option is to disable the rule if it is initialized in its own declaration, e.g., here are real-world examples:

private let mapViewDelegate = MapViewDelegate()

or

private let textFieldDelegate = PhoneNumberTextFieldDelegate()

It seems exceedingly confusing to produce warning says “should be weak” when (a) this is a constant and cannot be weak; and (b) if you did make this a weak var, this delegate object would immediately be deallocated. It seems that if it is being initialized in its declaration, that’s a good candidate for not applying the weak_delegate rule.

[marcelofabri]

While we’re thinking about alternatives, another option is to disable the rule if it is initialized in its own declaration

Or if it's a let declaration 🤔

[robertmryan]

Yeah, perhaps. When I first drafted my comment, I was considering that, but stepped back from it on further reflection. Consider:

class Foo {
    private let delegate: FooDelegate

    init(delegate: FooDelegate) {
        self.delegate = delegate
    }
}

In that case, weak_delegate rule still might be appropriate, with perhaps the following being better:

class Foo {
    private weak var delegate: FooDelegate?

    init(delegate: FooDelegate) {
        self.delegate = delegate
    }
}

That’s why I gravitated to declaration followed by = and some initialization...

[mime29]

In our project, we have:

struct One {
    let delegate: OneTwoDelegate
    init(delegate: OneTwoDelegate) {
        self.delegate = delegate
    }
}

Can the rule be deactivated when it's happening in struct please (as weak is not actually possible in a struct anyway)?

[lawgimenez]

It's 2020. I'm still getting this false positive.

[stale[bot]]

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!