Skip to content

Commit 155773c

Browse files
ahoppenahoppen
committed
Fix renaming enum case parameters with unnamed associated arguments
We treated enum case parameters the same way as function parameters and weren’t considering that they can be unlabeled. That caused us to insert eg. `_ ` in front of the case’s type, producing `case myCase(_ String)`, which is invalid. When we are inside an enum case parameter and the parameter label is empty, treat it the same as a function call, which will leave the label untouched if it isn’t modified and insert a label including a colon if a new label is introduced. swiftlang/sourcekit-lsp#1228
1 parent 91afad2 commit 155773c

8 files changed

+186
-10
lines changed

‎include/swift/IDE/IDEBridging.h

+7
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ enum class LabelRangeType {
3838
/// `func foo([a b]: Int)`
3939
Param,
4040

41+
/// The parameter of an enum case declaration
42+
///
43+
/// ### Examples
44+
/// - `case myCase([a]: Int)`
45+
/// - `case myCase([]Int)`
46+
EnumCaseParam,
47+
4148
/// Parameters of a function that can't be collapsed if they are the same.
4249
///
4350
/// This is the case for parameters of subscripts since subscripts have

‎include/swift/IDE/Utils.h

+28-9
Original file line numberDiff line numberDiff line change
@@ -465,15 +465,34 @@ enum class RegionType {
465465
};
466466

467467
enum class RefactoringRangeKind {
468-
BaseName, // func [foo](a b: Int)
469-
KeywordBaseName, // [init](a: Int)
470-
ParameterName, // func foo(a[ b]: Int)
471-
NoncollapsibleParameterName, // subscript(a[ a]: Int)
472-
DeclArgumentLabel, // func foo([a] b: Int)
473-
CallArgumentLabel, // foo([a]: 1)
474-
CallArgumentColon, // foo(a[: ]1)
475-
CallArgumentCombined, // foo([]1) could expand to foo([a: ]1)
476-
SelectorArgumentLabel, // foo([a]:)
468+
/// `func [foo](a b: Int)`
469+
BaseName,
470+
471+
/// `[init](a: Int)`
472+
KeywordBaseName,
473+
474+
/// `func foo(a[ b]: Int)`
475+
ParameterName,
476+
477+
/// `subscript(a[ a]: Int)`
478+
NoncollapsibleParameterName,
479+
480+
/// `func foo([a] b: Int)`
481+
DeclArgumentLabel,
482+
483+
/// `foo([a]: 1)`
484+
CallArgumentLabel,
485+
486+
/// `foo(a[: ]1)`
487+
CallArgumentColon,
488+
489+
/// `foo([]1) could expand to foo([a: ]1)`
490+
/// Also used for enum case declarations without a label, eg.
491+
/// `case foo([]String)` should expand to `case foo([a: ]String)`.
492+
CallArgumentCombined,
493+
494+
/// `foo([a]:)`
495+
SelectorArgumentLabel,
477496
};
478497

479498
struct NoteRegion {

‎lib/ASTGen/Sources/SwiftIDEUtilsBridging/NameMatcherBridging.swift

+2
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ fileprivate extension IDEBridging.LabelRangeType {
5050
case .noArguments: self = .None
5151
case .call: self = .CallArg
5252
case .parameters: self = .Param
53+
case .enumCaseParameters: self = .EnumCaseParam
5354
case .noncollapsibleParameters: self = .NoncollapsibleParam
5455
case .selector: self = .CompoundName
5556
#if RESILIENT_SWIFT_SYNTAX
@@ -72,6 +73,7 @@ extension BridgedResolvedLoc {
7273
case .noArguments: arguments = []
7374
case .call(let arguments2, _): arguments = arguments2
7475
case .parameters(let arguments2): arguments = arguments2
76+
case .enumCaseParameters(let arguments2): arguments = arguments2
7577
case .noncollapsibleParameters(let arguments2): arguments = arguments2
7678
case .selector(let arguments2): arguments = arguments2
7779
#if RESILIENT_SWIFT_SYNTAX

‎lib/Refactoring/SyntacticRenameRangeDetails.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,18 @@ void RenameRangeDetailCollector::splitAndRenameLabel(CharSourceRange Range,
146146
return splitAndRenameCallArg(Range, NameIndex);
147147
case LabelRangeType::Param:
148148
return splitAndRenameParamLabel(Range, NameIndex, /*IsCollapsible=*/true);
149+
case LabelRangeType::EnumCaseParam:
150+
if (Range.getByteLength() == 0) {
151+
// If the associated value currently doesn't have a label, emit a
152+
// `CallArgumentCombined` range, which will cause a new label followed by
153+
// `:` to be inserted in the same fashion that call arguments get inserted
154+
// to calls
155+
return addRenameRange(Range, RefactoringRangeKind::CallArgumentCombined, NameIndex);
156+
} else {
157+
// If the associated value has a label already, we are in the same case as
158+
// function parameters.
159+
return splitAndRenameParamLabel(Range, NameIndex, /*IsCollapsible=*/true);
160+
}
149161
case LabelRangeType::NoncollapsibleParam:
150162
return splitAndRenameParamLabel(Range, NameIndex,
151163
/*IsCollapsible=*/false);
@@ -248,6 +260,7 @@ bool RenameRangeDetailCollector::labelRangeMatches(CharSourceRange Range,
248260
LLVM_FALLTHROUGH;
249261
case LabelRangeType::CallArg:
250262
case LabelRangeType::Param:
263+
case LabelRangeType::EnumCaseParam:
251264
case LabelRangeType::CompoundName:
252265
return ExistingLabel == (Expected.empty() ? "_" : Expected);
253266
case LabelRangeType::None:

‎test/SourceKit/Refactoring/find_rename_ranges_enum_case_with_label.swift

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
// RUN: %sourcekitd-test -req=find-rename-ranges -rename-spec %t/rename-spec.json %t/input.swift | %FileCheck %s
4+
5+
// CHECK: source.edit.kind.active:
6+
// CHECK-NEXT: 2:8-2:14 source.refactoring.range.kind.basename
7+
// CHECK-NEXT: 2:15-2:20 source.refactoring.range.kind.decl-argument-label arg-index=0
8+
// CHECK-NEXT: 2:20-2:20 source.refactoring.range.kind.parameter-and-whitespace arg-index=0
9+
// CHECK-NEXT: source.edit.kind.active:
10+
// CHECK-NEXT: 6:14-6:20 source.refactoring.range.kind.basename
11+
// CHECK-NEXT: 6:21-6:26 source.refactoring.range.kind.call-argument-label arg-index=0
12+
// CHECK-NEXT: 6:26-6:28 source.refactoring.range.kind.call-argument-colon arg-index=0
13+
14+
//--- input.swift
15+
enum MyEnum {
16+
case myCase(label: String)
17+
}
18+
19+
func test() {
20+
_ = MyEnum.myCase(label: "abc")
21+
}
22+
23+
//--- rename-spec.json
24+
25+
[
26+
{
27+
"key.name": "myCase(label:)",
28+
"key.locations": [
29+
{
30+
"key.line": 2,
31+
"key.column": 8,
32+
"key.nametype": source.syntacticrename.definition
33+
}
34+
]
35+
},
36+
{
37+
"key.name": "myCase(label:)",
38+
"key.locations": [
39+
{
40+
"key.line": 6,
41+
"key.column": 14,
42+
"key.nametype": source.syntacticrename.call
43+
}
44+
]
45+
}
46+
]

‎test/SourceKit/Refactoring/find_rename_ranges_enum_case_without_label.swift

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
// RUN: %sourcekitd-test -req=find-rename-ranges -rename-spec %t/rename-spec.json %t/input.swift | %FileCheck %s
4+
5+
// CHECK: source.edit.kind.active:
6+
// CHECK-NEXT: 2:8-2:14 source.refactoring.range.kind.basename
7+
// CHECK-NEXT: 2:15-2:15 source.refactoring.range.kind.call-argument-combined arg-index=0
8+
// CHECK-NEXT: source.edit.kind.active:
9+
// CHECK-NEXT: 6:14-6:20 source.refactoring.range.kind.basename
10+
// CHECK-NEXT: 6:21-6:21 source.refactoring.range.kind.call-argument-combined arg-index=0
11+
12+
//--- input.swift
13+
enum MyEnum {
14+
case myCase(String)
15+
}
16+
17+
func test() {
18+
_ = MyEnum.myCase("abc")
19+
}
20+
21+
//--- rename-spec.json
22+
23+
[
24+
{
25+
"key.name": "myCase(_:)",
26+
"key.locations": [
27+
{
28+
"key.line": 2,
29+
"key.column": 8,
30+
"key.nametype": source.syntacticrename.definition
31+
}
32+
]
33+
},
34+
{
35+
"key.name": "myCase(_:)",
36+
"key.locations": [
37+
{
38+
"key.line": 6,
39+
"key.column": 14,
40+
"key.nametype": source.syntacticrename.call
41+
}
42+
]
43+
}
44+
]

‎test/SourceKit/Refactoring/find_rename_ranges_enum_case_without_underscore_as_first_name_label.swift

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
// RUN: %sourcekitd-test -req=find-rename-ranges -rename-spec %t/rename-spec.json %t/input.swift | %FileCheck %s
4+
5+
// CHECK: source.edit.kind.active:
6+
// CHECK-NEXT: 2:8-2:14 source.refactoring.range.kind.basename
7+
// CHECK-NEXT: 2:15-2:16 source.refactoring.range.kind.decl-argument-label arg-index=0
8+
// CHECK-NEXT: 2:16-2:22 source.refactoring.range.kind.parameter-and-whitespace arg-index=0
9+
// CHECK-NEXT: source.edit.kind.active:
10+
// CHECK-NEXT: 6:14-6:20 source.refactoring.range.kind.basename
11+
// CHECK-NEXT: 6:21-6:21 source.refactoring.range.kind.call-argument-combined arg-index=0
12+
13+
//--- input.swift
14+
enum MyEnum {
15+
case myCase(_ label: String)
16+
}
17+
18+
func test() {
19+
_ = MyEnum.myCase("abc")
20+
}
21+
22+
//--- rename-spec.json
23+
24+
[
25+
{
26+
"key.name": "myCase(:)",
27+
"key.locations": [
28+
{
29+
"key.line": 2,
30+
"key.column": 8,
31+
"key.nametype": source.syntacticrename.definition
32+
}
33+
]
34+
},
35+
{
36+
"key.name": "myCase(:)",
37+
"key.locations": [
38+
{
39+
"key.line": 6,
40+
"key.column": 14,
41+
"key.nametype": source.syntacticrename.call
42+
}
43+
]
44+
}
45+
]

‎test/refactoring/SyntacticRename/Outputs/types/case-other.swift.expected

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ extension /*class-Animal:def*/Animal {
2727
enum /*enum-Barcode:def*/Barcode {
2828
case upc(Int, Int, Int, Int)
2929
case /*case-qrCode:def*/qrCode(code: String)
30-
case /*case-other:def*/<base>other</base>(<arglabel index=0></arglabel><param index=0></param>Int)
30+
case /*case-other:def*/<base>other</base>(<callcombo index=0></callcombo>Int)
3131
case /*case-another:def*/another
3232
}
3333
var barCode: /*enum-Barcode*/Barcode = /*enum-Barcode*/Barcode.upc(1, 1, 1, 1)

0 commit comments

Comments
 (0)