Skip to content

Commit d756fd5

Browse files
authored
Add noForceUnwrapInTests rule (#2204)
1 parent 9e4caaa commit d756fd5

File tree

6 files changed

+1301
-0
lines changed

6 files changed

+1301
-0
lines changed

Rules.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@
109109
* [isEmpty](#isEmpty)
110110
* [markTypes](#markTypes)
111111
* [noExplicitOwnership](#noExplicitOwnership)
112+
* [noForceUnwrapInTests](#noForceUnwrapInTests)
112113
* [noGuardInTests](#noGuardInTests)
113114
* [organizeDeclarations](#organizeDeclarations)
114115
* [preferFinalClasses](#preferFinalClasses)
@@ -1633,6 +1634,48 @@ Don't use explicit ownership modifiers (borrowing / consuming).
16331634
</details>
16341635
<br/>
16351636

1637+
## noForceUnwrapInTests
1638+
1639+
Use XCTUnwrap or #require in test cases, rather than force unwrapping.
1640+
1641+
<details>
1642+
<summary>Examples</summary>
1643+
1644+
```diff
1645+
import Testing
1646+
1647+
struct MyFeatureTests {
1648+
- @Test func myFeature() {
1649+
- let myValue = foo.bar!.value as! Value
1650+
- let otherValue = (foo! as! Other).bar
1651+
- otherValue.manager!.prepare()
1652+
- #expect(myValue.property! == other)
1653+
+ @Test func myFeature() throws {
1654+
+ let myValue = try #require(foo.bar?.value as? Value)
1655+
+ let otherValue = try #require((foo as? Other)?.bar)
1656+
+ otherValue.manager?.prepare()
1657+
+ #expect(try #require(myValue.property) == other)
1658+
}
1659+
}
1660+
1661+
import XCTest
1662+
1663+
class MyFeatureTests: XCTestCase {
1664+
- func testMyFeature() {
1665+
- let myValue = foo.bar!.value as! Value
1666+
- let otherValue = (foo! as! Other).bar
1667+
- XCTAssertEqual(myValue.property, "foo")
1668+
+ func testMyFeature() throws {
1669+
+ let myValue = try XCTUnwrap(foo.bar?.value as? Value)
1670+
+ let otherValue = try XCTUnwrap((foo as? Other)?.bar)
1671+
+ XCTAssertEqual(try XCTUnwrap(myValue.property), otherValue)
1672+
}
1673+
}
1674+
```
1675+
1676+
</details>
1677+
<br/>
1678+
16361679
## noGuardInTests
16371680

16381681
Convert guard statements in unit tests to `try #require(...)` / `#expect(...)`

Sources/ParsingHelpers.swift

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,11 @@ extension Formatter {
549549
return false
550550
}
551551

552+
// If the code is in a string, then it could be inside a string interpolation
553+
if tokens[startOfScopeIndex] == .startOfScope("\"") || tokens[startOfScopeIndex] == .startOfScope("\"\"\"") {
554+
return false
555+
}
556+
552557
// If this is a function scope, but not the body of the function itself,
553558
// then this is some nested function.
554559
if lastSignificantKeyword(at: startOfScopeIndex, excluding: ["where"]) == "func",
@@ -1688,6 +1693,146 @@ extension Formatter {
16881693
return startIndex ... endOfExpression
16891694
}
16901695

1696+
/// Parses the expression ending at the given index.
1697+
///
1698+
/// This is the reverse counterpart to `parseExpressionRange(startingAt:)`.
1699+
/// It works backwards from an ending position to find where the expression starts.
1700+
func parseExpressionRange(
1701+
endingAt endIndex: Int
1702+
) -> ClosedRange<Int>? {
1703+
let token = tokens[endIndex]
1704+
1705+
// First, check what comes BEFORE this token to see if we're part of a larger expression
1706+
if let prevIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: endIndex) {
1707+
let prevToken = tokens[prevIndex]
1708+
1709+
// Check for dot notation (foo.bar)
1710+
if prevToken == .operator(".", .infix) || prevToken == .delimiter(".") {
1711+
guard let beforeDotIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: prevIndex),
1712+
let previousExpression = parseExpressionRange(endingAt: beforeDotIndex)
1713+
else { return nil }
1714+
return previousExpression.lowerBound ... endIndex
1715+
}
1716+
1717+
// Check for infix operators (foo + bar, but not foo = bar)
1718+
if prevToken.isOperator(ofType: .infix), prevToken.string != "=" {
1719+
guard let beforeOperatorIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: prevIndex),
1720+
let previousExpression = parseExpressionRange(endingAt: beforeOperatorIndex)
1721+
else { return nil }
1722+
return previousExpression.lowerBound ... endIndex
1723+
}
1724+
1725+
// Prefix operators have to be the start of a subexpression,
1726+
// but can come after infix operators like `foo == !bar`.
1727+
if prevToken.isOperator(ofType: .prefix),
1728+
let tokenBeforeOperator = index(of: .nonSpaceOrComment, before: prevIndex),
1729+
tokens[tokenBeforeOperator].isOperator(ofType: .infix),
1730+
let previousExpression = parseExpressionRange(endingAt: prevIndex)
1731+
{
1732+
return previousExpression.lowerBound ... endIndex
1733+
}
1734+
1735+
// Check for type casting keywords (as, is)
1736+
if prevToken == .keyword("as") || prevToken == .keyword("is") {
1737+
guard let beforeKeywordIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: prevIndex),
1738+
let previousExpression = parseExpressionRange(endingAt: beforeKeywordIndex)
1739+
else { return nil }
1740+
return previousExpression.lowerBound ... endIndex
1741+
}
1742+
1743+
// Check for as?, as! (unwrap operator after "as")
1744+
if prevToken.isUnwrapOperator,
1745+
let asIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: prevIndex),
1746+
tokens[asIndex] == .keyword("as")
1747+
{
1748+
guard let beforeAsIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: asIndex),
1749+
let previousExpression = parseExpressionRange(endingAt: beforeAsIndex)
1750+
else { return nil }
1751+
return previousExpression.lowerBound ... endIndex
1752+
}
1753+
1754+
// Check for prefix operators or keywords (!, try, await)
1755+
let prefixKeywords = ["await", "try", "repeat", "each"]
1756+
if prevToken.isOperator(ofType: .prefix) || prefixKeywords.contains(prevToken.string) {
1757+
return prevIndex ... endIndex
1758+
}
1759+
1760+
// Check for operators after prefix keywords (try!, try?, await!)
1761+
if prevToken.isUnwrapOperator || prevToken == .operator("?", .postfix) {
1762+
if let beforeOperatorIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: prevIndex),
1763+
prefixKeywords.contains(tokens[beforeOperatorIndex].string)
1764+
{
1765+
return beforeOperatorIndex ... endIndex
1766+
}
1767+
}
1768+
}
1769+
1770+
// Handle postfix and infix operators (!, ?, +) that can be preceded by other parts of the expression
1771+
if token.isOperator(ofType: .postfix) || token.isOperator(ofType: .infix) {
1772+
guard let prevIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: endIndex),
1773+
let previousExpression = parseExpressionRange(endingAt: prevIndex)
1774+
else { return nil }
1775+
return previousExpression.lowerBound ... endIndex
1776+
}
1777+
1778+
// Handle end of scope tokens ), ], }, "
1779+
if case .endOfScope = token {
1780+
guard let startOfScope = index(of: .startOfScope, before: endIndex) else { return nil }
1781+
1782+
// Check if there's something before the scope (method call, subscript)
1783+
if let prevIndex = index(of: .nonSpaceOrCommentOrLinebreak, before: startOfScope),
1784+
let previousExpression = parseExpressionRange(endingAt: prevIndex)
1785+
{
1786+
return previousExpression.lowerBound ... endIndex
1787+
}
1788+
1789+
// The scope itself is the expression (array literal, closure, etc)
1790+
return startOfScope ... endIndex
1791+
}
1792+
1793+
// Base cases: identifiers, numbers, literals, etc.
1794+
if token.isIdentifier || token.isNumber {
1795+
return endIndex ... endIndex
1796+
}
1797+
1798+
return nil
1799+
}
1800+
1801+
/// Parses the expression that contains the token at the given index.
1802+
func parseExpressionRange(
1803+
containing index: Int
1804+
) -> ClosedRange<Int>? {
1805+
// To find the complete expression, parse forwards from the input index to the end of the expression,
1806+
// and then parse backwards from the end of that expression to the start of the complete expression.
1807+
var forwardRange = parseExpressionRange(startingAt: index)
1808+
1809+
// If this is an operator that can't ever be the start of an expression, parse from some previous token
1810+
if forwardRange == nil,
1811+
tokens[index].isOperator(ofType: .postfix) || tokens[index].isOperator(ofType: .infix)
1812+
{
1813+
var parseToken = index
1814+
while let previousToken = self.index(of: .nonSpaceOrCommentOrLinebreak, before: parseToken) {
1815+
// Check if this previous token is the start of a subexpression that contains the given index
1816+
if let forwardRangeFromPreviousToken = parseExpressionRange(startingAt: parseToken),
1817+
forwardRangeFromPreviousToken.contains(index)
1818+
{
1819+
forwardRange = forwardRangeFromPreviousToken
1820+
break
1821+
}
1822+
1823+
parseToken = previousToken
1824+
}
1825+
forwardRange = parseExpressionRange(startingAt: parseToken)
1826+
}
1827+
1828+
guard let forwardRange,
1829+
let backwardRange = parseExpressionRange(endingAt: forwardRange.upperBound),
1830+
backwardRange.contains(index)
1831+
else { return nil }
1832+
1833+
return backwardRange
1834+
}
1835+
16911836
/// Parses all of the declarations in the source file.
16921837
func parseDeclarations() -> [Declaration] {
16931838
parseDeclarations(in: tokens.indices)

Sources/RuleRegistry.generated.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ let ruleRegistry: [String: FormatRule] = [
5555
"modifierOrder": .modifierOrder,
5656
"modifiersOnSameLine": .modifiersOnSameLine,
5757
"noExplicitOwnership": .noExplicitOwnership,
58+
"noForceUnwrapInTests": .noForceUnwrapInTests,
5859
"noGuardInTests": .noGuardInTests,
5960
"numberFormatting": .numberFormatting,
6061
"opaqueGenericParameters": .opaqueGenericParameters,

0 commit comments

Comments
 (0)