Add ConvertToDoCatch refactoring to SwiftRefactor by Clemo97 · Pull Request #3233 · swiftlang/swift-syntax

Clemo97 · 2026-01-12T23:34:54Z

Summary
This PR implements the ConvertToDoCatch refactoring in swift-syntax, which converts try! (force-try) expressions into proper do-catch blocks with error handling.

Related to swiftlang/sourcekit-lsp#2424

And replaces the sourcekitd implementation https://github.com/swiftlang/swift/blob/9b452820367ccc7b5d9effbc1565bcd945c81768/lib/Refactoring/ConvertToDoCatch.cpp

ahoppen

Very nice. I have a few comments on the test case but overall this looks good. Thanks for implementing it.

ahoppen · 2026-01-13T15:51:48Z

Sources/SwiftRefactor/ConvertToDoCatch.swift

+        leftBrace: .leftBraceToken(trailingTrivia: .newline),
+        statements: CodeBlockItemListSyntax([
+          CodeBlockItemSyntax(
+            leadingTrivia: .spaces(2),


Could we pass in the indentation of the source file through the context, similar to what we do in ExpandEditorPlaceholder?

Actually, sorry for my change of mind, but let’s infer the indentation using BasicFormat.inferIndentation. I just realized that that’s what we’re doing in the majority of other refactoring actions.

ahoppen · 2026-01-13T15:53:21Z

Sources/SwiftRefactor/ConvertToDoCatch.swift

+    let tryExpression = syntax.with(\.questionOrExclamationMark, nil)
+
+    // Create the do-catch statement with proper spacing
+    let doStatement = DoStmtSyntax(


Would it be easier to construct this statement using string interpolation? Ie.

let doStatement = DoStmtSyntax(""" do { … } """)

ahoppen · 2026-01-13T15:53:47Z

Tests/SwiftRefactorTest/ConvertToDoCatchTest.swift

+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testForceTryWithVariableBinding() throws {


This test doesn’t contain a variable binding, right?

ahoppen · 2026-01-13T15:55:00Z

Tests/SwiftRefactorTest/ConvertToDoCatchTest.swift

+  func testForceTryWithArguments() throws {
+    let baseline: ExprSyntax = """
+      try! processFile(at: path, with: options)
+      """
+
+    let expected: CodeBlockItemListSyntax = """
+      do {
+        try processFile(at: path, with: options)
+      } catch {
+        <#code#>
+      }
+      """
+
+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testForceTryWithChaining() throws {
+    let baseline: ExprSyntax = """
+      try! getData().process().save()
+      """
+
+    let expected: CodeBlockItemListSyntax = """
+      do {
+        try getData().process().save()
+      } catch {
+        <#code#>
+      }
+      """
+
+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testForceTryWithPropertyAccess() throws {
+    let baseline: ExprSyntax = """
+      try! object.riskyProperty.value
+      """
+
+    let expected: CodeBlockItemListSyntax = """
+      do {
+        try object.riskyProperty.value
+      } catch {
+        <#code#>
+      }
+      """
+
+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  // MARK: - Complex Expressions
+
+  func testForceTryWithSubscript() throws {
+    let baseline: ExprSyntax = """
+      try! array[index]
+      """
+
+    let expected: CodeBlockItemListSyntax = """
+      do {
+        try array[index]
+      } catch {
+        <#code#>
+      }
+      """
+
+    try assertRefactorConvert(baseline, expected: expected)
+  }


Since we just copy the expression verbatim, I don’t think that these tests provide much value. It’s the exact same code path that’s taken in the refactor action, so I’d remove them

ahoppen · 2026-01-13T15:56:55Z

Tests/SwiftRefactorTest/ConvertToDoCatchTest.swift

+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testForceTryWithMultilineExpression() throws {


This should already be covered by testForceTryWithClosure, which also contains a multi-line expression (which is what I think is relevant here)

ahoppen · 2026-01-13T15:58:37Z

Tests/SwiftRefactorTest/ConvertToDoCatchTest.swift

+  func testJSONDecodingExample() throws {
+    let baseline: ExprSyntax = """
+      try! JSONDecoder().decode(User.self, from: data)
+      """
+
+    let expected: CodeBlockItemListSyntax = """
+      do {
+        try JSONDecoder().decode(User.self, from: data)
+      } catch {
+        <#code#>
+      }
+      """
+
+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testFileReadingExample() throws {
+    let baseline: ExprSyntax = """
+      try! String(contentsOf: url, encoding: .utf8)
+      """
+
+    let expected: CodeBlockItemListSyntax = """
+      do {
+        try String(contentsOf: url, encoding: .utf8)
+      } catch {
+        <#code#>
+      }
+      """
+
+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testDataWritingExample() throws {
+    let baseline: ExprSyntax = """
+      try! data.write(to: fileURL)
+      """
+
+    let expected: CodeBlockItemListSyntax = """
+      do {
+        try data.write(to: fileURL)
+      } catch {
+        <#code#>
+      }
+      """
+
+    try assertRefactorConvert(baseline, expected: expected)
+  }


I think all of these are effectively covered by the tests you already have above, so I’d remove them. JSONDecoder is really no different than any other types to swift-syntax.

ahoppen · 2026-01-13T15:59:59Z

Tests/SwiftRefactorTest/ConvertToDoCatchTest.swift

+
+    try assertRefactorConvert(baseline, expected: expected)
+  }
+}


Could you add one test case where the try expression has a base indentation that needs to be applied to the do block? I think that would be valuable to test, ie. something like

func test() { try! print( "Hello" ) }

ahoppen · 2026-01-13T16:19:44Z

Sources/SwiftRefactor/ConvertToDoCatch.swift

+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors


😉

Suggested change

// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors

// Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors

Sources/SwiftRefactor/ConvertToDoCatch.swift

ahoppen

Very nice. Just a few more comments.

Sources/SwiftRefactor/ConvertToDoCatch.swift

ahoppen · 2026-01-29T17:20:45Z

Sources/SwiftRefactor/ConvertToDoCatch.swift

+    // Format the do-catch statement with the proper indentation
+    let format = BasicFormat(
+      indentationWidth: indentWidth,
+      initialIndentation: baseIndentation
+    )


This may change the user-provided formatting and I’d prefer to leave it as-is. Could you use Indenter instead to apply indentation to all code blocks that need indenting?

This may change the user-provided formatting and I’d prefer to leave it as-is. Could you use Indenter instead to apply indentation to all code blocks that need indenting?

Indenter in SwiftSyntaxBuilder has been deprecated in favor of the new indented(by:) in SwiftBasicFormat, so I used the newer .indented(by:) method.

ahoppen · 2026-01-29T17:21:52Z

Tests/SwiftRefactorTest/ConvertToDoCatchTest.swift

+    try assertRefactorConvert(baseline, expected: expected)
+  }
+
+  func testBasicForceTryExpression() throws {


This is exactly the same as testBasicForceTryConversion, right?

Yes, you are right, I've removed 'testBasicForceTryExpression'.

ahoppen · 2026-02-09T09:48:09Z

@Clemo97 Did you mean to push changes that addressed my last set of comments?

ahoppen · 2026-02-09T11:04:47Z

My last review comments are still not addressed in your latest version of the PR. Just thinking that you might have addressed them locally because you reacted to them.

Clemo97 · 2026-02-09T12:14:20Z

My last review comments are still not addressed in your latest version of the PR. Just thinking that you might have addressed them locally because you reacted to them.

Hello, sorry for the confusion I had forgotten to push my changes remotely, let me work on it.

…dentation management

ahoppen

Just one more small thing.

ahoppen · 2026-02-15T14:26:01Z

Sources/SwiftRefactor/ConvertToDoCatch.swift

+          )
+        )
+      ])
+    ).with(\.leadingTrivia, baseIndentation)


You can set leadingTrivai in the DoStmtSyntax initializer directly.

ahoppen · 2026-02-15T14:26:48Z

Sources/SwiftRefactor/ConvertToDoCatch.swift

+    }
+
+    // Remove leading trivia from the try expression since we'll manage indentation
+    let trimmedTryExpression = tryExpression.with(\.leadingTrivia, [])


This would also remove any comments in the leading trivia. Maybe I’m missing something but I think you don’t need to remove the leading trivia here if you only the CodeBlockItemListSyntax by indentationWidth in line 83.

A test case for this would be the following, which should keep the comment.

// Hope this doesn’t explode try! hopefullyNoBoom()

ahoppen · 2026-02-15T14:28:23Z

Tests/SwiftRefactorTest/ConvertToDoCatchTest.swift

+  guard let tryExpr = input.as(TryExprSyntax.self) else {
+    if expected != nil {
+      XCTFail("Input is not a TryExprSyntax", file: file, line: line)
+    }
+    return
+  }


Suggested change

guard let tryExpr = input.as(TryExprSyntax.self) else {

if expected != nil {

XCTFail("Input is not a TryExprSyntax", file: file, line: line)

}

return

}

let tryExpr = try XCTUnwrap(input.as(TryExprSyntax.self), file: file, line: line)

ahoppen · 2026-02-21T15:23:51Z

Looks like you merged main into your branch but didn’t address my changes. Just want to make sure that you didn’t forget to push anything. Also, could you rebase and squash your changes instead of merging main into your branch, as described in https://github.com/swiftlang/sourcekit-lsp/blob/main/CONTRIBUTING.md#authoring-commits?

Clemo97 added 2 commits January 13, 2026 02:21

Add ConvertToDoCatch refactoring provider and corresponding tests

6c43f1e

Merge branch 'swiftlang:main' into main

2a11589

Clemo97 requested review from ahoppen, hamishknight, hborla and rintaro as code owners January 12, 2026 23:34

ahoppen reviewed Jan 13, 2026

View reviewed changes

Sources/SwiftRefactor/ConvertToDoCatch.swift Show resolved Hide resolved

Clemo97 added 2 commits January 21, 2026 19:38

Add ConvertToDoCatch refactoring provider and update tests

e2437a1

Merge branch 'swiftlang:main' into main

70d3374

Clemo97 requested a review from ahoppen January 21, 2026 16:44

ahoppen reviewed Jan 29, 2026

View reviewed changes

Merge branch 'swiftlang:main' into main

970802c

Clemo97 added 2 commits February 9, 2026 17:58

Refactor ConvertToDoCatch to simplify context handling and improve in…

227afb8

…dentation management

fix identation

0cc9970

ahoppen reviewed Feb 15, 2026

View reviewed changes

Merge branch 'swiftlang:main' into main

47cbe5e

	// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
	// Copyright (c) 2014 - 2026 Apple Inc. and the Swift project authors

Comments

Conversation

Clemo97 commented Jan 12, 2026

Uh oh!

ahoppen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoppen commented Feb 9, 2026

Uh oh!

ahoppen commented Feb 9, 2026

Uh oh!

Clemo97 commented Feb 9, 2026

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoppen commented Feb 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ahoppen left a comment •

edited

Loading