Merge pull request #20849 from akyrtzi/5.0-api-checker-protocol-whitelist

[5.0][api-digester] Add '-protocol-requirement-white-list' option to the digester for when diagnosing API breakage in SDKs

Author: akyrtzi

test/api-digester/Inputs/Foo-new-version/foo.h

@@ -9,6 +9,11 @@
   -(void) someOptionalFunctionFromProt;
 @end
 
+@protocol AnotherObjcProt
+  -(void) anotherFunctionFromProt;
+  -(void) anotherFunctionFromProt2;
+@end
+
 @interface ClangInterface: NSObject <ObjcProt>
 - (void)someFunction;
 @end

test/api-digester/Inputs/Foo-prot-whitelist.txt

@@ -0,0 +1 @@
+AnotherObjcProt

test/api-digester/Inputs/Foo/foo.h

@@ -4,6 +4,10 @@
   -(void) someFunctionFromProt;
 @end
 
+@protocol AnotherObjcProt
+  -(void) anotherFunctionFromProt;
+@end
+
 @interface ClangInterface: NSObject <ObjcProt>
 - (void)someFunction;
 @end

test/api-digester/Outputs/clang-module-dump.txt

@@ -3,6 +3,47 @@
   "name": "TopLevel",
   "printedName": "TopLevel",
   "children": [
+    {
+      "kind": "TypeDecl",
+      "name": "AnotherObjcProt",
+      "printedName": "AnotherObjcProt",
+      "children": [
+        {
+          "kind": "Function",
+          "name": "anotherFunctionFromProt",
+          "printedName": "anotherFunctionFromProt()",
+          "children": [
+            {
+              "kind": "TypeNameAlias",
+              "name": "Void",
+              "printedName": "Void",
+              "children": [
+                {
+                  "kind": "TypeNominal",
+                  "name": "Void",
+                  "printedName": "()"
+                }
+              ]
+            }
+          ],
+          "declKind": "Func",
+          "usr": "c:objc(pl)AnotherObjcProt(im)anotherFunctionFromProt",
+          "moduleName": "Foo",
+          "genericSig": "<Self where Self : AnotherObjcProt>",
+          "protocolReq": true,
+          "declAttributes": [
+            "ObjC"
+          ],
+          "funcSelfKind": "NonMutating"
+        }
+      ],
+      "declKind": "Protocol",
+      "usr": "c:objc(pl)AnotherObjcProt",
+      "moduleName": "Foo",
+      "declAttributes": [
+        "ObjC"
+      ]
+    },
     {
       "kind": "TypeDecl",
       "name": "ClangInterface",

test/api-digester/compare-clang-dump.swift

@@ -1,7 +1,7 @@
 // RUN: %empty-directory(%t.module-cache)
 // RUN: %api-digester -dump-sdk -module Foo -o %t.dump1.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %S/Inputs/Foo -avoid-location
 // RUN: %api-digester -dump-sdk -module Foo -o %t.dump2.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %S/Inputs/Foo-new-version -avoid-location
-// RUN: %api-digester -diagnose-sdk -print-module --input-paths %t.dump1.json -input-paths %t.dump2.json -o %t.result
+// RUN: %api-digester -diagnose-sdk -protocol-requirement-white-list %S/Inputs/Foo-prot-whitelist.txt -print-module --input-paths %t.dump1.json -input-paths %t.dump2.json -o %t.result
 
 // RUN: %clang -E -P -x c %S/Outputs/Foo-diff.txt -o - | sed '/^\s*$/d' > %t.expected
 // RUN: %clang -E -P -x c %t.result -o - | sed '/^\s*$/d' > %t.result.tmp

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

@@ -63,6 +63,10 @@ static llvm::cl::opt<std::string>
 ModuleList("module-list-file",
            llvm::cl::desc("File containing a new-line separated list of modules"));
 
+static llvm::cl::opt<std::string>
+ProtReqWhiteList("protocol-requirement-white-list",
+           llvm::cl::desc("File containing a new-line separated list of protocol names"));
+
 static llvm::cl::opt<std::string>
 OutputFile("o", llvm::cl::desc("Output file"));
 
@@ -927,6 +931,7 @@ class PrunePass : public MatchedNodeListener, public SDKTreeDiffPass {
 
   SDKContext &Ctx;
   UpdatedNodesMap &UpdateMap;
+  llvm::StringSet<> ProtocolReqWhitelist;
 
   static void printSpaces(llvm::raw_ostream &OS, SDKNode *N) {
     assert(N);
@@ -959,6 +964,10 @@ class PrunePass : public MatchedNodeListener, public SDKTreeDiffPass {
 
 public:
   PrunePass(SDKContext &Ctx): Ctx(Ctx), UpdateMap(Ctx.getNodeUpdateMap()) {}
+  PrunePass(SDKContext &Ctx, llvm::StringSet<> prWhitelist):
+    Ctx(Ctx),
+    UpdateMap(Ctx.getNodeUpdateMap()),
+    ProtocolReqWhitelist(std::move(prWhitelist)) {}
 
   void foundMatch(NodePtr Left, NodePtr Right, NodeMatchReason Reason) override {
     if (options::Verbose)
@@ -985,6 +994,12 @@ class PrunePass : public MatchedNodeListener, public SDKTreeDiffPass {
             if (ATD->getDefault())
               ShouldComplain = false;
           }
+          if (ShouldComplain &&
+              ProtocolReqWhitelist.count(D->getParent()->getAs<SDKNodeDecl>()->getFullyQualifiedName())) {
+            // Ignore protocol requirement additions if the protocol has been added
+            // to the whitelist.
+            ShouldComplain = false;
+          }
           if (ShouldComplain)
             Ctx.getDiags().diagnose(SourceLoc(), diag::protocol_req_added,
                                     D->getScreenInfo());
@@ -2027,7 +2042,8 @@ static void findTypeMemberDiffs(NodePtr leftSDKRoot, NodePtr rightSDKRoot,
 
 static int diagnoseModuleChange(StringRef LeftPath, StringRef RightPath,
                                 StringRef OutputPath,
-                                CheckerOptions Opts) {
+                                CheckerOptions Opts,
+                                llvm::StringSet<> ProtocolReqWhitelist) {
   if (!fs::exists(LeftPath)) {
     llvm::errs() << LeftPath << " does not exist\n";
     return 1;
@@ -2055,7 +2071,7 @@ static int diagnoseModuleChange(StringRef LeftPath, StringRef RightPath,
   auto RightModule = RightCollector.getSDKRoot();
   TypeAliasDiffFinder(LeftModule, RightModule,
                       Ctx.getTypeAliasUpdateMap()).search();
-  PrunePass Prune(Ctx);
+  PrunePass Prune(Ctx, std::move(ProtocolReqWhitelist));
   Prune.pass(LeftModule, RightModule);
   ChangeRefinementPass RefinementPass(Ctx.getNodeUpdateMap());
   RefinementPass.pass(LeftModule, RightModule);
@@ -2182,7 +2198,7 @@ static int compareSDKs(StringRef LeftPath, StringRef RightPath,
 static int readFileLineByLine(StringRef Path, llvm::StringSet<> &Lines) {
   auto FileBufOrErr = llvm::MemoryBuffer::getFile(Path);
   if (!FileBufOrErr) {
-    llvm::errs() << "error opening file: "
+    llvm::errs() << "error opening file '" << Path << "': "
       << FileBufOrErr.getError().message() << '\n';
     return 1;
   }
@@ -2192,8 +2208,11 @@ static int readFileLineByLine(StringRef Path, llvm::StringSet<> &Lines) {
     StringRef Line;
     std::tie(Line, BufferText) = BufferText.split('\n');
     Line = Line.trim();
-    if (!Line.empty())
-      Lines.insert(Line);
+    if (Line.empty())
+      continue;
+    if (Line.startswith("// ")) // comment.
+      continue;
+    Lines.insert(Line);
   }
   return 0;
 }
@@ -2347,19 +2366,26 @@ int main(int argc, char *argv[]) {
     return (prepareForDump(argv[0], InitInvok, Modules)) ? 1 :
       dumpSDKContent(InitInvok, Modules, options::OutputFile, Opts);
   case ActionType::CompareSDKs:
-  case ActionType::DiagnoseSDKs:
+  case ActionType::DiagnoseSDKs: {
     if (options::SDKJsonPaths.size() != 2) {
       llvm::errs() << "Only two SDK versions can be compared\n";
       llvm::cl::PrintHelpMessage();
       return 1;
     }
+    llvm::StringSet<> protocolWhitelist;
+    if (!options::ProtReqWhiteList.empty()) {
+      if (readFileLineByLine(options::ProtReqWhiteList, protocolWhitelist))
+          return 1;
+    }
     if (options::Action == ActionType::CompareSDKs)
       return compareSDKs(options::SDKJsonPaths[0], options::SDKJsonPaths[1],
                          options::OutputFile, IgnoredUsrs, Opts);
     else
       return diagnoseModuleChange(options::SDKJsonPaths[0],
                                   options::SDKJsonPaths[1],
-                                  options::OutputFile, Opts);
+                                  options::OutputFile, Opts,
+                                  std::move(protocolWhitelist));
+  }
   case ActionType::DeserializeSDK:
   case ActionType::DeserializeDiffItems: {
     if (options::SDKJsonPaths.size() != 1) {