-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#2065 - New rule: Empty and comment lines in function bodies. #2066
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget the Tests/LinuxMain.swift
and Tests/SwiftLintFrameworkTests/RulesTests.swift
files as well.
CHANGELOG.md
Outdated
@@ -22,10 +22,9 @@ | |||
|
|||
#### Enhancements | |||
|
|||
* Adds `discouraged_optional_boolean` opt-in rule to discourage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to remove this rule entry from the change log?
CHANGELOG.md
Outdated
the use of optional booleans. | ||
[Ornithologist Coder](https://github.com/ornithocoder) | ||
[#2011](https://github.com/realm/SwiftLint/issues/2011) | ||
* Adds `function_body_whitespace_comment` opt-in rule to prohibit empty and comment lines in function bodies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note the length of these lines should not exceed 80 columns.
identifier: "function_body_whitespace_comment", | ||
name: "Function Body Empty Lines", | ||
description: "Functions bodies should not have whitespace and comment lines.", | ||
kind: .metrics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about .style
instead of .metrics
?
@@ -0,0 +1,44 @@ | |||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using triggering and nontTriggering examples when defining the rule (see other rules for example). With triggering and nontTriggering SwiftLint will:
- Add examples to the Rules.md file
- Build and run testes automatically looking for violations (making this test file unnecessary).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ornithocoder I have changed the tests file. Now it is similar to FuctionBodyLengthRuleTests.
case let contentsNSString = file.contents.bridge(), | ||
let startLine = contentsNSString.lineAndCharacter(forByteOffset: bodyOffset)?.line, | ||
let endLine = contentsNSString.lineAndCharacter(forByteOffset: bodyOffset + bodyLength)?.line | ||
else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The else {
/ return
indentation can be improved. Try to select the lines and use ^i (Control+i) to fix it.
else { | ||
return [] | ||
} | ||
for parameter in configuration.params { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could map/flatMap there, returning an array of Violations. This would eliminate the need to return an empty array on line 48.
@ornithocoder Thank you for the feedback! I have addressed your concerns. |
@@ -20,13 +23,6 @@ | |||
|
|||
* None. | |||
|
|||
#### Enhancements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't remove previous changlog entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ornithocoder Sorry about that. This is fixed.
I've merged #2068 which should fix the CI failure. Can you please rebase this PR? |
Replaced loop with flatMap
Edited rules.md
939fca4
to
bf1b8cc
Compare
@marcelofabri Done. |
Codecov Report
@@ Coverage Diff @@
## master #2066 +/- ##
==========================================
+ Coverage 89.68% 89.68% +<.01%
==========================================
Files 259 261 +2
Lines 15020 15100 +80
Branches 977 979 +2
==========================================
+ Hits 13471 13543 +72
- Misses 1532 1540 +8
Partials 17 17
Continue to review full report at Codecov.
|
@marcelofabri This PR is ready to be merged. |
As per #2065
In this PR I have implemented a new rule, which allows to prohibit the usage of whitespace and comment line in function bodies.