Explicit Enum Raw Value Rule #1788
Conversation
|
|
||
| let locs = substructureElements(of: dictionary, matching: .enumcase) | ||
| .flatMap { substructureElements(of: $0, matching: .enumelement) } | ||
| .flatMap(enumElementsMissingInitExpr(_:)) |
There was a problem hiding this comment.
You can safely remove the (_:) here.
| private func substructureElements(of dict: [String: SourceKitRepresentable], | ||
| matching kind: SwiftDeclarationKind) -> [[String: SourceKitRepresentable]] { | ||
| return dict.substructure | ||
| .filter { $0.kind.flatMap(SwiftDeclarationKind.init(rawValue:)) == kind } |
There was a problem hiding this comment.
Same here for (rawValue:)
| public init() {} | ||
|
|
||
| public static let description = RuleDescription( | ||
| identifier: "explicit_associated_enum_value", |
There was a problem hiding this comment.
this is not a good name, because when talking about enums, associated values are generally referred to cases with "parameters". Instead, we should use "raw values"
There was a problem hiding this comment.
I thought those were associated types, since jpsim called them associated values. But sure, I don't have any attachments to the name :)
There was a problem hiding this comment.
| } | ||
|
|
||
| // Check if it's an associated value enum | ||
| guard !dictionary.inheritedTypes.isEmpty else { |
There was a problem hiding this comment.
this is not enough, because enums can implement protocols, for example:
protocol Foo {}
enum Bar: Foo {
case bar
}Instead, we should keep a list of possible raw types that support implicit raw values. As far as I know, that means String and all numeric types (Int, Float, Int64, ...). Types like Character conform to RawRepresentable, but they need an explicit value.
| } | ||
| } | ||
|
|
||
| private func violatingOffsetsForEnum(dictionary: [String: SourceKitRepresentable], file: File) -> [Int] { |
There was a problem hiding this comment.
you don't actually use the file parameter here
|
I guess we should add |
|
After reading more on |
Mazyod
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1788 +/- ##
==========================================
+ Coverage 88.17% 88.21% +0.04%
==========================================
Files 223 224 +1
Lines 11027 11068 +41
==========================================
+ Hits 9723 9764 +41
Misses 1304 1304
Continue to review full report at Codecov.
|
| Int.self, Int8.self, Int16.self, Int32.self, Int64.self, | ||
| UInt.self, UInt8.self, UInt16.self, UInt32.self, UInt64.self, | ||
| Double.self, Float.self, Float80.self, Decimal.self, NSNumber.self, | ||
| NSDecimalNumber.self, "NSInteger", String.self |
There was a problem hiding this comment.
TIL that you can use NSNumber and other types as raw values 😅
I don't think so, because to add an explicit value, you still need to "inherit" from a raw value type. |
|
|
||
| private func violatingOffsetsForEnum(dictionary: [String: SourceKitRepresentable]) -> [Int] { | ||
|
|
||
| let locs = substructureElements(of: dictionary, matching: .enumcase) |
There was a problem hiding this comment.
Could you please combine the 3 flatMaps? I'd also recommend not storing the result in an intermediate variable, since it's unused other than just being returned right away, so doesn't make things any clearer IMHO.
There was a problem hiding this comment.
Sure, I can see this logic using a single loop with guards and continue
There was a problem hiding this comment.
Nevermind, I don't think my suggestion would improve things after all.
Hey, as @marcelofabri suggested, this was a good rule for new contributors to get their hands dirty (assuming I did this with reasonable correctness!).
I feel the PR is self-explanatory, but just to reiterate:
init_exprIntenums, andStringenums.Fixes #1778