-
Notifications
You must be signed in to change notification settings - Fork 379
Add Generate enum associated value accessors code action #2544
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
Changes from all commits
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,157 @@ | ||||||
| //===----------------------------------------------------------------------===// | ||||||
| // | ||||||
| // 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 | ||||||
| // | ||||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| @_spi(SourceKitLSP) import LanguageServerProtocol | ||||||
| import SourceKitLSP | ||||||
| import SwiftSyntax | ||||||
|
|
||||||
| /// A code action that generates computed properties to extract associated | ||||||
| /// values and check cases for an enum. | ||||||
| /// | ||||||
| /// For each case with associated values, generates: | ||||||
| /// - `asX: T?` — extracts the associated value, or `nil` if a different case | ||||||
| /// - `isX: Bool` — returns `true` if the value matches that case | ||||||
| /// | ||||||
| /// Example: | ||||||
| /// ```swift | ||||||
| /// enum Value { | ||||||
| /// case text(String) | ||||||
| /// case number(Int) | ||||||
| /// } | ||||||
| /// ``` | ||||||
| /// Generates `asText`, `isText`, `asNumber`, `isNumber` computed properties. | ||||||
| struct GenerateEnumAssociatedValueAccessors: SyntaxCodeActionProvider { | ||||||
| static func codeActions(in scope: SyntaxCodeActionScope) -> [CodeAction] { | ||||||
| guard let node = scope.innermostNodeContainingRange else { | ||||||
| return [] | ||||||
| } | ||||||
|
|
||||||
| guard let enumDecl = node.findParentOfSelf( | ||||||
| ofType: EnumDeclSyntax.self, | ||||||
| stoppingIf: { _ in false } | ||||||
|
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. We should stop at a |
||||||
| ) else { | ||||||
| return [] | ||||||
| } | ||||||
|
|
||||||
| // Collect all cases with associated values. | ||||||
| let casesWithAssociatedValues = enumDecl.memberBlock.members.compactMap { member -> EnumCaseElementSyntax? in | ||||||
| guard let caseDecl = member.decl.as(EnumCaseDeclSyntax.self), | ||||||
| let element = caseDecl.elements.first, | ||||||
|
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 run swift-format on your PR as described in CONTRIBUTING.md
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. We have
Suggested change
|
||||||
| caseDecl.elements.count == 1, | ||||||
| element.parameterClause != nil | ||||||
| else { | ||||||
| return nil | ||||||
| } | ||||||
| return element | ||||||
| } | ||||||
|
|
||||||
| if casesWithAssociatedValues.isEmpty { | ||||||
| return [] | ||||||
|
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. You should throw a
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. Is there any reason why we shouldn’t generate the |
||||||
| } | ||||||
|
|
||||||
| // Scan existing member names to avoid duplicates. | ||||||
| let existingMembers = Set( | ||||||
| enumDecl.memberBlock.members.compactMap { member -> String? in | ||||||
| guard let varDecl = member.decl.as(VariableDeclSyntax.self), | ||||||
| let binding = varDecl.bindings.first, | ||||||
| let pattern = binding.pattern.as(IdentifierPatternSyntax.self) | ||||||
| else { | ||||||
| return nil | ||||||
| } | ||||||
| return pattern.identifier.text | ||||||
| } | ||||||
| ) | ||||||
|
|
||||||
| var accessors: [String] = [] | ||||||
|
|
||||||
| for element in casesWithAssociatedValues { | ||||||
| let caseName = element.name.text | ||||||
| let capitalizedName = caseName.prefix(1).uppercased() + caseName.dropFirst() | ||||||
| let asName = "as\(capitalizedName)" | ||||||
| let isName = "is\(capitalizedName)" | ||||||
|
|
||||||
| guard let paramClause = element.parameterClause else { continue } | ||||||
| let params = Array(paramClause.parameters) | ||||||
|
|
||||||
| if params.count == 1 { | ||||||
|
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. You can use |
||||||
| let typeText = params[0].type.trimmedDescription | ||||||
|
|
||||||
| if !existingMembers.contains(asName) { | ||||||
| accessors.append( | ||||||
| """ | ||||||
| var \(asName): \(typeText)? { | ||||||
| if case let .\(caseName)(v) = self { return v } | ||||||
| return nil | ||||||
| } | ||||||
| """ | ||||||
| ) | ||||||
| } | ||||||
| } else { | ||||||
| let tupleTypes = params.map { $0.type.trimmedDescription } | ||||||
| let returnType = "(\(tupleTypes.joined(separator: ", ")))" | ||||||
| let bindingVars = (0..<params.count).map { "v\($0)" } | ||||||
|
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. We should attempt to pick more descriptive variable names here. If the associated value has a label, we should pick that. Otherwise, I’d use |
||||||
| let bindingPattern = bindingVars.joined(separator: ", ") | ||||||
|
Comment on lines
+99
to
+102
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. Why can’t this general case also handle the case where there’s a single associated value? |
||||||
|
|
||||||
| if !existingMembers.contains(asName) { | ||||||
| accessors.append( | ||||||
| """ | ||||||
| var \(asName): \(returnType)? { | ||||||
| if case let .\(caseName)(\(bindingPattern)) = self { return (\(bindingPattern)) } | ||||||
|
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. We should infer the indentation of the source file (see how other code actions do this) instead of hardcoding it to 4 spaces. |
||||||
| return nil | ||||||
| } | ||||||
| """ | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if !existingMembers.contains(isName) { | ||||||
| accessors.append( | ||||||
| """ | ||||||
| var \(isName): Bool { | ||||||
| if case .\(caseName) = self { return true } | ||||||
| return false | ||||||
| } | ||||||
| """ | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if accessors.isEmpty { | ||||||
| return [] | ||||||
| } | ||||||
|
|
||||||
| // Insert before the closing brace. | ||||||
| let closingBrace = enumDecl.memberBlock.rightBrace | ||||||
| let insertPosition = scope.snapshot.position(of: closingBrace.positionAfterSkippingLeadingTrivia) | ||||||
|
|
||||||
| let insertionText = "\n" + accessors.joined(separator: "\n\n") + "\n" | ||||||
|
|
||||||
| return [ | ||||||
| CodeAction( | ||||||
| title: "Generate enum associated value accessors", | ||||||
| kind: .refactorInline, | ||||||
| edit: WorkspaceEdit( | ||||||
| changes: [ | ||||||
| scope.snapshot.uri: [ | ||||||
| TextEdit( | ||||||
| range: Range( | ||||||
| uncheckedBounds: (lower: insertPosition, upper: insertPosition) | ||||||
| ), | ||||||
| newText: insertionText | ||||||
| ) | ||||||
| ] | ||||||
| ] | ||||||
| ) | ||||||
| ) | ||||||
| ] | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1925,3 +1925,52 @@ private func assertDeMorganTransform( | |
|
|
||
| XCTAssertEqual(result.description, expected, file: file, line: line) | ||
| } | ||
|
|
||
| // MARK: - Generate Enum Associated Value Accessors Tests | ||
|
|
||
| extension CodeActionTests { | ||
| func testGenerateEnumAssociatedValueAccessors() async throws { | ||
| try await assertCodeActions( | ||
| ##""" | ||
| 1️⃣enum Value { | ||
| case text(String) | ||
| case number(Int) | ||
| 2️⃣} | ||
| """##, | ||
| ranges: [("1️⃣", "2️⃣")], | ||
| exhaustive: false | ||
| ) { uri, positions in | ||
| [ | ||
| CodeAction( | ||
| title: "Generate enum associated value accessors", | ||
| kind: .refactorInline, | ||
| edit: WorkspaceEdit( | ||
| changes: [ | ||
| uri: [ | ||
| TextEdit( | ||
| range: positions["2️⃣"]..<positions["2️⃣"], | ||
| newText: "\n var asText: String? {\n if case let .text(v) = self { return v }\n return nil\n }\n\n var isText: Bool {\n if case .text = self { return true }\n return false\n }\n\n var asNumber: Int? {\n if case let .number(v) = self { return v }\n return nil\n }\n\n var isNumber: Bool {\n if case .number = self { return true }\n return false\n }\n" | ||
|
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. I think this would be a lot more readable as a multi-line string literal. |
||
| ) | ||
| ] | ||
| ] | ||
| ) | ||
| ) | ||
| ] | ||
| } | ||
| } | ||
|
|
||
| func testGenerateEnumNoAssociatedValues() async throws { | ||
| try await assertCodeActions( | ||
| ##""" | ||
| 1️⃣enum Direction { | ||
| case north | ||
| case south | ||
| 2️⃣} | ||
| """##, | ||
| ranges: [("1️⃣", "2️⃣")], | ||
| exhaustive: false | ||
|
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. You need to pass |
||
| ) { _, _ in | ||
| [] | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to implement this as a
SyntaxRefactoringCodeActionProvider?