Linting CRLF encoded files causes an empty line to be inserted between each line of code

ghost commented 7 years ago

New Issue Checklist

[x] Updated SwiftLint to the latest version
[x] I searched for existing GitHub issues

Issue Description

Complete output when running SwiftLint, including the stack trace and command used

$ swiftlint autocorrect

Environment

SwiftLint version 0.21.0
Installation method used Homebrew
Paste your configuration file:

# comments_space: # From https://github.com/brandenr/swiftlintconfig
#   name: "Space After Comment"
#   regex: '(^ *//\w+)'
#   message: "There should be a space after //"
#   severity: error
#
# force_https: # From https://github.com/Twigz/Game
#   name: "Force HTTPS over HTTP"
#   regex: "((?i)http(?!s))"
#   match_kinds: string
#   message: "HTTPS should be favored over HTTP"
#   severity: warning
#
# double_space: # From https://github.com/IBM-Swift/Package-Builder
#   include: "*.swift"
#   name: "Double space"
#   regex: '([a-z,A-Z] \s+)'
#   message: "Double space between keywords"
#   match_kinds: keyword
#   severity: warning

disabled_rules: # rule identifiers to exclude from running
  #Errors
  - force_try
  - force_cast
  - line_length
  - file_length
  - type_body_length
  - function_body_length
  - identifier_name
  - cyclomatic_complexity
  - large_tuple
  #Warnings
  - trailing_comma # Keep this exclusion
  - empty_parentheses_with_trailing_closure
  - function_parameter_count
  - vertical_whitespace # This rule is confused by Runes it seems

opt_in_rules: # some rules are only opt-in
  # Find all the available rules by running:
  # swiftlint rules

included: # paths to include during linting. `--path` is ignored if present.

excluded: # paths to ignore during linting. Takes precedence over `included`.
  - Carthage
  - Pods
  - fastlane

#reporter: "xcode" # reporter type (xcode, json, csv, checkstyle)

Are you using nested configurations? No
Which Xcode version are you using (check xcode-select -p)? 8.3.3
Do you have a sample example that shows the issue? No. Create any CRLF encoded swift file.

marcelofabri commented 7 years ago

It looks like vertical_whitespace is the one to blame. Could please post the result of swiftlint autocorrect to confirm?

(PS: it'd also be weird if it was, because you're disabling that rule in your configuration file)

ghost commented 7 years ago

I can't post any code that is actually reproducing the problem. It's company code. The problem immediately went away when I changed the file encoding from CRLF to LF.

I'm trying to make a sample file to reproduce the issue, but I can't get it to create the extra empty lines any more. Atom seems to have trouble with CRLF line endings, though. See image below.

crlfatom

ghost commented 7 years ago

We have SwiftLint run in a Build Phase in our iOS project using the script below. I never see the issue then. It only happens when I open terminal and run using the autocorrect command. Which brings up another point, running swiftlint autocorrect in the Xcode Build Phase corrects very little, whereas, running autocorrect via Terminal corrects a ton, but also produces this bug. That's a little frustrating.

Xcode Build Phase script

if which swiftlint >/dev/null; then
    swiftlint autocorrect
else
    echo "Warning: SwiftLint not installed, install using 'brew install swiftlint' or download from https://github.com/realm/SwiftLint"
fi

ghost commented 7 years ago

Here is another screenshot of Atom with an even better bug. The red indicator for the error isn't even near where the error occurs. Similar issues occur in terminal where the line number given for the warning or error is not even close to where the error occurs. crlfatom2

ghost commented 7 years ago

And here I have only changed the file encoding from CRLF to LF. All the warnings have gone away and the error correctly shows where the error is. You'll notice I haven't even saved the file yet after changing the encoding. crlfatom3

ghost commented 7 years ago

Sorry, I am unable to reproduce the original issue. I even tried converting the original file where I found this problem back to CRLF. Now I only see the issues mentioned above about the line numbers being wrong, but no extra empty lines.

Sorry to flood you with screenshots. I hope they are helpful. Here's one that shows all the Linter packages I have installed in Atom atomlinters

ornithocoder commented 7 years ago

Hi @chadcummings, these are the steps I've used to reproduce it without plugins and/or IDE extensions:

