Add joined_default_parameter opt-in rule #1746
Conversation
Generated by 🚫 Danger |
to discourage explicit usage of the default separator. Implements realm#1093.
ornithocoder
force-pushed
the
joined_default_parameter
branch
from
839497a
to
da823f5
Compare
Codecov Report
@@ Coverage Diff @@
## master #1746 +/- ##
==========================================
+ Coverage 87.75% 87.78% +0.02%
==========================================
Files 218 219 +1
Lines 10704 10730 +26
==========================================
+ Hits 9393 9419 +26
Misses 1311 1311
Continue to review full report at Codecov.
|
|
Should this one be opt-in or enabled by default @marcelofabri / @jpsim? |
|
I think it should be opt-in. By bet is that |
| let bodyOffset = dictionary.bodyOffset, | ||
| let bodyLength = dictionary.bodyLength else { return nil } | ||
|
|
||
| let body = file.contents.bridge().substringWithByteRange(start: bodyOffset, length: bodyLength) |
There was a problem hiding this comment.
what do you think about using the substructure instead?
{
"key.namelength": 10,
"key.nameoffset": 1,
"key.length": 25,
"key.substructure": [{
"key.namelength": 9,
"key.nameoffset": 12,
"key.length": 13,
"key.name": "separator",
"key.bodyoffset": 23,
"key.bodylength": 2,
"key.kind": "source.lang.swift.expr.argument",
"key.offset": 12
}],
"key.name": "bar.joined",
"key.bodyoffset": 12,
"key.bodylength": 13,
"key.kind": "source.lang.swift.expr.call",
"key.offset": 1
}It should be more reliable than the current implementation.
There was a problem hiding this comment.
Sure. Consider it done :-)
| let bodyOffset = dictionary.bodyOffset, | ||
| let bodyLength = dictionary.bodyLength else { return nil } | ||
| let argument = dictionary.substructure.first(where: { $0.name == "separator" }), | ||
| let argumentBodyOffset = argument.bodyOffset, |
There was a problem hiding this comment.
you can also check if argument.name == "separator"
There was a problem hiding this comment.
I've just checked in another version which check the number of arguments and the argument name.
ornithocoder
force-pushed
the
joined_default_parameter
branch
from
642c157
to
be2c121
Compare
| } | ||
|
|
||
| private func defaultSeparatorOffset(dictionary: [String: SourceKitRepresentable], file: File) -> Int? { | ||
| let arguments = dictionary.substructure.filter { $0.kind == SwiftExpressionKind.argument.rawValue } |
There was a problem hiding this comment.
you can use dictionary.enclosedArguments instead
|
|
||
| ### Examples | ||
|
|
||
| <details> |
There was a problem hiding this comment.
It looks like the examples are outdated too.
ornithocoder
force-pushed
the
joined_default_parameter
branch
from
be2c121
to
3252673
Compare
|
@ornithocoder what do you think about making it autocorrectable? If you think it's easy, we can do on this PR. Otherwise we can leave it as a future improvement 👍 |
|
@marcelofabri I'd do in a different PR, if that's OK. Thanks! :-) |
|
Sure, no problem. Thanks! 💯 |
as described in realm#1757 and realm#1746.
as described in realm#1757 and realm#1746.
as described in realm#1757 and realm#1746.
to discourage explicit usage of the default separator. Implements #1093.
[x] Adds opt-in rule
[x] Adds to tests (+ Linux)
[x] Adds to changelog
[x] Updates Rules.md