Skip to content

Commit cd87f41

Browse files
committed
swift-api-digester: add specific logic to detect optional dictionary's key changes.
We've also seen type changes in the frameworks from "[String: Any]?" to "[StringRepresentable: Any]?". This patch adds specific logic and attribute for this kind of change on the top of nonnull dictionary changes.
1 parent 924767d commit cd87f41

File tree

7 files changed

+88
-13
lines changed

7 files changed

+88
-13
lines changed

include/swift/IDE/DigesterEnums.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ NODE_ANNOTATION(NowMutating)
7575
NODE_ANNOTATION(StaticChange)
7676
NODE_ANNOTATION(OwnershipChange)
7777
NODE_ANNOTATION(DictionaryKeyUpdate)
78+
NODE_ANNOTATION(OptionalDictionaryKeyUpdate)
7879

7980
DECL_ATTR(deprecated)
8081
DECL_ATTR(fixedLayout)

test/api-digester/Inputs/APINotesLeft/APINotesTest.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ extern int ANTGlobalValue;
2020

2121
@interface AnimalStatusDescriptor
2222
- (nonnull AnimalStatusDescriptor *)animalStatusDescriptorByAddingAttributes:(nonnull NSDictionary<NSString*, id> *)attributes;
23+
- (nonnull AnimalStatusDescriptor *)animalStatusDescriptorByAddingOptionalAttributes:(nullable NSDictionary<NSString*, id> *)attributes;
2324
@end

test/api-digester/Inputs/APINotesRight/APINotesTest.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ typedef NSString * AnimalAttributeName NS_STRING_ENUM;
2020

2121
@interface AnimalStatusDescriptor
2222
- (nonnull AnimalStatusDescriptor *)animalStatusDescriptorByAddingAttributes:(nonnull NSDictionary<AnimalAttributeName, id> *)attributes;
23+
- (nonnull AnimalStatusDescriptor *)animalStatusDescriptorByAddingOptionalAttributes:(nullable NSDictionary<AnimalAttributeName, id> *)attributes;
2324
@end

test/api-digester/Outputs/apinotes-diags-3-4.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@ APINotesTest(APINotesTest.h): TypeAlias AnimalAttributeName(NSString) is now Str
1010

1111
/* Type Changes */
1212
APINotesTest(APINotesTest.h): Func AnimalStatusDescriptor.addingAttributes(_:) has parameter 0 type change from [String : Any] to [AnimalAttributeName : Any]
13+
APINotesTest(APINotesTest.h): Func AnimalStatusDescriptor.addingOptionalAttributes(_:) has parameter 0 type change from [String : Any]? to [AnimalAttributeName : Any]?
1314

1415
/* Decl Attribute changes */

test/api-digester/Outputs/apinotes-diags.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,6 @@ APINotesTest(APINotesTest.h): Var OldType.oldMember has been renamed to Var NewT
1414

1515
/* Type Changes */
1616
APINotesTest(APINotesTest.h): Func AnimalStatusDescriptor.addingAttributes(_:) has parameter 0 type change from [String : Any] to [AnimalAttributeName : Any]
17+
APINotesTest(APINotesTest.h): Func AnimalStatusDescriptor.addingOptionalAttributes(_:) has parameter 0 type change from [String : Any]? to [AnimalAttributeName : Any]?
1718

1819
/* Decl Attribute changes */

