-
Notifications
You must be signed in to change notification settings - Fork 491
Add ConvertToDoCatch refactoring to SwiftRefactor #3233
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
6c43f1e
2a11589
e2437a1
70d3374
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 | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,121 @@ | ||||||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||||
| // | ||||||||||||||
| // This source file is part of the Swift.org open source project | ||||||||||||||
| // | ||||||||||||||
| // Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors | ||||||||||||||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||||||||||||||
| // | ||||||||||||||
| // See https://swift.org/LICENSE.txt for license information | ||||||||||||||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||||||||||||||
| // | ||||||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||||
|
|
||||||||||||||
| #if compiler(>=6) | ||||||||||||||
| import SwiftBasicFormat | ||||||||||||||
| import SwiftParser | ||||||||||||||
| @_spi(RawSyntax) public import SwiftSyntax | ||||||||||||||
| import SwiftSyntaxBuilder | ||||||||||||||
| #else | ||||||||||||||
| import SwiftBasicFormat | ||||||||||||||
| import SwiftParser | ||||||||||||||
| @_spi(RawSyntax) import SwiftSyntax | ||||||||||||||
| import SwiftSyntaxBuilder | ||||||||||||||
| #endif | ||||||||||||||
|
|
||||||||||||||
| /// Converts a `try!` expression to a `do-catch` block. | ||||||||||||||
| /// | ||||||||||||||
| /// This refactoring transforms force-try expressions into proper error handling | ||||||||||||||
| /// using do-catch blocks with a placeholder for the catch body. | ||||||||||||||
| /// | ||||||||||||||
| /// Example: | ||||||||||||||
| /// ```swift | ||||||||||||||
| /// // Before: | ||||||||||||||
| /// let result = try! riskyFunction() | ||||||||||||||
| /// | ||||||||||||||
| /// // After: | ||||||||||||||
| /// do { | ||||||||||||||
| /// let result = try riskyFunction() | ||||||||||||||
| /// } catch { | ||||||||||||||
| /// <#code#> | ||||||||||||||
| /// } | ||||||||||||||
| /// ``` | ||||||||||||||
| public struct ConvertToDoCatch: SyntaxRefactoringProvider { | ||||||||||||||
| public struct Context { | ||||||||||||||
| /// The indentation width to use for the generated do-catch block. | ||||||||||||||
| /// If `nil`, a default indentation of 2 spaces will be used. | ||||||||||||||
| public let indentationWidth: Trivia? | ||||||||||||||
|
|
||||||||||||||
| public init(indentationWidth: Trivia? = nil) { | ||||||||||||||
| self.indentationWidth = indentationWidth | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| public static func refactor(syntax: TryExprSyntax, in context: Context = Context()) throws -> CodeBlockItemListSyntax | ||||||||||||||
| { | ||||||||||||||
|
Comment on lines
+53
to
+54
Member
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. Personal formatting preference
Suggested change
Author
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. Thanks, will remeber to run the formatter before pushing. |
||||||||||||||
| // Validate that this is a force-try (try!) expression | ||||||||||||||
| guard syntax.questionOrExclamationMark?.tokenKind == .exclamationMark else { | ||||||||||||||
| throw RefactoringNotApplicableError("not a force-try expression") | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Create the try expression without the exclamation mark | ||||||||||||||
| // Transfer any trailing trivia from the ! to the try keyword to preserve comments | ||||||||||||||
| let tryExpression: TryExprSyntax | ||||||||||||||
| if let exclamationMark = syntax.questionOrExclamationMark { | ||||||||||||||
| let newTryKeyword = syntax.tryKeyword.with( | ||||||||||||||
| \.trailingTrivia, | ||||||||||||||
| syntax.tryKeyword.trailingTrivia + exclamationMark.trailingTrivia | ||||||||||||||
| ) | ||||||||||||||
| tryExpression = syntax | ||||||||||||||
| .with(\.tryKeyword, newTryKeyword) | ||||||||||||||
| .with(\.questionOrExclamationMark, nil) | ||||||||||||||
| } else { | ||||||||||||||
| tryExpression = syntax.with(\.questionOrExclamationMark, nil) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Extract the base indentation from the source file | ||||||||||||||
| let baseIndentation = syntax.firstToken(viewMode: .sourceAccurate)?.indentationOfLine ?? [] | ||||||||||||||
|
|
||||||||||||||
| // Determine the indentation width (default to 2 spaces if not provided) | ||||||||||||||
| let indentWidth = context.indentationWidth ?? .spaces(2) | ||||||||||||||
|
|
||||||||||||||
| // Create the do-catch statement | ||||||||||||||
| let doStatement = DoStmtSyntax( | ||||||||||||||
|
Member
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. Would it be easier to construct this statement using string interpolation? Ie. let doStatement = DoStmtSyntax("""
do {
…
}
""") |
||||||||||||||
| doKeyword: .keyword(.do), | ||||||||||||||
| body: CodeBlockSyntax( | ||||||||||||||
| leftBrace: .leftBraceToken(), | ||||||||||||||
| statements: CodeBlockItemListSyntax([ | ||||||||||||||
| CodeBlockItemSyntax( | ||||||||||||||
| item: .expr(ExprSyntax(tryExpression)) | ||||||||||||||
| ) | ||||||||||||||
| ]), | ||||||||||||||
| rightBrace: .rightBraceToken() | ||||||||||||||
| ), | ||||||||||||||
| catchClauses: CatchClauseListSyntax([ | ||||||||||||||
| CatchClauseSyntax( | ||||||||||||||
| catchKeyword: .keyword(.catch), | ||||||||||||||
| body: CodeBlockSyntax( | ||||||||||||||
| leftBrace: .leftBraceToken(), | ||||||||||||||
| statements: CodeBlockItemListSyntax([ | ||||||||||||||
| CodeBlockItemSyntax( | ||||||||||||||
| item: .expr(ExprSyntax("<#code#>" as ExprSyntax)) | ||||||||||||||
| ) | ||||||||||||||
| ]), | ||||||||||||||
| rightBrace: .rightBraceToken() | ||||||||||||||
| ) | ||||||||||||||
| ) | ||||||||||||||
| ]) | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| // Format the do-catch statement with the proper indentation | ||||||||||||||
| let format = BasicFormat( | ||||||||||||||
| indentationWidth: indentWidth, | ||||||||||||||
| initialIndentation: baseIndentation | ||||||||||||||
| ) | ||||||||||||||
|
Comment on lines
+109
to
+113
Member
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 may change the user-provided formatting and I’d prefer to leave it as-is. Could you use |
||||||||||||||
|
|
||||||||||||||
| let formatted = doStatement.formatted(using: format).as(DoStmtSyntax.self)! | ||||||||||||||
|
|
||||||||||||||
| return CodeBlockItemListSyntax([ | ||||||||||||||
| CodeBlockItemSyntax(item: .stmt(StmtSyntax(formatted))) | ||||||||||||||
| ]) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This source file is part of the Swift.org open source project | ||
| // | ||
| // Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors | ||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||
| // | ||
| // See https://swift.org/LICENSE.txt for license information | ||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import SwiftRefactor | ||
| import SwiftSyntax | ||
| import SwiftSyntaxBuilder | ||
| import XCTest | ||
| import _SwiftSyntaxTestSupport | ||
|
|
||
| final class ConvertToDoCatchTest: XCTestCase { | ||
|
|
||
| // MARK: - Basic Conversions | ||
|
|
||
| func testBasicForceTryConversion() throws { | ||
| let baseline: ExprSyntax = """ | ||
| try! riskyFunction() | ||
| """ | ||
|
|
||
| let expected: CodeBlockItemListSyntax = """ | ||
| do { | ||
| try riskyFunction() | ||
| } catch { | ||
| <#code#> | ||
| } | ||
| """ | ||
|
|
||
| try assertRefactorConvert(baseline, expected: expected) | ||
| } | ||
|
|
||
| func testBasicForceTryExpression() throws { | ||
|
Member
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 is exactly the same as |
||
| let baseline: ExprSyntax = """ | ||
| try! fetchData() | ||
| """ | ||
|
|
||
| let expected: CodeBlockItemListSyntax = """ | ||
| do { | ||
| try fetchData() | ||
| } catch { | ||
| <#code#> | ||
| } | ||
| """ | ||
|
|
||
| try assertRefactorConvert(baseline, expected: expected) | ||
| } | ||
|
|
||
| func testForceTryWithClosure() throws { | ||
| let baseline: ExprSyntax = """ | ||
| try! performAsync { result in | ||
| print(result) | ||
| } | ||
| """ | ||
|
|
||
| let expected: CodeBlockItemListSyntax = """ | ||
| do { | ||
| try performAsync { result in | ||
| print(result) | ||
| } | ||
| } catch { | ||
| <#code#> | ||
| } | ||
| """ | ||
|
|
||
| try assertRefactorConvert(baseline, expected: expected) | ||
| } | ||
|
|
||
| func testForceTryWithAwait() throws { | ||
| let baseline: ExprSyntax = """ | ||
| try! await fetchRemoteData() | ||
| """ | ||
|
|
||
| let expected: CodeBlockItemListSyntax = """ | ||
| do { | ||
| try await fetchRemoteData() | ||
| } catch { | ||
| <#code#> | ||
| } | ||
| """ | ||
|
|
||
| try assertRefactorConvert(baseline, expected: expected) | ||
| } | ||
|
|
||
| // MARK: - Edge Cases | ||
|
|
||
| func testForceTryWithComments() throws { | ||
| let baseline: ExprSyntax = """ | ||
| try! /* important */ riskyFunction() | ||
| """ | ||
|
|
||
| let expected: CodeBlockItemListSyntax = """ | ||
| do { | ||
| try /* important */ riskyFunction() | ||
| } catch { | ||
| <#code#> | ||
| } | ||
| """ | ||
|
|
||
| try assertRefactorConvert(baseline, expected: expected) | ||
| } | ||
|
|
||
| // MARK: - Negative Tests (Should Not Apply) | ||
|
|
||
| func testOptionalTryShouldNotApply() throws { | ||
| let baseline: ExprSyntax = """ | ||
| try? riskyFunction() | ||
| """ | ||
|
|
||
| // Should throw RefactoringNotApplicableError | ||
| try assertRefactorConvert(baseline, expected: nil) | ||
| } | ||
|
|
||
| func testRegularTryShouldNotApply() throws { | ||
| let baseline: ExprSyntax = """ | ||
| try riskyFunction() | ||
| """ | ||
|
|
||
| // Should throw RefactoringNotApplicableError | ||
| try assertRefactorConvert(baseline, expected: nil) | ||
| } | ||
|
|
||
| func testNonTryExpressionShouldNotApply() throws { | ||
| let baseline: ExprSyntax = """ | ||
| regularFunction() | ||
| """ | ||
|
|
||
| // Should throw RefactoringNotApplicableError | ||
| try assertRefactorConvert(baseline, expected: nil) | ||
| } | ||
|
|
||
| // MARK: - Indentation Preservation | ||
|
|
||
| func testIndentationPreservation() throws { | ||
| let baseline: ExprSyntax = """ | ||
| try! riskyFunction() | ||
| """ | ||
|
|
||
| let expected: CodeBlockItemListSyntax = """ | ||
| do { | ||
| try riskyFunction() | ||
| } catch { | ||
| <#code#> | ||
| } | ||
| """ | ||
|
|
||
| let context = ConvertToDoCatch.Context(indentationWidth: .spaces(2)) | ||
| try assertRefactor( | ||
| baseline.as(TryExprSyntax.self)!, | ||
| context: context, | ||
| provider: ConvertToDoCatch.self, | ||
| expected: expected | ||
| ) | ||
| } | ||
| } | ||
|
Member
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. Could you add one test case where the func test() {
try! print(
"Hello"
)
} |
||
|
|
||
| // MARK: - Helper Functions | ||
|
|
||
| private func assertRefactorConvert( | ||
| _ input: ExprSyntax, | ||
| expected: CodeBlockItemListSyntax?, | ||
| file: StaticString = #filePath, | ||
| line: UInt = #line | ||
| ) throws { | ||
| guard let tryExpr = input.as(TryExprSyntax.self) else { | ||
| if expected != nil { | ||
| XCTFail("Input is not a TryExprSyntax", file: file, line: line) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| try assertRefactor( | ||
| tryExpr, | ||
| context: ConvertToDoCatch.Context(), | ||
| provider: ConvertToDoCatch.self, | ||
| expected: expected, | ||
| file: file, | ||
| line: line | ||
| ) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.