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

False positives and false negatives with unused_optional_binding #1376

Closed
drodriguez opened this issue Mar 20, 2017 · 5 comments
Closed

False positives and false negatives with unused_optional_binding #1376

drodriguez opened this issue Mar 20, 2017 · 5 comments
Labels

Comments

@drodriguez
Copy link

// Should not trigger, but triggers
if case .some(_) = self {
}

// Should trigger, but doesn’t
if case .some(let _) = self {
}

Seems that any usage of case is ignored.

Reviewing the rule code, the regular expressions used are quite broad and don’t even check for let being or not there.

Something like the following almost get it right:

        let underlineOutsideParenthesis = "(?<=let\\s)_\\s*="
        let underlineInsideParenthesis = "(?<=let\\s)\\((\\s*[_,]\\s*)+\\)"
        let pattern = "(("  + underlineOutsideParenthesis + ")|(" + underlineInsideParenthesis + "))"

But it doesn’t catch the case .some(let _) case.

/cc @marcelofabri

@crayment
Copy link

Getting false positive a lot for code like:

guard case .dictionary(_) = json else {

@marcelofabri
Copy link
Collaborator

@drodriguez Can you send a PR fixing the false positives? I think that is more important than catching the case .some(let _) case for now.

We can then create another issue just for that and address in the future.

drodriguez pushed a commit to drodriguez/SwiftLint that referenced this issue Mar 26, 2017
The rule for unused optional binding was false triggering for pattern matching where the associated value was just an underscore. Additionally, pattern matching mixed with unused optional binding was not triggering in some cases.

New non triggering and triggering examples have been added for the failing cases, and new regular expressions are used to catch all the cases.

Modify the code in `File+SwiftLint` to expose full `NSTextCheckingResult` (and not just ranges) and rewire some pieces to use the new functions.

Fixes realm#1376
@drodriguez
Copy link
Author

PR sent with the full solution (hopefully): #1396

drodriguez pushed a commit to drodriguez/SwiftLint that referenced this issue Mar 28, 2017
The rule for unused optional binding was false triggering for pattern matching where the associated value was just an underscore. Additionally, pattern matching mixed with unused optional binding was not triggering in some cases.

New non triggering and triggering examples have been added for the failing cases, and new regular expressions are used to catch all the cases.

Modify the code in `File+SwiftLint` to expose full `NSTextCheckingResult` (and not just ranges) and rewire some pieces to use the new functions.

Fixes realm#1376
@alejandro-isaza
Copy link

I'm getting a false positive with

if let point = state.find({ _ in true }) { ... }

@drodriguez
Copy link
Author

I think the fix in #1396 will avoid the false positive in @Aleph7 example. When I have a moment I will add it to the examples of that PR and check for sure.

@jpsim jpsim closed this as completed in c29cf9a Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants