[APIDiff] Improve API digester integration & polish of experimental-api-diff

Fixtures/Miscellaneous/APIDiff/Bar/Package.swift

@@ -0,0 +1,14 @@
+// swift-tools-version:4.2
+import PackageDescription
+
+let package = Package(
+    name: "Bar",
+    products: [
+        .library(name: "Baz", targets: ["Baz"]),
+        .library(name: "Qux", targets: ["Qux"]),
+    ],
+    targets: [
+        .target(name: "Baz"),
+        .target(name: "Qux")
+    ]
+)

Fixtures/Miscellaneous/APIDiff/Bar/Sources/Baz/Baz.swift

@@ -0,0 +1,3 @@
+public func bar() -> Int {
+    42
+}

Fixtures/Miscellaneous/APIDiff/Bar/Sources/Qux/Qux.swift

@@ -0,0 +1 @@
+public class Qux<T> { public let x = 1 }

Fixtures/Miscellaneous/APIDiff/CTargetDep/Package.swift

@@ -0,0 +1,13 @@
+// swift-tools-version:4.2
+import PackageDescription
+
+let package = Package(
+    name: "CLibrarySources",
+    products: [
+        .library(name: "Lib", targets: ["Bar"])
+    ],
+    targets: [
+        .target(name: "Foo"),
+        .target(name: "Bar", dependencies: ["Foo"])
+    ]
+)

Fixtures/Miscellaneous/APIDiff/CTargetDep/Sources/Bar/Bar.swift

@@ -0,0 +1,6 @@
+import Foo
+
+public func bar() -> Int {
+    foo()
+    return 42
+}

Fixtures/Miscellaneous/APIDiff/CTargetDep/Sources/Foo/Foo.c

@@ -0,0 +1,8 @@
+#include "Foo.h"
+
+int foo() {
+    int a = 5;
+    int b = a;
+    a = b;
+    return a;
+}

Fixtures/Miscellaneous/APIDiff/CTargetDep/Sources/Foo/include/Foo.h

@@ -0,0 +1 @@
+int foo();

Fixtures/Miscellaneous/APIDiff/Foo/Foo.swift

@@ -0,0 +1,3 @@
+public func foo() {
+	{}()
+}

Fixtures/Miscellaneous/APIDiff/Foo/Package.swift

@@ -0,0 +1,12 @@
+// swift-tools-version:4.2
+import PackageDescription
+
+let package = Package(
+    name: "Foo",
+    products: [
+        .library(name: "Foo", targets: ["Foo"]),
+    ],
+    targets: [
+        .target(name: "Foo", path: "./"),
+    ]
+)

Fixtures/Miscellaneous/APIDiff/NonAPILibraryTargets/Package.swift

@@ -0,0 +1,18 @@
+// swift-tools-version:4.2
+import PackageDescription
+
+let package = Package(
+    name: "NonAPILibraryTargets",
+    products: [
+        .library(name: "One", targets: ["Foo"]),
+        .library(name: "Two", targets: ["Bar", "Baz"]),
+        .executable(name: "Exec", targets: ["Exec", "Qux"])
+    ],
+    targets: [
+        .target(name: "Foo"),
+        .target(name: "Bar", dependencies: ["Baz"]),
+        .target(name: "Baz"),
+        .target(name: "Qux"),
+        .target(name: "Exec", dependencies: ["Qux"])
+    ]
+)

Fixtures/Miscellaneous/APIDiff/NonAPILibraryTargets/Sources/Bar/Bar.swift

@@ -0,0 +1,5 @@
+import Baz
+
+public func bar() -> Int {
+    42
+}

Fixtures/Miscellaneous/APIDiff/NonAPILibraryTargets/Sources/Baz/Baz.swift

@@ -0,0 +1,3 @@
+public enum Baz {
+    case a, c
+}

Fixtures/Miscellaneous/APIDiff/NonAPILibraryTargets/Sources/Exec/main.swift

@@ -0,0 +1,3 @@
+import Qux
+
+print("Hello, world!")

Fixtures/Miscellaneous/APIDiff/NonAPILibraryTargets/Sources/Foo/Foo.swift

