Skip to content

Commit 5c52400

Browse files
author
Nathan Hawes
authored
Merge pull request swiftlang#9211 from nathawes/migrator-type-change-fixes
[migrator] Don't walk into user-specified type aliases when applying API type changes
2 parents 155273a + 0d97d55 commit 5c52400

File tree

3 files changed

+80
-33
lines changed

3 files changed

+80
-33
lines changed

‎lib/Migrator/SyntacticMigratorPass.cpp

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ using namespace swift::ide::api;
3535

3636
struct FoundResult {
3737
SourceRange TokenRange;
38+
bool Optional; // Range has a trailing ? or ! included
3839
bool Suffixable; // No need to wrap parens when adding optionality
3940
bool isValid() const { return TokenRange.isValid(); }
4041
};
@@ -53,11 +54,12 @@ class ChildIndexFinder : public TypeReprVisitor<ChildIndexFinder, FoundResult> {
5354
return findChild(Func->getBodyResultTypeLoc());
5455
if (auto Init = dyn_cast<ConstructorDecl>(Parent)) {
5556
SourceLoc End = Init->getFailabilityLoc();
56-
if (End.isInvalid())
57+
bool Optional = End.isValid();
58+
if (!Optional)
5759
End = Init->getNameLoc();
58-
return {SourceRange(Init->getNameLoc(), End), true};
60+
return {SourceRange(Init->getNameLoc(), End), Optional, /*suffixable=*/true};
5961
}
60-
return {SourceRange(), false};
62+
return {SourceRange(), false, false};
6163
}
6264

