Skip to content

swift-api-digester: refine diagnostic messages for removed type alias. #15755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions test/api-digester/Inputs/APINotesRight/APINotesTest.apinotes
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,10 @@ Globals:
Protocols:
- Name: TypeWithMethod
SwiftName: SwiftTypeWithMethodRight

SwiftVersions:
- Version: 3
Typedefs:
- Name: AnimalAttributeName
SwiftName: AnimalAttributeName
SwiftWrapper: none
2 changes: 2 additions & 0 deletions test/api-digester/Outputs/Cake.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@

/* RawRepresentable Changes */

/* Removed Decls */
cake1: Constructor Somestruct2.init(_:) has been removed
cake1: Func C4.foo() has been removed
Expand Down
14 changes: 14 additions & 0 deletions test/api-digester/Outputs/apinotes-diags-3-4.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

/* RawRepresentable Changes */
APINotesTest(APINotesTest.h): TypeAlias AnimalAttributeName(NSString) is now String representable

/* Removed Decls */

/* Moved Decls */

/* Renamed Decls */

/* Type Changes */
APINotesTest(APINotesTest.h): Func AnimalStatusDescriptor.addingAttributes(_:) has parameter 0 type change from [String : Any] to [AnimalAttributeName : Any]

/* Decl Attribute changes */
2 changes: 2 additions & 0 deletions test/api-digester/Outputs/apinotes-diags.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@

/* RawRepresentable Changes */