@@ -0,0 +1,3 @@
+public struct Foo {
+    func doThing() {}
+}

Fixtures/Miscellaneous/APIDiff/NonAPILibraryTargets/Sources/Qux/Qux.swift

@@ -0,0 +1 @@
+public class Qux<T> { public let x = 1 }

Sources/Build/BuildPlan.swift

@@ -1919,14 +1919,7 @@ public class BuildPlan {
         // Add search paths from the system library targets.
         for target in graph.reachableTargets {
             if let systemLib = target.underlyingTarget as? SystemLibraryTarget {
-                for flag in self.pkgConfig(for: systemLib).cFlags {
-                    // The api-digester tool doesn't like `-I<Foo>` style for some reason.
-                    if flag.hasPrefix("-I") && flag.count > 2 {
-                        arguments += ["-I", String(flag.dropFirst(2))]
-                    } else {
-                        arguments.append(flag)
-                    }
-                }
+                arguments.append(contentsOf: self.pkgConfig(for: systemLib).cFlags)
                 // Add the path to the module map.
                 arguments += ["-I", systemLib.moduleMapPath.parentDirectory.pathString]
             }

Sources/Commands/APIDigester.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+ Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See http://swift.org/LICENSE.txt for license information
@@ -18,11 +18,10 @@ import PackageModel
 import SourceControl
 import Workspace
 
-/// Helper for dumping the SDK JSON file for the baseline.
+/// Helper for emitting a JSON API baseline for a module.
 struct APIDigesterBaselineDumper {
-    /// The baseline we're diffing against.
-    ///
-    /// This is the git treeish.
+
+    /// The git treeish to emit a baseline for.
     let baselineTreeish: String
 
     /// The root package path.
@@ -61,17 +60,18 @@ struct APIDigesterBaselineDumper {
         self.diags = diags
     }
 
-    /// Dump the baseline SDK JSON and return its path.
-    func dumpBaselineSDKJSON() throws -> AbsolutePath {
+    /// Emit the API baseline files and return the path to their directory.
+    func emitAPIBaseline() throws -> AbsolutePath {
         let apiDiffDir = inputBuildParameters.apiDiff
-        let sdkJSON = apiDiffDir.appending(component: baselineTreeish + ".json")
+        let baselineDir = apiDiffDir.appending(component: baselineTreeish)
 
-        // We're done if the JSON already exists on disk.
-        if localFileSystem.exists(sdkJSON) {
-            return sdkJSON
+        // We're done if the baseline already exists on disk.
+        if localFileSystem.exists(baselineDir) {
+            return baselineDir
         }
 
-        let baselinePackageRoot = apiDiffDir.appending(component: baselineTreeish)
+        // Setup a temporary directory where we can checkout and build the baseline treeish.
+        let baselinePackageRoot = apiDiffDir.appending(component: "\(baselineTreeish)-checkout")
         if localFileSystem.exists(baselinePackageRoot) {
             try localFileSystem.removeFileTree(baselinePackageRoot)
         }
@@ -116,66 +116,85 @@ struct APIDigesterBaselineDumper {
             stdoutStream: stdoutStream
         )
 
-        // FIXME: Need a way to register this build operation with the interrupt handler.
-
         try buildOp.build()
 
         // Dump the SDK JSON.
-        try apiDigesterTool.dumpSDKJSON(
-            at: sdkJSON,
-            modules: graph.apiDigesterModules,
-            additionalArgs: buildOp.buildPlan!.createAPIToolCommonArgs(includeLibrarySearchPaths: false)
-        )
+        for module in graph.apiDigesterModules {
+            let moduleBaselinePath = baselineDir.appending(component: module + ".json")
+            try apiDigesterTool.emitAPIBaseline(
+                to: moduleBaselinePath,
+                for: module,
+                buildPlan: buildOp.buildPlan!,
+                diagnosticsEngine: diags
+            )
+        }
 
-        return sdkJSON
+        return baselineDir
     }
 }
 
-/// A wrapper for swift-api-digester tool.
+/// A wrapper for the swift-api-digester tool.
 public struct SwiftAPIDigester {
+
+    /// The absolute path to `swift-api-digester` in the toolchain.
     let tool: AbsolutePath
 
     init(tool: AbsolutePath) {
         self.tool = tool
     }
 
-    public func dumpSDKJSON(
-        at json: AbsolutePath,
-        modules: [String],
-        additionalArgs: [String]
+    /// Emit an API baseline file for the specified module at the specified location.
+    public func emitAPIBaseline(
+        to outputPath: AbsolutePath,
+        for module: String,
+        buildPlan: BuildPlan,
+        diagnosticsEngine: DiagnosticsEngine
     ) throws {
         var args = ["-dump-sdk"]
-        args += additionalArgs
-        args += modules.flatMap { ["-module", $0] }
-        args += ["-o", json.pathString]
-        try localFileSystem.createDirectory(json.parentDirectory, recursive: true)
+        args += buildPlan.createAPIToolCommonArgs(includeLibrarySearchPaths: false)
+        args += ["-module", module, "-o", outputPath.pathString]
+        try localFileSystem.createDirectory(outputPath.parentDirectory, recursive: true)
 
         try runTool(args)
 
-        // FIXME: The tool doesn't exit with 1 if it fails.
-        if !localFileSystem.exists(json) {
+        if !localFileSystem.exists(outputPath) {
+            diagnosticsEngine.emit(error: "failed to generate baseline for \(module)")
             throw Diagnostics.fatalError
         }
     }
 
-    public func diagnoseSDK(
-        baselineSDKJSON: AbsolutePath,
-        apiToolArgs: [String],
-        modules: [String]
-    ) throws {
+    /// Compare the current package API to a provided baseline file.
+    public func compareAPIToBaseline(
+        at baselinePath: AbsolutePath,
+        for module: String,
+        buildPlan: BuildPlan,
+        diagnosticsEngine: DiagnosticsEngine
+    ) throws -> ComparisonResult {
         var args = [
             "-diagnose-sdk",
-            "-baseline-path", baselineSDKJSON.pathString,
+            "-baseline-path", baselinePath.pathString,
+            "-module", module
         ]
-        args.append(contentsOf: apiToolArgs)
-        for module in modules {
-            args.append(contentsOf: ["-module", module])
+        args.append(contentsOf: buildPlan.createAPIToolCommonArgs(includeLibrarySearchPaths: false))
+
+        return try withTemporaryFile(deleteOnClose: false) { file in
+            args.append(contentsOf: ["-serialize-diagnostics-path", file.path.pathString])
+            try runTool(args)
+            let contents = try localFileSystem.readFileContents(file.path)
+            guard contents.count > 0 else {
+                diagnosticsEngine.emit(error: "failed to read API digester output for \(module)")
+                throw Diagnostics.fatalError
+            }
+            let serializedDiagnostics = try SerializedDiagnostics(bytes: contents)
+            let apiDigesterCategory = "api-digester-breaking-change"
+            let apiBreakingChanges = serializedDiagnostics.diagnostics.filter { $0.category == apiDigesterCategory }
+            let otherDiagnostics = serializedDiagnostics.diagnostics.filter { $0.category != apiDigesterCategory }
+            return ComparisonResult(apiBreakingChanges: apiBreakingChanges,
+                                    otherDiagnostics: otherDiagnostics)
         }
-
-        try runTool(args)
     }
 
-    func runTool(_ args: [String]) throws {
+    private func runTool(_ args: [String]) throws {
         let arguments = [tool.pathString] + args
         let process = Process(
             arguments: arguments,
@@ -187,6 +206,21 @@ public struct SwiftAPIDigester {
     }
 }
 
+extension SwiftAPIDigester {
+    /// The result of comparing a module's API to a provided baseline.
+    public struct ComparisonResult {
+        /// Breaking changes made to the API since the baseline was generated.
+        var apiBreakingChanges: [SerializedDiagnostics.Diagnostic]
+        /// Other diagnostics emitted while comparing the current API to the baseline.
+        var otherDiagnostics: [SerializedDiagnostics.Diagnostic]
+
+        /// `true` if the comparison succeeded and no breaking changes were found, otherwise `false`.
+        var hasNoAPIBreakingChanges: Bool {
+            apiBreakingChanges.isEmpty && otherDiagnostics.filter { [.fatal, .error].contains($0.level) }.isEmpty
+        }
+    }
+}
+
 extension BuildParameters {
     /// The directory containing artifacts for API diffing operations.
     var apiDiff: AbsolutePath {
@@ -198,9 +232,16 @@ extension PackageGraph {
     /// The list of modules that should be used as an input to the API digester.
     var apiDigesterModules: [String] {
         self.rootPackages
-            .flatMap { $0.targets }
-            .filter { $0.type == .library }
+            .flatMap(\.products)
+            .filter { $0.type.isLibrary }
+            .flatMap(\.targets)
             .filter { $0.underlyingTarget is SwiftTarget }
             .map { $0.c99name }
     }
 }
+
+extension SerializedDiagnostics.SourceLocation: DiagnosticLocation {
+    public var description: String {
+        return "\(filename):\(line):\(column)"
+    }
+}

Sources/Commands/SwiftPackageTool.swift

@@ -319,14 +319,55 @@ extension SwiftPackageTool {
                 apiDigesterTool: apiDigesterTool,
                 diags: swiftTool.diagnostics
             )
-            let baselineSDKJSON = try baselineDumper.dumpBaselineSDKJSON()
+            let baselineDir = try baselineDumper.emitAPIBaseline()
+
+            var succeeded = true
+            for module in try buildOp.getPackageGraph().apiDigesterModules {
+                let moduleBaselinePath = baselineDir.appending(component: "\(module).json")
+                guard localFileSystem.exists(moduleBaselinePath) else {
+                    print("\nSkipping \(module) because it does not exist in the baseline")
+                    continue
+                }
+                let comparisonResult = try apiDigesterTool.compareAPIToBaseline(
+                    at: moduleBaselinePath,
+                    for: module,
+                    buildPlan: buildOp.buildPlan!,
+                    diagnosticsEngine: swiftTool.diagnostics
+                )
+                printComparisonResult(comparisonResult, moduleName: module, diagnosticsEngine: swiftTool.diagnostics)
+                succeeded = succeeded && comparisonResult.hasNoAPIBreakingChanges
+            }
 
-            // Run the diagnose tool which will print the diff.
-            try apiDigesterTool.diagnoseSDK(
-                baselineSDKJSON: baselineSDKJSON,
-                apiToolArgs: buildOp.buildPlan!.createAPIToolCommonArgs(includeLibrarySearchPaths: false),
-                modules: try buildOp.getPackageGraph().apiDigesterModules
-            )
+            guard succeeded else { throw ExitCode.failure }
+        }
+
+        private func printComparisonResult(_ comparisonResult: SwiftAPIDigester.ComparisonResult,
+                                           moduleName: String,
+                                           diagnosticsEngine: DiagnosticsEngine) {
+            for diagnostic in comparisonResult.otherDiagnostics {
+                switch diagnostic.level {
+                case .error, .fatal:
+                    diagnosticsEngine.emit(error: diagnostic.text, location: diagnostic.location)
+                case .warning:
+                    diagnosticsEngine.emit(warning: diagnostic.text, location: diagnostic.location)
+                case .note:
+                    diagnosticsEngine.emit(note: diagnostic.text, location: diagnostic.location)
+                case .remark:
+                    diagnosticsEngine.emit(remark: diagnostic.text, location: diagnostic.location)
+                case .ignored:
+                    break
+                }
+            }
+
+            if comparisonResult.apiBreakingChanges.isEmpty {
+                print("\nNo breaking changes detected in \(moduleName)")
+            } else {
+                let count = comparisonResult.apiBreakingChanges.count
+                print("\n\(count) breaking \(count > 1 ? "changes" : "change") detected in \(moduleName):")
+                for change in comparisonResult.apiBreakingChanges {
+                    print("  💔 \(change.text)")
+                }
+            }
         }
     }