Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions Sources/SwiftRefactor/MoveMembersToExtension.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
//===----------------------------------------------------------------------===//
//
// 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)
public import SwiftSyntax
#else
import SwiftSyntax
#endif

public struct MoveMembersToExtension: SyntaxRefactoringProvider {

public struct Context {
public let declName: TokenSyntax
public let selectedIdentifiers: [SyntaxIdentifier]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should specify a range of declarations to move, not their identifiers. The base identifier names are not sufficient to identify a declaration if it’s overloaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


public init(declName: TokenSyntax, selectedIdentifiers: [SyntaxIdentifier]) {
self.declName = declName
self.selectedIdentifiers = selectedIdentifiers
}
}

public static func refactor(syntax: SourceFileSyntax, in context: Context) throws -> SourceFileSyntax {
guard
let decl = syntax.statements.first(where: {
$0.item.asProtocol(NamedDeclSyntax.self)?.name == context.declName
}),
let declGroup = decl.item.asProtocol(DeclGroupSyntax.self),
let index = syntax.statements.index(of: decl)
else {
throw RefactoringNotApplicableError("Type declaration not found")
}

let selectedMembers = declGroup.memberBlock.members.filter { context.selectedIdentifiers.contains($0.id) }

for member in selectedMembers {
try validateMember(member)
}

let remainingMembers = declGroup.memberBlock.members.filter { !context.selectedIdentifiers.contains($0.id) }

let updatedMemberBlock = declGroup.memberBlock.with(\.members, remainingMembers)
let updatedDeclGroup = declGroup.with(\.memberBlock, updatedMemberBlock)
let updatedItem = decl.with(\.item, .decl(DeclSyntax(updatedDeclGroup)))

let extensionMemberBlockSyntax = declGroup.memberBlock.with(\.members, selectedMembers)

let extensionDecl = ExtensionDeclSyntax(
leadingTrivia: .newlines(2),
extendedType: IdentifierTypeSyntax(
leadingTrivia: .space,
name: context.declName
),
memberBlock: extensionMemberBlockSyntax
)

var updatedStatements = syntax.statements
updatedStatements.remove(at: index)
updatedStatements.insert(updatedItem, at: index)
updatedStatements.append(CodeBlockItemSyntax(item: .decl(DeclSyntax(extensionDecl))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we insert the declaration after the updated item instead of at the end of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


return syntax.with(\.statements, updatedStatements)
}

private static func validateMember(_ member: MemberBlockItemSyntax) throws {
if member.decl.is(AccessorDeclSyntax.self) || member.decl.is(DeinitializerDeclSyntax.self)
|| member.decl.is(EnumCaseDeclSyntax.self)
{
throw RefactoringNotApplicableError("Cannot move this type of declaration")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be a little more specific in the error message about what we can’t move. I would imagine that this error message is annoying to get if you selected 200 lines of code and you don’t know what this is.

Actually, just thinking about it, when you select 5 functions and one deinitializer, maybe we should just move the 5 functions and leave the deinitializer where it is. And only emit an error like deinitializers cannot be moved to an extension if the deinitializer is the only declaration that was selected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give some advice on how to handle error logging for this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just played around a little bit how I would model this. What do you think of the following?

private enum ValidationResult: CustomStringConvertible {
  case accessor
  case deinitializer
  case enumCase
  case storedProperty

  var description: String {
    switch self {
    case .accessor: return "accessor"
    case .deinitializer: return "deinitializer"
    case .enumCase: return "enum case"
    case .storedProperty: return "stored property"
    }

  }

  /// Validates that `member` can be moved to an extension. If it can, return `nil`, otherwise return the reason why
  /// `member` cannot be moved to an extension.
  init?(_ member: MemberBlockItemSyntax) {
    switch member.decl.kind {
    case .accessorDecl:
      self = .accessor
    case .deinitializerDecl:
      self = .deinitializer
    case .enumCaseDecl:
      self = .enumCase
    default:
      break
    }
    if let varDecl = member.decl.as(VariableDeclSyntax.self),
      varDecl.bindings.contains(where: { $0.accessorBlock == nil || $0.initializer != nil })
    {
      self = .storedProperty
    }

    return nil
  }
}

public struct MoveMembersToExtension: SyntaxRefactoringProvider {
  public struct Context {
    public let range: Range<AbsolutePosition>

    public init(range: Range<AbsolutePosition>) {
      self.range = range
    }
  }

  public static func refactor(syntax: SourceFileSyntax, in context: Context) throws -> SourceFileSyntax {
    guard
      let statement = syntax.statements.first(where: { $0.item.range.contains(context.range) }),
      let decl = statement.item.asProtocol(NamedDeclSyntax.self),
      let declGroup = statement.item.asProtocol(DeclGroupSyntax.self),
      let statementIndex = syntax.statements.index(of: statement)
    else {
      throw RefactoringNotApplicableError("Type declaration not found")
    }

    let selectedMembers = declGroup.memberBlock.members.filter { context.range.overlaps($0.trimmedRange) }
      .map { (member: $0, validationResult: ValidationResult($0)) }

    guard !selectedMembers.isEmpty else {
      throw RefactoringNotApplicableError("No members to move")
    }

    let membersToMove = selectedMembers.filter { $0.validationResult == nil }.map(\.member)
    guard !membersToMove.isEmpty else {
      throw RefactoringNotApplicableError(
        "Cannot move \(Set(selectedMembers.compactMap(\.validationResult)).map(\.description).sorted()) to extension"
      )
    }

    var updatedDeclGroup = declGroup
    updatedDeclGroup.memberBlock.members = declGroup.memberBlock.members.filter { !membersToMove.contains($0) }
    let updatedItem = statement.with(\.item, .decl(DeclSyntax(updatedDeclGroup)))

    let extensionMemberBlockSyntax = declGroup.memberBlock.with(\.members, MemberBlockItemListSyntax(membersToMove))

    var declName = decl.name
    declName.trailingTrivia = declName.trailingTrivia.merging(.space)

    let extensionDecl = ExtensionDeclSyntax(
      leadingTrivia: .newlines(2),
      extendedType: IdentifierTypeSyntax(
        leadingTrivia: .space,
        name: declName
      ),
      memberBlock: extensionMemberBlockSyntax
    )

    var syntax = syntax
    syntax.statements[statementIndex] = updatedItem
    syntax.statements.insert(
      CodeBlockItemSyntax(item: .decl(DeclSyntax(extensionDecl))),
      at: syntax.statements.index(after: statementIndex)
    )
    return syntax
  }
}

}

if let varDecl = member.decl.as(VariableDeclSyntax.self),
varDecl.bindings.contains(where: { $0.initializer == nil })
{
throw RefactoringNotApplicableError("Cannot move stored properties to extension")
}
}
}
90 changes: 90 additions & 0 deletions Tests/SwiftRefactorTest/MoveMembersToExtensionTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
//===----------------------------------------------------------------------===//
//
// 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 SwiftParser
import SwiftRefactor
import SwiftSyntax
import SwiftSyntaxBuilder
import XCTest
import _SwiftSyntaxTestSupport

final class MoveMembersToExtensionTests: XCTestCase {
func testMoveFunctionToExtension() throws {
let baseline: String = """
class Foo {1️⃣
func foo() {
print("Hello world!")
}2️⃣

func bar() {
print("Hello world!")
}
}
"""

let expected: SourceFileSyntax = """
class Foo {

func bar() {
print("Hello world!")
}
}

extension Foo {
func foo() {
print("Hello world!")
}
}
"""

let (markers, source) = extractMarkers(baseline)

var parser = Parser(source)
let tree = SourceFileSyntax.parse(from: &parser)
let context = makeContextFromClass(markers: markers, source: tree)
try assertRefactorConvert(tree, expected: expected, context: context)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make sure that we either disallow or correctly handle extracting members from nested types.

Suggested test cases:

struct Outer {
  struct Inner {
    func moveThis() {}
  }
}
struct Outer<T> {
  struct Inner {
    func moveThis() {}
  }
}
func outer() {
  struct Inner {
    func moveThis() {}
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a couple of tests, the rest are in progress.

}

private func makeContextFromClass(markers: [String: Int], source: SourceFileSyntax) -> MoveMembersToExtension.Context {
let classDecl = source.statements
.first(where: { $0.item.is(ClassDeclSyntax.self) })!
.item.cast(ClassDeclSyntax.self)
let members = classDecl.memberBlock.members

let selectedMembersId: [SyntaxIdentifier] = members.compactMap({
let offset = $0.positionAfterSkippingLeadingTrivia.utf8Offset
if let start = markers["1️⃣"], let end = markers["2️⃣"], offset > start && offset < end {
return $0.id
}
return nil
})

return MoveMembersToExtension.Context(declName: classDecl.name, selectedIdentifiers: selectedMembersId)
}

private func assertRefactorConvert(
_ callDecl: SourceFileSyntax,
expected: SourceFileSyntax?,
context: MoveMembersToExtension.Context,
file: StaticString = #filePath,
line: UInt = #line
) throws {
try assertRefactor(
callDecl,
context: context,
provider: MoveMembersToExtension.self,
expected: expected,
file: file,
line: line
)
}