Added fileprivateRule #1489
Added fileprivateRule #1489
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1489 +/- ##
==========================================
+ Coverage 88.08% 88.09% +0.01%
==========================================
Files 196 198 +2
Lines 9791 9844 +53
==========================================
+ Hits 8624 8672 +48
- Misses 1167 1172 +5
Continue to review full report at Codecov.
|
| let violationOffsets = toplevel.flatMap { (offSet) -> Int? in | ||
| let parts = syntaxTokens.partitioned { offSet <= $0.offset } | ||
| guard let lastKind = parts.first.last, | ||
| lastKind.length == "fileprivate".bridge().length, |
There was a problem hiding this comment.
Full disclosure here: There is another builtin that has the same length as fileprivate. nonmutating has the same length of characters but nonmutating can never be toplevel in the current version of swift AFAIK. We could just file.substring to find out exactly what the builtin key is but that would slow down this pass.
|
This doesn't close #1058 because my intention there is broader. For example, this should trigger: class Foo {
fileprivate var bar = ""
func something() {
print(bar)
}
} |
|
@marcelofabri That case is triggered in the |
My idea was to trigger only if we find out that the variable is only used in the current type scope (i.e. changing to Example1: class Foo {
fileprivate var bar = ""
func something() {
print(bar)
}
}Example2: class Foo {
fileprivate var bar = ""
}
extension Foo {
func printBar() {
print(bar)
}
}I know that there's the strict mode, but that comes with false positives as well. In a perfect scenario, I'd like this rule to always flag cases where I still think this is useful without implementing the full #1058 issue tough. |
|
I see. I was thinking |
masters3d
force-pushed
the
limit_fileprivate_usage
branch
2 times, most recently
from
79115ff to
7c0f637
Compare
|
@marcelofabri I rebased this. Let me know if there is anything that would be blocking this. |
masters3d
force-pushed
the
limit_fileprivate_usage
branch
from
7c0f637 to
985f9f7
Compare
Added
fileprivateRuleto check for top-level usages offileprivateand recommendprivateinstead. This is inline with SE-0169's goal "forfileprivateto be used rarely". There is a also an "strict" option that will mark everyfileprivateas a violation.closes #1469
benchmark