Create a file with CRLF line terminators (I've used VIM for that)

$ cat quick.swift 
internal struct Toto {
    func foo() -> Bool {
        return false
    }

    func bar(_ variable: Bool) -> Bool {
        return !variable
    }
}

Make sure it has CRLF terminators

$ file quick.swift 
quick.swift: ASCII text, with CRLF line terminators

Autocorrect the file

$ swiftlint autocorrect --path quick.swift
$ cat quick.swift 
internal struct Toto {

    func foo() -> Bool {

        return false

    }

    func bar(_ variable: Bool) -> Bool {

        return !variable

    }

}

After running autocorrect once, the file won't have CRLF line terminators anymore.

$ file quick.swift 
quick.swift: ASCII text

ornithocoder commented 7 years ago

For this example (w/ CRLF line terminators):

internal struct Toto {
    func foo() -> Bool {
        return false
    }

    func bar(_ variable: Bool) -> Bool {
        return !variable
    }
}

SourceKittenFramework's File.init(path:) returns 18 lines:

(lldb) po contents.bridge().lines().count
18

po contents.bridge().lines()
▿ 18 elements
  ▿ 0 : Line
    - index : 1
    - content : "internal struct Toto {"
    ▿ range : {0, 23}
      - location : 0
      - length : 23
    ▿ byteRange : {0, 23}
      - location : 0
      - length : 23
  ▿ 1 : Line
    - index : 2
    - content : ""
    ▿ range : {23, 1}
      - location : 23
      - length : 1
    ▿ byteRange : {23, 1}
      - location : 23
      - length : 1
  ▿ 2 : Line
    - index : 3
    - content : "    func foo() -> Bool {"
    ▿ range : {24, 25}
      - location : 24
      - length : 25
    ▿ byteRange : {24, 25}
      - location : 24
      - length : 25
  ▿ 3 : Line
    - index : 4
    - content : ""
    ▿ range : {49, 1}
      - location : 49
      - length : 1
    ▿ byteRange : {49, 1}
      - location : 49
      - length : 1
  ▿ 4 : Line
    - index : 5
    - content : "        return false"
    ▿ range : {50, 21}
      - location : 50
      - length : 21
    ▿ byteRange : {50, 21}
      - location : 50
      - length : 21
  ▿ 5 : Line
    - index : 6
    - content : ""
    ▿ range : {71, 1}
      - location : 71
      - length : 1
    ▿ byteRange : {71, 1}
      - location : 71
      - length : 1
  ▿ 6 : Line
    - index : 7
    - content : "    }"
    ▿ range : {72, 6}
      - location : 72
      - length : 6
    ▿ byteRange : {72, 6}
      - location : 72
      - length : 6
  ▿ 7 : Line
    - index : 8
    - content : ""
    ▿ range : {78, 1}
      - location : 78
      - length : 1
    ▿ byteRange : {78, 1}
      - location : 78
      - length : 1
  ▿ 8 : Line
    - index : 9
    - content : ""
    ▿ range : {79, 1}
      - location : 79
      - length : 1
    ▿ byteRange : {79, 1}
      - location : 79
      - length : 1
  ▿ 9 : Line
    - index : 10
    - content : ""
    ▿ range : {80, 1}
      - location : 80
      - length : 1
    ▿ byteRange : {80, 1}
      - location : 80
      - length : 1
  ▿ 10 : Line
    - index : 11
    - content : "    func bar(_ variable: Bool) -> Bool {"
    ▿ range : {81, 41}
      - location : 81
      - length : 41
    ▿ byteRange : {81, 41}
      - location : 81
      - length : 41
  ▿ 11 : Line
    - index : 12
    - content : ""
    ▿ range : {122, 1}
      - location : 122
      - length : 1
    ▿ byteRange : {122, 1}
      - location : 122
      - length : 1
  ▿ 12 : Line
    - index : 13
    - content : "        return !variable"
    ▿ range : {123, 25}
      - location : 123
      - length : 25
    ▿ byteRange : {123, 25}
      - location : 123
      - length : 25
  ▿ 13 : Line
    - index : 14
    - content : ""
    ▿ range : {148, 1}
      - location : 148
      - length : 1
    ▿ byteRange : {148, 1}
      - location : 148
      - length : 1
  ▿ 14 : Line
    - index : 15
    - content : "    }"
    ▿ range : {149, 6}
      - location : 149
      - length : 6
    ▿ byteRange : {149, 6}
      - location : 149
      - length : 6
  ▿ 15 : Line
    - index : 16
    - content : ""
    ▿ range : {155, 1}
      - location : 155
      - length : 1
    ▿ byteRange : {155, 1}
      - location : 155
      - length : 1
  ▿ 16 : Line
    - index : 17
    - content : "}"
    ▿ range : {156, 2}
      - location : 156
      - length : 2
    ▿ byteRange : {156, 2}
      - location : 156
      - length : 2
  ▿ 17 : Line
    - index : 18
    - content : ""
    ▿ range : {158, 1}
      - location : 158
      - length : 1
    ▿ byteRange : {158, 1}
      - location : 158
      - length : 1

marcelofabri commented 7 years ago

Nice investigation work, @ornithocoder 🕵️

Maybe this is the problem?

ornithocoder commented 7 years ago

Apparently .components(separatedBy: .newlines) returns the empty lines when the file has CRLF line terminators. So enumerator is already looping thru twice the number of lines.

ghost commented 7 years ago

Sounds like you all have it all under control! I'll let you take it from here. Just ping me if you need anything else from me. Happy Coding!

marcelofabri commented 7 years ago

I've come up with a fix for that in SourceKitten: https://github.com/marcelofabri/SourceKitten/commit/01195936a6251b269038e3276cfb097f6d7cdfcc

However, that API is not implemented on Linux yet 🙃

fatal error: enumerateSubstrings(in:options:using:) is not yet implemented: file Foundation/NSString.swift, line 770

masters3d commented 6 years ago

I just encountered this too. @marcelofabri any update on the status of Linux and SourceKitten? Thanks

marcelofabri commented 6 years ago

No updates yet.

mildm8nnered commented 1 week ago

I just verified this is still an issue as of 0.57.0

realm / SwiftLint