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

vertical_whitespace_between_cases gives false warning if blank line has whitespace #2538

Closed
BenStaveleyTaylor opened this Issue Jan 2, 2019 · 6 comments

Comments

Projects
None yet
2 participants
@BenStaveleyTaylor
Copy link
Contributor

BenStaveleyTaylor commented Jan 2, 2019

New Issue Checklist

Describe the bug

The vertical_whitespace_between_cases rule flags a warning even when there is a blank line before the case or default keyword, if that blank line contains any white space. Totally whitespace lines should be accepted as being blank.

Explanation: Xcode tends to insert trailing whitespace when it formats text and its Text Editing preference for "Automatically trim trailing whitespace" is not reliable. We live in a world where there frequently is trailing whitespace and so we disable the trailing_whitespace rule.

Complete output when running SwiftLint, including the stack trace and command used
$ ./swiftlint --config swiftlint.yml
Loading configuration from 'swiftlint.yml'
Linting Swift files at paths 
Linting 'vertical_whitespace_between_cases.swift' (1/1)
/Users/bestave/Desktop/vertical_whitespace_between_cases/vertical_whitespace_between_cases.swift:9:1: warning: Vertical Whitespace Between Cases Violation: Include a single empty line between switch cases. (vertical_whitespace_between_cases)
Done linting! Found 1 violation, 0 serious in 1 file.

Environment

  • SwiftLint version (run swiftlint version to be sure)? 0.29.2
  • Installation method used (Homebrew, CocoaPods, building from source, etc)? CocoaPods
  • Paste your configuration file:
disabled_rules:
  - trailing_whitespace
  
opt_in_rules:
  - vertical_whitespace_between_cases
  • Are you using nested configurations? No
  • Which Xcode version are you using? 10.1
  • Do you have a sample that shows the issue?
enum Test {

    static func caseTest(value: Int) -> String {

        switch value {
        case 1:
            return "One"
         
        default:
            return "Not One"
        }
    }

}

Note: the blank line before default contains some trailing whitespace

@BenStaveleyTaylor BenStaveleyTaylor changed the title vertical_whitespace_between_cases doesn't work if trailing_whitespace rule is disabled vertical_whitespace_between_cases gives false warning if blank line has whitespace Jan 2, 2019

@BenStaveleyTaylor

This comment has been minimized.

Copy link
Contributor

BenStaveleyTaylor commented Jan 2, 2019

@marcelofabri

This comment has been minimized.

Copy link
Collaborator

marcelofabri commented Jan 2, 2019

@BenStaveleyTaylor this should be a good starter bug if you wanna try fixing it!

@marcelofabri marcelofabri added the bug label Jan 3, 2019

@BenStaveleyTaylor

This comment has been minimized.

Copy link
Contributor

BenStaveleyTaylor commented Jan 3, 2019

@marcelofabri OK, that's a challenge then :-) I'll have a go shortly...

@marcelofabri

This comment has been minimized.

Copy link
Collaborator

marcelofabri commented Jan 3, 2019

Let me know if you have any questions! You should only have to change VerticalWhitespaceBetweenCasesRule.swift, so definitely start there.

@BenStaveleyTaylor

This comment has been minimized.

Copy link
Contributor

BenStaveleyTaylor commented Jan 3, 2019

@marcelofabri Thanks. I haven't been a contributor on any open source projects before so the process is my main concern. Are there some guidelines for new contributors I can read?

@marcelofabri

This comment has been minimized.

Copy link
Collaborator

marcelofabri commented Jan 3, 2019

We have some documentation in CONTRIBUTING.MD, but feel free to ask any questions!

kimdv pushed a commit to kimdv/SwiftLint that referenced this issue Jan 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment