Merge pull request #2080 from hzeller/20240128-improve-err-message

LanguageServer Improve error messages in symbol table log

common/util/file_util.cc

@@ -79,21 +79,22 @@ static absl::Status CreateErrorStatusFromSysError(absl::string_view filename,
                                                   const char *fallback_msg) {
   const char *const system_msg =
       sys_error == 0 ? fallback_msg : strerror(sys_error);
-  const std::string msg = filename.empty()
-                              ? std::string{system_msg}
-                              : absl::StrCat(filename, ": ", system_msg);
+  if (filename.empty()) filename = "<empty-filename>";
+  std::string msg = absl::StrCat(filename, ": ", system_msg);
   switch (sys_error) {
     case EPERM:
     case EACCES:
       return {absl::StatusCode::kPermissionDenied, msg};
     case ENOENT:
+    case ESRCH:  // Win32 returns this for fs::status() on non-existing file.
       return {absl::StatusCode::kNotFound, msg};
     case EEXIST:
       return {absl::StatusCode::kAlreadyExists, msg};
     case EINVAL:
     case EISDIR:
       return {absl::StatusCode::kInvalidArgument, msg};
     default:
+      absl::StrAppend(&msg, " (sys_error=", sys_error, ")");
       return {absl::StatusCode::kUnknown, msg};
   }
 }
@@ -127,15 +128,17 @@ absl::Status UpwardFileSearch(absl::string_view start,
     if (one_up == probe_dir) break;
     probe_dir = one_up;
   }
-  return absl::NotFoundError("No matching file found.");
+  return absl::NotFoundError(absl::StrCat("UpwardFileSearch: starting from '",
+                                          start, "', no file '", filename,
+                                          "' found'"));
 }
 
 absl::Status FileExists(const std::string &filename) {
   std::error_code err;
-  fs::file_status stat = fs::status(filename, err);
+  const fs::file_status stat = fs::status(filename, err);
 
   if (err.value() != 0) {
-    return absl::NotFoundError(absl::StrCat(filename, ": ", err.message()));
+    return CreateErrorStatusFromErr(filename, err, "file exists check");
   }
 
   if (fs::is_regular_file(stat) || fs::is_fifo(stat)) {

common/util/file_util_test.cc

@@ -248,6 +248,12 @@ TEST(FileUtil, ScopedTestFileEmplace) {
   }
 }
 
+TEST(FileUtil, FileNotExistsTests) {
+  absl::Status s = file::FileExists("not/an/existing/file");
+  EXPECT_FALSE(s.ok());
+  EXPECT_EQ(s.code(), absl::StatusCode::kNotFound);
+}
+
 TEST(FileUtil, FileExistsDirectoryErrorMessage) {
   absl::Status s;
   s = file::FileExists(testing::TempDir());

verilog/analysis/symbol_table_test.cc

@@ -8252,7 +8252,8 @@ TEST(BuildSymbolTableTest, ModuleInstancesFromProjectMissingFile) {
   symbol_table.BuildSingleTranslationUnit("file/not/found.txt",
                                           &build_diagnostics);
   ASSERT_FALSE(build_diagnostics.empty());
-  EXPECT_THAT(build_diagnostics.front().code(), absl::StatusCode::kNotFound);
+  EXPECT_THAT(build_diagnostics.front().code(), absl::StatusCode::kNotFound)
+      << build_diagnostics.front();
 }
 
 TEST(BuildSymbolTableTest, ModuleInstancesFromProjectFilesGood) {

verilog/analysis/verilog_project.cc

@@ -22,6 +22,7 @@
 #include <vector>
 
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_join.h"
 #include "absl/strings/string_view.h"
@@ -301,14 +302,14 @@ absl::StatusOr<VerilogSourceFile *> VerilogProject::OpenTranslationUnit(
 
 absl::Status VerilogProject::IncludeFileNotFoundError(
     absl::string_view referenced_filename) const {
-  return absl::NotFoundError(absl::StrCat(
-      "Unable to find '", referenced_filename,
-      "' among the included paths: ", absl::StrJoin(include_paths_, ", ")));
+  return absl::NotFoundError(
+      absl::StrCat("'", referenced_filename, "' not in any of the ",
+                   include_paths_.size(), " include paths"));
 }
 
 absl::StatusOr<VerilogSourceFile *> VerilogProject::OpenIncludedFile(
     absl::string_view referenced_filename) {
-  VLOG(1) << __FUNCTION__ << ", referenced: " << referenced_filename;
+  VLOG(2) << __FUNCTION__ << ", referenced: " << referenced_filename;
   // Check for a pre-existing entry to avoid duplicate files.
   {
     const auto opened_file = FindOpenedFile(referenced_filename);
@@ -329,18 +330,16 @@ absl::StatusOr<VerilogSourceFile *> VerilogProject::OpenIncludedFile(
 
   // Locate the file among the base paths.
   for (const auto &include_path : include_paths_) {
-    const std::string resolved_filename =
+    const std::string resolved =
         verible::file::JoinPath(include_path, referenced_filename);
-    if (verible::file::FileExists(resolved_filename).ok()) {
-      VLOG(2) << "File '" << resolved_filename << "' exists. Resolved from '"
-              << referenced_filename << "'";
-      return OpenFile(referenced_filename, resolved_filename, Corpus());
+    if (verible::file::FileExists(resolved).ok()) {
+      VLOG(2) << referenced_filename << " in incdir '" << resolved << "'";
+      return OpenFile(referenced_filename, resolved, Corpus());
     }
-    VLOG(2) << "Checked for file'" << resolved_filename
-            << "', but not found. Resolved from '" << referenced_filename
-            << "'";
+    VLOG(2) << referenced_filename << " not in incdir '" << resolved << "'";
   }
 
+  VLOG(1) << __FUNCTION__ << "': '" << referenced_filename << "' not found";
   // Not found in any path.  Cache this status.
   const auto inserted = files_.emplace(
       referenced_filename,