Skip to content

Fix renaming enum case parameters with unnamed associated arguments #74241

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

Merged
merged 1 commit into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 7 additions & 0 deletions include/swift/IDE/IDEBridging.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ enum class LabelRangeType {
/// `func foo([a b]: Int)`
Param,

/// The parameter of an enum case declaration
///
/// ### Examples
/// - `case myCase([a]: Int)`
/// - `case myCase([]Int)`
EnumCaseParam,

/// Parameters of a function that can't be collapsed if they are the same.
///
/// This is the case for parameters of subscripts since subscripts have
Expand Down
37 changes: 28 additions & 9 deletions include/swift/IDE/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -465,15 +465,34 @@ enum class RegionType {
};

enum class RefactoringRangeKind {
BaseName, // func [foo](a b: Int)
KeywordBaseName, // [init](a: Int)
ParameterName, // func foo(a[ b]: Int)
NoncollapsibleParameterName, // subscript(a[ a]: Int)
DeclArgumentLabel, // func foo([a] b: Int)
CallArgumentLabel, // foo([a]: 1)
CallArgumentColon, // foo(a[: ]1)
CallArgumentCombined, // foo([]1) could expand to foo([a: ]1)
SelectorArgumentLabel, // foo([a]:)
/// `func [foo](a b: Int)`
BaseName,

/// `[init](a: Int)`
KeywordBaseName,

/// `func foo(a[ b]: Int)`
ParameterName,

/// `subscript(a[ a]: Int)`
NoncollapsibleParameterName,

/// `func foo([a] b: Int)`
DeclArgumentLabel,

/// `foo([a]: 1)`
CallArgumentLabel,

/// `foo(a[: ]1)`
CallArgumentColon,

/// `foo([]1) could expand to foo([a: ]1)`
/// Also used for enum case declarations without a label, eg.
/// `case foo([]String)` should expand to `case foo([a: ]String)`.
CallArgumentCombined,

/// `foo([a]:)`
SelectorArgumentLabel,
};

struct NoteRegion {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ fileprivate extension IDEBridging.LabelRangeType {
case .noArguments: self = .None
case .call: self = .CallArg
case .parameters: self = .Param
case .enumCaseParameters: self = .EnumCaseParam
case .noncollapsibleParameters: self = .NoncollapsibleParam
case .selector: self = .CompoundName
#if RESILIENT_SWIFT_SYNTAX
Expand All @@ -72,6 +73,7 @@ extension BridgedResolvedLoc {
case .noArguments: arguments = []
case .call(let arguments2, _): arguments = arguments2
case .parameters(let arguments2): arguments = arguments2
case .enumCaseParameters(let arguments2): arguments = arguments2
case .noncollapsibleParameters(let arguments2): arguments = arguments2
case .selector(let arguments2): arguments = arguments2
#if RESILIENT_SWIFT_SYNTAX
Expand Down
13 changes: 13 additions & 0 deletions lib/Refactoring/SyntacticRenameRangeDetails.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,18 @@ void RenameRangeDetailCollector::splitAndRenameLabel(CharSourceRange Range,
return splitAndRenameCallArg(Range, NameIndex);
case LabelRangeType::Param:
return splitAndRenameParamLabel(Range, NameIndex, /*IsCollapsible=*/true);
case LabelRangeType::EnumCaseParam:
if (Range.getByteLength() == 0) {
// If the associated value currently doesn't have a label, emit a
// `CallArgumentCombined` range, which will cause a new label followed by
// `:` to be inserted in the same fashion that call arguments get inserted
// to calls
return addRenameRange(Range, RefactoringRangeKind::CallArgumentCombined, NameIndex);
} else {
// If the associated value has a label already, we are in the same case as
// function parameters.
return splitAndRenameParamLabel(Range, NameIndex, /*IsCollapsible=*/true);
}
case LabelRangeType::NoncollapsibleParam:
return splitAndRenameParamLabel(Range, NameIndex,
/*IsCollapsible=*/false);
Expand Down Expand Up @@ -248,6 +260,7 @@ bool RenameRangeDetailCollector::labelRangeMatches(CharSourceRange Range,
LLVM_FALLTHROUGH;
case LabelRangeType::CallArg:
case LabelRangeType::Param:
case LabelRangeType::EnumCaseParam:
case LabelRangeType::CompoundName:
return ExistingLabel == (Expected.empty() ? "_" : Expected);
case LabelRangeType::None:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t
// RUN: %sourcekitd-test -req=find-rename-ranges -rename-spec %t/rename-spec.json %t/input.swift | %FileCheck %s

// CHECK: source.edit.kind.active:
// CHECK-NEXT: 2:8-2:14 source.refactoring.range.kind.basename
// CHECK-NEXT: 2:15-2:20 source.refactoring.range.kind.decl-argument-label arg-index=0
// CHECK-NEXT: 2:20-2:20 source.refactoring.range.kind.parameter-and-whitespace arg-index=0
// CHECK-NEXT: source.edit.kind.active:
// CHECK-NEXT: 6:14-6:20 source.refactoring.range.kind.basename
// CHECK-NEXT: 6:21-6:26 source.refactoring.range.kind.call-argument-label arg-index=0
// CHECK-NEXT: 6:26-6:28 source.refactoring.range.kind.call-argument-colon arg-index=0

//--- input.swift
enum MyEnum {
case myCase(label: String)
}

func test() {
_ = MyEnum.myCase(label: "abc")
}

//--- rename-spec.json

[
{
"key.name": "myCase(label:)",
"key.locations": [
{
"key.line": 2,
"key.column": 8,
"key.nametype": source.syntacticrename.definition
}
]
},
{
"key.name": "myCase(label:)",
"key.locations": [
{
"key.line": 6,
"key.column": 14,
"key.nametype": source.syntacticrename.call
}
]
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t
// RUN: %sourcekitd-test -req=find-rename-ranges -rename-spec %t/rename-spec.json %t/input.swift | %FileCheck %s

// CHECK: source.edit.kind.active:
// CHECK-NEXT: 2:8-2:14 source.refactoring.range.kind.basename
// CHECK-NEXT: 2:15-2:15 source.refactoring.range.kind.call-argument-combined arg-index=0
// CHECK-NEXT: source.edit.kind.active:
// CHECK-NEXT: 6:14-6:20 source.refactoring.range.kind.basename
// CHECK-NEXT: 6:21-6:21 source.refactoring.range.kind.call-argument-combined arg-index=0

//--- input.swift
enum MyEnum {
case myCase(String)
}

func test() {
_ = MyEnum.myCase("abc")
}

//--- rename-spec.json

[
{
"key.name": "myCase(_:)",
"key.locations": [
{
"key.line": 2,
"key.column": 8,
"key.nametype": source.syntacticrename.definition
}
]
},
{
"key.name": "myCase(_:)",
"key.locations": [
{
"key.line": 6,
"key.column": 14,
"key.nametype": source.syntacticrename.call
}
]
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t
// RUN: %sourcekitd-test -req=find-rename-ranges -rename-spec %t/rename-spec.json %t/input.swift | %FileCheck %s

// CHECK: source.edit.kind.active:
// CHECK-NEXT: 2:8-2:14 source.refactoring.range.kind.basename
// CHECK-NEXT: 2:15-2:16 source.refactoring.range.kind.decl-argument-label arg-index=0
// CHECK-NEXT: 2:16-2:22 source.refactoring.range.kind.parameter-and-whitespace arg-index=0
// CHECK-NEXT: source.edit.kind.active:
// CHECK-NEXT: 6:14-6:20 source.refactoring.range.kind.basename
// CHECK-NEXT: 6:21-6:21 source.refactoring.range.kind.call-argument-combined arg-index=0

//--- input.swift
enum MyEnum {
case myCase(_ label: String)
}

func test() {
_ = MyEnum.myCase("abc")
}

//--- rename-spec.json

[
{
"key.name": "myCase(:)",
"key.locations": [
{
"key.line": 2,
"key.column": 8,
"key.nametype": source.syntacticrename.definition
}
]
},
{
"key.name": "myCase(:)",
"key.locations": [
{
"key.line": 6,
"key.column": 14,
"key.nametype": source.syntacticrename.call
}
]
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ extension /*class-Animal:def*/Animal {
enum /*enum-Barcode:def*/Barcode {
case upc(Int, Int, Int, Int)
case /*case-qrCode:def*/qrCode(code: String)
case /*case-other:def*/<base>other</base>(<arglabel index=0></arglabel><param index=0></param>Int)
case /*case-other:def*/<base>other</base>(<callcombo index=0></callcombo>Int)
case /*case-another:def*/another
}
var barCode: /*enum-Barcode*/Barcode = /*enum-Barcode*/Barcode.upc(1, 1, 1, 1)
Expand Down