From 874ec2a6e4407e5e8044a7b925ba22e2071936cc Mon Sep 17 00:00:00 2001 From: Ueeek Date: Sat, 28 Dec 2024 13:47:41 +0900 Subject: [PATCH 1/3] Fix: TipKit rule triggers an empty_count violation --- CHANGELOG.md | 4 ++++ .../Rules/Performance/EmptyCountRule.swift | 24 +++++++++++++++++++ .../EmptyCountRuleTests.swift | 4 ++++ 3 files changed, 32 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6538b218b1..a4bef52fb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,10 @@ #### Bug Fixes +* Ignore TipKit Rule Macro in `empty_count` rule. + [Ueeek](https://github.com/Ueeek) + [#5883](https://github.com/realm/SwiftLint/issues/5883) + * Ignore super calls with trailing closures in `unneeded_override` rule. [SimplyDanny](https://github.com/SimplyDanny) [#5886](ttps://github.com/realm/SwiftLint/issues/5886) diff --git a/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift b/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift index 3901237f2a..c599000b80 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift @@ -20,6 +20,7 @@ struct EmptyCountRule: Rule { Example("[Int]().count == 0o07"), Example("discount == 0"), Example("order.discount == 0"), + Example("let rule = #Rule(Tips.Event(id: \"someTips\")) { $0.donations.count == 0 }"), ], triggeringExamples: [ Example("[Int]().↓count == 0"), @@ -32,6 +33,7 @@ struct EmptyCountRule: Rule { Example("[Int]().↓count == 0b00"), Example("[Int]().↓count == 0o00"), Example("↓count == 0"), + Example("let predicate = #Predicate { $0.list.↓count == 0 }"), ], corrections: [ Example("[].↓count == 0"): @@ -62,6 +64,8 @@ struct EmptyCountRule: Rule { Example("isEmpty && [Int]().isEmpty"), Example("[Int]().count != 3 && [Int]().↓count != 0 || ↓count == 0 && [Int]().count > 2"): Example("[Int]().count != 3 && ![Int]().isEmpty || isEmpty && [Int]().count > 2"), + Example("let predicate = #Predicate { $0.list.↓count == 0 }"): + Example("let predicate = #Predicate { $0.list.isEmpty }"), ] ) } @@ -77,6 +81,10 @@ private extension EmptyCountRule { violations.append(position) } } + + override func visit(_ node: MacroExpansionExprSyntax) -> SyntaxVisitorContinueKind { + node.isTipsRuleMacro ? .skipChildren : .visitChildren + } } final class Rewriter: ViolationsSyntaxRewriter { @@ -105,6 +113,14 @@ private extension EmptyCountRule { } return super.visit(node) } + + override func visit(_ node: MacroExpansionExprSyntax) -> ExprSyntax { + if node.isTipsRuleMacro { + ExprSyntax(Syntax(node).cast(MacroExpansionExprSyntax.self)) + } else { + super.visit(node) + } + } } } @@ -137,6 +153,14 @@ private extension TokenSyntax { } } +private extension MacroExpansionExprSyntax { + var isTipsRuleMacro: Bool { + self.macroName.text == "Rule" && + self.arguments.isNotEmpty && + self.trailingClosure != nil + } +} + private extension ExprSyntaxProtocol { var negated: ExprSyntax { ExprSyntax(PrefixOperatorExprSyntax(operator: .prefixOperator("!"), expression: self)) diff --git a/Tests/BuiltInRulesTests/EmptyCountRuleTests.swift b/Tests/BuiltInRulesTests/EmptyCountRuleTests.swift index 7df9e49108..683ec8447f 100644 --- a/Tests/BuiltInRulesTests/EmptyCountRuleTests.swift +++ b/Tests/BuiltInRulesTests/EmptyCountRuleTests.swift @@ -15,6 +15,7 @@ final class EmptyCountRuleTests: SwiftLintTestCase { Example("discount == 0\n"), Example("order.discount == 0\n"), Example("count == 0\n"), + Example("let rule = #Rule(Tips.Event(id: \"someTips\")) { $0.donations.isEmpty }"), ] let triggeringExamples = [ Example("[Int]().↓count == 0\n"), @@ -24,6 +25,7 @@ final class EmptyCountRuleTests: SwiftLintTestCase { Example("[Int]().↓count == 0x00_00\n"), Example("[Int]().↓count == 0b00\n"), Example("[Int]().↓count == 0o00\n"), + Example("let predicate = #Predicate { $0.list.↓count == 0 }"), ] let corrections = [ @@ -55,6 +57,8 @@ final class EmptyCountRuleTests: SwiftLintTestCase { Example("count == 0 && [Int]().isEmpty"), Example("[Int]().count != 3 && [Int]().↓count != 0 || count == 0 && [Int]().count > 2"): Example("[Int]().count != 3 && ![Int]().isEmpty || count == 0 && [Int]().count > 2"), + Example("let predicate = #Predicate { $0.list.↓count == 0 }"): + Example("let predicate = #Predicate { $0.list.isEmpty }"), ] let description = EmptyCountRule.description From bc3d87833e10bc05a84f7f43726e873873352908 Mon Sep 17 00:00:00 2001 From: Ueeek Date: Sun, 5 Jan 2025 23:29:50 +0900 Subject: [PATCH 2/3] fix review points: Use more strict condition, use simpler example --- .../Rules/Performance/EmptyCountRule.swift | 14 +++++++------- Tests/BuiltInRulesTests/EmptyCountRuleTests.swift | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift b/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift index c599000b80..5b2d409589 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift @@ -33,7 +33,7 @@ struct EmptyCountRule: Rule { Example("[Int]().↓count == 0b00"), Example("[Int]().↓count == 0o00"), Example("↓count == 0"), - Example("let predicate = #Predicate { $0.list.↓count == 0 }"), + Example("#ExampleMacro { $0.list.↓count == 0 }"), ], corrections: [ Example("[].↓count == 0"): @@ -64,8 +64,8 @@ struct EmptyCountRule: Rule { Example("isEmpty && [Int]().isEmpty"), Example("[Int]().count != 3 && [Int]().↓count != 0 || ↓count == 0 && [Int]().count > 2"): Example("[Int]().count != 3 && ![Int]().isEmpty || isEmpty && [Int]().count > 2"), - Example("let predicate = #Predicate { $0.list.↓count == 0 }"): - Example("let predicate = #Predicate { $0.list.isEmpty }"), + Example("#ExampleMacro { $0.list.↓count == 0 }"): + Example("#ExampleMacro { $0.list.isEmpty }"), ] ) } @@ -116,7 +116,7 @@ private extension EmptyCountRule { override func visit(_ node: MacroExpansionExprSyntax) -> ExprSyntax { if node.isTipsRuleMacro { - ExprSyntax(Syntax(node).cast(MacroExpansionExprSyntax.self)) + ExprSyntax(node) } else { super.visit(node) } @@ -155,9 +155,9 @@ private extension TokenSyntax { private extension MacroExpansionExprSyntax { var isTipsRuleMacro: Bool { - self.macroName.text == "Rule" && - self.arguments.isNotEmpty && - self.trailingClosure != nil + macroName.text == "Rule" && + arguments.count == 1 && + trailingClosure.map { $0.statements.onlyElement?.item.is(ReturnStmtSyntax.self) == false } ?? false } } diff --git a/Tests/BuiltInRulesTests/EmptyCountRuleTests.swift b/Tests/BuiltInRulesTests/EmptyCountRuleTests.swift index 683ec8447f..75d50cf456 100644 --- a/Tests/BuiltInRulesTests/EmptyCountRuleTests.swift +++ b/Tests/BuiltInRulesTests/EmptyCountRuleTests.swift @@ -25,7 +25,7 @@ final class EmptyCountRuleTests: SwiftLintTestCase { Example("[Int]().↓count == 0x00_00\n"), Example("[Int]().↓count == 0b00\n"), Example("[Int]().↓count == 0o00\n"), - Example("let predicate = #Predicate { $0.list.↓count == 0 }"), + Example("#ExampleMacro { $0.list.↓count == 0 }"), ] let corrections = [ @@ -57,8 +57,8 @@ final class EmptyCountRuleTests: SwiftLintTestCase { Example("count == 0 && [Int]().isEmpty"), Example("[Int]().count != 3 && [Int]().↓count != 0 || count == 0 && [Int]().count > 2"): Example("[Int]().count != 3 && ![Int]().isEmpty || count == 0 && [Int]().count > 2"), - Example("let predicate = #Predicate { $0.list.↓count == 0 }"): - Example("let predicate = #Predicate { $0.list.isEmpty }"), + Example("#ExampleMacro { $0.list.↓count == 0 }"): + Example("#ExampleMacro { $0.list.isEmpty }"), ] let description = EmptyCountRule.description From 3b51cccba11082100679c7cb329234003777a291 Mon Sep 17 00:00:00 2001 From: Ueeek Date: Mon, 6 Jan 2025 23:32:08 +0900 Subject: [PATCH 3/3] add more unit tests --- .../Rules/Performance/EmptyCountRule.swift | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift b/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift index 5b2d409589..ef52d6674b 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift @@ -21,6 +21,7 @@ struct EmptyCountRule: Rule { Example("discount == 0"), Example("order.discount == 0"), Example("let rule = #Rule(Tips.Event(id: \"someTips\")) { $0.donations.count == 0 }"), + Example("#Rule(param1: \"param1\")", excludeFromDocumentation: true), ], triggeringExamples: [ Example("[Int]().↓count == 0"), @@ -34,6 +35,22 @@ struct EmptyCountRule: Rule { Example("[Int]().↓count == 0o00"), Example("↓count == 0"), Example("#ExampleMacro { $0.list.↓count == 0 }"), + Example("#Rule { $0.donations.↓count == 0 }", excludeFromDocumentation: true), + Example( + "#Rule(param1: \"param1\", param2: \"param2\") { $0.donations.↓count == 0 }", + excludeFromDocumentation: true + ), + Example( + "#Rule(param1: \"param1\") { $0.donations.↓count == 0 } closure2: { doSomething() }", + excludeFromDocumentation: true + ), + Example("#Rule(param1: \"param1\") { return $0.donations.↓count == 0 }", excludeFromDocumentation: true), + Example(""" + #Rule(param1: "param1") { + doSomething() + return $0.donations.↓count == 0 + } + """, excludeFromDocumentation: true), ], corrections: [ Example("[].↓count == 0"): @@ -66,6 +83,38 @@ struct EmptyCountRule: Rule { Example("[Int]().count != 3 && ![Int]().isEmpty || isEmpty && [Int]().count > 2"), Example("#ExampleMacro { $0.list.↓count == 0 }"): Example("#ExampleMacro { $0.list.isEmpty }"), + Example("#Rule { $0.donations.↓count == 0 }", excludeFromDocumentation: true): + Example("#Rule { $0.donations.isEmpty }"), + Example( + "#Rule(param1: \"param1\", param2: \"param2\") { $0.donations.↓count == 0 }", + excludeFromDocumentation: true + ): Example( + "#Rule(param1: \"param1\", param2: \"param2\") { $0.donations.isEmpty }" + ), + Example( + "#Rule(param1: \"param1\") { $0.donations.↓count == 0 } closure2: { doSomething() }", + excludeFromDocumentation: true + ): Example( + "#Rule(param1: \"param1\") { $0.donations.isEmpty } closure2: { doSomething() }" + ), + Example( + "#Rule(param1: \"param1\") { return $0.donations.↓count == 0 }", + excludeFromDocumentation: true + ): Example( + "#Rule(param1: \"param1\") { return $0.donations.isEmpty }" + ), + Example(""" + #Rule(param1: "param1") { + doSomething() + return $0.donations.↓count == 0 + } + """, excludeFromDocumentation: true): + Example(""" + #Rule(param1: "param1") { + doSomething() + return $0.donations.isEmpty + } + """), ] ) } @@ -156,6 +205,7 @@ private extension TokenSyntax { private extension MacroExpansionExprSyntax { var isTipsRuleMacro: Bool { macroName.text == "Rule" && + additionalTrailingClosures.isEmpty && arguments.count == 1 && trailingClosure.map { $0.statements.onlyElement?.item.is(ReturnStmtSyntax.self) == false } ?? false }