Refactor command line errors to mirror diagnostic style (#4568)

This changes to an `Error` return to let the driver do the "error: "
prefix, except for one case with `help` that needs more work to change
(I'm not planning on picking up that TODO). It also changes
capitalization, backtick use, and a few minor punctuation things to try
to better match the diagnostic style.

This also adds `Error` matchers so that the changes to command line
testing are clearer.

common/BUILD

@@ -41,6 +41,7 @@ cc_library(
     hdrs = ["command_line.h"],
     deps = [
         ":check",
+        ":error",
         ":ostream",
         "@llvm-project//llvm:Support",
     ],
@@ -52,6 +53,7 @@ cc_test(
     srcs = ["command_line_test.cpp"],
     deps = [
         ":command_line",
+        ":error_test_helpers",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
         "@googletest//:gtest",
@@ -122,12 +124,23 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "error_test_helpers",
+    testonly = 1,
+    hdrs = ["error_test_helpers.h"],
+    deps = [
+        ":error",
+        "@googletest//:gtest",
+    ],
+)
+
 cc_test(
     name = "error_test",
     size = "small",
     srcs = ["error_test.cpp"],
     deps = [
         ":error",
+        ":error_test_helpers",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
         "@googletest//:gtest",

common/command_line.cpp

@@ -15,8 +15,6 @@ namespace Carbon::CommandLine {
 auto operator<<(llvm::raw_ostream& output, ParseResult result)
     -> llvm::raw_ostream& {
   switch (result) {
-    case ParseResult::Error:
-      return output << "Error";
     case ParseResult::MetaSuccess:
       return output << "MetaSuccess";
     case ParseResult::Success:
@@ -311,8 +309,10 @@ void MetaPrinter::PrintHelpForSubcommandName(
     }
   }
 
-  *out_ << "ERROR: Could not find a subcommand named '" << subcommand_name
-        << "'.\n";
+  // TODO: This should really be connected up so that parsing can return an
+  // Error instead of ParseResult::MetaSuccess in this case.
+  *out_ << "error: could not find a subcommand named '" << subcommand_name
+        << "'\n";
 }
 
 void MetaPrinter::PrintVersion(const Command& command) const {
@@ -330,12 +330,12 @@ void MetaPrinter::PrintVersion(const Command& command) const {
 void MetaPrinter::PrintSubcommands(const Command& command) const {
   for (const auto& subcommand :
        llvm::ArrayRef(command.subcommands).drop_back()) {
-    *out_ << "'" << subcommand->info.name << "', ";
+    *out_ << "`" << subcommand->info.name << "`, ";
   }
   if (command.subcommands.size() > 1) {
     *out_ << "or ";
   }
-  *out_ << "'" << command.subcommands.back()->info.name << "'";
+  *out_ << "`" << command.subcommands.back()->info.name << "`";
 }
 
 void MetaPrinter::PrintRawVersion(const Command& command,
@@ -632,7 +632,7 @@ void MetaPrinter::PrintUsage(const Command& command) const {
   if (!command.parent) {
     *out_ << "Usage:\n";
   } else {
-    *out_ << "Subcommand '" << command.info.name << "' usage:\n";
+    *out_ << "Subcommand `" << command.info.name << "` usage:\n";
   }
   PrintRawUsage(command, "  ");
 }
@@ -648,7 +648,7 @@ void MetaPrinter::PrintHelpSubcommands(const Command& command) const {
       if (!command.parent) {
         *out_ << "\nSubcommands:";
       } else {
-        *out_ << "\nSubcommand '" << command.info.name << "' subcommands:";
+        *out_ << "\nSubcommand `" << command.info.name << "` subcommands:";
       }
     }
     *out_ << "\n";
@@ -668,8 +668,8 @@ void MetaPrinter::PrintHelpPositionalArgs(const Command& command) const {
       if (!command.parent) {
         *out_ << "\nPositional arguments:";
       } else {
-        *out_ << "\nSubcommand '" << command.info.name
-              << "' positional arguments:";
+        *out_ << "\nSubcommand `" << command.info.name
+              << "` positional arguments:";
       }
     }
     *out_ << "\n";
@@ -692,7 +692,7 @@ void MetaPrinter::PrintHelpOptions(const Command& command) const {
       } else if (!command.parent) {
         *out_ << "\nCommand options:";
       } else {
-        *out_ << "\nSubcommand '" << command.info.name << "' options:";
+        *out_ << "\nSubcommand `" << command.info.name << "` options:";
       }
     }
     *out_ << "\n";
@@ -711,12 +711,12 @@ void MetaPrinter::PrintHelpOptions(const Command& command) const {
 
 class Parser {
  public:
-  // `out` and `errors` must not be null.
-  explicit Parser(llvm::raw_ostream* out, llvm::raw_ostream* errors,
-                  CommandInfo command_info,
+  // `out` must not be null.
+  explicit Parser(llvm::raw_ostream* out, CommandInfo command_info,
                   llvm::function_ref<void(CommandBuilder&)> build);
 
-  auto Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args) -> ParseResult;
+  auto Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args)
+      -> ErrorOr<ParseResult>;
 
  private:
   friend CommandBuilder;
@@ -739,39 +739,38 @@ class Parser {
   void SetOptionDefault(const Arg& option);
 
   auto ParseNegatedFlag(const Arg& flag, std::optional<llvm::StringRef> value)
-      -> bool;
-  auto ParseFlag(const Arg& flag, std::optional<llvm::StringRef> value) -> bool;
-  auto ParseIntegerArgValue(const Arg& arg, llvm::StringRef value) -> bool;
-  auto ParseStringArgValue(const Arg& arg, llvm::StringRef value) -> bool;
-  auto ParseOneOfArgValue(const Arg& arg, llvm::StringRef value) -> bool;
+      -> ErrorOr<Success>;
+  auto ParseFlag(const Arg& flag, std::optional<llvm::StringRef> value)
+      -> ErrorOr<Success>;
+  auto ParseIntegerArgValue(const Arg& arg, llvm::StringRef value)
+      -> ErrorOr<Success>;
+  auto ParseStringArgValue(const Arg& arg, llvm::StringRef value)
+      -> ErrorOr<Success>;
+  auto ParseOneOfArgValue(const Arg& arg, llvm::StringRef value)
+      -> ErrorOr<Success>;
   auto ParseArg(const Arg& arg, bool short_spelling,
                 std::optional<llvm::StringRef> value, bool negated_name = false)
-      -> bool;
+      -> ErrorOr<Success>;
 
   auto SplitValue(llvm::StringRef& unparsed_arg)
       -> std::optional<llvm::StringRef>;
-  auto ParseLongOption(llvm::StringRef unparsed_arg) -> bool;
-  auto ParseShortOptionSeq(llvm::StringRef unparsed_arg) -> bool;
-  auto FinalizeParsedOptions() -> bool;
+  auto ParseLongOption(llvm::StringRef unparsed_arg) -> ErrorOr<Success>;
+  auto ParseShortOptionSeq(llvm::StringRef unparsed_arg) -> ErrorOr<Success>;
+  auto FinalizeParsedOptions() -> ErrorOr<Success>;
 
-  auto ParsePositionalArg(llvm::StringRef unparsed_arg) -> bool;
-  auto ParseSubcommand(llvm::StringRef unparsed_arg) -> bool;
+  auto ParsePositionalArg(llvm::StringRef unparsed_arg) -> ErrorOr<Success>;
+  auto ParseSubcommand(llvm::StringRef unparsed_arg) -> ErrorOr<Success>;
 
   auto ParsePositionalSuffix(llvm::ArrayRef<llvm::StringRef> unparsed_args)
-      -> bool;
+      -> ErrorOr<Success>;
 
-  auto FinalizeParse() -> ParseResult;
+  auto FinalizeParse() -> ErrorOr<ParseResult>;
 
   // When building a command, it registers arguments and potentially subcommands
   // that are meta actions to print things to standard out, so we build a meta
   // printer for that here.
   MetaPrinter meta_printer_;
 
-  // Most parsing output goes to an error stream, and we also provide an
-  // error-oriented meta printer for when that is useful during parsing.
-  llvm::raw_ostream* errors_;
-  MetaPrinter error_meta_printer_;
-
   Command root_command_;
 
   const Command* command_;
@@ -833,89 +832,92 @@ void Parser::SetOptionDefault(const Arg& option) {
 }
 
 auto Parser::ParseNegatedFlag(const Arg& flag,
-                              std::optional<llvm::StringRef> value) -> bool {
+                              std::optional<llvm::StringRef> value)
+    -> ErrorOr<Success> {
   if (flag.kind != Arg::Kind::Flag) {
-    *errors_ << "ERROR: Cannot use a negated flag name by prefixing it with "
-                "'no-' when it isn't a boolean flag argument.\n";
-    return false;
+    return Error(
+        "cannot use a negated flag name by prefixing it with `no-` when it "
+        "isn't a boolean flag argument");
   }
   if (value) {
-    *errors_ << "ERROR: Cannot specify a value when using a flag name prefixed "
-                "with 'no-' -- that prefix implies a value of 'false'.\n";
-    return false;
+    return Error(
+        "cannot specify a value when using a flag name prefixed with `no-` -- "
+        "that prefix implies a value of `false`");
   }
   *flag.flag_storage = false;
-  return true;
+  return Success();
 }
 
 auto Parser::ParseFlag(const Arg& flag, std::optional<llvm::StringRef> value)
-    -> bool {
+    -> ErrorOr<Success> {
   CARBON_CHECK(flag.kind == Arg::Kind::Flag, "Incorrect kind: {0}", flag.kind);
   if (!value || *value == "true") {
     *flag.flag_storage = true;
   } else if (*value == "false") {
     *flag.flag_storage = false;
   } else {
-    *errors_ << "ERROR: Invalid value specified for the boolean flag '--"
-             << flag.info.name << "': " << *value << "\n";
-
-    return false;
+    return Error(llvm::formatv(
+        "invalid value specified for the boolean flag `--{0}`: {1}",
+        flag.info.name, *value));
   }
-  return true;
+  return Success();
 }
 
 auto Parser::ParseIntegerArgValue(const Arg& arg, llvm::StringRef value)
-    -> bool {
+    -> ErrorOr<Success> {
   CARBON_CHECK(arg.kind == Arg::Kind::Integer, "Incorrect kind: {0}", arg.kind);
   int integer_value;
   // Note that this method returns *true* on error!
   if (value.getAsInteger(/*Radix=*/0, integer_value)) {
-    *errors_ << "ERROR: Cannot parse value for option '--" << arg.info.name
-             << "' as an integer: " << value << "\n";
-    return false;
+    return Error(llvm::formatv(
+        "cannot parse value for option `--{0}` as an integer: {1}",
+        arg.info.name, value));
   }
   if (!arg.is_append) {
     *arg.integer_storage = integer_value;
   } else {
     arg.integer_sequence->push_back(integer_value);
   }
-  return true;
+  return Success();
 }
 
 auto Parser::ParseStringArgValue(const Arg& arg, llvm::StringRef value)
-    -> bool {
+    -> ErrorOr<Success> {
   CARBON_CHECK(arg.kind == Arg::Kind::String, "Incorrect kind: {0}", arg.kind);
   if (!arg.is_append) {
     *arg.string_storage = value;
   } else {
     arg.string_sequence->push_back(value);
   }
-  return true;
+  return Success();
 }
 
-auto Parser::ParseOneOfArgValue(const Arg& arg, llvm::StringRef value) -> bool {
+auto Parser::ParseOneOfArgValue(const Arg& arg, llvm::StringRef value)
+    -> ErrorOr<Success> {
   CARBON_CHECK(arg.kind == Arg::Kind::OneOf, "Incorrect kind: {0}", arg.kind);
   if (!arg.value_action(arg, value)) {
-    *errors_ << "ERROR: Option '--" << arg.info.name << "=";
-    llvm::printEscapedString(value, *errors_);
-    *errors_ << "' has an invalid value '";
-    llvm::printEscapedString(value, *errors_);
-    *errors_ << "'; valid values are: ";
+    std::string error_str;
+    llvm::raw_string_ostream error(error_str);
+    error << "option `--" << arg.info.name << "=";
+    llvm::printEscapedString(value, error);
+    error << "` has an invalid value `";
+    llvm::printEscapedString(value, error);
+    error << "`; valid values are: ";
     for (auto value_string : arg.value_strings.drop_back()) {
-      *errors_ << "'" << value_string << "', ";
+      error << "`" << value_string << "`, ";
     }
     if (arg.value_strings.size() > 1) {
-      *errors_ << "or ";
+      error << "or ";
     }
-    *errors_ << "'" << arg.value_strings.back() << "'\n";
-    return false;
+    error << "`" << arg.value_strings.back() << "`";
+    return Error(error_str);
   }
-  return true;
+  return Success();
 }
 
 auto Parser::ParseArg(const Arg& arg, bool short_spelling,
                       std::optional<llvm::StringRef> value, bool negated_name)
-    -> bool {
+    -> ErrorOr<Success> {
   // If this argument has a meta action, replace the current meta action with
   // it.
   if (arg.meta_action) {
@@ -932,10 +934,10 @@ auto Parser::ParseArg(const Arg& arg, bool short_spelling,
 
   std::string name;
   if (short_spelling) {
-    name = llvm::formatv("'-{0}' (short for '--{1}')", arg.info.short_name,
+    name = llvm::formatv("`-{0}` (short for `--{1}`)", arg.info.short_name,
                          arg.info.name);
   } else {
-    name = llvm::formatv("'--{0}'", arg.info.name);
+    name = llvm::formatv("`--{0}`", arg.info.name);
   }
 
   if (!value) {
@@ -943,15 +945,14 @@ auto Parser::ParseArg(const Arg& arg, bool short_spelling,
     // an option and handle it as such.
     if (arg.kind == Arg::Kind::MetaActionOnly) {
       // Nothing further to do here, this is only a meta-action.
-      return true;
+      return Success();
     }
     if (!arg.has_default) {
-      *errors_ << "ERROR: Option " << name
-               << " requires a value to be provided and none was.\n";
-      return false;
+      return Error(llvm::formatv(
+          "option {0} requires a value to be provided and none was", name));
     }
     SetOptionDefault(arg);
-    return true;
+    return Success();
   }
 
   // There is a value to parse as part of the argument.
@@ -963,11 +964,10 @@ auto Parser::ParseArg(const Arg& arg, bool short_spelling,
     case Arg::Kind::OneOf:
       return ParseOneOfArgValue(arg, *value);
     case Arg::Kind::MetaActionOnly:
-      *errors_ << "ERROR: Option " << name
-               << " cannot be used with a value, and '" << *value
-               << "' was provided.\n";
-      // TODO: improve message
-      return false;
+      // TODO: Improve message.
+      return Error(llvm::formatv(
+          "option {0} cannot be used with a value, and '{1}' was provided",
+          name, value));
     case Arg::Kind::Flag:
     case Arg::Kind::Invalid:
       CARBON_FATAL("Invalid kind!");
@@ -986,7 +986,7 @@ auto Parser::SplitValue(llvm::StringRef& unparsed_arg)
   return value;
 }
 
-auto Parser::ParseLongOption(llvm::StringRef unparsed_arg) -> bool {
+auto Parser::ParseLongOption(llvm::StringRef unparsed_arg) -> ErrorOr<Success> {
   CARBON_CHECK(unparsed_arg.starts_with("--") && unparsed_arg.size() > 2,
                "Must only be called on a potential long option.");
 
@@ -997,10 +997,9 @@ auto Parser::ParseLongOption(llvm::StringRef unparsed_arg) -> bool {
 
   auto option_it = option_map_.find(unparsed_arg);
   if (option_it == option_map_.end()) {
-    *errors_ << "ERROR: Unknown option '--" << (negated_name ? "no-" : "")
-             << unparsed_arg << "'\n";
-    // TODO: improve error
-    return false;
+    // TODO: Improve error.
+    return Error(llvm::formatv("unknown option `--{0}{1}`",
+                               negated_name ? "no-" : "", unparsed_arg));
   }
 
   // Mark this option as parsed.
@@ -1011,41 +1010,38 @@ auto Parser::ParseLongOption(llvm::StringRef unparsed_arg) -> bool {
   return ParseArg(option, /*short_spelling=*/false, value, negated_name);
 }
 
-auto Parser::ParseShortOptionSeq(llvm::StringRef unparsed_arg) -> bool {
+auto Parser::ParseShortOptionSeq(llvm::StringRef unparsed_arg)
+    -> ErrorOr<Success> {
   CARBON_CHECK(unparsed_arg.starts_with("-") && unparsed_arg.size() > 1,
                "Must only be called on a potential short option sequence.");
 
   unparsed_arg = unparsed_arg.drop_front();
   std::optional<llvm::StringRef> value = SplitValue(unparsed_arg);
   if (value && unparsed_arg.size() != 1) {
-    *errors_ << "ERROR: Cannot provide a value to the group of multiple short "
-                "options '-"
-             << unparsed_arg
-             << "=...'; values must be provided to a single option, using "
-                "either the short or long spelling.\n";
-    return false;
+    return Error(llvm::formatv(
+        "cannot provide a value to the group of multiple short options "
+        "`-{0}=...`; values must be provided to a single option, using "
+        "either the short or long spelling",
+        unparsed_arg));
   }
 
   for (unsigned char c : unparsed_arg) {
     auto* arg_entry =
         (c < short_option_table_.size()) ? short_option_table_[c] : nullptr;
     if (!arg_entry) {
-      *errors_ << "ERROR: Unknown short option '" << c << "'\n";
-      return false;
+      return Error(llvm::formatv("unknown short option `{0}`", c));
     }
     // Mark this argument as parsed.
     arg_entry->setInt(true);
 
     // Parse the argument, including the value if this is the last.
     const Arg& arg = *arg_entry->getPointer();
-    if (!ParseArg(arg, /*short_spelling=*/true, value)) {
-      return false;
-    }
+    CARBON_RETURN_IF_ERROR(ParseArg(arg, /*short_spelling=*/true, value));
   }
-  return true;
+  return Success();
 }
 
-auto Parser::FinalizeParsedOptions() -> bool {
+auto Parser::FinalizeParsedOptions() -> ErrorOr<Success> {
   llvm::SmallVector<const Arg*> missing_options;
   for (const auto& option_entry : option_map_) {
     const Arg* option = option_entry.second.getPointer();
@@ -1062,7 +1058,7 @@ auto Parser::FinalizeParsedOptions() -> bool {
     }
   }
   if (missing_options.empty()) {
-    return true;
+    return Success();
   }
 
   // Sort the missing arguments by name to provide a stable and deterministic
@@ -1073,23 +1069,24 @@ auto Parser::FinalizeParsedOptions() -> bool {
               return lhs->info.name < rhs->info.name;
             });
 
+  std::string error_str = "required options not provided: ";
+  llvm::raw_string_ostream error(error_str);
+  llvm::ListSeparator sep;
   for (const Arg* option : missing_options) {
-    *errors_ << "ERROR: Required option '--" << option->info.name
-             << "' not provided.\n";
+    error << sep << "--" << option->info.name;
   }
 
-  return false;
+  return Error(error_str);
 }
 
-auto Parser::ParsePositionalArg(llvm::StringRef unparsed_arg) -> bool {
+auto Parser::ParsePositionalArg(llvm::StringRef unparsed_arg)
+    -> ErrorOr<Success> {
   if (static_cast<size_t>(positional_arg_index_) >=
       command_->positional_args.size()) {
-    *errors_ << "ERROR: Completed parsing all "
-             << command_->positional_args.size()
-             << " configured positional arguments, and found an additional "
-                "positional argument: '"
-             << unparsed_arg << "'\n";
-    return false;
+    return Error(llvm::formatv(
+        "completed parsing all {0} configured positional arguments, and found "
+        "an additional positional argument: `{1}`",
+        command_->positional_args.size(), unparsed_arg));
   }
 
   const Arg& arg = *command_->positional_args[positional_arg_index_];
@@ -1107,29 +1104,28 @@ auto Parser::ParsePositionalArg(llvm::StringRef unparsed_arg) -> bool {
   return ParseArg(arg, /*short_spelling=*/false, unparsed_arg);
 }
 
-auto Parser::ParseSubcommand(llvm::StringRef unparsed_arg) -> bool {
+auto Parser::ParseSubcommand(llvm::StringRef unparsed_arg) -> ErrorOr<Success> {
   auto subcommand_it = subcommand_map_.find(unparsed_arg);
   if (subcommand_it == subcommand_map_.end()) {
-    *errors_ << "ERROR: Invalid subcommand '" << unparsed_arg
-             << "'. Available subcommands: ";
-    error_meta_printer_.PrintSubcommands(*command_);
-    *errors_ << "\n";
-    return false;
+    std::string error_str;
+    llvm::raw_string_ostream error(error_str);
+    error << "invalid subcommand `" << unparsed_arg
+          << "`; available subcommands: ";
+    MetaPrinter(&error).PrintSubcommands(*command_);
+    return Error(error_str);
   }
 
   // Before we recurse into the subcommand, verify that all the required
   // arguments for this command were in fact parsed.
-  if (!FinalizeParsedOptions()) {
-    return false;
-  }
+  CARBON_RETURN_IF_ERROR(FinalizeParsedOptions());
 
   // Recurse into the subcommand, tracking the active command.
   command_ = subcommand_it->second;
   PopulateMaps(*command_);
-  return true;
+  return Success();
 }
 
-auto Parser::FinalizeParse() -> ParseResult {
+auto Parser::FinalizeParse() -> ErrorOr<ParseResult> {
   // If an argument action is provided, we run that and consider the parse
   // meta-successful rather than verifying required arguments were provided and
   // the (sub)command action.
@@ -1139,9 +1135,7 @@ auto Parser::FinalizeParse() -> ParseResult {
   }
 
   // Verify we're not missing any arguments.
-  if (!FinalizeParsedOptions()) {
-    return ParseResult::Error;
-  }
+  CARBON_RETURN_IF_ERROR(FinalizeParsedOptions());
 
   // If we were appending to a positional argument, mark that as complete.
   llvm::ArrayRef positional_args = command_->positional_args;
@@ -1159,10 +1153,10 @@ auto Parser::FinalizeParse() -> ParseResult {
     // There are un-parsed positional arguments, make sure they aren't required.
     const Arg& missing_arg = *unparsed_positional_args.front();
     if (missing_arg.is_required) {
-      *errors_ << "ERROR: Not all required positional arguments were provided. "
-                  "First missing and required positional argument: '"
-               << missing_arg.info.name << "'\n";
-      return ParseResult::Error;
+      return Error(
+          llvm::formatv("not all required positional arguments were provided; "
+                        "first missing and required positional argument: `{0}`",
+                        missing_arg.info.name));
     }
     for (const auto& arg_ptr : unparsed_positional_args) {
       CARBON_CHECK(
@@ -1174,11 +1168,13 @@ auto Parser::FinalizeParse() -> ParseResult {
   switch (command_->kind) {
     case Command::Kind::Invalid:
       CARBON_FATAL("Should never have a parser with an invalid command!");
-    case Command::Kind::RequiresSubcommand:
-      *errors_ << "ERROR: No subcommand specified. Available subcommands: ";
-      error_meta_printer_.PrintSubcommands(*command_);
-      *errors_ << "\n";
-      return ParseResult::Error;
+    case Command::Kind::RequiresSubcommand: {
+      std::string error_str;
+      llvm::raw_string_ostream error(error_str);
+      error << "no subcommand specified; available subcommands: ";
+      MetaPrinter(&error).PrintSubcommands(*command_);
+      return Error(error_str);
+    }
     case Command::Kind::Action:
       // All arguments have been successfully parsed, run any action for the
       // most specific selected command. Only the leaf command's action is run.
@@ -1191,7 +1187,7 @@ auto Parser::FinalizeParse() -> ParseResult {
 }
 
 auto Parser::ParsePositionalSuffix(
-    llvm::ArrayRef<llvm::StringRef> unparsed_args) -> bool {
+    llvm::ArrayRef<llvm::StringRef> unparsed_args) -> ErrorOr<Success> {
   CARBON_CHECK(
       !command_->positional_args.empty(),
       "Cannot do positional suffix parsing without positional arguments!");
@@ -1207,9 +1203,7 @@ auto Parser::ParsePositionalSuffix(
     unparsed_args = unparsed_args.drop_front();
 
     if (unparsed_arg != "--") {
-      if (!ParsePositionalArg(unparsed_arg)) {
-        return false;
-      }
+      CARBON_RETURN_IF_ERROR(ParsePositionalArg(unparsed_arg));
       empty_positional = false;
       continue;
     }
@@ -1218,28 +1212,23 @@ auto Parser::ParsePositionalSuffix(
       ++positional_arg_index_;
       if (static_cast<size_t>(positional_arg_index_) >=
           command_->positional_args.size()) {
-        *errors_
-            << "ERROR: Completed parsing all "
-            << command_->positional_args.size()
-            << " configured positional arguments, but found a subsequent `--` "
-               "and have no further positional arguments to parse beyond it.\n";
-        return false;
+        return Error(
+            llvm::formatv("completed parsing all {0} configured positional "
+                          "arguments, but found a subsequent `--` and have no "
+                          "further positional arguments to parse beyond it",
+                          command_->positional_args.size()));
       }
     }
     appending_to_positional_arg_ = false;
     empty_positional = true;
   }
 
-  return true;
+  return Success();
 }
 
-Parser::Parser(llvm::raw_ostream* out, llvm::raw_ostream* errors,
-               CommandInfo command_info,
+Parser::Parser(llvm::raw_ostream* out, CommandInfo command_info,
                llvm::function_ref<void(CommandBuilder&)> build)
-    : meta_printer_(out),
-      errors_(errors),
-      error_meta_printer_(errors),
-      root_command_(command_info) {
+    : meta_printer_(out), root_command_(command_info) {
   // Run the command building lambda on a builder for the root command.
   CommandBuilder builder(&root_command_, &meta_printer_);
   build(builder);
@@ -1248,7 +1237,7 @@ Parser::Parser(llvm::raw_ostream* out, llvm::raw_ostream* errors,
 }
 
 auto Parser::Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args)
-    -> ParseResult {
+    -> ErrorOr<ParseResult> {
   PopulateMaps(*command_);
 
   while (!unparsed_args.empty()) {
@@ -1258,22 +1247,18 @@ auto Parser::Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args)
     // positional suffix parsing without dropping this argument.
     if (unparsed_arg == "--") {
       if (command_->positional_args.empty()) {
-        *errors_ << "ERROR: Cannot meaningfully end option and subcommand "
-                    "arguments with a `--` argument when there are no "
-                    "positional arguments to parse.\n";
-        return ParseResult::Error;
+        return Error(
+            "cannot meaningfully end option and subcommand arguments with a "
+            "`--` argument when there are no positional arguments to parse");
       }
       if (static_cast<size_t>(positional_arg_index_) >=
           command_->positional_args.size()) {
-        *errors_
-            << "ERROR: Switched to purely positional arguments with a `--` "
-               "argument despite already having parsed all positional "
-               "arguments for this command.\n";
-        return ParseResult::Error;
-      }
-      if (!ParsePositionalSuffix(unparsed_args)) {
-        return ParseResult::Error;
+        return Error(
+            "switched to purely positional arguments with a `--` argument "
+            "despite already having parsed all positional arguments for this "
+            "command");
       }
+      CARBON_RETURN_IF_ERROR(ParsePositionalSuffix(unparsed_args));
       // No more unparsed arguments to handle.
       break;
     }
@@ -1284,16 +1269,12 @@ auto Parser::Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args)
 
     if (unparsed_arg.starts_with("--")) {
       // Note that the exact argument "--" has been handled above already.
-      if (!ParseLongOption(unparsed_arg)) {
-        return ParseResult::Error;
-      }
+      CARBON_RETURN_IF_ERROR(ParseLongOption(unparsed_arg));
       continue;
     }
 
     if (unparsed_arg.starts_with("-") && unparsed_arg.size() > 1) {
-      if (!ParseShortOptionSeq(unparsed_arg)) {
-        return ParseResult::Error;
-      }
+      CARBON_RETURN_IF_ERROR(ParseShortOptionSeq(unparsed_arg));
       continue;
     }
 
@@ -1301,20 +1282,16 @@ auto Parser::Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args)
         command_->positional_args.empty() || command_->subcommands.empty(),
         "Cannot have both positional arguments and subcommands!");
     if (command_->positional_args.empty() && command_->subcommands.empty()) {
-      *errors_ << "ERROR: Found unexpected positional argument or subcommand: '"
-               << unparsed_arg << "'\n";
-      return ParseResult::Error;
+      return Error(llvm::formatv(
+          "found unexpected positional argument or subcommand: `{0}`",
+          unparsed_arg));
     }
 
     if (!command_->positional_args.empty()) {
-      if (!ParsePositionalArg(unparsed_arg)) {
-        return ParseResult::Error;
-      }
+      CARBON_RETURN_IF_ERROR(ParsePositionalArg(unparsed_arg));
       continue;
     }
-    if (!ParseSubcommand(unparsed_arg)) {
-      return ParseResult::Error;
-    }
+    CARBON_RETURN_IF_ERROR(ParseSubcommand(unparsed_arg));
   }
 
   return FinalizeParse();
@@ -1531,12 +1508,12 @@ void CommandBuilder::Finalize() {
 }
 
 auto Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args,
-           llvm::raw_ostream& out, llvm::raw_ostream& errors,
-           CommandInfo command_info,
-           llvm::function_ref<void(CommandBuilder&)> build) -> ParseResult {
+           llvm::raw_ostream& out, CommandInfo command_info,
+           llvm::function_ref<void(CommandBuilder&)> build)
+    -> ErrorOr<ParseResult> {
   // Build a parser, which includes building the command description provided by
   // the user.
-  Parser parser(&out, &errors, command_info, build);
+  Parser parser(&out, command_info, build);
 
   // Now parse the arguments provided using that parser.
   return parser.Parse(unparsed_args);

common/command_line.h

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "common/check.h"
+#include "common/error.h"
 #include "common/ostream.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
@@ -220,15 +221,8 @@ class MetaPrinter;
 class Parser;
 class CommandBuilder;
 
-// The result of parsing arguments can be a parse error, a successfully parsed
-// command line, or a meta-success due to triggering a meta-action during the
-// parse such as rendering help text.
+// The result of parsing arguments.
 enum class ParseResult : int8_t {
-  // Signifies an error parsing arguments. It will have been diagnosed using
-  // the streams provided to the parser, and no useful parsed arguments are
-  // available.
-  Error,
-
   // Signifies that program arguments were successfully parsed and can be
   // used.
   Success,
@@ -660,9 +654,9 @@ class CommandBuilder {
 // are printed to `errors`, but meta-actions like printing a command's help go
 // to `out`.
 auto Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args,
-           llvm::raw_ostream& out, llvm::raw_ostream& errors,
-           CommandInfo command_info,
-           llvm::function_ref<void(CommandBuilder&)> build) -> ParseResult;
+           llvm::raw_ostream& out, CommandInfo command_info,
+           llvm::function_ref<void(CommandBuilder&)> build)
+    -> ErrorOr<ParseResult>;
 
 // Implementation details only below.
 

common/command_line_test.cpp

@@ -7,17 +7,18 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "common/error_test_helpers.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "testing/base/test_raw_ostream.h"
 
 namespace Carbon::CommandLine {
 namespace {
 
+using ::Carbon::Testing::IsError;
+using ::Carbon::Testing::IsSuccess;
 using ::Carbon::Testing::TestRawOstream;
 using ::testing::ElementsAre;
 using ::testing::Eq;
-using ::testing::HasSubstr;
-using ::testing::Not;
 using ::testing::StrEq;
 
 constexpr CommandInfo TestCommandInfo = {
@@ -41,19 +42,18 @@ TEST(ArgParserTest, BasicCommand) {
   int integer_option = -1;
   llvm::StringRef string_option = "";
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args) {
-    return Parse(
-        args, llvm::errs(), llvm::errs(), TestCommandInfo, [&](auto& b) {
-          b.AddFlag({.name = "flag"}, [&](auto& arg_b) { arg_b.Set(&flag); });
-          b.AddIntegerOption({.name = "option1"},
-                             [&](auto& arg_b) { arg_b.Set(&integer_option); });
-          b.AddStringOption({.name = "option2"},
-                            [&](auto& arg_b) { arg_b.Set(&string_option); });
-          b.Do([] {});
-        });
+    return Parse(args, llvm::errs(), TestCommandInfo, [&](auto& b) {
+      b.AddFlag({.name = "flag"}, [&](auto& arg_b) { arg_b.Set(&flag); });
+      b.AddIntegerOption({.name = "option1"},
+                         [&](auto& arg_b) { arg_b.Set(&integer_option); });
+      b.AddStringOption({.name = "option2"},
+                        [&](auto& arg_b) { arg_b.Set(&string_option); });
+      b.Do([] {});
+    });
   };
 
   EXPECT_THAT(parse({"--flag", "--option2=value", "--option1=42"}),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_THAT(integer_option, Eq(42));
   EXPECT_THAT(string_option, StrEq("value"));
@@ -62,47 +62,46 @@ TEST(ArgParserTest, BasicCommand) {
 TEST(ArgParserTest, BooleanFlags) {
   bool flag = false;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddFlag({.name = "flag"}, [&](auto& arg_b) { arg_b.Set(&flag); });
       b.Do([] {});
     });
   };
 
-  EXPECT_THAT(parse({"--no-flag"}, llvm::errs()), Eq(ParseResult::Success));
+  EXPECT_THAT(parse({"--no-flag"}, llvm::errs()),
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_FALSE(flag);
 
   EXPECT_THAT(parse({"--flag", "--no-flag"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_FALSE(flag);
 
   EXPECT_THAT(parse({"--no-flag", "--flag"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
 
   EXPECT_THAT(parse({"--no-flag", "--flag", "--flag=false"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_FALSE(flag);
 
   EXPECT_THAT(parse({"--no-flag", "--flag=true"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
 
   EXPECT_THAT(parse({"--no-flag", "--flag=true", "--no-flag"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_FALSE(flag);
 
   TestRawOstream os;
-  EXPECT_THAT(parse({"--no-flag=true"}, os), Eq(ParseResult::Error));
   EXPECT_THAT(
-      os.TakeStr(),
-      StrEq("ERROR: Cannot specify a value when using a flag name prefixed "
-            "with 'no-' -- that prefix implies a value of 'false'.\n"));
+      parse({"--no-flag=true"}, os),
+      IsError(StrEq("cannot specify a value when using a flag name prefixed "
+                    "with `no-` -- that prefix implies a value of `false`")));
 
-  EXPECT_THAT(parse({"--no-flag=false"}, os), Eq(ParseResult::Error));
   EXPECT_THAT(
-      os.TakeStr(),
-      StrEq("ERROR: Cannot specify a value when using a flag name prefixed "
-            "with 'no-' -- that prefix implies a value of 'false'.\n"));
+      parse({"--no-flag=false"}, os),
+      IsError(StrEq("cannot specify a value when using a flag name prefixed "
+                    "with `no-` -- that prefix implies a value of `false`")));
 }
 
 TEST(ArgParserTest, ArgDefaults) {
@@ -110,7 +109,7 @@ TEST(ArgParserTest, ArgDefaults) {
   int integer_option = -1;
   llvm::StringRef string_option = "";
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddFlag({.name = "flag"}, [&](auto& arg_b) {
         arg_b.Default(true);
         arg_b.Set(&flag);
@@ -127,19 +126,19 @@ TEST(ArgParserTest, ArgDefaults) {
     });
   };
 
-  EXPECT_THAT(parse({}, llvm::errs()), Eq(ParseResult::Success));
+  EXPECT_THAT(parse({}, llvm::errs()), IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_THAT(integer_option, Eq(7));
   EXPECT_THAT(string_option, StrEq("default"));
 
   EXPECT_THAT(parse({"--option1", "--option2"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_THAT(integer_option, Eq(7));
   EXPECT_THAT(string_option, StrEq("default"));
 
   EXPECT_THAT(parse({"--option1=42", "--option2=explicit"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_THAT(integer_option, Eq(42));
   EXPECT_THAT(string_option, StrEq("explicit"));
@@ -147,7 +146,7 @@ TEST(ArgParserTest, ArgDefaults) {
   EXPECT_THAT(
       parse({"--option1=42", "--option2=explicit", "--option1", "--option2"},
             llvm::errs()),
-      Eq(ParseResult::Success));
+      IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_THAT(integer_option, Eq(7));
   EXPECT_THAT(string_option, StrEq("default"));
@@ -158,7 +157,7 @@ TEST(ArgParserTest, ShortArgs) {
   bool example = false;
   int integer_option = -1;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddFlag({.name = "flag", .short_name = "f"},
                 [&](auto& arg_b) { arg_b.Set(&flag); });
       b.AddFlag({.name = "example", .short_name = "x"},
@@ -175,7 +174,7 @@ TEST(ArgParserTest, ShortArgs) {
   };
 
   EXPECT_THAT(parse({"-f", "-o=42", "-x"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_TRUE(example);
   EXPECT_THAT(integer_option, Eq(42));
@@ -183,27 +182,27 @@ TEST(ArgParserTest, ShortArgs) {
   flag = false;
   example = false;
   EXPECT_THAT(parse({"--option1=13", "-xfo"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_TRUE(example);
   EXPECT_THAT(integer_option, Eq(123));
 
   TestRawOstream os;
-  EXPECT_THAT(parse({"-xzf"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(),
-              StrEq("ERROR: Option '-z' (short for '--option2') requires a "
-                    "value to be provided and none was.\n"));
+  EXPECT_THAT(
+      parse({"-xzf"}, os),
+      IsError(StrEq("option `-z` (short for `--option2`) requires a value to "
+                    "be provided and none was")));
 
-  EXPECT_THAT(parse({"--option2"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq("ERROR: Option '--option2' requires a value "
-                                  "to be provided and none was.\n"));
+  EXPECT_THAT(
+      parse({"--option2"}, os),
+      IsError(StrEq(
+          "option `--option2` requires a value to be provided and none was")));
 
-  EXPECT_THAT(parse({"-xz=123"}, os), Eq(ParseResult::Error));
   EXPECT_THAT(
-      os.TakeStr(),
-      StrEq("ERROR: Cannot provide a value to the group of multiple short "
-            "options '-xz=...'; values must be provided to a single option, "
-            "using either the short or long spelling.\n"));
+      parse({"-xz=123"}, os),
+      IsError(StrEq("cannot provide a value to the group of multiple short "
+                    "options `-xz=...`; values must be provided to a single "
+                    "option, using either the short or long spelling")));
 }
 
 TEST(ArgParserTest, PositionalArgs) {
@@ -212,7 +211,7 @@ TEST(ArgParserTest, PositionalArgs) {
   llvm::StringRef source_string;
   llvm::StringRef dest_string;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddFlag({.name = "flag"}, [&](auto& arg_b) { arg_b.Set(&flag); });
       b.AddStringOption({.name = "option"},
                         [&](auto& arg_b) { arg_b.Set(&string_option); });
@@ -229,25 +228,25 @@ TEST(ArgParserTest, PositionalArgs) {
   };
 
   TestRawOstream os;
-  EXPECT_THAT(parse({"--flag", "--option=x"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(
-      os.TakeStr(),
-      StrEq("ERROR: Not all required positional arguments were provided. First "
-            "missing and required positional argument: 'source'\n"));
+  EXPECT_THAT(parse({"--flag", "--option=x"}, os),
+              IsError(StrEq(
+                  "not all required positional arguments were provided; first "
+                  "missing and required positional argument: `source`")));
 
   EXPECT_THAT(parse({"src", "--flag", "--option=value", "dst"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_THAT(string_option, StrEq("value"));
   EXPECT_THAT(source_string, StrEq("src"));
   EXPECT_THAT(dest_string, StrEq("dst"));
 
   EXPECT_THAT(parse({"src2", "--", "dst2"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(source_string, StrEq("src2"));
   EXPECT_THAT(dest_string, StrEq("dst2"));
 
-  EXPECT_THAT(parse({"-", "--", "-"}, llvm::errs()), Eq(ParseResult::Success));
+  EXPECT_THAT(parse({"-", "--", "-"}, llvm::errs()),
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(source_string, StrEq("-"));
   EXPECT_THAT(dest_string, StrEq("-"));
 }
@@ -258,7 +257,7 @@ TEST(ArgParserTest, PositionalAppendArgs) {
   llvm::SmallVector<llvm::StringRef> source_strings;
   llvm::SmallVector<llvm::StringRef> dest_strings;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddFlag({.name = "flag"}, [&](auto& arg_b) { arg_b.Set(&flag); });
       b.AddStringOption({.name = "option"},
                         [&](auto& arg_b) { arg_b.Set(&string_option); });
@@ -275,16 +274,15 @@ TEST(ArgParserTest, PositionalAppendArgs) {
   };
 
   TestRawOstream os;
-  EXPECT_THAT(parse({"--flag", "--option=x"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(
-      os.TakeStr(),
-      StrEq("ERROR: Not all required positional arguments were provided. First "
-            "missing and required positional argument: 'sources'\n"));
+  EXPECT_THAT(parse({"--flag", "--option=x"}, os),
+              IsError(StrEq(
+                  "not all required positional arguments were provided; first "
+                  "missing and required positional argument: `sources`")));
 
   EXPECT_THAT(
       parse({"src1", "--flag", "src2", "--option=value", "--", "--dst--"},
             llvm::errs()),
-      Eq(ParseResult::Success));
+      IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(flag);
   EXPECT_THAT(string_option, StrEq("value"));
   EXPECT_THAT(source_strings, ElementsAre(StrEq("src1"), StrEq("src2")));
@@ -294,7 +292,7 @@ TEST(ArgParserTest, PositionalAppendArgs) {
   dest_strings.clear();
   EXPECT_THAT(
       parse({"--", "--src1--", "--src2--", "--", "dst1", "dst2"}, llvm::errs()),
-      Eq(ParseResult::Success));
+      IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(source_strings,
               ElementsAre(StrEq("--src1--"), StrEq("--src2--")));
   EXPECT_THAT(dest_strings, ElementsAre(StrEq("dst1"), StrEq("dst2")));
@@ -302,7 +300,7 @@ TEST(ArgParserTest, PositionalAppendArgs) {
   source_strings.clear();
   dest_strings.clear();
   EXPECT_THAT(parse({"--", "--", "dst1", "dst2"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(source_strings, ElementsAre());
   EXPECT_THAT(dest_strings, ElementsAre(StrEq("dst1"), StrEq("dst2")));
 }
@@ -316,7 +314,7 @@ TEST(ArgParserTest, BasicSubcommands) {
   bool subsub_command = false;
 
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddSubcommand({.name = "sub1"}, [&](auto& sub_b) {
         sub_b.AddFlag({.name = "flag"}, [&](auto& arg_b) { arg_b.Set(&flag); });
         sub_b.AddStringOption({.name = "option"},
@@ -338,39 +336,36 @@ TEST(ArgParserTest, BasicSubcommands) {
   };
 
   TestRawOstream os;
-  EXPECT_THAT(parse({}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq("ERROR: No subcommand specified. Available "
-                                  "subcommands: 'sub1', 'sub2', or 'help'\n"));
+  EXPECT_THAT(parse({}, os),
+              IsError(StrEq("no subcommand specified; available subcommands: "
+                            "`sub1`, `sub2`, or `help`")));
 
-  EXPECT_THAT(parse({"--flag"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq("ERROR: Unknown option '--flag'\n"));
+  EXPECT_THAT(parse({"--flag"}, os), IsError(StrEq("unknown option `--flag`")));
 
-  EXPECT_THAT(parse({"sub3"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq("ERROR: Invalid subcommand 'sub3'. Available "
-                                  "subcommands: 'sub1', 'sub2', or 'help'\n"));
+  EXPECT_THAT(parse({"sub3"}, os),
+              IsError(StrEq("invalid subcommand `sub3`; available subcommands: "
+                            "`sub1`, `sub2`, or `help`")));
 
-  EXPECT_THAT(parse({"--flag", "sub1"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq("ERROR: Unknown option '--flag'\n"));
+  EXPECT_THAT(parse({"--flag", "sub1"}, os),
+              IsError(StrEq("unknown option `--flag`")));
 
   EXPECT_THAT(parse({"sub1", "--flag"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(subcommand, Eq(TestSubcommand::Sub1));
   EXPECT_TRUE(flag);
 
   EXPECT_THAT(parse({"sub2", "--option=value"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(subcommand, Eq(TestSubcommand::Sub2));
   EXPECT_THAT(option1, StrEq("value"));
 
   EXPECT_THAT(parse({"sub2", "--option=abc", "subsub42", "--option=xyz"}, os),
-              Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(),
-              StrEq("ERROR: Invalid subcommand 'subsub42'. Available "
-                    "subcommands: 'subsub', or 'help'\n"));
+              IsError(StrEq("invalid subcommand `subsub42`; available "
+                            "subcommands: `subsub`, or `help`")));
 
   EXPECT_THAT(
       parse({"sub2", "--option=abc", "subsub", "--option=xyz"}, llvm::errs()),
-      Eq(ParseResult::Success));
+      IsSuccess(Eq(ParseResult::Success)));
   EXPECT_TRUE(subsub_command);
   EXPECT_THAT(option1, StrEq("abc"));
   EXPECT_THAT(option2, StrEq("xyz"));
@@ -380,7 +375,7 @@ TEST(ArgParserTest, Appending) {
   llvm::SmallVector<int> integers;
   llvm::SmallVector<llvm::StringRef> strings;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddIntegerOption({.name = "int"},
                          [&](auto& arg_b) { arg_b.Append(&integers); });
       b.AddStringOption({.name = "str"},
@@ -389,22 +384,22 @@ TEST(ArgParserTest, Appending) {
     });
   };
 
-  EXPECT_THAT(parse({}, llvm::errs()), Eq(ParseResult::Success));
+  EXPECT_THAT(parse({}, llvm::errs()), IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(integers, ElementsAre());
   EXPECT_THAT(strings, ElementsAre());
 
   EXPECT_THAT(parse({"--int=1", "--int=2", "--int=3"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(integers, ElementsAre(Eq(1), Eq(2), Eq(3)));
   EXPECT_THAT(strings, ElementsAre());
 
   EXPECT_THAT(parse({"--str=a", "--str=b", "--str=c"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(integers, ElementsAre(Eq(1), Eq(2), Eq(3)));
   EXPECT_THAT(strings, ElementsAre(StrEq("a"), StrEq("b"), StrEq("c")));
 
   EXPECT_THAT(parse({"--str=d", "--int=4", "--str=e"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(integers, ElementsAre(Eq(1), Eq(2), Eq(3), Eq(4)));
   EXPECT_THAT(strings, ElementsAre(StrEq("a"), StrEq("b"), StrEq("c"),
                                    StrEq("d"), StrEq("e")));
@@ -416,7 +411,7 @@ TEST(ArgParserTest, PositionalAppending) {
   llvm::SmallVector<llvm::StringRef> strings2;
   llvm::SmallVector<llvm::StringRef> strings3;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddStringOption({.name = "opt"},
                         [&](auto& arg_b) { arg_b.Append(&option_strings); });
       b.AddStringPositionalArg({.name = "s1"},
@@ -432,7 +427,7 @@ TEST(ArgParserTest, PositionalAppending) {
   EXPECT_THAT(parse({"a", "--opt=x", "b", "--opt=y", "--", "c", "--opt=z", "d",
                      "--", "e", "f"},
                     llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(option_strings, ElementsAre(StrEq("x"), StrEq("y")));
   EXPECT_THAT(strings1, ElementsAre(StrEq("a"), StrEq("b")));
   EXPECT_THAT(strings2, ElementsAre(StrEq("c"), StrEq("--opt=z"), StrEq("d")));
@@ -442,7 +437,7 @@ TEST(ArgParserTest, PositionalAppending) {
 TEST(ArgParserTest, OneOfOption) {
   int value = 0;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddOneOfOption({.name = "option"}, [&](auto& arg_b) {
         arg_b.SetOneOf(
             {
@@ -456,45 +451,48 @@ TEST(ArgParserTest, OneOfOption) {
     });
   };
 
-  EXPECT_THAT(parse({"--option=x"}, llvm::errs()), Eq(ParseResult::Success));
+  EXPECT_THAT(parse({"--option=x"}, llvm::errs()),
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(value, Eq(1));
 
-  EXPECT_THAT(parse({"--option=y"}, llvm::errs()), Eq(ParseResult::Success));
+  EXPECT_THAT(parse({"--option=y"}, llvm::errs()),
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(value, Eq(2));
 
-  EXPECT_THAT(parse({"--option=z"}, llvm::errs()), Eq(ParseResult::Success));
+  EXPECT_THAT(parse({"--option=z"}, llvm::errs()),
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(value, Eq(3));
 
   TestRawOstream os;
 
-  EXPECT_THAT(parse({"--option"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq("ERROR: Option '--option' requires a value "
-                                  "to be provided and none was.\n"));
+  EXPECT_THAT(
+      parse({"--option"}, os),
+      IsError(StrEq(
+          "option `--option` requires a value to be provided and none was")));
 
   constexpr const char* ErrorStr =
-      "ERROR: Option '--option={0}' has an invalid value '{0}'; valid values "
-      "are: 'x', 'y', or 'z'\n";
-  EXPECT_THAT(parse({"--option=a"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq(llvm::formatv(ErrorStr, "a")));
+      "option `--option={0}` has an invalid value `{0}`; valid values are: "
+      "`x`, `y`, or `z`";
+  EXPECT_THAT(parse({"--option=a"}, os),
+              IsError(StrEq(llvm::formatv(ErrorStr, "a"))));
 
-  EXPECT_THAT(parse({"--option="}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq(llvm::formatv(ErrorStr, "")));
+  EXPECT_THAT(parse({"--option="}, os),
+              IsError(StrEq(llvm::formatv(ErrorStr, ""))));
 
-  EXPECT_THAT(parse({"--option=xx"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq(llvm::formatv(ErrorStr, "xx")));
+  EXPECT_THAT(parse({"--option=xx"}, os),
+              IsError(StrEq(llvm::formatv(ErrorStr, "xx"))));
 
-  EXPECT_THAT(parse({"--option=\xFF"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq(llvm::formatv(ErrorStr, "\\FF")));
+  EXPECT_THAT(parse({"--option=\xFF"}, os),
+              IsError(StrEq(llvm::formatv(ErrorStr, "\\FF"))));
 
   EXPECT_THAT(parse({llvm::StringRef("--option=\0", 10)}, os),
-              Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq(llvm::formatv(ErrorStr, "\\00")));
+              IsError(StrEq(llvm::formatv(ErrorStr, "\\00"))));
 }
 
 TEST(ArgParserTest, OneOfOptionAppending) {
   llvm::SmallVector<int> values;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddOneOfOption({.name = "option"}, [&](auto& arg_b) {
         arg_b.AppendOneOf(
             {
@@ -508,15 +506,15 @@ TEST(ArgParserTest, OneOfOptionAppending) {
     });
   };
 
-  EXPECT_THAT(parse({}, llvm::errs()), Eq(ParseResult::Success));
+  EXPECT_THAT(parse({}, llvm::errs()), IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(values, ElementsAre());
 
   EXPECT_THAT(parse({"--option=x", "--option=y", "--option=z"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(values, ElementsAre(Eq(1), Eq(2), Eq(3)));
 
   EXPECT_THAT(parse({"--option=y", "--option=y", "--option=x"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(values, ElementsAre(Eq(1), Eq(2), Eq(3), Eq(2), Eq(2), Eq(1)));
 }
 
@@ -530,7 +528,7 @@ TEST(ArgParserTest, RequiredArgs) {
   TestEnum enum_sub_option;
 
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
-    return Parse(args, s, s, TestCommandInfo, [&](auto& b) {
+    return Parse(args, s, TestCommandInfo, [&](auto& b) {
       b.AddIntegerOption({.name = "opt1"}, [&](auto& arg_b) {
         arg_b.Required(true);
         arg_b.Set(&integer_option);
@@ -574,46 +572,35 @@ TEST(ArgParserTest, RequiredArgs) {
     });
   };
 
-  constexpr const char* ErrorStr =
-      "ERROR: Required option '{0}' not provided.\n";
   TestRawOstream os;
-  EXPECT_THAT(parse({}, os), Eq(ParseResult::Error));
-  auto errors = os.TakeStr();
-  EXPECT_THAT(errors, HasSubstr(llvm::formatv(ErrorStr, "--opt1")));
-  EXPECT_THAT(errors, HasSubstr(llvm::formatv(ErrorStr, "--opt2")));
+  EXPECT_THAT(parse({}, os),
+              IsError(StrEq("required options not provided: --opt1, --opt2")));
 
-  EXPECT_THAT(parse({"--opt2=xyz"}, os), Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq(llvm::formatv(ErrorStr, "--opt1")));
+  EXPECT_THAT(parse({"--opt2=xyz"}, os),
+              IsError(StrEq("required options not provided: --opt1")));
 
   EXPECT_THAT(parse({"--opt2=xyz", "--opt1=42"}, llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(integer_option, Eq(42));
   EXPECT_THAT(string_option, StrEq("xyz"));
 
   EXPECT_THAT(parse({"--opt2=xyz", "--opt1=42", "sub2"}, os),
-              Eq(ParseResult::Error));
-  errors = os.TakeStr();
-  EXPECT_THAT(errors, HasSubstr(llvm::formatv(ErrorStr, "--sub-opt1")));
-  EXPECT_THAT(errors, HasSubstr(llvm::formatv(ErrorStr, "--sub-opt2")));
-  EXPECT_THAT(errors, HasSubstr(llvm::formatv(ErrorStr, "--sub-opt3")));
-
-  EXPECT_THAT(parse({"--opt2=xyz", "--opt1=42", "sub2", "--sub-opt3=b"}, os),
-              Eq(ParseResult::Error));
-  errors = os.TakeStr();
-  EXPECT_THAT(errors, HasSubstr(llvm::formatv(ErrorStr, "--sub-opt1")));
-  EXPECT_THAT(errors, HasSubstr(llvm::formatv(ErrorStr, "--sub-opt2")));
-  EXPECT_THAT(errors, Not(HasSubstr(llvm::formatv(ErrorStr, "--sub-opt3"))));
+              IsError(StrEq("required options not provided: --sub-opt1, "
+                            "--sub-opt2, --sub-opt3")));
+
+  EXPECT_THAT(
+      parse({"--opt2=xyz", "--opt1=42", "sub2", "--sub-opt3=b"}, os),
+      IsError(StrEq("required options not provided: --sub-opt1, --sub-opt2")));
 
   EXPECT_THAT(parse({"--opt2=xyz", "--opt1=42", "sub2", "--sub-opt3=b",
                      "--sub-opt1=13"},
                     os),
-              Eq(ParseResult::Error));
-  EXPECT_THAT(os.TakeStr(), StrEq(llvm::formatv(ErrorStr, "--sub-opt2")));
+              IsError(StrEq("required options not provided: --sub-opt2")));
 
   EXPECT_THAT(parse({"--opt2=xyz", "--opt1=42", "sub2", "--sub-opt3=b",
                      "--sub-opt1=13", "--sub-opt2=abc"},
                     llvm::errs()),
-              Eq(ParseResult::Success));
+              IsSuccess(Eq(ParseResult::Success)));
   EXPECT_THAT(integer_option, Eq(42));
   EXPECT_THAT(string_option, StrEq("xyz"));
   EXPECT_THAT(subcommand, Eq(TestSubcommand::Sub2));
@@ -628,7 +615,7 @@ TEST(ArgParserTest, HelpAndVersion) {
   llvm::StringRef string = "";
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
     return Parse(  // Force line break.
-        args, s, s,
+        args, s,
         {
             .name = "test",
             .version = "Test Command -- version 1.2.3",
@@ -747,7 +734,8 @@ A hidden subcommand.
 
   TestRawOstream os;
 
-  EXPECT_THAT(parse({"--flag", "--help"}, os), Eq(ParseResult::MetaSuccess));
+  EXPECT_THAT(parse({"--flag", "--help"}, os),
+              IsSuccess(Eq(ParseResult::MetaSuccess)));
   std::string help_flag_output = os.TakeStr();
   EXPECT_THAT(help_flag_output, StrEq(llvm::StringRef(R"""(
 Test Command -- version 1.2.3
@@ -789,10 +777,11 @@ Closing remarks.
 
 )""")
                                           .ltrim('\n')));
-  EXPECT_THAT(parse({"help"}, os), Eq(ParseResult::MetaSuccess));
+  EXPECT_THAT(parse({"help"}, os), IsSuccess(Eq(ParseResult::MetaSuccess)));
   EXPECT_THAT(os.TakeStr(), StrEq(help_flag_output));
 
-  EXPECT_THAT(parse({"--version"}, os), Eq(ParseResult::MetaSuccess));
+  EXPECT_THAT(parse({"--version"}, os),
+              IsSuccess(Eq(ParseResult::MetaSuccess)));
   std::string version_flag_output = os.TakeStr();
   EXPECT_THAT(version_flag_output, StrEq(llvm::StringRef(R"""(
 Test Command -- version 1.2.3
@@ -801,21 +790,21 @@ Build timestamp: )""" __TIMESTAMP__ R"""(
 Build config: test-config-info
 )""")
                                              .ltrim('\n')));
-  EXPECT_THAT(parse({"version"}, os), Eq(ParseResult::MetaSuccess));
+  EXPECT_THAT(parse({"version"}, os), IsSuccess(Eq(ParseResult::MetaSuccess)));
   EXPECT_THAT(os.TakeStr(), StrEq(version_flag_output));
 
   EXPECT_THAT(parse({"--flag", "edit", "--option=42", "--help"}, os),
-              Eq(ParseResult::MetaSuccess));
+              IsSuccess(Eq(ParseResult::MetaSuccess)));
   std::string edit_help_output = os.TakeStr();
   EXPECT_THAT(edit_help_output, StrEq(llvm::StringRef(R"""(
 Edit the widget.
 
 This will take the provided widgets and edit them.
 
-Subcommand 'edit' usage:
+Subcommand `edit` usage:
   test [-f] edit [--option=...]
 
-Subcommand 'edit' options:
+Subcommand `edit` options:
   -o, --option=...
           An integer option.
 
@@ -831,21 +820,22 @@ That's all.
 )""")
                                           .ltrim('\n')));
 
-  EXPECT_THAT(parse({"help", "edit"}, os), Eq(ParseResult::MetaSuccess));
+  EXPECT_THAT(parse({"help", "edit"}, os),
+              IsSuccess(Eq(ParseResult::MetaSuccess)));
   EXPECT_THAT(os.TakeStr(), StrEq(edit_help_output));
 
   EXPECT_THAT(parse({"--flag", "run", "--option=abc", "--help"}, os),
-              Eq(ParseResult::MetaSuccess));
+              IsSuccess(Eq(ParseResult::MetaSuccess)));
   std::string run_help_output = os.TakeStr();
   EXPECT_THAT(run_help_output, StrEq(llvm::StringRef(R"""(
 Run wombats across the screen.
 
 This will cause several wombats to run across your screen.
 
-Subcommand 'run' usage:
+Subcommand `run` usage:
   test [-f] run [OPTIONS]
 
-Subcommand 'run' options:
+Subcommand `run` options:
   -o, --option=...
           A string option.
 
@@ -869,7 +859,8 @@ Or it won't, who knows.
 )""")
                                          .ltrim('\n')));
 
-  EXPECT_THAT(parse({"help", "run"}, os), Eq(ParseResult::MetaSuccess));
+  EXPECT_THAT(parse({"help", "run"}, os),
+              IsSuccess(Eq(ParseResult::MetaSuccess)));
   EXPECT_THAT(os.TakeStr(), StrEq(run_help_output));
 }
 
@@ -877,7 +868,7 @@ TEST(ArgParserTest, HelpMarkdownLike) {
   bool flag = false;
   auto parse = [&](llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s) {
     return Parse(  // Force line break.
-        args, s, s, {.name = "test"}, [&](auto& b) {
+        args, s, {.name = "test"}, [&](auto& b) {
           b.AddFlag(
               {
                   .name = "flag",
@@ -907,7 +898,7 @@ x
   };
 
   TestRawOstream os;
-  EXPECT_THAT(parse({"--help"}, os), Eq(ParseResult::MetaSuccess));
+  EXPECT_THAT(parse({"--help"}, os), IsSuccess(Eq(ParseResult::MetaSuccess)));
   EXPECT_THAT(os.TakeStr(), StrEq(llvm::StringRef(R"""(
 Usage:
   test [--flag]

common/error.h

@@ -17,7 +17,9 @@ namespace Carbon {
 // Success values should be represented as the presence of a value in ErrorOr,
 // using `ErrorOr<Success>` and `return Success();` if no value needs to be
 // returned.
-struct Success {};
+struct Success : public Printable<Success> {
+  void Print(llvm::raw_ostream& out) const { out << "Success"; }
+};
 
 // Tracks an error message.
 //

common/error_test.cpp

@@ -6,11 +6,16 @@
 
 #include <gtest/gtest.h>
 
+#include "common/error_test_helpers.h"
 #include "testing/base/test_raw_ostream.h"
 
 namespace Carbon {
 namespace {
 
+using ::Carbon::Testing::IsError;
+using ::Carbon::Testing::IsSuccess;
+using ::testing::Eq;
+
 TEST(ErrorTest, Error) {
   Error err("test");
   EXPECT_EQ(err.message(), "test");
@@ -26,8 +31,8 @@ TEST(ErrorTest, IndirectError) { EXPECT_EQ(IndirectError().message(), "test"); }
 
 TEST(ErrorTest, ErrorOr) {
   ErrorOr<int> err(Error("test"));
-  EXPECT_FALSE(err.ok());
-  EXPECT_EQ(err.error().message(), "test");
+
+  EXPECT_THAT(err, IsError("test"));
 }
 
 TEST(ErrorTest, ErrorOrValue) { EXPECT_TRUE(ErrorOr<int>(0).ok()); }
@@ -66,8 +71,7 @@ TEST(ErrorTest, ReturnIfErrorHasError) {
     CARBON_RETURN_IF_ERROR(ErrorOr<Success>(Error("error")));
     return Success();
   }();
-  ASSERT_FALSE(result.ok());
-  EXPECT_EQ(result.error().message(), "error");
+  EXPECT_THAT(result, IsError("error"));
 }
 
 TEST(ErrorTest, AssignOrReturnNoError) {
@@ -78,8 +82,7 @@ TEST(ErrorTest, AssignOrReturnNoError) {
     CARBON_ASSIGN_OR_RETURN(c, ErrorOr<int>(3));
     return a + b + c;
   }();
-  ASSERT_TRUE(result.ok());
-  EXPECT_EQ(6, *result);
+  EXPECT_THAT(result, IsSuccess(Eq(6)));
 }
 
 TEST(ErrorTest, AssignOrReturnHasDirectError) {
@@ -87,7 +90,7 @@ TEST(ErrorTest, AssignOrReturnHasDirectError) {
     CARBON_RETURN_IF_ERROR(ErrorOr<int>(Error("error")));
     return 0;
   }();
-  ASSERT_FALSE(result.ok());
+  EXPECT_THAT(result, IsError("error"));
 }
 
 TEST(ErrorTest, AssignOrReturnHasErrorInExpected) {
@@ -95,14 +98,12 @@ TEST(ErrorTest, AssignOrReturnHasErrorInExpected) {
     CARBON_ASSIGN_OR_RETURN(int a, ErrorOr<int>(Error("error")));
     return a;
   }();
-  ASSERT_FALSE(result.ok());
-  EXPECT_EQ(result.error().message(), "error");
+  EXPECT_THAT(result, IsError("error"));
 }
 
 TEST(ErrorTest, ErrorBuilderOperatorImplicitCast) {
   ErrorOr<int> result = ErrorBuilder() << "msg";
-  ASSERT_FALSE(result.ok());
-  EXPECT_EQ(result.error().message(), "msg");
+  EXPECT_THAT(result, IsError("msg"));
 }
 
 TEST(ErrorTest, StreamError) {

common/error_test_helpers.h

@@ -0,0 +1,111 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_COMMON_ERROR_TEST_HELPERS_H_
+#define CARBON_COMMON_ERROR_TEST_HELPERS_H_
+
+#include <gmock/gmock.h>
+
+#include "common/error.h"
+
+namespace Carbon::Testing {
+
+// Matches the message for an error state of `ErrorOr<T>`. For example:
+//   EXPECT_THAT(my_result, IsError(StrEq("error message")));
+class IsError {
+ public:
+  // NOLINTNEXTLINE(readability-identifier-naming)
+  using is_gtest_matcher = void;
+
+  explicit IsError(::testing::Matcher<std::string> matcher)
+      : matcher_(std::move(matcher)) {}
+
+  template <typename T>
+  auto MatchAndExplain(const ErrorOr<T>& result,
+                       ::testing::MatchResultListener* listener) const -> bool {
+    if (result.ok()) {
+      *listener->stream() << "is a success";
+      return false;
+    } else {
+      return matcher_.MatchAndExplain(result.error().message(), listener);
+    }
+  }
+
+  auto DescribeTo(std::ostream* os) const -> void {
+    *os << "is an error and matches ";
+    matcher_.DescribeTo(os);
+  }
+
+  auto DescribeNegationTo(std::ostream* os) const -> void {
+    *os << "is a success or does not match ";
+    matcher_.DescribeTo(os);
+  }
+
+ private:
+  ::testing::Matcher<std::string> matcher_;
+};
+
+// Matches the value for a non-error state of `ErrorOr<T>`. For example:
+//   EXPECT_THAT(my_result, IsSuccess(Eq(3)));
+template <typename InnerMatcher>
+class IsSuccessMatcher {
+ public:
+  // NOLINTNEXTLINE(readability-identifier-naming)
+  using is_gtest_matcher = void;
+
+  explicit IsSuccessMatcher(InnerMatcher matcher)
+      : matcher_(std::move(matcher)) {}
+
+  template <typename T>
+  auto MatchAndExplain(const ErrorOr<T>& result,
+                       ::testing::MatchResultListener* listener) const -> bool {
+    if (result.ok()) {
+      return ::testing::Matcher<T>(matcher_).MatchAndExplain(*result, listener);
+    } else {
+      *listener->stream() << "is an error with `" << result.error().message()
+                          << "`";
+      return false;
+    }
+  }
+
+  auto DescribeTo(std::ostream* os) const -> void {
+    *os << "is a success and matches ";
+    matcher_.DescribeTo(os);
+  }
+
+  auto DescribeNegationTo(std::ostream* os) const -> void {
+    *os << "is an error or does not match ";
+    matcher_.DescribeTo(os);
+  }
+
+ private:
+  InnerMatcher matcher_;
+};
+
+// Wraps `IsSuccessMatcher` for the inner matcher deduction.
+template <typename InnerMatcher>
+auto IsSuccess(InnerMatcher matcher) -> IsSuccessMatcher<InnerMatcher> {
+  return IsSuccessMatcher<InnerMatcher>(matcher);
+}
+
+}  // namespace Carbon::Testing
+
+namespace Carbon {
+
+// Supports printing `ErrorOr<T>` to `std::ostream` in tests.
+template <typename T>
+auto operator<<(std::ostream& out, const ErrorOr<T>& error_or)
+    -> std::ostream& {
+  if (error_or.ok()) {
+    out << llvm::formatv("ErrorOr{{.value = `{0}`}}", *error_or);
+  } else {
+    out << llvm::formatv("ErrorOr{{.error = \"{0}\"}}",
+                         error_or.error().message());
+  }
+  return out;
+}
+
+}  // namespace Carbon
+
+#endif  // CARBON_COMMON_ERROR_TEST_HELPERS_H_

testing/base/source_gen_main.cpp

@@ -54,9 +54,8 @@ auto Run(llvm::ArrayRef<llvm::StringRef> args) -> bool {
   int lines = 10'000;
   SourceGen::Language language;
 
-  CommandLine::ParseResult parsed_args = CommandLine::Parse(
-      args, llvm::outs(), llvm::errs(), Info,
-      [&](CommandLine::CommandBuilder& b) {
+  auto parse_result = CommandLine::Parse(
+      args, llvm::outs(), Info, [&](CommandLine::CommandBuilder& b) {
         b.AddStringOption(OutputArgInfo,
                           [&](auto& arg_b) { arg_b.Set(&output_filename); });
         b.AddIntegerOption(LinesArgInfo,
@@ -74,9 +73,10 @@ auto Run(llvm::ArrayRef<llvm::StringRef> args) -> bool {
         // No-op action as there is only one operation for this command.
         b.Do([] {});
       });
-  if (parsed_args == CommandLine::ParseResult::Error) {
+  if (!parse_result.ok()) {
+    llvm::errs() << "error: " << *parse_result << "\n";
     return false;
-  } else if (parsed_args == CommandLine::ParseResult::MetaSuccess) {
+  } else if (*parse_result == CommandLine::ParseResult::MetaSuccess) {
     // Fully handled by the CLI library.
     return true;
   }

toolchain/driver/driver.cpp

@@ -84,19 +84,21 @@ auto Options::Build(CommandLine::CommandBuilder& b) -> void {
 
 auto Driver::RunCommand(llvm::ArrayRef<llvm::StringRef> args) -> DriverResult {
   if (driver_env_.installation->error()) {
-    llvm::errs() << "error: " << *driver_env_.installation->error() << "\n";
+    driver_env_.error_stream << "error: " << *driver_env_.installation->error()
+                             << "\n";
     return {.success = false};
   }
 
   Options options;
 
-  CommandLine::ParseResult result = CommandLine::Parse(
-      args, driver_env_.output_stream, driver_env_.error_stream, Options::Info,
+  ErrorOr<CommandLine::ParseResult> result = CommandLine::Parse(
+      args, driver_env_.output_stream, Options::Info,
       [&](CommandLine::CommandBuilder& b) { options.Build(b); });
 
-  if (result == CommandLine::ParseResult::Error) {
+  if (!result.ok()) {
+    driver_env_.error_stream << "error: " << result.error() << "\n";
     return {.success = false};
-  } else if (result == CommandLine::ParseResult::MetaSuccess) {
+  } else if (*result == CommandLine::ParseResult::MetaSuccess) {
     return {.success = true};
   }
 

toolchain/driver/driver_test.cpp

@@ -105,13 +105,13 @@ class DriverTest : public testing::Test {
 
 TEST_F(DriverTest, BadCommandErrors) {
   EXPECT_FALSE(driver_.RunCommand({}).success);
-  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("error:"));
 
   EXPECT_FALSE(driver_.RunCommand({"foo"}).success);
-  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("error:"));
 
   EXPECT_FALSE(driver_.RunCommand({"foo --bar --baz"}).success);
-  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("ERROR"));
+  EXPECT_THAT(test_error_stream_.TakeStr(), HasSubstr("error:"));
 }
 
 TEST_F(DriverTest, CompileCommandErrors) {
@@ -119,8 +119,8 @@ TEST_F(DriverTest, CompileCommandErrors) {
   EXPECT_FALSE(driver_.RunCommand({"compile"}).success);
   EXPECT_THAT(
       test_error_stream_.TakeStr(),
-      StrEq("ERROR: Not all required positional arguments were provided. First "
-            "missing and required positional argument: 'FILE'\n"));
+      StrEq("error: not all required positional arguments were provided; first "
+            "missing and required positional argument: `FILE`\n"));
 
   // Invalid output filename. No reliably error message here.
   // TODO: Likely want a different filename on Windows.

toolchain/driver/testdata/fail_flag.carbon

@@ -0,0 +1,12 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// ARGS: compile --non-existent-flag
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/fail_flag.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/fail_flag.carbon
+// CHECK:STDERR: error: unknown option `--non-existent-flag`