Add private_only to prefixed_toplevel_constant #2315
Conversation
Generated by 🚫 Danger |
Codecov Report
@@ Coverage Diff @@
## master #2315 +/- ##
==========================================
- Coverage 92.02% 91.93% -0.09%
==========================================
Files 292 293 +1
Lines 14763 14782 +19
==========================================
+ Hits 13585 13590 +5
- Misses 1178 1192 +14
Continue to review full report at Codecov.
|
keith
force-pushed
the
ks/toplevel-private
branch
from
f18b518
to
845b616
Compare
| @@ -54,6 +54,11 @@ public struct PrefixedTopLevelConstantRule: ASTRule, OptInRule, ConfigurationPro | |||
| public func validate(file: File, | |||
| kind: SwiftDeclarationKind, | |||
| dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { | |||
| if self.configuration.onlyPrivateMembers, | |||
There was a problem hiding this comment.
Maybe this could be combined with the other guard below, since both are used for early exiting.
There was a problem hiding this comment.
I thought about that but since we only want to take into account if the ACL is private if this configuration is set, it felt like it was getting too complicated to be in that one as well.
| @@ -54,6 +54,11 @@ public struct PrefixedTopLevelConstantRule: ASTRule, OptInRule, ConfigurationPro | |||
| public func validate(file: File, | |||
| kind: SwiftDeclarationKind, | |||
| dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { | |||
| if self.configuration.onlyPrivateMembers, | |||
There was a problem hiding this comment.
You can safely drop the self. here.
| @@ -54,6 +54,11 @@ public struct PrefixedTopLevelConstantRule: ASTRule, OptInRule, ConfigurationPro | |||
| public func validate(file: File, | |||
| kind: SwiftDeclarationKind, | |||
| dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { | |||
| if self.configuration.onlyPrivateMembers, | |||
| let acl = dictionary.accessibility.flatMap(AccessControlLevel.init(identifier:)), !acl.isPrivate { | |||
| return [] | |||
There was a problem hiding this comment.
The indentation here looks a little bit odd.
There was a problem hiding this comment.
huh yea, tab got in here some how
You can do it in a similar way as this commit: https://github.com/realm/SwiftLint/pull/2308/files/73fafd5c071f59b7ed8c09a2fe493f5e202215ea..dc44fbcac37900283ea54072f2fa75c881dc950c. You can remove the |
This allows users to opt in to only validate top level constants have the given prefix if the constant is private or fileprivate.
keith
force-pushed
the
ks/toplevel-private
branch
from
999abf1
to
61844ea
Compare
|
Awesome, thanks for the example, added tests for this specific configuration |
keith
force-pushed
the
ks/toplevel-private
branch
from
61844ea
to
50c7165
Compare
| "public let Foo = 20.0" | ||
| ] | ||
|
|
||
| let alwaysOnSameLineDescription = PrefixedTopLevelConstantRule.description |
There was a problem hiding this comment.
can you rename this variable? (I just noticed it's also wrong on the linked PR 😅 )
* Add private_only to prefixed_toplevel_constant This allows users to opt in to only validate top level constants have the given prefix if the constant is private or fileprivate.
* Add private_only to prefixed_toplevel_constant This allows users to opt in to only validate top level constants have the given prefix if the constant is private or fileprivate.
This allows users to opt in to only validate top level constants have
the given prefix if the constant is private or fileprivate.
I'm not sure how to add a test with a configuration difference, but I'm happy to do that if it's possible.