[parsing] Use DiagnosticPolicy for model directives errors

multibody/parsing/BUILD.bazel

@@ -402,6 +402,7 @@ drake_cc_library(
     hdrs = ["model_directives.h"],
     visibility = ["//visibility:public"],
     deps = [
+        "//common:diagnostic_policy",
         "//common:essential",
         "//common:name_value",
         "//common/schema:transform",

multibody/parsing/detail_dmd_parser.cc

@@ -16,6 +16,7 @@ namespace drake {
 namespace multibody {
 namespace internal {
 
+using drake::internal::DiagnosticPolicy;
 using parsing::GetScopedFrameByName;
 using parsing::ModelDirectives;
 using parsing::ModelInstanceInfo;
@@ -29,7 +30,8 @@ void AddWeld(const Frame<double>& parent_frame,
              const Frame<double>& child_frame,
              const math::RigidTransform<double>& X_PC,
              MultibodyPlant<double>* plant,
-             std::vector<ModelInstanceInfo>* added_models) {
+             std::vector<ModelInstanceInfo>* added_models,
+             const DiagnosticPolicy& diagnostic) {
   plant->WeldFrames(parent_frame, child_frame, X_PC);
   if (added_models) {
     // Record weld info into crappy ModelInstanceInfo struct.
@@ -43,7 +45,13 @@ void AddWeld(const Frame<double>& parent_frame,
         info.X_PC = X_PC;
       }
     }
-    DRAKE_THROW_UNLESS(found);
+    if (!found) {
+      diagnostic.Error(fmt::format(
+          "add_weld: child frame '{}' (model instance '{}') does not match "
+          "any model added by these directives",
+          child_frame.name(),
+          plant->GetModelInstanceName(child_frame.model_instance())));
+    }
   }
 }
 
@@ -201,9 +209,10 @@ void ParseModelDirectivesImpl(const ModelDirectives& directives,
       auto& frame = *directive.add_frame;
       drake::log()->debug("  add_frame: {}", frame.name);
       if (!frame.X_PF.base_frame) {
-        // This would be caught elsewhere, but it is clearer to throw here.
-        throw std::logic_error(
+        // This would be caught elsewhere, but it is clearer to diagnose here.
+        diagnostic.Error(
             "add_frame directive with empty base frame is ambiguous");
+        continue;
       }
       // Only override instance if scope is explicitly specified.
       std::optional<ModelInstanceIndex> instance;
@@ -234,7 +243,7 @@ void ParseModelDirectivesImpl(const ModelDirectives& directives,
       }
       AddWeld(get_scoped_frame(directive.add_weld->parent),
               get_scoped_frame(directive.add_weld->child), X_PC, plant,
-              added_models);
+              added_models, diagnostic);
 
     } else if (directive.add_collision_filter_group) {
       // If there's no geometry registered, there's nothing to be done with
@@ -277,9 +286,10 @@ void ParseModelDirectivesImpl(const ModelDirectives& directives,
       drake::log()->debug("    new_model_namespace: {}", new_model_namespace);
       if (!new_model_namespace.empty() &&
           !plant->HasModelInstanceNamed(new_model_namespace)) {
-        throw std::runtime_error(
+        diagnostic.Error(
             fmt::format("Namespace '{}' does not exist as model instance",
                         new_model_namespace));
+        continue;
       }
       const ResolveUriResult resolved =
           ResolveUri(diagnostic, sub.file, package_map, {});
@@ -288,9 +298,12 @@ void ParseModelDirectivesImpl(const ModelDirectives& directives,
         continue;
       }
       const std::string filename = resolved.full_path.string();
-      auto sub_directives =
-          LoadModelDirectives({DataSource::kFilename, &filename});
-      ParseModelDirectivesImpl(sub_directives, new_model_namespace, workspace,
+      std::optional<ModelDirectives> sub_directives =
+          LoadModelDirectives({DataSource::kFilename, &filename}, diagnostic);
+      if (!sub_directives.has_value()) {
+        continue;
+      }
+      ParseModelDirectivesImpl(*sub_directives, new_model_namespace, workspace,
                                added_models);
     }
   }
@@ -304,7 +317,8 @@ ScopedName DmdScopedNameJoin(const std::string& namespace_name,
   return ScopedName::Join(namespace_name, element_name);
 }
 
-ModelDirectives LoadModelDirectives(const DataSource& data_source) {
+std::optional<ModelDirectives> LoadModelDirectives(
+    const DataSource& data_source, const DiagnosticPolicy& diagnostic) {
   // Even though the 'defaults' we use to start parsing here are empty, by
   // providing any defaults at all, the effect during parsing will be that any
   // of the users' ModelDirective structs and sub-structs will _also_ start
@@ -318,8 +332,9 @@ ModelDirectives LoadModelDirectives(const DataSource& data_source) {
     drake::log()->debug("LoadModelDirectives: {}", filename);
 
     if (!std::filesystem::exists({filename})) {
-      throw std::runtime_error(
+      diagnostic.Error(
           fmt::format("No such file {} during LoadModelDirectives", filename));
+      return std::nullopt;
     }
 
     directives = yaml::LoadYamlFile<ModelDirectives>(
@@ -329,7 +344,9 @@ ModelDirectives LoadModelDirectives(const DataSource& data_source) {
     directives = yaml::LoadYamlString<ModelDirectives>(
         data_source.contents(), std::nullopt /* child_name */, defaults);
   }
-  DRAKE_THROW_UNLESS(directives.IsValid());
+  if (!directives.IsValid(diagnostic)) {
+    return std::nullopt;
+  }
   return directives;
 }
 
@@ -365,11 +382,13 @@ std::vector<ModelInstanceIndex> DmdParserWrapper::AddAllModels(
     const DataSource& data_source,
     const std::optional<std::string>& parent_model_name,
     const ParsingWorkspace& workspace) {
-  // TODO(#18052): diagnostic policy?
-  ModelDirectives directives = LoadModelDirectives(data_source);
-  // TODO(#18052): diagnostic policy?
+  std::optional<ModelDirectives> directives =
+      LoadModelDirectives(data_source, workspace.diagnostic);
+  if (!directives.has_value()) {
+    return {};
+  }
   const std::vector<ModelInstanceInfo> infos =
-      ParseModelDirectives(directives, parent_model_name, workspace);
+      ParseModelDirectives(*directives, parent_model_name, workspace);
   std::vector<ModelInstanceIndex> results;
   results.reserve(infos.size());
   for (const auto& info : infos) {

multibody/parsing/detail_dmd_parser.h

@@ -4,6 +4,7 @@
 #include <string>
 #include <vector>
 
+#include "drake/common/diagnostic_policy.h"
 #include "drake/multibody/parsing/detail_common.h"
 #include "drake/multibody/parsing/detail_parsing_workspace.h"
 #include "drake/multibody/parsing/model_directives.h"
@@ -20,8 +21,12 @@ namespace internal {
 ScopedName DmdScopedNameJoin(const std::string& namespace_name,
                              const std::string& element_name);
 
-// TODO(#18052): diagnostic policy?
-parsing::ModelDirectives LoadModelDirectives(const DataSource& data_source);
+// Loads model directives from the given data source, reporting errors via
+// @p diagnostic. Returns std::nullopt if an error was reported (and the
+// diagnostic policy did not throw).
+std::optional<parsing::ModelDirectives> LoadModelDirectives(
+    const DataSource& data_source,
+    const drake::internal::DiagnosticPolicy& diagnostic);
 
 std::vector<parsing::ModelInstanceInfo> ParseModelDirectives(
     const parsing::ModelDirectives& directives,

multibody/parsing/model_directives.cc

@@ -6,23 +6,44 @@ namespace drake {
 namespace multibody {
 namespace parsing {
 
+using drake::internal::DiagnosticDetail;
+using drake::internal::DiagnosticPolicy;
+
+namespace {
+
+/* Preserves the historical IsValid() behavior: log errors and return false
+without throwing. */
+DiagnosticPolicy MakeLoggingPolicy() {
+  DiagnosticPolicy policy;
+  policy.SetActionForErrors([](const DiagnosticDetail& detail) {
+    drake::log()->error(detail.message);
+  });
+  return policy;
+}
+
+}  // namespace
+
 bool AddWeld::IsValid() const {
+  return IsValid(MakeLoggingPolicy());
+}
+
+bool AddWeld::IsValid(const DiagnosticPolicy& diagnostic) const {
   if (parent.empty()) {
-    drake::log()->error("add_weld: `parent` must be non-empty");
+    diagnostic.Error("add_weld: `parent` must be non-empty");
     return false;
   } else if (child.empty()) {
-    drake::log()->error("add_weld: `child` must be non-empty");
+    diagnostic.Error("add_weld: `child` must be non-empty");
     return false;
   }
   if (X_PC) {
     if (X_PC->base_frame) {
-      drake::log()->error(
+      diagnostic.Error(
           "add_weld: `X_PC` must not specify a `base_frame`; the pose is "
           "always in the parent frame.");
       return false;
     }
     if (!X_PC->IsDeterministic()) {
-      drake::log()->error(
+      diagnostic.Error(
           "add_weld: `X_PC` must specify a deterministic transform, not a "
           "distribution.");
       return false;
@@ -32,16 +53,20 @@ bool AddWeld::IsValid() const {
 }
 
 bool AddModel::IsValid() const {
+  return IsValid(MakeLoggingPolicy());
+}
+
+bool AddModel::IsValid(const DiagnosticPolicy& diagnostic) const {
   if (file.empty()) {
-    drake::log()->error("add_model: `file` must be non-empty");
+    diagnostic.Error("add_model: `file` must be non-empty");
     return false;
   } else if (name.empty()) {
-    drake::log()->error("add_model: `name` must be non-empty");
+    diagnostic.Error("add_model: `name` must be non-empty");
     return false;
   }
   for (const auto& [body_name, pose] : default_free_body_pose) {
     if (!pose.IsDeterministic()) {
-      drake::log()->error(
+      diagnostic.Error(
           "add_model: `default_free_body_pose` must specify a "
           "deterministic transform, not a distribution.");
       return false;
@@ -51,22 +76,30 @@ bool AddModel::IsValid() const {
 }
 
 bool AddModelInstance::IsValid() const {
+  return IsValid(MakeLoggingPolicy());
+}
+
+bool AddModelInstance::IsValid(const DiagnosticPolicy& diagnostic) const {
   if (name.empty()) {
-    drake::log()->error("add_model_instance: `name` must be non-empty");
+    diagnostic.Error("add_model_instance: `name` must be non-empty");
     return false;
   }
   return true;
 }
 
 bool AddFrame::IsValid() const {
+  return IsValid(MakeLoggingPolicy());
+}
+
+bool AddFrame::IsValid(const DiagnosticPolicy& diagnostic) const {
   if (name.empty()) {
-    drake::log()->error("add_frame: `name` must be non-empty");
+    diagnostic.Error("add_frame: `name` must be non-empty");
     return false;
   } else if (!X_PF.base_frame || X_PF.base_frame->empty()) {
-    drake::log()->error("add_frame: `X_PF.base_frame` must be defined");
+    diagnostic.Error("add_frame: `X_PF.base_frame` must be defined");
     return false;
   } else if (!X_PF.IsDeterministic()) {
-    drake::log()->error(
+    diagnostic.Error(
         "add_frame: `X_PF` must specify a deterministic transform, not a "
         "distribution.");
     return false;
@@ -75,11 +108,16 @@ bool AddFrame::IsValid() const {
 }
 
 bool AddCollisionFilterGroup::IsValid() const {
+  return IsValid(MakeLoggingPolicy());
+}
+
+bool AddCollisionFilterGroup::IsValid(
+    const DiagnosticPolicy& diagnostic) const {
   if (name.empty()) {
-    drake::log()->error("add_collision_filter_group: `name` must be non-empty");
+    diagnostic.Error("add_collision_filter_group: `name` must be non-empty");
     return false;
   } else if (members.empty() && member_groups.empty()) {
-    drake::log()->error(
+    diagnostic.Error(
         "add_collision_filter_group:"
         " at least one of `members` or `member_groups` must be non-empty");
     return false;
@@ -88,41 +126,53 @@ bool AddCollisionFilterGroup::IsValid() const {
 }
 
 bool AddDirectives::IsValid() const {
+  return IsValid(MakeLoggingPolicy());
+}
+
+bool AddDirectives::IsValid(const DiagnosticPolicy& diagnostic) const {
   if (file.empty()) {
-    drake::log()->error("add_directives: `file` must be non-empty");
+    diagnostic.Error("add_directives: `file` must be non-empty");
     return false;
   }
   return true;
 }
 
 bool ModelDirective::IsValid() const {
+  return IsValid(MakeLoggingPolicy());
+}
+
+bool ModelDirective::IsValid(const DiagnosticPolicy& diagnostic) const {
   const bool unique = (add_model.has_value() + add_model_instance.has_value() +
                        add_frame.has_value() + add_weld.has_value() +
                        add_collision_filter_group.has_value() +
                        add_directives.has_value()) == 1;
   if (!unique) {
-    drake::log()->error(
+    diagnostic.Error(
         "directive: Specify one of `add_model`, `add_model_instance`, "
         "`add_frame`, `add_collision_filter_group`, or `add_directives`");
     return false;
   } else if (add_model) {
-    return add_model->IsValid();
+    return add_model->IsValid(diagnostic);
   } else if (add_model_instance) {
-    return add_model_instance->IsValid();
+    return add_model_instance->IsValid(diagnostic);
   } else if (add_frame) {
-    return add_frame->IsValid();
+    return add_frame->IsValid(diagnostic);
   } else if (add_weld) {
-    return add_weld->IsValid();
+    return add_weld->IsValid(diagnostic);
   } else if (add_collision_filter_group) {
-    return add_collision_filter_group->IsValid();
+    return add_collision_filter_group->IsValid(diagnostic);
   } else {
-    return add_directives->IsValid();
+    return add_directives->IsValid(diagnostic);
   }
 }
 
 bool ModelDirectives::IsValid() const {
+  return IsValid(MakeLoggingPolicy());
+}
+
+bool ModelDirectives::IsValid(const DiagnosticPolicy& diagnostic) const {
   for (auto& directive : directives) {
-    if (!directive.IsValid()) return false;
+    if (!directive.IsValid(diagnostic)) return false;
   }
   return true;
 }