krzysztofzablocki / Sourcery

Meta-programming for Swift, stop writing boilerplate code.
http://merowing.info
MIT License
7.7k stars 621 forks source link

Crash on multibyte enum case #69

Closed toshi0383 closed 7 years ago

toshi0383 commented 7 years ago

Arguments

/Users/toshi0383/github/Sourcery/test/ /Users/toshi0383/github/Sourcery/test/autoequatable.swift.template /Users/toshi0383/github/Sourcery/out/

source: test/Sample.swift

protocol AutoEquatable {}
enum JapaneseEnum {
    case アイウエオ
}
struct Cat: AutoEquatable {
    let name: String
    let age: Int
    let flag: Bool
}
struct Dog: AutoEquatable {
    let name: String
    let age: Int
    let flag: Bool
}

template: test/autoequatable.swift.template

{% for type in types.implementing.AutoEquatable %}
extension {{ type.name }}: Equatable {}

func == (lhs: {{ type.name }}, rhs: {{ type.name }}) -> Bool {
    {% for variable in type.storedVariables %} if lhs.{{ variable.name }} != rhs.{{ variable.name }} { return false }
    {% endfor %}
    return true
}
{% endfor %}

crash

xcode-crash

Looks like it's inside SourceKitten. SwiftLint doesn't crash on same code. I don't know why.

krzysztofzablocki commented 7 years ago

I pushed a fix to master, would you mind taking a look whether it works for your codebase? The commit with changes

Edit: according to @jpsim, treating key.offset / length as byteOffset is correct, so my fix of using that as characterOffset even if it passes sounds like a wrong approach. Need to take a look at this again and see if there is anything there that we might be doing wrong, or is this sourcekitten bug and in that case fill a report for them

jpsim commented 7 years ago

Yeah, this is likely a bug in SourceKitten, and likely the same issue as realm/SwiftLint#1006.

We're investigating the following re-implementation, but since that breaks other things we haven't fully determined the scope of this issue:

func lineAndCharacter(forByteOffset offset: Int) -> (line: Int, character: Int)? {
    let characterOffset = location(fromByteOffset: offset)
    return lineAndCharacter(forCharacterOffset: characterOffset)
}
jpsim commented 7 years ago

And to reiterate from twitter, yes all offsets and lengths reported by SourceKit are in bytes.

krzysztofzablocki commented 7 years ago

ok I think the master is working now, I used the suggestion for translating the byteOffset, once sourcekitten solves the internal issue we can go back to using old function. @toshi0383 can you confirm current master works for you? I added test but just to be sure

toshi0383 commented 7 years ago

Confirmed fix at master. 👍