test/api-digester/Outputs/apinotes-migrator-gen.json

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,50 @@
4343
"RightComment": "AnimalAttributeName",
4444
"ModuleName": "APINotesTest"
4545
},
46+
{
47+
"DiffItemKind": "CommonDiffItem",
48+
"NodeKind": "Function",
49+
"NodeAnnotation": "TypeRewritten",
50+
"ChildIndex": "1:0:0",
51+
"LeftUsr": "c:objc(cs)AnimalStatusDescriptor(im)animalStatusDescriptorByAddingOptionalAttributes:",
52+
"LeftComment": "String",
53+
"RightUsr": "",
54+
"RightComment": "AnimalAttributeName",
55+
"ModuleName": "APINotesTest"
56+
},
57+
{
58+
"DiffItemKind": "CommonDiffItem",
59+
"NodeKind": "Function",
60+
"NodeAnnotation": "OptionalDictionaryKeyUpdate",
61+
"ChildIndex": "1",
62+
"LeftUsr": "c:objc(cs)AnimalStatusDescriptor(im)animalStatusDescriptorByAddingOptionalAttributes:",
63+
"LeftComment": "",
64+
"RightUsr": "",
65+
"RightComment": "AnimalAttributeName",
66+
"ModuleName": "APINotesTest"
67+
},
68+
{
69+
"DiffItemKind": "CommonDiffItem",
70+
"NodeKind": "Function",
71+
"NodeAnnotation": "TypeRewritten",
72+
"ChildIndex": "1:0:0",
73+
"LeftUsr": "c:objc(cs)AnimalStatusDescriptor(im)animalStatusDescriptorByAddingOptionalAttributes:",
74+
"LeftComment": "String",
75+
"RightUsr": "",
76+
"RightComment": "AnimalAttributeName",
77+
"ModuleName": "APINotesTest"
78+
},
79+
{
80+
"DiffItemKind": "CommonDiffItem",
81+
"NodeKind": "Function",
82+
"NodeAnnotation": "OptionalDictionaryKeyUpdate",
83+
"ChildIndex": "1",
84+
"LeftUsr": "c:objc(cs)AnimalStatusDescriptor(im)animalStatusDescriptorByAddingOptionalAttributes:",
85+
"LeftComment": "",
86+
"RightUsr": "",
87+
"RightComment": "AnimalAttributeName",
88+
"ModuleName": "APINotesTest"
89+
},
4690
{
4791
"DiffItemKind": "CommonDiffItem",
4892
"NodeKind": "Function",

tools/swift-api-digester/swift-api-digester.cpp

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ class SDKNode {
409409
ArrayRef<SDKNode*> getChildren() const;
410410
bool hasSameChildren(const SDKNode &Other) const;
411411
unsigned getChildIndex(NodePtr Child) const;
412-
const SDKNode* getOnlyChild() const;
412+
SDKNode* getOnlyChild() const;
413413
SDKContext &getSDKContext() const { return Ctx; }
414414
SDKNodeRoot *getRootNode() const;
415415
template <typename T> const T *getAs() const;
@@ -575,7 +575,7 @@ unsigned SDKNode::getChildIndex(NodePtr Child) const {
575575
return std::find(Children.begin(), Children.end(), Child) - Children.begin();
576576
}
577577

578-
const SDKNode* SDKNode::getOnlyChild() const {
578+
SDKNode* SDKNode::getOnlyChild() const {
579579
assert(Children.size() == 1 && "more that one child.");
580580
return *Children.begin();
581581
}
@@ -2693,12 +2693,11 @@ class ChangeRefinementPass : public SDKTreeDiffPass, public SDKNodeVisitor {
26932693
return false;
26942694
}
26952695

2696-
bool detectDictionaryKeyChange(SDKNodeType *L, SDKNodeType *R) {
2697-
if (!IsVisitingLeft)
2698-
return false;
2696+
static StringRef detectDictionaryKeyChangeInternal(SDKNodeType *L,
2697+
SDKNodeType *R) {
26992698
if (L->getTypeKind() != KnownTypeKind::Dictionary ||
27002699
R->getTypeKind() != KnownTypeKind::Dictionary)
2701-
return false;
2700+
return StringRef();
27022701
auto *Left = dyn_cast<SDKNodeTypeNominal>(L);
27032702
auto *Right = dyn_cast<SDKNodeTypeNominal>(R);
27042703
assert(Left && Right);
@@ -2707,20 +2706,45 @@ class ChangeRefinementPass : public SDKTreeDiffPass, public SDKNodeVisitor {
27072706
auto* LKey = dyn_cast<SDKNodeTypeNominal>(*Left->getChildBegin());
27082707
auto* RKey = dyn_cast<SDKNodeTypeNominal>(*Right->getChildBegin());
27092708
if (!LKey || !RKey)
2710-
return false;
2709+
return StringRef();
27112710
if (LKey->getTypeKind() != KnownTypeKind::String)
2712-
return false;
2711+
return StringRef();
27132712
auto Results = RKey->getRootNode()->getDescendantsByUsr(RKey->getUsr());
27142713
if (Results.empty())
2715-
return false;
2714+
return StringRef();
27162715
if (auto DT = dyn_cast<SDKNodeDeclType>(Results.front())) {
27172716
if (DT->isConformingTo(KnownProtocolKind::RawRepresentable)) {
2718-
L->annotate(NodeAnnotation::DictionaryKeyUpdate);
2719-
L->annotate(NodeAnnotation::TypeRewrittenRight,
2720-
DT->getFullyQualifiedName());
2721-
return true;
2717+
return DT->getFullyQualifiedName();
27222718
}
27232719
}
2720+
return StringRef();
2721+
}
2722+
2723+
bool detectDictionaryKeyChange(SDKNodeType *L, SDKNodeType *R) {
2724+
if (!IsVisitingLeft)
2725+
return false;
2726+
2727+
// We only care if this the top-level type node.
2728+
if (!isa<SDKNodeDecl>(L->getParent()) || !isa<SDKNodeDecl>(R->getParent()))
2729+
return false;
2730+
StringRef KeyChangedTo;
2731+
if (L->getTypeKind() == KnownTypeKind::Optional &&
2732+
R->getTypeKind() == KnownTypeKind::Optional) {
2733+
// Detect [String: Any]? to [StringRepresentableStruct: Any]? Chnage
2734+
KeyChangedTo =
2735+
detectDictionaryKeyChangeInternal(L->getOnlyChild()->getAs<SDKNodeType>(),
2736+
R->getOnlyChild()->getAs<SDKNodeType>());
2737+
} else {
2738+
// Detect [String: Any] to [StringRepresentableStruct: Any] Chnage
2739+
KeyChangedTo = detectDictionaryKeyChangeInternal(L, R);
2740+
}
2741+
if (!KeyChangedTo.empty()) {
2742+
L->annotate(L->getTypeKind() == KnownTypeKind::Optional ?
2743+
NodeAnnotation::OptionalDictionaryKeyUpdate :
2744+
NodeAnnotation::DictionaryKeyUpdate);
2745+
L->annotate(NodeAnnotation::TypeRewrittenRight, KeyChangedTo);
2746+
return true;
2747+
}
27242748
return false;
27252749
}
27262750

@@ -2864,6 +2888,7 @@ class DiffItemEmitter : public SDKNodeVisitor {
28642888
static StringRef getRightComment(NodePtr Node, NodeAnnotation Anno) {
28652889
switch (Anno) {
28662890
case NodeAnnotation::DictionaryKeyUpdate:
2891+
case NodeAnnotation::OptionalDictionaryKeyUpdate:
28672892
case NodeAnnotation::TypeRewritten:
28682893
return Node->getAnnotateComment(NodeAnnotation::TypeRewrittenRight);
28692894
case NodeAnnotation::ModernizeEnum:
@@ -2922,6 +2947,7 @@ class DiffItemEmitter : public SDKNodeVisitor {
29222947
NodeAnnotation::Rename,
29232948
NodeAnnotation::NowThrowing,
29242949
NodeAnnotation::DictionaryKeyUpdate,
2950+
NodeAnnotation::OptionalDictionaryKeyUpdate,
29252951
});
29262952
}
29272953

0 commit comments

Comments
 (0)