Repository: carbon-language/carbon-lang
File: testing/file_test/test_file.cpp (C++)

Code before:

  auto add_content(llvm::StringRef line) -> void {
    content.append(line.str());
    content.append("\n");
  }

  // Whether content has been found. Only updated before a file split is found
  // (which may be never).
  bool found_code_pre_split = false;

  // The current file name, considering splits. Empty for the default file.
  llvm::StringRef filename = "";

  // The accumulated content for the file being built. This may elide some of
  // the original content, such as conflict markers.
  std::string content;

  // The current file index.
  int file_index = 0;
};

static auto ExtractFilePathFromUri(llvm::StringRef uri)
    -> std::optional<llvm::StringRef> {
  static constexpr llvm::StringRef FilePrefix = "file:/";
  if (!uri.starts_with(FilePrefix)) {
    return std::nullopt;
  }
  return uri.drop_front(FilePrefix.size());
}

static auto AutoFillDidOpenParams(llvm::json::Object* params,
                                  llvm::ArrayRef<TestFile::Split> splits)
    -> ErrorOr<Success> {
  auto* text_document = params->getObject("textDocument");
  if (text_document == nullptr) {
    return Success{};
  }

  auto attr_it = text_document->find("text");
  if (attr_it == text_document->end() || attr_it->second != "FROM_FILE_SPLIT") {
    return Success{};
  }

  auto uri = text_document->getString("uri");
  if (!uri) {
    return ErrorBuilder() << "missing uri in params.textDocument";
  }

  auto file_path = ExtractFilePathFromUri(*uri);
  if (!file_path) {
    return ErrorBuilder() << "uri `" << *uri << "` is not a file uri";

Review comment (refactor):
Rather than generating the error at the callsite, had you considered using `ErrorOr` here? For example:

```suggestion
    -> ErrorOr<llvm::StringRef> {
  static constexpr llvm::StringRef FilePrefix = "file:/";
  if (!uri.starts_with(FilePrefix)) {
    return ErrorBuilder() << "uri `" << *uri << "` is not a file uri";
```


At the callsite, this should allow reducing the call to just a macro assignment (just dropping a couple LOC overall):

```
  CARBON_ASSIGN_OR_RETURN(auto file_path, ExtractFilePathFromUri(*uri));
```

(I've made a corresponding suggestion below)

Code after:
  auto has_splits() const -> bool { return file_index > 0; }

  auto add_content(llvm::StringRef line) -> void {
    content.append(line.str());
    content.append("\n");
  }

  // Whether content has been found. Only updated before a file split is found
  // (which may be never).
  bool found_code_pre_split = false;

  // The current file name, considering splits. Empty for the default file.
  llvm::StringRef filename = "";

  // The accumulated content for the file being built. This may elide some of
  // the original content, such as conflict markers.
  std::string content;

  // The current file index.
  int file_index = 0;
};

// Given a `file:/<filename>` URI, returns the filename.
static auto ExtractFilePathFromUri(llvm::StringRef uri)
    -> ErrorOr<llvm::StringRef> {
  static constexpr llvm::StringRef FilePrefix = "file:/";
  if (!uri.starts_with(FilePrefix)) {
    return ErrorBuilder("uri `") << uri << "` is not a file uri";
  }
  return uri.drop_front(FilePrefix.size());
}

// When `FROM_FILE_SPLIT` is used in path `textDocument.text`, populate the
// value from the split matching the `uri`. Only used for
// `textDocument/didOpen`.
static auto AutoFillDidOpenParams(llvm::json::Object* params,
                                  llvm::ArrayRef<TestFile::Split> splits)
    -> ErrorOr<Success> {
  auto* text_document = params->getObject("textDocument");
  if (text_document == nullptr) {
    return Success();
  }

  auto attr_it = text_document->find("text");
  if (attr_it == text_document->end() || attr_it->second != "FROM_FILE_SPLIT") {
    return Success();
  }

  auto uri = text_document->getString("uri");
  if (!uri) {
    return Error("missing uri in params.textDocument");