Skip to content

Commit 859d918

Browse files
committed
Re-apply "[migrator] migrator: avoid inserting base name while renaming
if users' member access doesn't specify the base name. rdar://40373279" Previously, we saw that unconditionally omitting type names can lead to build errors (rdar://40458118). This revised fix omits type names only when the new member name is identical to the old one.
1 parent a378536 commit 859d918

File tree

5 files changed

+48
-9
lines changed

5 files changed

+48
-9
lines changed

lib/Migrator/APIDiffMigratorPass.cpp

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,21 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
273273
SF->getASTContext().SourceMgr, Range).str() == "nil";
274274
}
275275

276+
bool isDotMember(CharSourceRange Range) {
277+
auto S = Range.str();
278+
return S.startswith(".") && S.substr(1).find(".") == StringRef::npos;
279+
}
280+
281+
bool isDotMember(SourceRange Range) {
282+
return isDotMember(Lexer::getCharSourceRangeFromSourceRange(
283+
SF->getASTContext().SourceMgr, Range));
284+
}
285+
286+
bool isDotMember(Expr *E) {
287+
auto Range = E->getSourceRange();
288+
return Range.isValid() && isDotMember(Range);
289+
}
290+
276291
std::vector<APIDiffItem*> getRelatedDiffItems(ValueDecl *VD) {
277292
std::vector<APIDiffItem*> results;
278293
if (!VD)
@@ -323,11 +338,13 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
323338
}
324339

325340

326-
bool isSimpleReplacement(APIDiffItem *Item, std::string &Text) {
341+
bool isSimpleReplacement(APIDiffItem *Item, bool isDotMember, std::string &Text) {
327342
if (auto *MD = dyn_cast<TypeMemberDiffItem>(Item)) {
328343
if (MD->Subkind == TypeMemberDiffItemSubKind::SimpleReplacement) {
329-
Text = (llvm::Twine(MD->newTypeName) + "." + MD->getNewName().base()).
330-
str();
344+
bool NeedNoTypeName = isDotMember &&
345+
MD->oldPrintedName == MD->newPrintedName;
346+
Text = (llvm::Twine(NeedNoTypeName ? "" : MD->newTypeName) + "." +
347+
MD->getNewName().base()).str();
331348
return true;
332349
}
333350
}
@@ -423,7 +440,7 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
423440
Type T, ReferenceMetaData Data) override {
424441
for (auto *Item: getRelatedDiffItems(CtorTyRef ? CtorTyRef: D)) {
425442
std::string RepText;
426-
if (isSimpleReplacement(Item, RepText)) {
443+
if (isSimpleReplacement(Item, isDotMember(Range), RepText)) {
427444
Editor.replace(Range, RepText);
428445
return true;
429446
}
@@ -498,8 +515,11 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
498515
for (auto *I: getRelatedDiffItems(VD)) {
499516
if (auto *Item = dyn_cast<TypeMemberDiffItem>(I)) {
500517
if (Item->Subkind == TypeMemberDiffItemSubKind::QualifiedReplacement) {
501-
Editor.replace(ToReplace, (llvm::Twine(Item->newTypeName) + "." +
502-
Item->getNewName().base()).str());
518+
bool NeedNoTypeName = isDotMember(ToReplace) &&
519+
Item->oldPrintedName == Item->newPrintedName;
520+
Editor.replace(ToReplace,
521+
(llvm::Twine(NeedNoTypeName ? "" : Item->newTypeName) + "." +
522+
Item->getNewName().base()).str());
503523
return true;
504524
}
505525
}
@@ -773,7 +793,7 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
773793
StringRef LeftComment;
774794
StringRef RightComment;
775795
for (auto *Item: getRelatedDiffItems(RD)) {
776-
if (isSimpleReplacement(Item, Rename)) {
796+
if (isSimpleReplacement(Item, isDotMember(Reference), Rename)) {
777797
} else if (auto *CI = dyn_cast<CommonDiffItem>(Item)) {
778798
if (CI->isStringRepresentableChange() &&
779799
CI->NodeKind == SDKNodeKind::DeclVar) {

test/Migrator/Inputs/qualified.json

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,13 @@
3838
"OldTypeName": "",
3939
"NewPrintedName": "internalType",
4040
"NewTypeName": "ToplevelWrapper"
41-
}
41+
},
42+
{
43+
"DiffItemKind": "TypeMemberDiffItem",
44+
"Usr": "c:@E@FooComparisonResult@FooOrderedMemberSame",
45+
"OldPrintedName": "orderedMemberSame",
46+
"OldTypeName": "FooComparisonResult",
47+
"NewPrintedName": "orderedMemberSame",
48+
"NewTypeName": "NewFooComparisonResult.Nested"
49+
},
4250
]

test/Migrator/mock-sdk/Bar.framework/Headers/Bar.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ typedef SomeItemSet SomeEnvironment;
5353
typedef NS_ENUM(long, FooComparisonResult) {
5454
FooOrderedAscending = -1L,
5555
FooOrderedSame,
56-
FooOrderedDescending
56+
FooOrderedDescending,
57+
FooOrderedMemberSame,
5758
};
5859

5960
@interface BarBase

test/Migrator/qualified-replacement.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ func foo() {
1414
_ = Cities.CityKind.Town
1515
_ = ToplevelType()
1616
_ = ToplevelType(recordName: "")
17+
bar(.orderedSame)
18+
bar(.orderedMemberSame)
19+
bar(FooComparisonResult.orderedSame)
20+
bar(FooComparisonResult.orderedMemberSame)
1721
}
1822

1923
func foo(_: ToplevelType) {}
24+
func bar(_ : FooComparisonResult) {}

test/Migrator/qualified-replacement.swift.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ func foo() {
1414
_ = NewCityKind.NewTown
1515
_ = ToplevelWrapper.internalType()
1616
_ = ToplevelWrapper.internalType(recordName: "")
17+
bar(NewFooComparisonResult.NewFooOrderedSame)
18+
bar(.orderedMemberSame)
19+
bar(NewFooComparisonResult.NewFooOrderedSame)
20+
bar(NewFooComparisonResult.Nested.orderedMemberSame)
1721
}
1822

1923
func foo(_: ToplevelWrapper.internalType) {}
24+
func bar(_ : FooComparisonResult) {}

0 commit comments

Comments
 (0)