refactor(diags): delete Diag_Reporter::report_impl

Remove the virtual function in the base class, and also inline the implementations which are called only in one place.
quick-lint · Sep 20, 2024 · cfc8d3d · cfc8d3d
1 parent de12486
commit cfc8d3d
Show file tree

Hide file tree

Showing 16 changed files with 51 additions and 99 deletions.
diff --git a/plugin/vscode/quick-lint-js/vscode/vscode-diag-reporter.h b/plugin/vscode/quick-lint-js/vscode/vscode-diag-reporter.h
@@ -143,21 +143,17 @@ class VSCode_Diag_Reporter final : public Diag_Reporter {
 
   void report(const Diag_List& diags) override {
     diags.for_each([&](Diag_Type type, void* diag) -> void {
-      this->report_impl(type, diag);
+      VSCode_Diag_Formatter formatter(
+          /*vscode=*/this->vscode_,
+          /*env=*/this->env_,
+          /*diagnostics=*/this->diagnostics_,
+          /*locator=*/this->locator_,
+          /*document_uri=*/this->document_uri_,
+          /*message_translator=*/this->message_translator_);
+      formatter.format(get_diagnostic_info(type), diag);
     });
   }
 
-  void report_impl(Diag_Type type, void* diag) override {
-    VSCode_Diag_Formatter formatter(
-        /*vscode=*/this->vscode_,
-        /*env=*/this->env_,
-        /*diagnostics=*/this->diagnostics_,
-        /*locator=*/this->locator_,
-        /*document_uri=*/this->document_uri_,
-        /*message_translator=*/this->message_translator_);
-    formatter.format(get_diagnostic_info(type), diag);
-  }
-
  private:
   VSCode_Module* vscode_;
   ::Napi::Env env_;

diff --git a/src/quick-lint-js/c-api-diag-reporter.cpp b/src/quick-lint-js/c-api-diag-reporter.cpp
@@ -42,17 +42,11 @@ void C_API_Diag_Reporter<Diagnostic, Locator>::set_translator(Translator t) {
 template <class Diagnostic, class Locator>
 void C_API_Diag_Reporter<Diagnostic, Locator>::report(const Diag_List &diags) {
   diags.for_each([&](Diag_Type type, void *diag) -> void {
-    this->report_impl(type, diag);
+    C_API_Diag_Formatter formatter(this);
+    formatter.format(get_diagnostic_info(type), diag);
   });
 }
 
-template <class Diagnostic, class Locator>
-void C_API_Diag_Reporter<Diagnostic, Locator>::report_impl(Diag_Type type,
-                                                           void *diag) {
-  C_API_Diag_Formatter formatter(this);
-  formatter.format(get_diagnostic_info(type), diag);
-}
-
 template <class Diagnostic, class Locator>
 const Diagnostic *C_API_Diag_Reporter<Diagnostic, Locator>::get_diagnostics() {
   // Null-terminate the returned diagnostics.

diff --git a/src/quick-lint-js/c-api-diag-reporter.h b/src/quick-lint-js/c-api-diag-reporter.h
@@ -36,7 +36,6 @@ class C_API_Diag_Reporter final : public Diag_Reporter {
   const Diagnostic *get_diagnostics();
 
   void report(const Diag_List &) override;
-  void report_impl(Diag_Type type, void *diag) override;
 
  private:
   Char8 *allocate_c_string(String8_View);

diff --git a/src/quick-lint-js/cli/emacs-lisp-diag-reporter.cpp b/src/quick-lint-js/cli/emacs-lisp-diag-reporter.cpp
@@ -25,19 +25,15 @@ void Emacs_Lisp_Diag_Reporter::set_source(Padded_String_View input) {
 }
 
 void Emacs_Lisp_Diag_Reporter::report(const Diag_List &diags) {
+  QLJS_ASSERT(this->locator_.has_value());
   diags.for_each([&](Diag_Type type, void *diag) -> void {
-    this->report_impl(type, diag);
+    Emacs_Lisp_Diag_Formatter formatter(this->translator_,
+                                        /*output=*/&this->output_,
+                                        /*locator=*/*this->locator_);
+    formatter.format(get_diagnostic_info(type), diag);
   });
 }
 
-void Emacs_Lisp_Diag_Reporter::report_impl(Diag_Type type, void *diag) {
-  QLJS_ASSERT(this->locator_.has_value());
-  Emacs_Lisp_Diag_Formatter formatter(this->translator_,
-                                      /*output=*/&this->output_,
-                                      /*locator=*/*this->locator_);
-  formatter.format(get_diagnostic_info(type), diag);
-}
-
 Emacs_Lisp_Diag_Formatter::Emacs_Lisp_Diag_Formatter(Translator t,
                                                      Output_Stream *output,
                                                      Emacs_Locator &locator)

diff --git a/src/quick-lint-js/cli/emacs-lisp-diag-reporter.h b/src/quick-lint-js/cli/emacs-lisp-diag-reporter.h
@@ -26,7 +26,6 @@ class Emacs_Lisp_Diag_Reporter final : public Diag_Reporter {
   void finish();
 
   void report(const Diag_List &) override;
-  void report_impl(Diag_Type type, void *diag) override;
 
  private:
   Output_Stream &output_;

diff --git a/src/quick-lint-js/cli/text-diag-reporter.cpp b/src/quick-lint-js/cli/text-diag-reporter.cpp
@@ -29,21 +29,17 @@ void Text_Diag_Reporter::set_source(Padded_String_View input,
 }
 
 void Text_Diag_Reporter::report(const Diag_List &diags) {
-  diags.for_each([&](Diag_Type type, void *diag) -> void {
-    this->report_impl(type, diag);
-  });
-}
-
-void Text_Diag_Reporter::report_impl(Diag_Type type, void *diag) {
   QLJS_ASSERT(this->file_path_);
   QLJS_ASSERT(this->locator_.has_value());
-  Text_Diag_Formatter formatter(
-      this->translator_,
-      /*output=*/&this->output_,
-      /*file_path=*/this->file_path_,
-      /*locator=*/*this->locator_,
-      /*format_escape_errors=*/this->format_escape_errors_);
-  formatter.format(get_diagnostic_info(type), diag);
+  diags.for_each([&](Diag_Type type, void *diag) -> void {
+    Text_Diag_Formatter formatter(
+        this->translator_,
+        /*output=*/&this->output_,
+        /*file_path=*/this->file_path_,
+        /*locator=*/*this->locator_,
+        /*format_escape_errors=*/this->format_escape_errors_);
+    formatter.format(get_diagnostic_info(type), diag);
+  });
 }
 
 Text_Diag_Formatter::Text_Diag_Formatter(Translator t, Output_Stream *output,

diff --git a/src/quick-lint-js/cli/text-diag-reporter.h b/src/quick-lint-js/cli/text-diag-reporter.h
@@ -27,7 +27,6 @@ class Text_Diag_Reporter final : public Diag_Reporter {
   void set_source(Padded_String_View input, const char *file_name);
 
   void report(const Diag_List &) override;
-  void report_impl(Diag_Type type, void *diag) override;
 
  private:
   Output_Stream &output_;

diff --git a/src/quick-lint-js/cli/vim-qflist-json-diag-reporter.cpp b/src/quick-lint-js/cli/vim-qflist-json-diag-reporter.cpp
@@ -54,24 +54,20 @@ void Vim_QFList_JSON_Diag_Reporter::finish() {
 
 void Vim_QFList_JSON_Diag_Reporter::report(const Diag_List &diags) {
   diags.for_each([&](Diag_Type type, void *diag) -> void {
-    this->report_impl(type, diag);
+    if (this->need_comma_) {
+      this->output_.append_literal(u8",\n"_sv);
+    }
+    this->need_comma_ = true;
+    QLJS_ASSERT(this->locator_.has_value());
+    Vim_QFList_JSON_Diag_Formatter formatter(this->translator_,
+                                             /*output=*/&this->output_,
+                                             /*locator=*/*this->locator_,
+                                             /*file_name=*/this->file_name_,
+                                             /*bufnr=*/this->bufnr_);
+    formatter.format(get_diagnostic_info(type), diag);
   });
 }
 
-void Vim_QFList_JSON_Diag_Reporter::report_impl(Diag_Type type, void *diag) {
-  if (this->need_comma_) {
-    this->output_.append_literal(u8",\n"_sv);
-  }
-  this->need_comma_ = true;
-  QLJS_ASSERT(this->locator_.has_value());
-  Vim_QFList_JSON_Diag_Formatter formatter(this->translator_,
-                                           /*output=*/&this->output_,
-                                           /*locator=*/*this->locator_,
-                                           /*file_name=*/this->file_name_,
-                                           /*bufnr=*/this->bufnr_);
-  formatter.format(get_diagnostic_info(type), diag);
-}
-
 Vim_QFList_JSON_Diag_Formatter::Vim_QFList_JSON_Diag_Formatter(
     Translator t, Output_Stream *output, Vim_Locator &locator,
     std::string_view file_name, std::string_view bufnr)

diff --git a/src/quick-lint-js/cli/vim-qflist-json-diag-reporter.h b/src/quick-lint-js/cli/vim-qflist-json-diag-reporter.h
@@ -31,7 +31,6 @@ class Vim_QFList_JSON_Diag_Reporter final : public Diag_Reporter {
   void finish();
 
   void report(const Diag_List &) override;
-  void report_impl(Diag_Type type, void *diag) override;
 
  private:
   Output_Stream &output_;

diff --git a/src/quick-lint-js/diag/diag-reporter.h b/src/quick-lint-js/diag/diag-reporter.h
@@ -21,17 +21,13 @@ class Diag_Reporter {
   virtual ~Diag_Reporter() = default;
 
   virtual void report(const Diag_List &) = 0;
-
-  // TODO(#1154): Delete this in favor of report(const Diag_List&).
-  virtual void report_impl(Diag_Type type, void *diag) = 0;
 };
 
 class Null_Diag_Reporter : public Diag_Reporter {
  public:
   static Null_Diag_Reporter instance;
 
   void report(const Diag_List &) override {}
-  void report_impl(Diag_Type, void *) override {}
 };
 }
 

diff --git a/src/quick-lint-js/diag/reported-diag-statistics.h b/src/quick-lint-js/diag/reported-diag-statistics.h
@@ -23,16 +23,12 @@ class Reported_Diag_Statistics final : public Diag_Reporter {
   bool found_matching_diag() const { return this->found_matching_diag_; }
 
   void report(const Diag_List &diags) override final {
-    diags.for_each([&](Diag_Type type, void *diag) -> void {
-      this->report_impl(type, diag);
+    diags.for_each([&](Diag_Type type, [[maybe_unused]] void *diag) -> void {
+      if (this->predicate_->is_present(type)) {
+        this->found_matching_diag_ = true;
+      }
     });
-  }
-
-  void report_impl(Diag_Type type, void *diag) override final {
-    if (this->predicate_->is_present(type)) {
-      this->found_matching_diag_ = true;
-    }
-    this->reporter_.report_impl(type, diag);
+    this->reporter_.report(diags);
   }
 
  private:

diff --git a/src/quick-lint-js/lsp/lsp-diag-reporter.cpp b/src/quick-lint-js/lsp/lsp-diag-reporter.cpp
@@ -30,20 +30,16 @@ void LSP_Diag_Reporter::finish() { this->output_.append_copy(u8"]"_sv); }
 
 void LSP_Diag_Reporter::report(const Diag_List &diags) {
   diags.for_each([&](Diag_Type type, void *diag) -> void {
-    this->report_impl(type, diag);
+    if (this->need_comma_) {
+      this->output_.append_copy(u8",\n"_sv);
+    }
+    this->need_comma_ = true;
+    LSP_Diag_Formatter formatter(/*output=*/this->output_,
+                                 /*locator=*/this->locator_, this->translator_);
+    formatter.format(get_diagnostic_info(type), diag);
   });
 }
 
-void LSP_Diag_Reporter::report_impl(Diag_Type type, void *diag) {
-  if (this->need_comma_) {
-    this->output_.append_copy(u8",\n"_sv);
-  }
-  this->need_comma_ = true;
-  LSP_Diag_Formatter formatter(/*output=*/this->output_,
-                               /*locator=*/this->locator_, this->translator_);
-  formatter.format(get_diagnostic_info(type), diag);
-}
-
 LSP_Diag_Formatter::LSP_Diag_Formatter(Byte_Buffer &output,
                                        LSP_Locator &locator, Translator t)
     : Diagnostic_Formatter(t), output_(output), locator_(locator) {}

diff --git a/src/quick-lint-js/lsp/lsp-diag-reporter.h b/src/quick-lint-js/lsp/lsp-diag-reporter.h
@@ -30,7 +30,6 @@ class LSP_Diag_Reporter final : public Diag_Reporter {
   void finish();
 
   void report(const Diag_List &) override;
-  void report_impl(Diag_Type type, void *diag) override;
 
  private:
   Byte_Buffer &output_;

diff --git a/test/quick-lint-js/failing-diag-reporter.cpp b/test/quick-lint-js/failing-diag-reporter.cpp
@@ -7,14 +7,10 @@
 
 namespace quick_lint_js {
 void Failing_Diag_Reporter::report(const Diag_List& diags) {
-  diags.for_each([&](Diag_Type type, void* diag) -> void {
-    this->report_impl(type, diag);
+  diags.for_each([&](Diag_Type type, [[maybe_unused]] void* diag) -> void {
+    ADD_FAILURE() << "expected no errors, but got: " << type;
   });
 }
-
-void Failing_Diag_Reporter::report_impl(Diag_Type type, void*) {
-  ADD_FAILURE() << "expected no errors, but got: " << type;
-}
 }
 
 // quick-lint-js finds bugs in JavaScript programs.

diff --git a/test/quick-lint-js/failing-diag-reporter.h b/test/quick-lint-js/failing-diag-reporter.h
@@ -10,7 +10,6 @@ namespace quick_lint_js {
 class Failing_Diag_Reporter : public Diag_Reporter {
  public:
   void report(const Diag_List &) override;
-  void report_impl(Diag_Type type, void *diag) override;
 };
 }
 

diff --git a/test/test-translation.cpp b/test/test-translation.cpp
@@ -50,15 +50,11 @@ class Basic_Text_Diag_Reporter final : public Diag_Reporter {
 
   void report(const Diag_List &diags) override {
     diags.for_each([&](Diag_Type type, void *diag) -> void {
-      this->report_impl(type, diag);
+      Basic_Text_Diag_Formatter formatter(this);
+      formatter.format(get_diagnostic_info(type), diag);
     });
   }
 
-  void report_impl(Diag_Type type, void *diag) override {
-    Basic_Text_Diag_Formatter formatter(this);
-    formatter.format(get_diagnostic_info(type), diag);
-  }
-
  private:
   std::vector<String8> messages_;
   Translator translator_;
-Original file line number
+Diff line change
@@ Expand Up / @@ -10,7 +10,6 @@ namespace quick_lint_js { @@
     class Failing_Diag_Reporter : public Diag_Reporter {
      public:
       void report(const Diag_List &) override;
-      void report_impl(Diag_Type type, void *diag) override;
     };
     }
@@ Expand Down @@