TypeBodyLengthRule should not count comment and whitespace lines #377
Conversation
|
👍 other than change log entry |
|
Fixed |
| @@ -181,4 +181,31 @@ extension File { | |||
| } | |||
| return violatingRanges | |||
| } | |||
|
|
|||
| internal func numberOfCommentOnlyLines(startLine: Int, endLine: Int) -> Int { | |||
There was a problem hiding this comment.
I thought these functions also excluded whitespace only lines? It doesn't seem like they do. I'm not sure what I'm missing.
There was a problem hiding this comment.
They do, but probably their names could be changed to reflect that.
syntaxKindsByLine() adds the whitespace only lines with an empty SyntaxKind array. Then numberOfCommentOnlyLines keeps these lines because of
kinds.filter { !commentKinds.contains($0) }.isEmptyas kinds will be empty, .filter won't have any effect, keeping the line on the resulting array of the outer .filter.
There was a problem hiding this comment.
Right, that line could use a short comment explaining that, and these functions should be renamed accordingly.
|
Renamed the functions in 1ad7e76 |
|
👍 |
|
I've rebased this with master. Is there anything else that needs to be done to get this merged? |
|
I think it would be better if it follows the 04267a4's example. |
|
You can check compilation time by |
|
OK, I'l try to refactor that later today |
| @@ -6,6 +6,10 @@ | |||
|
|
|||
| ##### Enhancements | |||
|
|
|||
| * TypeBodyLengthRule now does not count comment lines. | |||
There was a problem hiding this comment.
"comment or whitespace lines"? Same with the one for FunctionBodyLengthRule below.
|
✨ |
|
👍 |
|
👍 nicely done! |
TypeBodyLengthRule should not count comment and whitespace lines
|
🎉 |
|
Hmm, I just noticed that the tests now take much longer to run than before, and it's because @marcelofabri could you please look a bit more into this to see if the tests are doing tons of unnecessary work, or if it's the rule that's quite slow? |
|
@jpsim I did notice about the test issue and investigated. |
|
Sorry, it seems #399 did not reduce the duration of test as https://travis-ci.org/realm/SwiftLint/builds/104243015#L583. 😞 |
|
I've investigated a little and found that it seems that most of time is spent in I'm afraid that's as far as I got. If you guys have any ideas on how to make that faster, I'd be happy to implement. |
|
Current |
|
That apparently worked:
|
Fixes #369.