SwiftLint icon indicating copy to clipboard operation
SwiftLint copied to clipboard

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

Open ghost opened this issue 8 years ago • 15 comments

New Issue Checklist

Issue Description

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

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.

ghost avatar Aug 18 '17 06:08 ghost

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)

marcelofabri avatar Aug 18 '17 09:08 marcelofabri

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 avatar Aug 18 '17 23:08 ghost

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 avatar Aug 18 '17 23:08 ghost

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 avatar Aug 19 '17 00:08 ghost

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 avatar Aug 19 '17 00:08 ghost

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

ghost avatar Aug 19 '17 00:08 ghost

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 avatar Aug 19 '17 07:08 ornithocoder

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

ornithocoder avatar Aug 19 '17 15:08 ornithocoder

Nice investigation work, @ornithocoder 🕵️

Maybe this is the problem?

marcelofabri avatar Aug 21 '17 09:08 marcelofabri

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.

ornithocoder avatar Aug 21 '17 12:08 ornithocoder

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!

ghost avatar Aug 21 '17 19:08 ghost

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

marcelofabri avatar Aug 22 '17 13:08 marcelofabri

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

masters3d avatar Mar 31 '18 05:03 masters3d

No updates yet.

marcelofabri avatar Mar 31 '18 17:03 marcelofabri

I just verified this is still an issue as of 0.57.0

mildm8nnered avatar Oct 30 '24 23:10 mildm8nnered