-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Navigate substructure even for different kinds on ASTRule #1248
Conversation
let offset = dictionary.offset, | ||
let length = dictionary.length, | ||
let range = file.contents.bridge().byteRangeToNSRange(start: offset, length: length) else { | ||
let elements = dictionary.elements?.filter({ $0.kind == conditionKind }) else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was needed to make sure that the pattern was in the condition, not inside an if
body
Codecov Report
@@ Coverage Diff @@
## master #1248 +/- ##
=========================================
Coverage ? 81.93%
=========================================
Files ? 168
Lines ? 8348
Branches ? 0
=========================================
Hits ? 6840
Misses ? 1508
Partials ? 0
Continue to review full report at Codecov.
|
Thanks for submitting this one! I'll test this locally shortly, FWIW the |
@keith I think that might be another issue because that rule isn't an |
Sorry, wrong rule, I meant the |
It looks like with this test case in
This still fails on this branch |
Ah, it looks like there's some duplicate custom logic in that rule that we'll need to fix as well. |
From a quick look I haven't found a motive to fail. My bet is that it's the same problem as #136. |
In fact, running {
"key.diagnostic_stage" : "source.diagnostic.stage.swift.parse",
"key.substructure" : [
{
"key.offset" : 0,
"key.nameoffset" : 5,
"key.accessibility" : "source.lang.swift.accessibility.internal",
"key.length" : 47,
"key.name" : "foo()",
"key.bodyoffset" : 12,
"key.kind" : "source.lang.swift.decl.function.free",
"key.namelength" : 5,
"key.bodylength" : 34
}
],
"key.offset" : 0,
"key.length" : 48
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. @keith it's hard to tell if you still have some reservations about this.
Nope, I think this fixes what it meant to. This just didn't fix what I was hoping, but that's not the fault of this code, just my misunderstanding! |
🚢 ❗️ |
Fixes #1237
I expect oss-check to introduce some violations that weren't being caught before.