Segmentation fault on trailing_comma with Swift 2.3

[csjones]

Currently exiting with code 139 on lint.

2016-12-01 22:06:49.888 swiftlint[87645:2364960] *** -[__NSCFString substringWithRange:]: Range {2127, 18446744073709551614} out of bounds; string length 3640. This will become an exception for apps linked after 10.10 and iOS 8. Warning shown once per app execution.

[jpsim]

@csjones how can we reproduce this? Can you share a repro case?

[jpsim]

Can you please also confirm that this issue is still present on master?

[csjones]

Offending line:

let unlockRequestSetupError = NSError(domain: "UnlockSetup_Error",
                    code: 22,
                    userInfo: [NSLocalizedDescriptionKey: "unlock setup error \(error)"])

[davemorrissey]

Just to add we're seeing exactly the same problem - builds succeed from the command line but fail with code 139 within xcode. Disabling the trailing_comma rule solves this for us too.

In case it helps, we run swiftlint as a build phase using the following command:

type swiftlint &> /dev/null && swiftlint lint

Here's the output.

2016-12-02 09:03:34.850 swiftlint[66635:12208042] *** -[__NSCFString substringWithRange:]: Range {1431, 18446744073709551615} out of bounds; string length 2370. This will become an exception for apps linked after 10.10 and iOS 8. Warning shown once per app execution.
/Users/dave/Library/Developer/Xcode/DerivedData/MyProject-alzojasizhzaabfvovkewbjzheyh/Build/Intermediates/MyProject.build/Debug-iphoneos/MyProject.build/Script-286C48BD1D08288300EF4DF0.sh: line 2: 66635 Segmentation fault: 11  swiftlint lint
Command /bin/sh failed with exit code 139

[jpsim]

Again, I'd appreciate steps to reproduce or a confirmation that this issue also exists on master.

[davemorrissey]

I've installed the master version and the problem exists there too.

Rough steps to reproduce:

1. Start with an existing swift project.
2. Edit a swift file to contain a trailing comma, as per @csjones' example
3. Add a "Run Script" build phase, with shell /bin/sh and command type swiftlint &> /dev/null && swiftlint lint. Leave checkboxes unticked.
4. Run Project > Build

There may be more to this but I didn't set up the project I'm working with so can't be certain. Note we've tried specifying /bin/bash but xcode seems to ignore it.

[marcelofabri]

I couldn't reproduce this on master. Can someone upload a full sample project that demonstrates the issue just to be sure I've tested properly?

[davemorrissey]

Sorry I'd have to create that from scratch but this may help:

The problem occurs on line 73/74 of TrailingCommaRule.swift:

        let contentsAfterLastElement = file.contents
            .substringWithByteRange(start: lastPosition, length: length) ?? ""

Specifically it seems to be in String+SourceKitten.swift within the byteRangeToNSRange function, but I haven't been able to determine exactly why.

I've determined that this will only occur if the source file contains an extended ASCII character - we have © in our header but any other character in the 128-255 range seems to cause it.

[jpsim]

Could you please try again with SwiftLint 0.14, which was just released?

[davemorrissey]

0.14 crashes with the same error. Have you tried adding a © character and running swiftlint as a build phase?

[jpsim]

Adding © to every Swift file in SwiftLint in the comment header (// SwiftLint ©) and running as a build phase doesn't cause any issues... 🤔

[davemorrissey]

Hmm, we have a mystery. I'll try to find some time to debug byteRangeToNSRange.

[marcelofabri]

@davemorrissey @csjones I've opened #1032 which is an attempt to fix the issue. Could you try that?

[davemorrissey]

Yes I think that does fix it. However I think there may be a deeper problem. In xcode we're now seeing over a thousand false positives for Unused Closure Parameter, and many more for Closure Parameter Position, none of which appear when swiftlint is run from the command line. I haven't got much closer to figuring out why this is, but then I'm not really an iOS developer so the intricacies of string encoding are beyond me.

[jpsim]

Is it possible you have more than one Xcode version installed? Or multiple Swift toolchains?

I had Xcode 7.3.1 xcode-selected earlier and was getting some Unused Closure Parameter issues when running on a Swift 3 codebase, so that might explain things.

[marcelofabri]

BTW, I run SwiftLint 0.14 against all projects listed in #1025 and all trigged unused_closure_parameter, trailing_comma and closure_parameter_position violations seem to be legit.

[davemorrissey]

I removed an old Xcode beta with no luck. Our builds in CircleCI were also affected and I assume that's a clean environment. I had no false positives building and testing the swiftlint project. This suggests it's something in our project config, but I have no idea where to look.

[marcelofabri]

@davemorrissey What's your Xcode and Swift version? Maybe that can help us.

[davemorrissey]

I have xcode 8.2.1 but I'm not sure what CircleCI uses. Our project is swift 2.3.

[kalambet]

Same problem with CircleCI and swiftlint 0.15.
We are also using 8.2.1 locally and Swift 2.3

[marcelofabri]

I was able to reproduce the issue using Xcode 7.3.x and the snippet @csjones shared. I'll try to investigate later.

[marcelofabri]

I don't think #1032 is the proper fix for this issue.

Here's what I get with Swift 3:

Linting '921.swift' (1/1)
element: NSLocalizedDescriptionKey
element: "unlock setup error \(error)"
End positions: [133, 164]
bodyLength: 56
bodyOffset: 108
lastPosition: 164
length: 0

And Swift 2.3:

Linting '921.swift' (1/1)
element: NSLocalizedDescriptionKey
element: "unlock setup error \(error)"])
End positions: [133, 166]
bodyLength: 56
bodyOffset: 108
lastPosition: 166
length: -2

For some reason it includes an ]) in the last element.

[marcelofabri]

For the false positives in unused_closure_parameter and closure_parameter_position rules (which is being tracked on #1019), I've opened #1081 which I think solves the issue.

[marcelofabri]

I've taken another look at this and it seems to happen only when using interpolation inside the dictionary. I was able to reduce to this:

foo(["a": "\(error)" ])

I think I'm OK with merging #1032 and having false negatives on these cases. Swift 2.3 will not survive long anyways.

[ryanfitz]

@marcelofabri is it possible to push out a new release version with these swift 2.3 fixes?

[marcelofabri]

@ryanfitz I think we're releasing a new version after #1073, #1144 and #1145 are merged.

In the meantime you can either use the version from master or temporally disable the rule.