Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

redundant_discardable_let rule causes a compilation error for things that are wrapped in a result-builder annotation #3855

Closed
2 tasks done
jeremypearson opened this issue Feb 15, 2022 · 7 comments · Fixed by #5929
Closed
2 tasks done

redundant_discardable_let rule causes a compilation error for things that are wrapped in a result-builder annotation #3855

jeremypearson opened this issue Feb 15, 2022 · 7 comments · Fixed by #5929
Labels
acceptable-false-positive False positives caused by rules that are unavoidable due to missing type information. bug Unexpected and reproducible misbehavior.

Comments

@jeremypearson
Copy link

jeremypearson commented Feb 15, 2022 •
edited
Loading

New Issue Checklist

Describe the bug

redundant_discardable_let rule causes a compilation error for things that are wrapped in a result-builder annotation.

Here's a link to the rule on GitHub which contains the bug: https://github.com/realm/SwiftLint/blob/master/Source/SwiftLintFramework/Rules/Style/RedundantDiscardableLetRule.swift

It's unsafe, for say, the body property of a struct which conforms to the View protocol from SwiftUI (and which therefore has a body property that's implicitly-annotated with the @ViewBuilder result-builder annotation), to replace this code snippet:

// Inside struct conforming to `View` protocol from `SwiftUI`.
var body: some View {
    let _ = 10

    Text("Hello, world!")
        .padding()
}

with this code snippet:

// Inside struct conforming to `View` protocol from `SwiftUI`.
var body: some View {
    _ = 10

    Text("Hello, world!")
        .padding()
}

as making such a replacement, would result in a compilation error. In this specific example, that compilation error will be: "Type '()' cannot conform to 'View'".

Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint lint /Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift
Linting Swift files at paths /Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift
Linting 'ContentView.swift' (1/1)
/Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift:13:9: warning: Redundant Discardable Let Violation: Prefer `_ = foo()` over `let _ = foo()` when discarding a result from a function. (redundant_discardable_let)
Done linting! Found 1 violation, 0 serious in 1 file.

Environments

  • SwiftLint version: 0.46.2

  • Installation method used: Homebrew

  • Paste your configuration file:
    N/A - default settings were used.

  • Nested configurations are not being used.

  • Xcode version: Xcode 13.2 Build version 13C5066c

  • Running the command: swiftlint lint --path /Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift --no-cache --enable-all-rules produced the following output:

/Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift:12:5: warning: Explicit ACL Violation: All declarations should specify Access Control Level keywords explicitly. (explicit_acl)
/Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift:11:1: warning: Explicit ACL Violation: All declarations should specify Access Control Level keywords explicitly. (explicit_acl)
/Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift:21:5: warning: Explicit ACL Violation: All declarations should specify Access Control Level keywords explicitly. (explicit_acl)
/Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift:20:1: warning: Explicit ACL Violation: All declarations should specify Access Control Level keywords explicitly. (explicit_acl)
/Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift:11:1: warning: Explicit Top Level ACL Violation: Top-level declarations should specify Access Control Level keywords explicitly. (explicit_top_level_acl)
/Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift:20:1: warning: Explicit Top Level ACL Violation: Top-level declarations should specify Access Control Level keywords explicitly. (explicit_top_level_acl)
/Users/jeremy/dev/xcode/SwiftLintBugDemoApp/SwiftLintBugDemoApp/ContentView.swift:13:9: warning: Redundant Discardable Let Violation: Prefer `_ = foo()` over `let _ = foo()` when discarding a result from a function. (redundant_discardable_let)
Done linting! Found 7 violations, 0 serious in 1 file.
import SwiftUI

struct ContentView: View {
    var body: some View {
        let _ = 10 // This line triggers a violation.

        Text("Hello, world!")
            .padding()
    }
}
@jpsim jpsim added the bug Unexpected and reproducible misbehavior. label Mar 9, 2022
@jpsim
Copy link
Collaborator

jpsim commented Mar 9, 2022

Thanks for the report. It should be possible to check to see if we're in a result builder.

@jeremypearson
Copy link
Author

jeremypearson commented Mar 10, 2022 •
edited
Loading

@jpsim you're welcome!

Do you think it would be a good idea to temporarily suppress the autofix-support for this rule and/or temporarily change it from being an opt-out rule to being an opt-in rule, just until this bug is fixed?

Perhaps such temporary changes would cause more problems than they're worth though. What are your thoughts :)?

@jpsim
Copy link
Collaborator

jpsim commented Mar 16, 2022

Perhaps such temporary changes would cause more problems than they're worth though

That's my thoughts on this. I suggest you disable the rule with // swiftlint:disable comments where you're encountering this until we find a resolution.

@jeremypearson
Copy link
Author

@jpsim Thanks. I've already opted to disable the rule by adding it to my disabled rules list in my config file, as in my projects, whenever I have a let _ = <expression> it's usually in an @ViewBuilder-annotated computed property variable - but for say, non-SwiftUI projects, I'd probably prefer to disable it in each individual case via // swiftlint:disable comments as you've recommended :).

@ZevEisenberg
Copy link
Contributor

Did anyone make any progress in detecting a builder context? I'm having this problem as well, and we are separately having this problem in Muter.

@SimplyDanny SimplyDanny added the acceptable-false-positive False positives caused by rules that are unavoidable due to missing type information. label Dec 3, 2023
@cabeca
Copy link

cabeca commented Dec 30, 2024

Just hit this problem myself. Any updates on the resolution/closing of this issue? I'm not sure if I agree with the acceptable-false-positive because if you have auto-fix enabled like we have for our project, this rule introduces compilation errors.

@SimplyDanny
Copy link
Collaborator

I'm not sure if I agree with the acceptable-false-positive because if you have auto-fix enabled like we have for our project, this rule introduces compilation errors.

The meaning of this label is that this is a bug we have to (not we want to) accept due to insufficient syntactic information available to reliably detect the cases where removing the let leads to a compilation error.

The described issue is such a bug. There is no way to know if the declaration is part of a result builder context.

The only improvement I can think of is to explicitly ignore the body: some View property. But fixing this for all kinds of result builders is impossible.

@SimplyDanny SimplyDanny mentioned this issue Dec 31, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
acceptable-false-positive False positives caused by rules that are unavoidable due to missing type information. bug Unexpected and reproducible misbehavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add option to disable redundant_discardable_let in SwiftUI view bodies SimplyDanny/SwiftLint
5 participants
@cabeca @ZevEisenberg @jpsim @jeremypearson @SimplyDanny