Files
SwiftLint/Source/SwiftLintFramework/Rules/Style/IdentifierNameRule.swift
T
JP Simard fa6bf50a22 Rethink body line count calculation (#4369)
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.
2022-10-14 03:16:26 -04:00

150 lines
5.8 KiB
Swift

import Foundation
import SourceKittenFramework
public struct IdentifierNameRule: ASTRule, ConfigurationProviderRule {
public var configuration = NameConfiguration(minLengthWarning: 3,
minLengthError: 2,
maxLengthWarning: 40,
maxLengthError: 60,
excluded: ["id"])
public init() {}
public static let description = RuleDescription(
identifier: "identifier_name",
name: "Identifier Name",
description: "Identifier names should only contain alphanumeric characters and " +
"start with a lowercase character or should only contain capital letters. " +
"In an exception to the above, variable names may start with a capital letter " +
"when they are declared static and immutable. Variable names should not be too " +
"long or too short.",
kind: .style,
nonTriggeringExamples: IdentifierNameRuleExamples.nonTriggeringExamples,
triggeringExamples: IdentifierNameRuleExamples.triggeringExamples,
deprecatedAliases: ["variable_name"]
)
// swiftlint:disable:next function_body_length
public func validate(
file: SwiftLintFile,
kind: SwiftDeclarationKind,
dictionary: SourceKittenDictionary
) -> [StyleViolation] {
guard !dictionary.enclosedSwiftAttributes.contains(.override) else {
return []
}
return validateName(dictionary: dictionary, kind: kind).map { name, offset in
guard !configuration.excluded.contains(name), let firstCharacter = name.first else {
return []
}
let isFunction = SwiftDeclarationKind.functionKinds.contains(kind)
let description = Self.description
let type = self.type(for: kind)
if !isFunction {
let allowedSymbols = configuration.allowedSymbols.union(.alphanumerics)
if !allowedSymbols.isSuperset(of: CharacterSet(charactersIn: name)) {
return [
StyleViolation(ruleDescription: description,
severity: .error,
location: Location(file: file, byteOffset: offset),
reason: "\(type) name should only contain alphanumeric " +
"characters: '\(name)'")
]
}
if let severity = severity(forLength: name.count) {
let reason = "\(type) name should be between " +
"\(configuration.minLengthThreshold) and " +
"\(configuration.maxLengthThreshold) characters long: '\(name)'"
return [
StyleViolation(ruleDescription: Self.description,
severity: severity,
location: Location(file: file, byteOffset: offset),
reason: reason)
]
}
}
let firstCharacterIsAllowed = configuration.allowedSymbols
.isSuperset(of: CharacterSet(charactersIn: String(firstCharacter)))
guard !firstCharacterIsAllowed else {
return []
}
let requiresCaseCheck = configuration.validatesStartWithLowercase
if requiresCaseCheck &&
kind != .varStatic && name.isViolatingCase && !name.isOperator {
let reason = "\(type) name should start with a lowercase character: '\(name)'"
return [
StyleViolation(ruleDescription: description,
severity: .error,
location: Location(file: file, byteOffset: offset),
reason: reason)
]
}
return []
} ?? []
}
private func validateName(
dictionary: SourceKittenDictionary,
kind: SwiftDeclarationKind
) -> (name: String, offset: ByteCount)? {
guard
var name = dictionary.name,
let offset = dictionary.offset,
kinds.contains(kind),
!name.hasPrefix("$")
else { return nil }
if
kind == .enumelement,
let parenIndex = name.firstIndex(of: "("),
parenIndex > name.startIndex
{
let index = name.index(before: parenIndex)
name = String(name[...index])
}
return (name.nameStrippingLeadingUnderscoreIfPrivate(dictionary), offset)
}
private let kinds: Set<SwiftDeclarationKind> = {
return SwiftDeclarationKind.variableKinds
.union(SwiftDeclarationKind.functionKinds)
.union([.enumelement])
}()
private func type(for kind: SwiftDeclarationKind) -> String {
if SwiftDeclarationKind.functionKinds.contains(kind) {
return "Function"
} else if kind == .enumelement {
return "Enum element"
} else {
return "Variable"
}
}
}
private extension String {
var isViolatingCase: Bool {
let firstCharacter = String(self[startIndex])
guard firstCharacter.isUppercase() else {
return false
}
guard count > 1 else {
return true
}
let secondCharacter = String(self[index(after: startIndex)])
return secondCharacter.isLowercase()
}
var isOperator: Bool {
let operators = ["/", "=", "-", "+", "!", "*", "|", "^", "~", "?", ".", "%", "<", ">", "&"]
return operators.contains(where: hasPrefix)
}
}