Merge pull request #84132 from rmaz/jsonerr

Add error messages for Swift module map parser

Author: rmaz

include/swift/AST/DiagnosticsFrontend.def

@@ -360,8 +360,8 @@ ERROR(explicit_swift_module_map_missing,none,
       (StringRef))
 
 ERROR(explicit_swift_module_map_corrupted,none,
-      "explicit Swift module map from %0 is malformed",
-      (StringRef))
+      "explicit Swift module map from %0 is malformed: %1",
+      (StringRef, StringRef))
 
 ERROR(const_extract_protocol_list_input_file_missing,none,
       "cannot open constant extraction protocol list input file from %0",

include/swift/Frontend/ModuleInterfaceLoader.h

@@ -111,6 +111,8 @@
 #include "swift/Frontend/Frontend.h"
 #include "swift/Frontend/ModuleInterfaceSupport.h"
 #include "swift/Serialization/SerializedModuleLoader.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -317,7 +319,7 @@ class ExplicitModuleMapParser {
 public:
   ExplicitModuleMapParser(llvm::BumpPtrAllocator &Allocator) : Saver(Allocator) {}
 
-  std::error_code parseSwiftExplicitModuleMap(
+  llvm::Error parseSwiftExplicitModuleMap(
       llvm::MemoryBufferRef BufferRef,
       llvm::StringMap<ExplicitSwiftModuleInputInfo> &swiftModuleMap,
       llvm::StringMap<ExplicitClangModuleInputInfo> &clangModuleMap,
@@ -331,16 +333,15 @@ class ExplicitModuleMapParser {
       assert(DI != Stream.end() && "Failed to read a document");
       if (auto *MN = dyn_cast_or_null<SequenceNode>(DI->getRoot())) {
         for (auto &entry : *MN) {
-          if (parseSingleModuleEntry(entry, swiftModuleMap, clangModuleMap,
-                                     moduleAliases)) {
-            return std::make_error_code(std::errc::invalid_argument);
-          }
+          if (auto Err = parseSingleModuleEntry(entry, swiftModuleMap,
+                                                clangModuleMap, moduleAliases))
+            return Err;
         }
       } else {
-        return std::make_error_code(std::errc::invalid_argument);
+        return llvm::createStringError("invalid JSON root object");
       }
     }
-    return std::error_code{}; // success
+    return llvm::Error::success(); // success
   }
 
 private:
@@ -360,15 +361,15 @@ class ExplicitModuleMapParser {
       llvm_unreachable("Unexpected JSON value for isFramework");
   }
 
-  bool parseSingleModuleEntry(
+  llvm::Error parseSingleModuleEntry(
       llvm::yaml::Node &node,
       llvm::StringMap<ExplicitSwiftModuleInputInfo> &swiftModuleMap,
       llvm::StringMap<ExplicitClangModuleInputInfo> &clangModuleMap,
       llvm::StringMap<std::string> &moduleAliases) {
     using namespace llvm::yaml;
     auto *mapNode = dyn_cast<MappingNode>(&node);
     if (!mapNode)
-      return true;
+      return llvm::createStringError("incorrect entry type");
     StringRef moduleName;
     std::optional<std::string> swiftModulePath, swiftModuleDocPath,
         swiftModuleSourceInfoPath, swiftModuleCacheKey, clangModuleCacheKey,
@@ -418,7 +419,7 @@ class ExplicitModuleMapParser {
       }
     }
     if (moduleName.empty())
-      return true;
+      return llvm::createStringError("entry is missing module name");
 
     bool didInsert;
     if (swiftModulePath.has_value()) {
@@ -445,11 +446,15 @@ class ExplicitModuleMapParser {
                                          clangModuleCacheKey);
       didInsert = clangModuleMap.try_emplace(moduleName, std::move(entry)).second;
     }
-    if (didInsert && moduleAlias.has_value()) {
+    if (!didInsert)
+      return llvm::createStringError(llvm::formatv(
+          "duplicate {0} module with name {1}",
+          swiftModulePath.has_value() ? "Swift" : "Clang", moduleName));
+
+    if (moduleAlias.has_value()) {
       moduleAliases[*moduleAlias] = moduleName;
     }
-    // Prevent duplicate module names.
-    return !didInsert;
+    return llvm::Error::success();
   }
 
   llvm::StringSaver Saver;

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -2258,13 +2258,14 @@ struct ExplicitSwiftModuleLoader::Implementation {
       return;
     }
 
-    auto hasError = parser.parseSwiftExplicitModuleMap(
+    auto error = parser.parseSwiftExplicitModuleMap(
         (*fileBufOrErr)->getMemBufferRef(), ExplicitModuleMap,
         ExplicitClangModuleMap, ModuleAliases);
-
-    if (hasError)
+    llvm::handleAllErrors(std::move(error), [this, &fileName](
+                                                const llvm::StringError &E) {
       Ctx.Diags.diagnose(SourceLoc(), diag::explicit_swift_module_map_corrupted,
-                         fileName);
+                         fileName, E.getMessage());
+    });
 
     // A single module map can define multiple modules; keep track of the ones
     // we've seen so that we don't generate duplicate flags.
@@ -2536,13 +2537,14 @@ struct ExplicitCASModuleLoader::Implementation {
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> fileBufOrErr =
         llvm::MemoryBuffer::getFile(ID);
 
-    auto hasError = parser.parseSwiftExplicitModuleMap(
+    auto error = parser.parseSwiftExplicitModuleMap(
         buf->getMemBufferRef(), ExplicitModuleMap, ExplicitClangModuleMap,
         ModuleAliases);
-
-    if (hasError)
+    llvm::handleAllErrors(std::move(error), [this,
+                                             &ID](const llvm::StringError &E) {
       Ctx.Diags.diagnose(SourceLoc(), diag::explicit_swift_module_map_corrupted,
-                         ID);
+                         ID, E.getMessage());
+    });
 
     std::set<std::string> moduleMapsSeen;
     std::vector<std::string> &extraClangArgs = Ctx.ClangImporterOpts.ExtraArgs;

lib/FrontendTool/FrontendTool.cpp

@@ -1604,8 +1604,12 @@ static bool generateReproducer(CompilerInstance &Instance,
     }
     auto map = llvm::json::parse(mapProxy->getData());
     if (!map) {
-      diags.diagnose(SourceLoc(), diag::explicit_swift_module_map_corrupted,
-                     mapOpts);
+      llvm::handleAllErrors(
+          map.takeError(), [&diags, &mapOpts](const llvm::json::ParseError &E) {
+            diags.diagnose(SourceLoc(),
+                           diag::explicit_swift_module_map_corrupted, mapOpts,
+                           E.message());
+          });
       return true;
     }
     if (auto array = map->getAsArray()) {

test/Frontend/explicit-swift-module-map-file-parser.swift

@@ -0,0 +1,78 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: not %target-swift-frontend -typecheck %t/test.swift \
+// RUN:   -explicit-swift-module-map-file %t/invalid_root_object.json \
+// RUN:   2>&1 | %FileCheck %s -check-prefix=CHECK-ROOT
+
+// CHECK-ROOT: malformed: invalid JSON root object
+
+// RUN: not %target-swift-frontend -typecheck %t/test.swift \
+// RUN:   -explicit-swift-module-map-file %t/invalid_entry_type.json \
+// RUN:   2>&1 | %FileCheck %s -check-prefix=CHECK-ENTRY-TYPE
+
+// CHECK-ENTRY-TYPE: malformed: incorrect entry type
+
+// RUN: not %target-swift-frontend -typecheck %t/test.swift \
+// RUN:   -explicit-swift-module-map-file %t/missing_module_name.json \
+// RUN:   2>&1 | %FileCheck %s -check-prefix=CHECK-NAME
+
+// CHECK-NAME: malformed: entry is missing module name
+
+// RUN: not %target-swift-frontend -typecheck %t/test.swift \
+// RUN:   -explicit-swift-module-map-file %t/duplicate_swift_module.json \
+// RUN:   2>&1 | %FileCheck %s -check-prefix=CHECK-DUP-SWIFT
+
+// CHECK-DUP-SWIFT: malformed: duplicate Swift module with name SwiftMod
+
+// RUN: not %target-swift-frontend -typecheck %t/test.swift \
+// RUN:   -explicit-swift-module-map-file %t/duplicate_clang_module.json \
+// RUN:   2>&1 | %FileCheck %s -check-prefix=CHECK-DUP-CLANG
+
+// CHECK-DUP-CLANG: malformed: duplicate Clang module with name ClangMod
+
+//--- invalid_root_object.json
+{
+    "some_key": "some_val"
+}
+//--- invalid_entry_type.json
+[
+    [
+        {"some_key": "some_val"}
+    ]
+]
+//--- missing_module_name.json
+[
+    {
+        "isFramework": false,
+        "modulePath": "/some/path"
+    }
+]
+//--- duplicate_swift_module.json
+[
+    {
+        "isFramework": false,
+        "moduleName": "SwiftMod",
+        "modulePath": "/some/path"
+    },
+    {
+        "isFramework": false,
+        "moduleName": "SwiftMod",
+        "modulePath": "/some/path"
+    }
+]
+//--- duplicate_clang_module.json
+[
+    {
+        "isFramework": false,
+        "moduleName": "ClangMod",
+        "clangModulePath": "/some/path"
+    },
+    {
+        "isFramework": false,
+        "moduleName": "ClangMod",
+        "clangModulePath": "/some/path"
+    }
+]
+//--- test.swift
+import Swift