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.

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

Rule request: No explicit == true or == false #1502

Open
mdiep opened this issue May 8, 2017 · 10 comments
Open

Rule request: No explicit == true or == false #1502

mdiep opened this issue May 8, 2017 · 10 comments
Labels
rule-request Requests for a new rules.

Comments

@mdiep
Copy link

mdiep commented May 8, 2017

I occasionally come across code like this:

if condition == true {  }

which just be written as:

if condition {  }

I'd love to have a rule that disallowed == true and == false. (And != true, != false, which I haven't seen, but would be far worse.)

@jpsim jpsim added the rule-request Requests for a new rules. label May 8, 2017
@jpsim
Copy link
Collaborator

jpsim commented May 8, 2017

Agreed. This is a good starter rule to write if someone's interested in contributing to SwiftLint but finds other requested rules daunting.

Might want to also validate this rule with the boolean literal on the left hand side:

if true == condition {}
if false != condition {}

@SDGGiesbrecht
Copy link
Contributor

SDGGiesbrecht commented May 8, 2017

This is a good starter rule.

I’m not so sure. It’s more complicated than it looks at first glance.

For example, what about Bool??

func someFunction() -> Bool? {
    return true
}

if someFunction() == true {
    // Should the rule trigger?
}

if someFunction() {
    // This will not compile.
}

if someFunction() ?? false {
    // This would satisfy both the rule and the compiler,
    // but is this really what we want?
}

@masters3d
Copy link
Contributor

I'd like to add to also throw a violation when comparing to the same object.

if someObj == someObj {} // always true
if objA != objA {} // always false

@jpsim
Copy link
Collaborator

jpsim commented May 8, 2017

Ah, cursed optionals strike again! @masters3d could you please file a new ticket for that, as its implementation could be entirely different than the original request.

@mohamede1945
Copy link

For the case where comparing with Optional<Bool> . I think it's clearer to use ?? true|false operator than just compare with == true|false.
If I look again at @SDGGiesbrecht 's example at the call site.

if someFunction() == true {
}

It's not clear for the reader that someFunction() might return nil.

But

if someFunction() ?? false {
}

It's very clear that the function might return nil and also clear what to do in that case.

@marcelofabri
Copy link
Collaborator

You could also use an extension (since Swift 3.1):

extension Optional where Wrapped == Bool {
    var isTrue: Bool {
        switch self {
        case .none:
            return false
        case .some(let value):
            return value
        }
    }
}

@mohamede1945
Copy link

But that extension assumes nil is equivalent to false which isn't.

@marcelofabri
Copy link
Collaborator

IMO it doesn't assume. It'd be the same as using == true or ?? false. If you need another behavior based on if it's nil, it's just a matter of don't using the extension.

@koenpunt
Copy link

Ternary operations should also be considered;

index < activeIndex ? true : false

@starFelix
Copy link

it is 2021 now. will this rule be added?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule-request Requests for a new rules.
Projects
None yet
Development

No branches or pull requests

8 participants