Add single_test_class opt-in rule #1780
Conversation
ornithocoder
changed the title
ornithocoder
force-pushed
the
quick_spec_limit
branch
from
457aea0 to
ec9f687
Compare
Implements realm#1779 (Limit number of QuickSpecs in file to one).
ornithocoder
force-pushed
the
quick_spec_limit
branch
from
cef1f5f to
f68c362
Compare
ornithocoder
force-pushed
the
quick_spec_limit
branch
from
f68c362 to
d3a063e
Compare
|
|
||
| private func toViolation(in file: File, | ||
| configuration: SeverityConfiguration, | ||
| numberOfSpecs: Int) -> ([String: SourceKitRepresentable]) -> StyleViolation? { |
There was a problem hiding this comment.
change numberOfSpecs to numberOfTestClasses (or any other name to avoid using specs)
There was a problem hiding this comment.
not applicable anymore.
| } | ||
| } | ||
|
|
||
| private func toViolation(in file: File, |
There was a problem hiding this comment.
what do you think about changing this method name? For me, toSomething usually means that the method will use self as input and convert it to Something.
There was a problem hiding this comment.
Actually, just now I've realized that it returns a function. Do we really need that? I think it just makes harder to read and the benefits are not so evident.
There was a problem hiding this comment.
agree. it's a simple function and doesn't need to be isolated. changing it to a closure.
| } | ||
|
|
||
| private func toViolation(in file: File, | ||
| configuration: SeverityConfiguration, |
There was a problem hiding this comment.
you don't need to pass the configuration here since it's a property anyway
There was a problem hiding this comment.
not applicable anymore.
| } | ||
| } | ||
|
|
||
| private func testClasses() -> [String] { |
There was a problem hiding this comment.
this could be a let to avoid being created everytime (or even declared in testClasses(in:)).
|
|
||
| private func testClasses(in file: File) -> [[String: SourceKitRepresentable]] { | ||
| return file.structure.dictionary.substructure.filter { | ||
| !$0.inheritedTypes.filter { testClasses().contains($0) }.isEmpty |
There was a problem hiding this comment.
you probably should check if the type is a class as well (just in case ¯\(ツ)/¯)
|
Thanks @ornithocoder! 🚀 |
|
You're welcome @marcelofabri! :-) Thank you for the code review! |
Implements #1779 (Limit number of QuickSpecs and XCTestCases in file to one).