A long-standing limitation with SourceKit's "editor open" request is
that we weren't able to get certain tokens, such as braces, brackets and
parentheses.
This meant that this code block would be counted as two lines:
```swift
print(
"hi"
)
```
because the trailing `)` would be treated as a whitespace line.
This meant that our "body length" family of rules that measure the
effective line count of declarations like functions, types or closures
would often significantly under-count the number of content lines in a
body.
Now with SwiftSyntax, we can get all tokens, including the ones
SourceKit was previously ignoring, so we can get much more accurate line
counts when ignoring whitespace and comments.
In addition, we weren't very thorough in how we measured body length.
As an exercise, how many lines long would you say the body of this
function is?
```swift
func hello() {
print("hello")
}
```
Does the body span one line or three lines?
I propose that we consistently ignore the left and right brace lines
when calculating the body line count of these scopes so that we measure
body line counts like this:
```swift
// 1 line
{ print("foo") }
// 1 line
{
}
// 1 line
{
print("foo")
}
// 2 lines
{
let sum = 1 + 2
print(sum)
}
```
Now with those changes in place, in order to keep the default
configuration thresholds to similar levels as before, we need to adjust
them slightly. Here's what I'm suggesting:
|Rule|Before|After|
|-|-|-|
|closure_body_length|20/100|30/100|
|function_body_length|40/100|50/100|
|type_body_length|200/350|250/350|
This is a pretty significant breaking change and I suspect we'll hear
from users who are surprised that some of their declarations now exceed
the rule limits, but I believe this new approach to calculating body
lines is more correct and intuitive compared to what we've had until
now.
OSSCheck is also going to report a bazillion changes with this, which is
expected given the scope of this change.
Sometimes we hit issues with SourceKit that cause this rule to produce
false positives of some imports. While we would prefer to fix all of
these cases whenever possible, this gives us a temporary escape hatch to
tell the rule never to remove imports of specific modules until the
fixes land in SwiftLint or Swift itself.
Currently this is for us to opt out of https://bugs.swift.org/browse/SR-13120
For example, if `CGFloat` is used in a file where only `UIKit` is imported but not `CoreGraphics`, this will be a violation even if the file previously compiled.
This is because Swift allows referencing some declarations that are only transitively imported.
Enabling the `require_explicit_imports` configuration option will require that the module of every declaration referenced in a source file be explicitly imported.
This will add significant noise to the imports list, but has a few advantages:
1. It will be easier to understand all the dependencies explicitly referenced in a source file.
2. Correcting the `unused_import` rule will no longer introduce compilation errors in files that compiled prior to the correction.
If missing imports are added to a file when correcting it, the `sorted_imports` rule will be automatically run on that file.
If you with to allow some imports to be implicitly importable transitively, you may specify the `allowed_transitive_imports` configuration:
```yaml
unused_import:
require_explicit_imports: true
allowed_transitive_imports:
- module: Foundation
allowed_transitive_imports:
- CoreFoundation
- Darwin
- ObjectiveC
```
Danny Mösch