unused_closure_parameter false positive on lazy var

[ignazioc]

When a closure is used in this way, swiftlint raise a warning about the variable lbl not being used.

    lazy var label: UILabel = { (lbl: UILabel) -> UILabel in
        lbl.backgroundColor = UIColor.red
        return lbl
    }(UILabel())

[jpsim]

Thanks for reporting this.

[ignazioc]

It would be nice to have kind of a tutor to fix on my own :)

…

-- Ignazio Calo'

On 11 January 2017 at 18:43:32, JP Simard ***@***.***) wrote: Thanks for reporting this. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1161 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAnaUyShMTx1wqqcjmJslE0zao4uJW-Lks5rRRTEgaJpZM4LgirT> .

[jpsim]

Some tips to debug this in SwiftLint yourself:

1. Clone and update submodules.
2. Build and run the swiftlint scheme from SwiftLint.xcworkspace, setting the scheme's "Working Directory" and arguments.
3. Set a breakpoint in the rule you're trying to debug so you can step through an inspect the state as you step through to identify why the code you shared is triggering a violation.

[marcelofabri]

You can also add your example to the rule examples. Another useful thing might be updating the rule to conform to OptInRule so your breakpoints only will be triggered with the examples (and not all files from the integration tests)

[marcelofabri]

Another tip is using SourceKitten to get the information about your example so you can understand the problem better.

[ignazioc]

Thanks for the suggestions. I was able to debug the issue and investigate a little bit using SourceKitten. Here what I've found.

This is the test input:

3.times { number in
	 number
}


{ ( lbl: UILabel) -> UILabel in
	lbl
}(obj)

SwiftLint raises a warning on the second closure

example.swift:7:5: warning: Unused Closure Parameter Violation: Unused parameter "lbl" in a closure should be replaced with _. (unused_closure_parameter)

Here the relevant output from SourceKitten:

"key.length" : 30,
"key.name" : "3.times",
"key.bodyoffset" : 10,
"key.kind" : "source.lang.swift.expr.call",
"key.offset" : 1,
"key.namelength" : 7

"key.length" : 43,
"key.name" : "{ ( lbl: UILabel) -> UILabel in\n\tlbl\n}",
"key.bodyoffset" : 73,
"key.kind" : "source.lang.swift.expr.call",
"key.offset" : 34,
"key.namelength" : 38

On the first example the closure's body can be found between key.bodyoffset and key.lenghth and SwiftLint code scans that part of the file to find any usage of the parameters.

Unfortunately on the second example the same indexes point outside of the definition of the closure, to the call ((obj)) part. Therefor SwifLint doesn't find any usage of the parameter and raise the (wrong) warning.

The first solution coming to my mind is to check if the key.name is actually a closure and in that case search there any usage of the parameter.
It doesn't look really clean so maybe some of you expert can give me an hint?

Thanks!

[marcelofabri]

I think it's hacky to try to use key.name because we don't have any AST information about its contents. What if the parameter is used in a string or comment?

IMHO, we should just detect this case (i.e. detect that key.name is a closure) and skip any validations for now.

[marcelofabri]

After all, this rule doesn't work with closure declarations as well (see #1082).

[ghost]

Not to hijack this thread, but a number of the suggestions given should probably be added to the CONTRIBUTING.md file to help future contributors.

[ignazioc]

agree.

…

On 16 January 2017 at 10:20, Aaron McTavish ***@***.***> wrote: Not to hijack this thread, but a number of the suggestions given should probably be added to the CONTRIBUTING.md file to help future contributors. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1161 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAnaUyUVziuD6463OHJ1B8U59C8snI3Fks5rSzZugaJpZM4LgirT> .

[marcelofabri]

@ignazioc I've opened #1252 to ignore this case for now and we can deal with it later in #1082.