diff --git a/EditorExtension/SwiftRefactorExtension/SourceEditorCommand.swift b/EditorExtension/SwiftRefactorExtension/SourceEditorCommand.swift index 777119368c5..012134909c6 100644 --- a/EditorExtension/SwiftRefactorExtension/SourceEditorCommand.swift +++ b/EditorExtension/SwiftRefactorExtension/SourceEditorCommand.swift @@ -40,7 +40,7 @@ final class SourceEditorCommand: NSObject, XCSourceEditorCommand { } override func visitAny(_ node: Syntax) -> Syntax? { - func withOpenedRefactoringProvider(_ providerType: T.Type) -> Syntax? { + func withOpenedRefactoringProvider(_ providerType: T.Type) throws -> Syntax? { guard let input = node.as(providerType.Input.self), providerType.Context.self == Void.self @@ -48,10 +48,10 @@ final class SourceEditorCommand: NSObject, XCSourceEditorCommand { return nil } let context = unsafeBitCast(Void(), to: providerType.Context.self) - return providerType.refactor(syntax: input, in: context).map { Syntax($0) } + return try Syntax(providerType.refactor(syntax: input, in: context)) } - return _openExistential(self.provider, do: withOpenedRefactoringProvider) + return try? _openExistential(self.provider, do: withOpenedRefactoringProvider) } } diff --git a/Sources/SwiftRefactor/AddSeparatorsToIntegerLiteral.swift b/Sources/SwiftRefactor/AddSeparatorsToIntegerLiteral.swift index 9d9afc2646c..b87fff1d09c 100644 --- a/Sources/SwiftRefactor/AddSeparatorsToIntegerLiteral.swift +++ b/Sources/SwiftRefactor/AddSeparatorsToIntegerLiteral.swift @@ -37,11 +37,12 @@ import SwiftSyntax /// 0b1_010 /// ``` public struct AddSeparatorsToIntegerLiteral: SyntaxRefactoringProvider { - public static func refactor(syntax lit: IntegerLiteralExprSyntax, in context: Void) -> IntegerLiteralExprSyntax? { + public static func refactor( + syntax lit: IntegerLiteralExprSyntax, + in context: Void + ) throws -> IntegerLiteralExprSyntax { if lit.literal.text.contains("_") { - guard let strippedLiteral = RemoveSeparatorsFromIntegerLiteral.refactor(syntax: lit) else { - return nil - } + let strippedLiteral = try RemoveSeparatorsFromIntegerLiteral.refactor(syntax: lit) return self.addSeparators(to: strippedLiteral) } else { return self.addSeparators(to: lit) diff --git a/Sources/SwiftRefactor/CallToTrailingClosures.swift b/Sources/SwiftRefactor/CallToTrailingClosures.swift index 858c1185b61..3641494334e 100644 --- a/Sources/SwiftRefactor/CallToTrailingClosures.swift +++ b/Sources/SwiftRefactor/CallToTrailingClosures.swift @@ -55,39 +55,43 @@ public struct CallToTrailingClosures: SyntaxRefactoringProvider { /// Apply the refactoring to a given syntax node. If either a /// non-function-like syntax node is passed, or the refactoring fails, - /// `nil` is returned. - // TODO: Rather than returning nil, we should consider throwing errors with - // appropriate messages instead. + /// an error is thrown. public static func refactor( syntax: Syntax, in context: Context = Context() - ) -> Syntax? { - guard let call = syntax.asProtocol(CallLikeSyntax.self) else { return nil } - return Syntax(fromProtocol: _refactor(syntax: call, in: context)) + ) throws -> Syntax { + guard let call = syntax.asProtocol(CallLikeSyntax.self) else { + throw RefactoringNotApplicableError("not a call") + } + return try Syntax(fromProtocol: _refactor(syntax: call, in: context)) } @available(*, deprecated, message: "Pass a Syntax argument instead of FunctionCallExprSyntax") public static func refactor( syntax call: FunctionCallExprSyntax, in context: Context = Context() - ) -> FunctionCallExprSyntax? { - _refactor(syntax: call, in: context) + ) throws -> FunctionCallExprSyntax { + try _refactor(syntax: call, in: context) } internal static func _refactor( syntax call: C, in context: Context = Context() - ) -> C? { - let converted = call.convertToTrailingClosures(from: context.startAtArgument) - return converted?.formatted().as(C.self) + ) throws -> C { + let converted = try call.convertToTrailingClosures(from: context.startAtArgument) + + guard let formatted = converted.formatted().as(C.self) else { + throw RefactoringNotApplicableError("format error") + } + + return formatted } } extension CallLikeSyntax { - fileprivate func convertToTrailingClosures(from startAtArgument: Int) -> Self? { + fileprivate func convertToTrailingClosures(from startAtArgument: Int) throws -> Self { guard trailingClosure == nil, additionalTrailingClosures.isEmpty, leftParen != nil, rightParen != nil else { - // Already have trailing closures - return nil + throw RefactoringNotApplicableError("call already uses trailing closures") } var closures = [(original: LabeledExprSyntax, closure: ClosureExprSyntax)]() @@ -106,7 +110,7 @@ extension CallLikeSyntax { } guard !closures.isEmpty else { - return nil + throw RefactoringNotApplicableError("no arguments to convert to closures") } // First trailing closure won't have label/colon. Transfer their trivia. diff --git a/Sources/SwiftRefactor/ConvertComputedPropertyToStored.swift b/Sources/SwiftRefactor/ConvertComputedPropertyToStored.swift index fd7685317b4..f2c14b7378e 100644 --- a/Sources/SwiftRefactor/ConvertComputedPropertyToStored.swift +++ b/Sources/SwiftRefactor/ConvertComputedPropertyToStored.swift @@ -17,11 +17,15 @@ import SwiftSyntax #endif public struct ConvertComputedPropertyToStored: SyntaxRefactoringProvider { - public static func refactor(syntax: VariableDeclSyntax, in context: ()) -> VariableDeclSyntax? { - guard syntax.bindings.count == 1, let binding = syntax.bindings.first, - let accessorBlock = binding.accessorBlock, case let .getter(body) = accessorBlock.accessors, !body.isEmpty + public static func refactor(syntax: VariableDeclSyntax, in context: ()) throws -> VariableDeclSyntax { + guard syntax.bindings.count == 1, let binding = syntax.bindings.first else { + throw RefactoringNotApplicableError("unsupported variable declaration") + } + + guard let accessorBlock = binding.accessorBlock, + case let .getter(body) = accessorBlock.accessors, !body.isEmpty else { - return nil + throw RefactoringNotApplicableError("getter is missing or empty") } let refactored = { (initializer: InitializerClauseSyntax) -> VariableDeclSyntax in @@ -55,7 +59,7 @@ public struct ConvertComputedPropertyToStored: SyntaxRefactoringProvider { } guard body.count == 1, let item = body.first?.item else { - return nil + throw RefactoringNotApplicableError("getter body is not a single expression") } if let item = item.as(ReturnStmtSyntax.self), let expression = item.expression { @@ -79,6 +83,6 @@ public struct ConvertComputedPropertyToStored: SyntaxRefactoringProvider { ) } - return nil + throw RefactoringNotApplicableError("could not extract initial value of stored property") } } diff --git a/Sources/SwiftRefactor/ConvertComputedPropertyToZeroParameterFunction.swift b/Sources/SwiftRefactor/ConvertComputedPropertyToZeroParameterFunction.swift index 60aa341ea44..e6224fe7f31 100644 --- a/Sources/SwiftRefactor/ConvertComputedPropertyToZeroParameterFunction.swift +++ b/Sources/SwiftRefactor/ConvertComputedPropertyToZeroParameterFunction.swift @@ -17,17 +17,17 @@ import SwiftSyntax #endif public struct ConvertComputedPropertyToZeroParameterFunction: SyntaxRefactoringProvider { - public static func refactor(syntax: VariableDeclSyntax, in context: Void) -> FunctionDeclSyntax? { + public static func refactor(syntax: VariableDeclSyntax, in context: Void) throws -> FunctionDeclSyntax { guard syntax.bindings.count == 1, let binding = syntax.bindings.first, let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) - else { return nil } + else { throw RefactoringNotApplicableError("unsupported variable declaration") } var statements: CodeBlockItemListSyntax guard let typeAnnotation = binding.typeAnnotation, var accessorBlock = binding.accessorBlock - else { return nil } + else { throw RefactoringNotApplicableError("no type annotation or stored") } var effectSpecifiers: AccessorEffectSpecifiersSyntax? @@ -35,7 +35,7 @@ public struct ConvertComputedPropertyToZeroParameterFunction: SyntaxRefactoringP case .accessors(let accessors): guard accessors.count == 1, let accessor = accessors.first, accessor.accessorSpecifier.tokenKind == .keyword(.get), let codeBlock = accessor.body - else { return nil } + else { throw RefactoringNotApplicableError("not a getter-only declaration") } effectSpecifiers = accessor.effectSpecifiers statements = codeBlock.statements let accessorSpecifier = accessor.accessorSpecifier diff --git a/Sources/SwiftRefactor/ConvertStoredPropertyToComputed.swift b/Sources/SwiftRefactor/ConvertStoredPropertyToComputed.swift index 2ea0976e675..625432e603c 100644 --- a/Sources/SwiftRefactor/ConvertStoredPropertyToComputed.swift +++ b/Sources/SwiftRefactor/ConvertStoredPropertyToComputed.swift @@ -17,9 +17,9 @@ import SwiftSyntax #endif public struct ConvertStoredPropertyToComputed: SyntaxRefactoringProvider { - public static func refactor(syntax: VariableDeclSyntax, in context: ()) -> VariableDeclSyntax? { + public static func refactor(syntax: VariableDeclSyntax, in context: ()) throws -> VariableDeclSyntax { guard syntax.bindings.count == 1, let binding = syntax.bindings.first, let initializer = binding.initializer else { - return nil + throw RefactoringNotApplicableError("unsupported variable declaration") } var codeBlockSyntax: CodeBlockItemListSyntax @@ -27,7 +27,11 @@ public struct ConvertStoredPropertyToComputed: SyntaxRefactoringProvider { if let functionExpression = initializer.value.as(FunctionCallExprSyntax.self), let closureExpression = functionExpression.calledExpression.as(ClosureExprSyntax.self) { - guard functionExpression.arguments.isEmpty else { return nil } + guard functionExpression.arguments.isEmpty else { + throw RefactoringNotApplicableError( + "initializer is a closure that takes arguments" + ) + } codeBlockSyntax = closureExpression.statements codeBlockSyntax.leadingTrivia = diff --git a/Sources/SwiftRefactor/ConvertZeroParameterFunctionToComputedProperty.swift b/Sources/SwiftRefactor/ConvertZeroParameterFunctionToComputedProperty.swift index 7027b0f5d93..7215b2fe028 100644 --- a/Sources/SwiftRefactor/ConvertZeroParameterFunctionToComputedProperty.swift +++ b/Sources/SwiftRefactor/ConvertZeroParameterFunctionToComputedProperty.swift @@ -17,10 +17,10 @@ import SwiftSyntax #endif public struct ConvertZeroParameterFunctionToComputedProperty: SyntaxRefactoringProvider { - public static func refactor(syntax: FunctionDeclSyntax, in context: ()) -> VariableDeclSyntax? { + public static func refactor(syntax: FunctionDeclSyntax, in context: ()) throws -> VariableDeclSyntax { guard syntax.signature.parameterClause.parameters.isEmpty, let body = syntax.body - else { return nil } + else { throw RefactoringNotApplicableError("not a zero parameter function") } let variableName = PatternSyntax( IdentifierPatternSyntax( diff --git a/Sources/SwiftRefactor/ExpandEditorPlaceholder.swift b/Sources/SwiftRefactor/ExpandEditorPlaceholder.swift index ab416e83bdc..a9ef7deaf9b 100644 --- a/Sources/SwiftRefactor/ExpandEditorPlaceholder.swift +++ b/Sources/SwiftRefactor/ExpandEditorPlaceholder.swift @@ -275,13 +275,19 @@ public struct ExpandEditorPlaceholdersToLiteralClosures: SyntaxRefactoringProvid public static func refactor( syntax: Syntax, in context: Context = Context() - ) -> Syntax? { - guard let call = syntax.asProtocol(CallLikeSyntax.self) else { return nil } - let expanded = Self.expandClosurePlaceholders( - in: call, - ifIncluded: nil, - context: context - ) + ) throws -> Syntax { + guard let call = syntax.asProtocol(CallLikeSyntax.self) else { + throw RefactoringNotApplicableError("not a call") + } + guard + let expanded = Self.expandClosurePlaceholders( + in: call, + ifIncluded: nil, + context: context + ) + else { + throw RefactoringNotApplicableError("could not expand closure placeholders") + } return Syntax(fromProtocol: expanded) } @@ -325,7 +331,7 @@ public struct ExpandEditorPlaceholdersToLiteralClosures: SyntaxRefactoringProvid let callToTrailingContext = CallToTrailingClosures.Context( startAtArgument: call.arguments.count - expanded.numClosures ) - return CallToTrailingClosures._refactor(syntax: expanded.expr, in: callToTrailingContext) + return try? CallToTrailingClosures._refactor(syntax: expanded.expr, in: callToTrailingContext) } } } diff --git a/Sources/SwiftRefactor/FormatRawStringLiteral.swift b/Sources/SwiftRefactor/FormatRawStringLiteral.swift index 50c13366fd0..c8208b56ec3 100644 --- a/Sources/SwiftRefactor/FormatRawStringLiteral.swift +++ b/Sources/SwiftRefactor/FormatRawStringLiteral.swift @@ -35,7 +35,7 @@ import SwiftSyntax /// "Hello World" /// ``` public struct FormatRawStringLiteral: SyntaxRefactoringProvider { - public static func refactor(syntax lit: StringLiteralExprSyntax, in context: Void) -> StringLiteralExprSyntax? { + public static func refactor(syntax lit: StringLiteralExprSyntax, in context: Void) -> StringLiteralExprSyntax { var maximumHashes = 0 for segment in lit.segments { switch segment { diff --git a/Sources/SwiftRefactor/MigrateToNewIfLetSyntax.swift b/Sources/SwiftRefactor/MigrateToNewIfLetSyntax.swift index 6c98f890c18..268d6a51c46 100644 --- a/Sources/SwiftRefactor/MigrateToNewIfLetSyntax.swift +++ b/Sources/SwiftRefactor/MigrateToNewIfLetSyntax.swift @@ -39,7 +39,7 @@ import SwiftSyntax /// // ... /// } public struct MigrateToNewIfLetSyntax: SyntaxRefactoringProvider { - public static func refactor(syntax node: IfExprSyntax, in context: ()) -> IfExprSyntax? { + public static func refactor(syntax node: IfExprSyntax, in context: ()) -> IfExprSyntax { // Visit all conditions in the node. let newConditions = node.conditions.enumerated().map { (index, condition) -> ConditionElementListSyntax.Element in var conditionCopy = condition diff --git a/Sources/SwiftRefactor/OpaqueParameterToGeneric.swift b/Sources/SwiftRefactor/OpaqueParameterToGeneric.swift index a98a9bb7cc3..c001aa3127b 100644 --- a/Sources/SwiftRefactor/OpaqueParameterToGeneric.swift +++ b/Sources/SwiftRefactor/OpaqueParameterToGeneric.swift @@ -179,7 +179,7 @@ public struct OpaqueParameterToGeneric: SyntaxRefactoringProvider { public static func refactor( syntax decl: DeclSyntax, in context: Void - ) -> DeclSyntax? { + ) throws -> DeclSyntax { // Function declaration. if let funcSyntax = decl.as(FunctionDeclSyntax.self) { guard @@ -188,7 +188,7 @@ public struct OpaqueParameterToGeneric: SyntaxRefactoringProvider { augmenting: funcSyntax.genericParameterClause ) else { - return nil + throw RefactoringNotApplicableError("found no parameters to rewrite") } return DeclSyntax( @@ -206,7 +206,7 @@ public struct OpaqueParameterToGeneric: SyntaxRefactoringProvider { augmenting: initSyntax.genericParameterClause ) else { - return nil + throw RefactoringNotApplicableError("found no parameters to rewrite") } return DeclSyntax( @@ -224,7 +224,7 @@ public struct OpaqueParameterToGeneric: SyntaxRefactoringProvider { augmenting: subscriptSyntax.genericParameterClause ) else { - return nil + throw RefactoringNotApplicableError("found no parameters to rewrite") } return DeclSyntax( @@ -234,6 +234,6 @@ public struct OpaqueParameterToGeneric: SyntaxRefactoringProvider { ) } - return nil + throw RefactoringNotApplicableError("unsupported declaration") } } diff --git a/Sources/SwiftRefactor/RefactoringProvider.swift b/Sources/SwiftRefactor/RefactoringProvider.swift index 63bd8b1c400..d8a7781784e 100644 --- a/Sources/SwiftRefactor/RefactoringProvider.swift +++ b/Sources/SwiftRefactor/RefactoringProvider.swift @@ -16,6 +16,18 @@ public import SwiftSyntax import SwiftSyntax #endif +/// Error that refactoring actions can throw when the refactoring fails. +/// +/// The reason should start with a single lowercase character and +/// should be formatted similarly to compiler error messages. +public struct RefactoringNotApplicableError: Error, CustomStringConvertible { + public var description: String + + public init(_ reason: String) { + self.description = reason + } +} + /// A refactoring expressed as textual edits on the original syntax tree. In /// general clients should prefer `SyntaxRefactoringProvider` where possible. public protocol EditRefactoringProvider { @@ -29,10 +41,10 @@ public protocol EditRefactoringProvider { /// - Parameters: /// - syntax: The syntax to transform. /// - context: Contextual information used by the refactoring action. + /// - Throws: Throws an error if the refactoring action fails or is not applicable. /// - Returns: Textual edits that describe how to apply the result of the - /// refactoring action on locations within the original tree. An - /// empty array if the refactoring could not be performed. - static func textRefactor(syntax: Input, in context: Context) -> [SourceEdit] + /// refactoring action on locations within the original tree. + static func textRefactor(syntax: Input, in context: Context) throws -> [SourceEdit] } extension EditRefactoringProvider where Context == Void { @@ -41,9 +53,10 @@ extension EditRefactoringProvider where Context == Void { /// /// - Parameters: /// - syntax: The syntax to transform. + /// - Throws: Throws an error if the refactoring action fails or is not applicable. /// - Returns: Textual edits describing the refactoring to perform. - public static func textRefactor(syntax: Input) -> [SourceEdit] { - return self.textRefactor(syntax: syntax, in: ()) + public static func textRefactor(syntax: Input) throws -> [SourceEdit] { + return try self.textRefactor(syntax: syntax, in: ()) } } @@ -84,9 +97,9 @@ public protocol SyntaxRefactoringProvider: EditRefactoringProvider { /// - Parameters: /// - syntax: The syntax to transform. /// - context: Contextual information used by the refactoring action. - /// - Returns: The result of applying the refactoring action, or `nil` if the - /// action could not be performed. - static func refactor(syntax: Input, in context: Context) -> Output? + /// - Throws: Throws an error if the refactoring action fails or is not applicable. + /// - Returns: The result of applying the refactoring action. + static func refactor(syntax: Input, in context: Context) throws -> Output } extension SyntaxRefactoringProvider where Context == Void { @@ -95,10 +108,10 @@ extension SyntaxRefactoringProvider where Context == Void { /// /// - Parameters: /// - syntax: The syntax to transform. - /// - Returns: The result of applying the refactoring action, or `nil` if the - /// action could not be performed. - public static func refactor(syntax: Input) -> Output? { - return self.refactor(syntax: syntax, in: ()) + /// - Throws: Throws an error if the refactoring action fails or is not applicable. + /// - Returns: The result of applying the refactoring action. + public static func refactor(syntax: Input) throws -> Output { + return try self.refactor(syntax: syntax, in: ()) } } @@ -106,10 +119,8 @@ extension SyntaxRefactoringProvider { /// Provides a default implementation for /// `EditRefactoringProvider.textRefactor(syntax:in:)` that produces an edit /// to replace the input of `refactor(syntax:in:)` with its returned output. - public static func textRefactor(syntax: Input, in context: Context) -> [SourceEdit] { - guard let output = refactor(syntax: syntax, in: context) else { - return [] - } + public static func textRefactor(syntax: Input, in context: Context) throws -> [SourceEdit] { + let output = try refactor(syntax: syntax, in: context) return [SourceEdit.replace(syntax, with: output.description)] } } diff --git a/Sources/SwiftRefactor/RemoveSeparatorsFromIntegerLiteral.swift b/Sources/SwiftRefactor/RemoveSeparatorsFromIntegerLiteral.swift index 038d531ac79..ea0e1aa95d8 100644 --- a/Sources/SwiftRefactor/RemoveSeparatorsFromIntegerLiteral.swift +++ b/Sources/SwiftRefactor/RemoveSeparatorsFromIntegerLiteral.swift @@ -31,10 +31,8 @@ import SwiftSyntax /// 0xFFFFFFFFF /// ``` public struct RemoveSeparatorsFromIntegerLiteral: SyntaxRefactoringProvider { - public static func refactor(syntax lit: IntegerLiteralExprSyntax, in context: Void) -> IntegerLiteralExprSyntax? { - guard lit.literal.text.contains("_") else { - return lit - } + public static func refactor(syntax lit: IntegerLiteralExprSyntax, in context: Void) -> IntegerLiteralExprSyntax { + guard lit.literal.text.contains("_") else { return lit } let formattedText = lit.literal.text.filter({ $0 != "_" }) return lit.with(\.literal, lit.literal.with(\.tokenKind, .integerLiteral(formattedText))) } diff --git a/Tests/SwiftRefactorTest/RefactorTestUtils.swift b/Tests/SwiftRefactorTest/RefactorTestUtils.swift index 8650d03e323..721327dca61 100644 --- a/Tests/SwiftRefactorTest/RefactorTestUtils.swift +++ b/Tests/SwiftRefactorTest/RefactorTestUtils.swift @@ -25,7 +25,7 @@ func assertRefactor( file: StaticString = #filePath, line: UInt = #line ) throws { - let edits = R.textRefactor(syntax: input, in: context) + let edits = try R.textRefactor(syntax: input, in: context) guard !edits.isEmpty else { if !expected.isEmpty { XCTFail( @@ -83,12 +83,12 @@ func assertRefactor( file: StaticString = #filePath, line: UInt = #line ) throws { - let refactored = R.refactor(syntax: input, in: context) + let refactored = try? R.refactor(syntax: input, in: context) guard let refactored = refactored else { if expected != nil { XCTFail( """ - Refactoring produced nil result, expected: + Refactoring failed, expected: \(expected?.description ?? "") """, file: file,