Skip to content

Commit 99a3a8b

Browse files
committed
swift-api-digester: refine diagnostic messages for removed type alias.
Framework authors can use SwiftWraper:none to bring back string enums to type alias of String. When diagnosing source breaking changes, these type alias are shown as removed. Therefore, it's hard to tell whether these changes are automatically migratable. This patch refines the removed-type-alias by further analyzing whether a RawRepresentable with the same usr appeared in the later version of SDK. If there is, another kind of message is emitted for differentiation.
1 parent 7c82fe0 commit 99a3a8b

File tree

7 files changed

+151
-13
lines changed

7 files changed

+151
-13
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,10 @@ Globals:
55
Protocols:
66
- Name: TypeWithMethod
77
SwiftName: SwiftTypeWithMethodRight
8+
9+
SwiftVersions:
10+
- Version: 3
11+
Typedefs:
12+
- Name: AnimalAttributeName
13+
SwiftName: AnimalAttributeName
14+
SwiftWrapper: none

test/api-digester/Outputs/Cake.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11

2+
/* RawRepresentable Changes */
3+
24
/* Removed Decls */
35
cake1: Constructor Somestruct2.init(_:) has been removed
46
cake1: Func C4.foo() has been removed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
/* RawRepresentable Changes */
3+
APINotesTest(APINotesTest.h): TypeAlias AnimalAttributeName(NSString) is now String representable
4+
5+
/* Removed Decls */
6+
7+
/* Moved Decls */
8+
9+
/* Renamed Decls */
10+
11+
/* Type Changes */
12+
APINotesTest(APINotesTest.h): Func AnimalStatusDescriptor.addingAttributes(_:) has parameter 0 type change from [String : Any] to [AnimalAttributeName : Any]
13+
14+
/* Decl Attribute changes */

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11

2+
/* RawRepresentable Changes */
3+
24
/* Removed Decls */
35
APINotesTest(APINotesTest.h): Func ObjcProt.protMemberFunc2() has been removed
46
APINotesTest(APINotesTest.h): Func ObjcProt.protMemberFunc3() has been removed

test/api-digester/apinotes-diags.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
// RUN: %empty-directory(%t.mod)
33
// RUN: %empty-directory(%t.sdk)
44
// RUN: %empty-directory(%t.module-cache)
5-
// RUN: %api-digester %clang-importer-sdk-nosource -dump-sdk -module APINotesTest -o %t.dump1.json -module-cache-path %t.module-cache -swift-version 3 -I %S/Inputs/APINotesLeft
5+
// RUN: %api-digester %clang-importer-sdk-nosource -dump-sdk -module APINotesTest -o %t.dump1.json -module-cache-path %t.module-cache -swift-version 4 -I %S/Inputs/APINotesLeft
66
// RUN: %api-digester %clang-importer-sdk-nosource -dump-sdk -module APINotesTest -o %t.dump2.json -module-cache-path %t.module-cache -swift-version 3 -I %S/Inputs/APINotesRight
7-
// RUN: %api-digester -diagnose-sdk -print-module -input-paths %t.dump1.json -input-paths %t.dump2.json > %t.result
7+
// RUN: %api-digester %clang-importer-sdk-nosource -dump-sdk -module APINotesTest -o %t.dump3.json -module-cache-path %t.module-cache -swift-version 4 -I %S/Inputs/APINotesRight
8+
// RUN: %api-digester -diagnose-sdk -print-module -input-paths %t.dump1.json -input-paths %t.dump3.json > %t.result
89
// RUN: diff -u %S/Outputs/apinotes-diags.txt %t.result
10+
// RUN: %api-digester -diagnose-sdk -print-module -input-paths %t.dump2.json -input-paths %t.dump3.json > %t.result
11+
// RUN: diff -u %S/Outputs/apinotes-diags-3-4.txt %t.result