6365
for (auto *Params: Parent->getParameterLists()) {
@@ -73,7 +75,7 @@ class ChildIndexFinder : public TypeReprVisitor<ChildIndexFinder, FoundResult> {
7375
}
7476

7577
private:
76-
bool hasNextIndex() {
78+
bool hasNextIndex() const {
7779
return !ChildIndices.empty();
7880
}
7981

@@ -83,44 +85,56 @@ class ChildIndexFinder : public TypeReprVisitor<ChildIndexFinder, FoundResult> {
8385
return Next;
8486
}
8587

88+
bool isUserTypeAlias(TypeRepr *T) const {
89+
if (auto Ident = dyn_cast<ComponentIdentTypeRepr>(T)) {
90+
if (auto Bound = Ident->getBoundDecl()) {
91+
return isa<TypeAliasDecl>(Bound) &&
92+
!Bound->getModuleContext()->isSystemModule();
93+
}
94+
}
95+
return false;
96+
}
97+
8698
FoundResult findChild(TypeLoc Loc) {
8799
if (!Loc.hasLocation())
88-
return {SourceRange(), false};
100+
return {SourceRange(), false, false};
89101
return visit(Loc.getTypeRepr());
90102
}
91103

92104
public:
93-
FoundResult handleParent(TypeRepr *Parent, TypeRepr *FirstChild,
94-
TypeRepr *SecondChild, bool Suffixable = true) {
95-
if (!hasNextIndex())
96-
return {Parent->getSourceRange(), Suffixable};
97-
auto NextIndex = consumeNext();
98-
assert(NextIndex < 2 && "child index out of bounds");
99-
return visit(NextIndex ? SecondChild : FirstChild);
100-
}
101105

102106
template<typename T>
103107
FoundResult handleParent(TypeRepr *Parent, const ArrayRef<T> Children,
104-
bool Suffixable = true) {
108+
bool Optional = false, bool Suffixable = true) {
105109
if (!hasNextIndex())
106-
return {Parent->getSourceRange(), Suffixable};
110+
return {Parent->getSourceRange(), Optional, Suffixable};
107111
auto NextIndex = consumeNext();
112+
if (isUserTypeAlias(Parent))
113+
return {SourceRange(), false, false};
108114
assert(NextIndex < Children.size());
109115
TypeRepr *Child = Children[NextIndex];
110116
return visit(Child);
111117
}
112118

113-
FoundResult handleParent(TypeRepr *Parent, TypeRepr *Base,
119+
FoundResult handleParent(TypeRepr *Parent, TypeRepr *FirstChild,
120+
TypeRepr *SecondChild, bool Optional = false,
114121
bool Suffixable = true) {
115-
return handleParent(Parent, llvm::makeArrayRef(Base), Suffixable);
122+
TypeRepr *Children[] = {FirstChild, SecondChild};
123+
return handleParent(Parent, llvm::makeArrayRef(Children), Optional,
124+
Suffixable);
125+
}
126+
127+
FoundResult handleParent(TypeRepr *Parent, TypeRepr *Base,
128+
bool Optional = false, bool Suffixable = true) {
129+
return handleParent(Parent, llvm::makeArrayRef(Base), Optional, Suffixable);
116130
}
117131

118132
FoundResult visitTypeRepr(TypeRepr *T) {
119133
llvm_unreachable("unexpected typerepr");
120134
}
121135

122136
FoundResult visitErrorTypeRepr(ErrorTypeRepr *T) {
123-
return {SourceRange(), false};
137+
return {SourceRange(), false, false};
124138
}
125139

126140
FoundResult visitAttributedTypeRepr(AttributedTypeRepr *T) {
@@ -149,36 +163,32 @@ class ChildIndexFinder : public TypeReprVisitor<ChildIndexFinder, FoundResult> {
149163

150164
FoundResult visitFunctionTypeRepr(FunctionTypeRepr *T) {
151165
return handleParent(T, T->getResultTypeRepr(), T->getArgsTypeRepr(),
152-
/*Suffixable=*/false);
166+
/*Optional=*/false, /*Suffixable=*/false);
153167
}
154168

155169
FoundResult visitCompositionTypeRepr(CompositionTypeRepr *T) {
156-
return handleParent(T, T->getTypes(), /*Suffixable=*/false);
170+
return handleParent(T, T->getTypes(), /*Optional=*/false,
171+
/*Suffixable=*/false);
157172
}
158173

159174
FoundResult visitSimpleIdentTypeRepr(SimpleIdentTypeRepr *T) {
160-
if (!hasNextIndex())
161-
return {T->getSourceRange(), true};
162-
// This may be a typealias so report no match
163-
return {SourceRange(), false};
175+
return handleParent(T, ArrayRef<TypeRepr*>());
164176
}
165177

166178
FoundResult visitGenericIdentTypeRepr(GenericIdentTypeRepr *T) {
167-
// FIXME: This could be a generic type alias
168179
return handleParent(T, T->getGenericArgs());
169180
}
170181

171182
FoundResult visitCompoundIdentTypeRepr(CompoundIdentTypeRepr *T) {
172-
// FIXME: this could be a nested typealias
173-
return handleParent(T, T->Components);
183+
return visit(T->Components.back());
174184
}
175185

176186
FoundResult visitOptionalTypeRepr(OptionalTypeRepr *T) {
177-
return handleParent(T, T->getBase());
187+
return handleParent(T, T->getBase(), /*Optional=*/true);
178188
}
179189

180190
FoundResult visitImplicitlyUnwrappedOptionalTypeRepr(ImplicitlyUnwrappedOptionalTypeRepr *T) {
181-
return handleParent(T, T->getBase());
191+
return handleParent(T, T->getBase(), /*Optional=*/true);
182192
}
183193

184194
FoundResult visitProtocolTypeRepr(ProtocolTypeRepr *T) {
@@ -190,8 +200,7 @@ class ChildIndexFinder : public TypeReprVisitor<ChildIndexFinder, FoundResult> {
190200
}
191201

192202
FoundResult visitFixedTypeRepr(FixedTypeRepr *T) {
193-
assert(!hasNextIndex());
194-
return {T->getSourceRange(), true};
203+
return handleParent(T, ArrayRef<TypeRepr*>());
195204
}
196205
};
197206

@@ -450,10 +459,12 @@ struct SyntacticMigratorPass::Implementation : public SourceEntityWalker {
450459
}
451460
break;
452461
case ide::api::NodeAnnotation::UnwrapOptional:
453-
Editor.remove(Result.TokenRange.End);
462+
if (Result.Optional)
463+
Editor.remove(Result.TokenRange.End);
454464
break;
455465
case ide::api::NodeAnnotation::ImplicitOptionalToOptional:
456-
Editor.replace(Result.TokenRange.End, "?");
466+
if (Result.Optional)
467+
Editor.replace(Result.TokenRange.End, "?");
457468
break;
458469
case ide::api::NodeAnnotation::TypeRewritten:
459470
Editor.replace(Result.TokenRange, DiffItem->RightComment);

‎test/Migrator/wrap_optional.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,21 @@ class MyExtraCities : ExtraCities {
3333
func blibli(x: (String?, String) -> String!) {}
3434
func currimundi(x: (Int, (Int, Int))!) {}
3535
}
36+
37+
typealias IntAnd<T> = (Int, T)
38+
class Outer {
39+
typealias Inner = (String?, String) -> String!
40+
}
41+
42+
class MyExtraCitiesWithAliases : ExtraCities {
43+
func blibli(x: Outer.Inner) {}
44+
func currimundi(x: (Int, IntAnd<Int>)!) {}
45+
}
46+
47+
typealias OptString = String?
48+
typealias ImplicitlyUnwrapped<T> = T!
49+
50+
class MyExtraCitiesWithMoreAliases : ExtraCities {
51+
func blibli(x: (OptString, String) -> String!) {}
52+
func currimundi(x: ImplicitlyUnwrapped<(Int, (Int, Int))>) {}
53+
}

‎test/Migrator/wrap_optional.swift.expected

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,21 @@ class MyExtraCities : ExtraCities {
3333
func blibli(x: ((String, String) -> String?)?) {}
3434
func currimundi(x: (Int, (Int?, Int))?) {}
3535
}
36+
37+
typealias IntAnd<T> = (Int, T)
38+
class Outer {
39+
typealias Inner = (String?, String) -> String!
40+
}
41+
42+
class MyExtraCitiesWithAliases : ExtraCities {
43+
func blibli(x: Outer.Inner?) {}
44+
func currimundi(x: (Int, IntAnd<Int>)?) {}
45+
}
46+
47+
typealias OptString = String?
48+
typealias ImplicitlyUnwrapped<T> = T!
49+
50+
class MyExtraCitiesWithMoreAliases : ExtraCities {
51+
func blibli(x: ((OptString, String) -> String?)?) {}
52+
func currimundi(x: ImplicitlyUnwrapped<(Int, (Int, Int))>) {}
53+
}

0 commit comments

Comments
 (0)