/* Removed Decls */
APINotesTest(APINotesTest.h): Func ObjcProt.protMemberFunc2() has been removed
APINotesTest(APINotesTest.h): Func ObjcProt.protMemberFunc3() has been removed
Expand Down
7 changes: 5 additions & 2 deletions test/api-digester/apinotes-diags.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
// RUN: %empty-directory(%t.mod)
// RUN: %empty-directory(%t.sdk)
// RUN: %empty-directory(%t.module-cache)
// 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
// 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
// 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
// RUN: %api-digester -diagnose-sdk -print-module -input-paths %t.dump1.json -input-paths %t.dump2.json > %t.result
// 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
// RUN: %api-digester -diagnose-sdk -print-module -input-paths %t.dump1.json -input-paths %t.dump3.json > %t.result
// RUN: diff -u %S/Outputs/apinotes-diags.txt %t.result
// RUN: %api-digester -diagnose-sdk -print-module -input-paths %t.dump2.json -input-paths %t.dump3.json > %t.result
// RUN: diff -u %S/Outputs/apinotes-diags-3-4.txt %t.result
4 changes: 2 additions & 2 deletions test/api-digester/apinotes-migrator-gen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// RUN: %empty-directory(%t.mod)
// RUN: %empty-directory(%t.sdk)
// RUN: %empty-directory(%t.module-cache)
// 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
// 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
// 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
// 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
// RUN: %api-digester -compare-sdk --input-paths %t.dump1.json -input-paths %t.dump2.json -o %t.result -json
// RUN: diff -u %S/Outputs/apinotes-migrator-gen.json %t.result
128 changes: 119 additions & 9 deletions tools/swift-api-digester/swift-api-digester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ bool contains(ArrayRef<T> container, T instance) {
class SDKNode;
typedef SDKNode* NodePtr;
typedef std::map<NodePtr, NodePtr> ParentMap;
typedef std::map<NodePtr, NodePtr> NodeMap;
typedef std::vector<NodePtr> NodeVector;

// The interface used to visit the SDK tree.
Expand Down Expand Up @@ -231,6 +232,7 @@ class SDKContext {
llvm::StringSet<> TextData;
llvm::BumpPtrAllocator Allocator;
UpdatedNodesMap UpdateMap;
NodeMap TypeAliasUpdateMap;

public:
llvm::BumpPtrAllocator &allocator() {
Expand All @@ -242,6 +244,10 @@ class SDKContext {
UpdatedNodesMap &getNodeUpdateMap() {
return UpdateMap;
}
NodeMap &getTypeAliasUpdateMap() {
return TypeAliasUpdateMap;
}

};

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

SDKNodeType *getRawValueType() const {
if (isConformingTo(KnownProtocolKind::RawRepresentable)) {
if (auto RV = lookupChildByPrintedName("rawValue")) {
return (*(*RV)->getChildBegin())->getAs<SDKNodeType>();
}
}
return nullptr;
}

bool isConformingTo(KnownProtocolKind Kind) const {
StringRef Usr;
switch (Kind) {
Expand All @@ -855,6 +870,9 @@ class SDKNodeTypeAlias : public SDKNodeDecl {
public:
SDKNodeTypeAlias(SDKNodeInitInfo Info) : SDKNodeDecl(Info,
SDKNodeKind::TypeAlias) {}
const SDKNodeType* getUnderlyingType() const {
return getOnlyChild()->getAs<SDKNodeType>();
}
static bool classof(const SDKNode *N);
};

Expand Down Expand Up @@ -1592,8 +1610,8 @@ class SwiftDeclCollector : public VisibleDeclConsumer {

// After collecting decls, either from imported modules or from a previously
// serialized JSON file, using this function to get the root of the SDK.
NodePtr getSDKRoot() {
return RootNode;
SDKNodeRoot* getSDKRoot() {
return static_cast<SDKNodeRoot*>(RootNode);
}

void printTopLevelNames() {
Expand Down Expand Up @@ -2517,6 +2535,57 @@ class TypeMemberDiffFinder : public SDKNodeVisitor {

};

/// This is to find type alias of raw types being changed to RawRepresentable.
/// e.g. AttributeName was a typealias of String in the old SDK however it becomes
/// a RawRepresentable struct in the new SDK.
/// This happens typically when we use apinotes to preserve API stability by
/// using SwiftWrapper:none in the old SDK.
class TypeAliasDiffFinder: public SDKNodeVisitor {
SDKNodeRoot *leftRoot;
SDKNodeRoot *rightRoot;
NodeMap &result;

static bool checkTypeMatch(const SDKNodeType* aliasType,
const SDKNodeType* rawType) {
StringRef Left = aliasType->getPrintedName();
StringRef Right = rawType->getPrintedName();
if (Left == "NSString" && Right == "String")
return true;
if (Left == "String" && Right == "String")
return true;
if (Left == "Int" && Right == "Int")
return true;
if (Left == "UInt" && Right == "UInt")
return true;
return false;
}

void visit(NodePtr node) override {
auto alias = dyn_cast<SDKNodeTypeAlias>(node);
if (!alias)
return;
const SDKNodeType* aliasType = alias->getUnderlyingType();
for (auto *counter: rightRoot->getDescendantsByUsr(alias->getUsr())) {
if (auto DT = dyn_cast<SDKNodeTypeDecl>(counter)) {
if (auto *rawType = DT->getRawValueType()) {
if (checkTypeMatch(aliasType, rawType)) {
result.insert({alias, DT});
return;
}
}
}
}
}
public:
TypeAliasDiffFinder(SDKNodeRoot *leftRoot, SDKNodeRoot *rightRoot,
NodeMap &result): leftRoot(leftRoot), rightRoot(rightRoot),
result(result) {}

void search() {
SDKNode::preorderVisit(leftRoot, *this);
}
};

// Given a condition, search whether a node satisfies that condition exists
// in a tree.
class SearchVisitor : public SDKNodeVisitor {
Expand Down Expand Up @@ -2999,21 +3068,47 @@ class DiagnosisEmitter : public SDKNodeVisitor {
static void theme(raw_ostream &OS) { OS << "Type Changes"; };
};

struct RawRepresentableChangeDiag: public DiagBase {
DeclKind Kind;
StringRef DeclName;
StringRef UnderlyingType;
StringRef RawTypeName;
RawRepresentableChangeDiag(MetaInfo Info, DeclKind Kind, StringRef DeclName,
StringRef UnderlyingType, StringRef RawTypeName): DiagBase(Info),
Kind(Kind), DeclName(DeclName), UnderlyingType(UnderlyingType),
RawTypeName(RawTypeName) {}
bool operator<(RawRepresentableChangeDiag Other) const {
if (Kind != Other.Kind)
return Kind < Other.Kind;
return DeclName.compare(Other.DeclName) < 0;
}
void output() const override {
llvm::outs() << Kind << " " << printName(DeclName)
<< "(" << UnderlyingType << ")"
<< " is now " << RawTypeName << " representable\n";
}
static void theme(raw_ostream &OS) { OS << "RawRepresentable Changes"; };
};

std::set<SDKNodeDecl*> AddedDecls;
DiagBag<DeclAttrDiag> AttrChangedDecls;
DiagBag<DeclTypeChangeDiag> TypeChangedDecls;
DiagBag<RenamedDeclDiag> RenamedDecls;
DiagBag<MovedDeclDiag> MovedDecls;
DiagBag<RemovedDeclDiag> RemovedDecls;
DiagBag<RawRepresentableChangeDiag> RawRepresentableDecls;

UpdatedNodesMap &UpdateMap;
NodeMap &TypeAliasUpdateMap;
TypeMemberDiffVector &MemberChanges;
DiagnosisEmitter(UpdatedNodesMap &UpdateMap,
TypeMemberDiffVector &MemberChanges):
UpdateMap(UpdateMap), MemberChanges(MemberChanges) {}
DiagnosisEmitter(SDKContext &Ctx, TypeMemberDiffVector &MemberChanges):
UpdateMap(Ctx.getNodeUpdateMap()),
TypeAliasUpdateMap(Ctx.getTypeAliasUpdateMap()),
MemberChanges(MemberChanges){}

public:
static void diagnosis(NodePtr LeftRoot, NodePtr RightRoot,
UpdatedNodesMap &UpdateMap);
SDKContext &Ctx);
};

void DiagnosisEmitter::collectAddedDecls(NodePtr Root,
Expand Down Expand Up @@ -3130,11 +3225,11 @@ void DiagnosisEmitter::DeclAttrDiag::output() const {
}

void DiagnosisEmitter::diagnosis(NodePtr LeftRoot, NodePtr RightRoot,
UpdatedNodesMap &UpdateMap) {
SDKContext &Ctx) {
// Find member hoist changes to help refine diagnostics.
TypeMemberDiffVector MemberChanges;
findTypeMemberDiffs(LeftRoot, RightRoot, MemberChanges);
DiagnosisEmitter Emitter(UpdateMap, MemberChanges);
DiagnosisEmitter Emitter(Ctx, MemberChanges);
collectAddedDecls(RightRoot, Emitter.AddedDecls);
SDKNode::postorderVisit(LeftRoot, Emitter);
}
Expand Down Expand Up @@ -3171,6 +3266,19 @@ void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
return;
}

// If a type alias of a raw type has been changed to a struct/enum that
// conforms to RawRepresentable in the later version of SDK, we show the
// refine diagnostics message instead of showing the type alias has been
// removed.
if (TypeAliasUpdateMap.find((SDKNode*)Node) != TypeAliasUpdateMap.end()) {
RawRepresentableDecls.Diags.emplace_back(ScreenInfo, Node->getDeclKind(),
Node->getFullyQualifiedName(),
Node->getAs<SDKNodeTypeAlias>()->getUnderlyingType()->getPrintedName(),
TypeAliasUpdateMap[(SDKNode*)Node]->getAs<SDKNodeTypeDecl>()->
getRawValueType()->getPrintedName());
return;
}

// We should exlude those declarations that are pulled up to the super classes.
bool FoundInSuperclass = false;
if (auto PD = dyn_cast<SDKNodeDecl>(Node->getParent())) {
Expand Down Expand Up @@ -3438,11 +3546,13 @@ static int diagnoseModuleChange(StringRef LeftPath, StringRef RightPath) {
RightCollector.deSerialize(RightPath);
auto LeftModule = LeftCollector.getSDKRoot();
auto RightModule = RightCollector.getSDKRoot();
TypeAliasDiffFinder(LeftModule, RightModule,
Ctx.getTypeAliasUpdateMap()).search();
PrunePass Prune(Ctx.getNodeUpdateMap());
Prune.pass(LeftModule, RightModule);
ChangeRefinementPass RefinementPass(Ctx.getNodeUpdateMap());
RefinementPass.pass(LeftModule, RightModule);
DiagnosisEmitter::diagnosis(LeftModule, RightModule, Ctx.getNodeUpdateMap());
DiagnosisEmitter::diagnosis(LeftModule, RightModule, Ctx);
return 0;
}

Expand Down