test/api-digester/apinotes-migrator-gen.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// RUN: %empty-directory(%t.mod)
33
// RUN: %empty-directory(%t.sdk)
44
// RUN: %empty-directory(%t.module-cache)
5-
// RUN: %api-digester -dump-sdk -module APINotesTest -o %t.dump1.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -swift-version 3 -I %S/Inputs/APINotesLeft
6-
// RUN: %api-digester -dump-sdk -module APINotesTest -o %t.dump2.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -swift-version 3 -I %S/Inputs/APINotesRight
5+
// RUN: %api-digester -dump-sdk -module APINotesTest -o %t.dump1.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -swift-version 4 -I %S/Inputs/APINotesLeft
6+
// RUN: %api-digester -dump-sdk -module APINotesTest -o %t.dump2.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -swift-version 4 -I %S/Inputs/APINotesRight
77
// RUN: %api-digester -compare-sdk --input-paths %t.dump1.json -input-paths %t.dump2.json -o %t.result -json
88
// RUN: diff -u %S/Outputs/apinotes-migrator-gen.json %t.result

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

Lines changed: 119 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ bool contains(ArrayRef<T> container, T instance) {
179179
class SDKNode;
180180
typedef SDKNode* NodePtr;
181181
typedef std::map<NodePtr, NodePtr> ParentMap;
182+
typedef std::map<NodePtr, NodePtr> NodeMap;
182183
typedef std::vector<NodePtr> NodeVector;
183184

184185
// The interface used to visit the SDK tree.
@@ -231,6 +232,7 @@ class SDKContext {
231232
llvm::StringSet<> TextData;
232233
llvm::BumpPtrAllocator Allocator;
233234
UpdatedNodesMap UpdateMap;
235+
NodeMap TypeAliasUpdateMap;
234236

235237
public:
236238
llvm::BumpPtrAllocator &allocator() {
@@ -242,6 +244,10 @@ class SDKContext {
242244
UpdatedNodesMap &getNodeUpdateMap() {
243245
return UpdateMap;
244246
}
247+
NodeMap &getTypeAliasUpdateMap() {
248+
return TypeAliasUpdateMap;
249+
}
250+
245251
};
246252

247253
// A node matcher will traverse two trees of SDKNode and find matched nodes
@@ -838,6 +844,15 @@ class SDKNodeTypeDecl : public SDKNodeDecl {
838844
return None;
839845
}
840846

847+
SDKNodeType *getRawValueType() const {
848+
if (isConformingTo(KnownProtocolKind::RawRepresentable)) {
849+
if (auto RV = lookupChildByPrintedName("rawValue")) {
850+
return (*(*RV)->getChildBegin())->getAs<SDKNodeType>();
851+
}
852+
}
853+
return nullptr;
854+
}
855+
841856
bool isConformingTo(KnownProtocolKind Kind) const {
842857
StringRef Usr;
843858
switch (Kind) {
@@ -855,6 +870,9 @@ class SDKNodeTypeAlias : public SDKNodeDecl {
855870
public:
856871
SDKNodeTypeAlias(SDKNodeInitInfo Info) : SDKNodeDecl(Info,
857872
SDKNodeKind::TypeAlias) {}
873+
const SDKNodeType* getUnderlyingType() const {
874+
return getOnlyChild()->getAs<SDKNodeType>();
875+
}
858876
static bool classof(const SDKNode *N);
859877
};
860878

@@ -1592,8 +1610,8 @@ class SwiftDeclCollector : public VisibleDeclConsumer {
15921610

15931611
// After collecting decls, either from imported modules or from a previously
15941612
// serialized JSON file, using this function to get the root of the SDK.
1595-
NodePtr getSDKRoot() {
1596-
return RootNode;
1613+
SDKNodeRoot* getSDKRoot() {
1614+
return static_cast<SDKNodeRoot*>(RootNode);
15971615
}
15981616

15991617
void printTopLevelNames() {
@@ -2517,6 +2535,57 @@ class TypeMemberDiffFinder : public SDKNodeVisitor {
25172535

25182536
};
25192537

2538+
/// This is to find type alias of raw types being changed to RawRepresentable.
2539+
/// e.g. AttributeName was a typealias of String in the old SDK however it becomes
2540+
/// a RawRepresentable struct in the new SDK.
2541+
/// This happens typically when we use apinotes to preserve API stability by
2542+
/// using SwiftWrapper:none in the old SDK.
2543+
class TypeAliasDiffFinder: public SDKNodeVisitor {
2544+
SDKNodeRoot *leftRoot;
2545+
SDKNodeRoot *rightRoot;
2546+
NodeMap &result;
2547+
2548+
static bool checkTypeMatch(const SDKNodeType* aliasType,
2549+
const SDKNodeType* rawType) {
2550+
StringRef Left = aliasType->getPrintedName();
2551+
StringRef Right = rawType->getPrintedName();
2552+
if (Left == "NSString" && Right == "String")
2553+
return true;
2554+
if (Left == "String" && Right == "String")
2555+
return true;
2556+
if (Left == "Int" && Right == "Int")
2557+
return true;
2558+
if (Left == "UInt" && Right == "UInt")
2559+
return true;
2560+
return false;
2561+
}
2562+
2563+
void visit(NodePtr node) override {
2564+
auto alias = dyn_cast<SDKNodeTypeAlias>(node);
2565+
if (!alias)
2566+
return;
2567+
const SDKNodeType* aliasType = alias->getUnderlyingType();
2568+
for (auto *counter: rightRoot->getDescendantsByUsr(alias->getUsr())) {
2569+
if (auto DT = dyn_cast<SDKNodeTypeDecl>(counter)) {
2570+
if (auto *rawType = DT->getRawValueType()) {
2571+
if (checkTypeMatch(aliasType, rawType)) {
2572+
result.insert({alias, DT});
2573+
return;
2574+
}
2575+
}
2576+
}
2577+
}
2578+
}
2579+
public:
2580+
TypeAliasDiffFinder(SDKNodeRoot *leftRoot, SDKNodeRoot *rightRoot,
2581+
NodeMap &result): leftRoot(leftRoot), rightRoot(rightRoot),
2582+
result(result) {}
2583+
2584+
void search() {
2585+
SDKNode::preorderVisit(leftRoot, *this);
2586+
}
2587+
};
2588+
25202589
// Given a condition, search whether a node satisfies that condition exists
25212590
// in a tree.
25222591
class SearchVisitor : public SDKNodeVisitor {
@@ -2999,21 +3068,47 @@ class DiagnosisEmitter : public SDKNodeVisitor {
29993068
static void theme(raw_ostream &OS) { OS << "Type Changes"; };
30003069
};
30013070

3071+
struct RawRepresentableChangeDiag: public DiagBase {
3072+
DeclKind Kind;
3073+
StringRef DeclName;
3074+
StringRef UnderlyingType;
3075+
StringRef RawTypeName;
3076+
RawRepresentableChangeDiag(MetaInfo Info, DeclKind Kind, StringRef DeclName,
3077+
StringRef UnderlyingType, StringRef RawTypeName): DiagBase(Info),
3078+
Kind(Kind), DeclName(DeclName), UnderlyingType(UnderlyingType),
3079+
RawTypeName(RawTypeName) {}
3080+
bool operator<(RawRepresentableChangeDiag Other) const {
3081+
if (Kind != Other.Kind)
3082+
return Kind < Other.Kind;
3083+
return DeclName.compare(Other.DeclName) < 0;
3084+
}
3085+
void output() const override {
3086+
llvm::outs() << Kind << " " << printName(DeclName)
3087+
<< "(" << UnderlyingType << ")"
3088+
<< " is now " << RawTypeName << " representable\n";
3089+
}
3090+
static void theme(raw_ostream &OS) { OS << "RawRepresentable Changes"; };
3091+
};
3092+
30023093
std::set<SDKNodeDecl*> AddedDecls;
30033094
DiagBag<DeclAttrDiag> AttrChangedDecls;
30043095
DiagBag<DeclTypeChangeDiag> TypeChangedDecls;
30053096
DiagBag<RenamedDeclDiag> RenamedDecls;
30063097
DiagBag<MovedDeclDiag> MovedDecls;
30073098
DiagBag<RemovedDeclDiag> RemovedDecls;
3099+
DiagBag<RawRepresentableChangeDiag> RawRepresentableDecls;
30083100

30093101
UpdatedNodesMap &UpdateMap;
3102+
NodeMap &TypeAliasUpdateMap;
30103103
TypeMemberDiffVector &MemberChanges;
3011-
DiagnosisEmitter(UpdatedNodesMap &UpdateMap,
3012-
TypeMemberDiffVector &MemberChanges):
3013-
UpdateMap(UpdateMap), MemberChanges(MemberChanges) {}
3104+
DiagnosisEmitter(SDKContext &Ctx, TypeMemberDiffVector &MemberChanges):
3105+
UpdateMap(Ctx.getNodeUpdateMap()),
3106+
TypeAliasUpdateMap(Ctx.getTypeAliasUpdateMap()),
3107+
MemberChanges(MemberChanges){}
3108+
30143109
public:
30153110
static void diagnosis(NodePtr LeftRoot, NodePtr RightRoot,
3016-
UpdatedNodesMap &UpdateMap);
3111+
SDKContext &Ctx);
30173112
};
30183113

30193114
void DiagnosisEmitter::collectAddedDecls(NodePtr Root,
@@ -3130,11 +3225,11 @@ void DiagnosisEmitter::DeclAttrDiag::output() const {
31303225
}
31313226

31323227
void DiagnosisEmitter::diagnosis(NodePtr LeftRoot, NodePtr RightRoot,
3133-
UpdatedNodesMap &UpdateMap) {
3228+
SDKContext &Ctx) {
31343229
// Find member hoist changes to help refine diagnostics.
31353230
TypeMemberDiffVector MemberChanges;
31363231
findTypeMemberDiffs(LeftRoot, RightRoot, MemberChanges);
3137-
DiagnosisEmitter Emitter(UpdateMap, MemberChanges);
3232+
DiagnosisEmitter Emitter(Ctx, MemberChanges);
31383233
collectAddedDecls(RightRoot, Emitter.AddedDecls);
31393234
SDKNode::postorderVisit(LeftRoot, Emitter);
31403235
}
@@ -3171,6 +3266,19 @@ void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
31713266
return;
31723267
}
31733268

3269+
// If a type alias of a raw type has been changed to a struct/enum that
3270+
// conforms to RawRepresentable in the later version of SDK, we show the
3271+
// refine diagnostics message instead of showing the type alias has been
3272+
// removed.
3273+
if (TypeAliasUpdateMap.find((SDKNode*)Node) != TypeAliasUpdateMap.end()) {
3274+
RawRepresentableDecls.Diags.emplace_back(ScreenInfo, Node->getDeclKind(),
3275+
Node->getFullyQualifiedName(),
3276+
Node->getAs<SDKNodeTypeAlias>()->getUnderlyingType()->getPrintedName(),
3277+
TypeAliasUpdateMap[(SDKNode*)Node]->getAs<SDKNodeTypeDecl>()->
3278+
getRawValueType()->getPrintedName());
3279+
return;
3280+
}
3281+
31743282
// We should exlude those declarations that are pulled up to the super classes.
31753283
bool FoundInSuperclass = false;
31763284
if (auto PD = dyn_cast<SDKNodeDecl>(Node->getParent())) {
@@ -3438,11 +3546,13 @@ static int diagnoseModuleChange(StringRef LeftPath, StringRef RightPath) {
34383546
RightCollector.deSerialize(RightPath);
34393547
auto LeftModule = LeftCollector.getSDKRoot();
34403548
auto RightModule = RightCollector.getSDKRoot();
3549+
TypeAliasDiffFinder(LeftModule, RightModule,
3550+
Ctx.getTypeAliasUpdateMap()).search();
34413551
PrunePass Prune(Ctx.getNodeUpdateMap());
34423552
Prune.pass(LeftModule, RightModule);
34433553
ChangeRefinementPass RefinementPass(Ctx.getNodeUpdateMap());
34443554
RefinementPass.pass(LeftModule, RightModule);
3445-
DiagnosisEmitter::diagnosis(LeftModule, RightModule, Ctx.getNodeUpdateMap());
3555+
DiagnosisEmitter::diagnosis(LeftModule, RightModule, Ctx);
34463556
return 0;
34473557
}
34483558

0 commit comments

Comments
 (0)