-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Allow prefer_key_paths
to ignore identity closures ({ $0 }
)
#6068
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,10 @@ | |
|
||
### Enhancements | ||
|
||
* None. | ||
* Add new `ignore_identity_closures` parameter to `prefer_key_paths` rule to skip conversion of identity closures | ||
(`{ $0 }`) to identity key paths (`\self`). | ||
Add a small note to the rule description stating that identity key path conversion is Swift 6+ only. | ||
[p4checo](https://github.com/p4checo) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please reference the issue here as well. |
||
|
||
### Bug Fixes | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -6,11 +6,21 @@ struct PreferKeyPathRule: Rule { | |||||||||||
var configuration = PreferKeyPathConfiguration() | ||||||||||||
|
||||||||||||
private static let extendedMode = ["restrict_to_standard_functions": false] | ||||||||||||
private static let ignoreIdentity = ["ignore_identity_closures": true] | ||||||||||||
private static let extendedModeAndIgnoreIdentity = [ | ||||||||||||
"restrict_to_standard_functions": false, | ||||||||||||
"ignore_identity_closures": true, | ||||||||||||
] | ||||||||||||
Comment on lines
+10
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should work, shouldn't it?
Suggested change
|
||||||||||||
|
||||||||||||
static let description = RuleDescription( | ||||||||||||
identifier: "prefer_key_path", | ||||||||||||
name: "Prefer Key Path", | ||||||||||||
description: "Use a key path argument instead of a closure with property access", | ||||||||||||
description: """ | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep the existing description as it's currently also used as the violation message and add a |
||||||||||||
Use a key path argument instead of a closure with property access | ||||||||||||
|
||||||||||||
Note: Swift 5 doesn't support identity key path conversion (`{ $0 }` -> `(\\.self)`), regardless of | ||||||||||||
`ignore_identity_closures` parameter value | ||||||||||||
""", | ||||||||||||
kind: .idiomatic, | ||||||||||||
minSwiftVersion: .fiveDotTwo, | ||||||||||||
nonTriggeringExamples: [ | ||||||||||||
|
@@ -25,6 +35,8 @@ struct PreferKeyPathRule: Rule { | |||||||||||
Example("f { (a, b) in a.b }", configuration: extendedMode), | ||||||||||||
Example("f { $0.a } g: { $0.b }", configuration: extendedMode), | ||||||||||||
Example("[1, 2, 3].reduce(1) { $0 + $1 }", configuration: extendedMode), | ||||||||||||
Example("f { $0 }", configuration: extendedModeAndIgnoreIdentity), | ||||||||||||
Example("f.map { $0 }", configuration: ignoreIdentity), | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also add a |
||||||||||||
Example("f.map(1) { $0.a }"), | ||||||||||||
Example("f.filter({ $0.a }, x)"), | ||||||||||||
Example("#Predicate { $0.a }"), | ||||||||||||
|
@@ -92,8 +104,12 @@ private extension PreferKeyPathRule { | |||||||||||
return | ||||||||||||
} | ||||||||||||
if let onlyStmt = node.onlyExprStmt, | ||||||||||||
SwiftVersion.current >= .six || !onlyStmt.is(DeclReferenceExprSyntax.self), | ||||||||||||
onlyStmt.accesses(identifier: node.onlyParameter) { | ||||||||||||
if onlyStmt.is(DeclReferenceExprSyntax.self), | ||||||||||||
configuration.ignoreIdentityClosures || SwiftVersion.current < .six { | ||||||||||||
return | ||||||||||||
} | ||||||||||||
|
||||||||||||
violations.append(node.positionAfterSkippingLeadingTrivia) | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
@@ -106,7 +122,7 @@ private extension PreferKeyPathRule { | |||||||||||
!closure.isInvalid(restrictToStandardFunctions: configuration.restrictToStandardFunctions), | ||||||||||||
let expr = closure.onlyExprStmt, | ||||||||||||
expr.accesses(identifier: closure.onlyParameter) == true, | ||||||||||||
let replacement = expr.asKeyPath, | ||||||||||||
let replacement = expr.asKeyPath(ignoreIdentityClosures: configuration.ignoreIdentityClosures), | ||||||||||||
let calleeName = node.calleeName else { | ||||||||||||
return super.visit(node) | ||||||||||||
} | ||||||||||||
|
@@ -136,7 +152,7 @@ private extension PreferKeyPathRule { | |||||||||||
} | ||||||||||||
if let expr = node.onlyExprStmt, | ||||||||||||
expr.accesses(identifier: node.onlyParameter) == true, | ||||||||||||
let replacement = expr.asKeyPath { | ||||||||||||
let replacement = expr.asKeyPath(ignoreIdentityClosures: configuration.ignoreIdentityClosures) { | ||||||||||||
numberOfCorrections += 1 | ||||||||||||
let node = replacement | ||||||||||||
.with(\.leadingTrivia, node.leadingTrivia) | ||||||||||||
|
@@ -226,7 +242,7 @@ private extension FunctionCallExprSyntax { | |||||||||||
} | ||||||||||||
|
||||||||||||
private extension ExprSyntax { | ||||||||||||
var asKeyPath: ExprSyntax? { | ||||||||||||
func asKeyPath(ignoreIdentityClosures: Bool) -> ExprSyntax? { | ||||||||||||
if let memberAccess = `as`(MemberAccessExprSyntax.self) { | ||||||||||||
var this = memberAccess.base | ||||||||||||
var elements = [memberAccess.declName] | ||||||||||||
|
@@ -238,7 +254,8 @@ private extension ExprSyntax { | |||||||||||
} | ||||||||||||
return "\\.\(raw: elements.reversed().map(\.baseName.text).joined(separator: "."))" as ExprSyntax | ||||||||||||
} | ||||||||||||
if SwiftVersion.current >= .six, `is`(DeclReferenceExprSyntax.self) { | ||||||||||||
|
||||||||||||
if !ignoreIdentityClosures, SwiftVersion.current >= .six, `is`(DeclReferenceExprSyntax.self) { | ||||||||||||
return "\\.self" | ||||||||||||
} | ||||||||||||
return nil | ||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,11 @@ import XCTest | |
|
||
final class PreferKeyPathRuleTests: SwiftLintTestCase { | ||
private static let extendedMode = ["restrict_to_standard_functions": false] | ||
private static let ignoreIdentity = ["ignore_identity_closures": true] | ||
private static let extendedModeAndIgnoreIdentity = [ | ||
"restrict_to_standard_functions": false, | ||
"ignore_identity_closures": true, | ||
] | ||
|
||
func testIdentityExpressionInSwift6() throws { | ||
try XCTSkipIf(SwiftVersion.current < .six) | ||
|
@@ -12,6 +17,8 @@ final class PreferKeyPathRuleTests: SwiftLintTestCase { | |
.with(nonTriggeringExamples: [ | ||
Example("f.filter { a in b }"), | ||
Example("f.g { $1 }", configuration: Self.extendedMode), | ||
Example("f { $0 }", configuration: Self.extendedModeAndIgnoreIdentity), | ||
Example("f.map { $0 }", configuration: Self.ignoreIdentity), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also add a |
||
]) | ||
.with(triggeringExamples: [ | ||
Example("f.compactMap ↓{ $0 }"), | ||
|
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.
Let's have the note here as well instead of a reference to the note: