From f064f25795c55876a1a68a96a621f3f0baa06f05 Mon Sep 17 00:00:00 2001 From: "Matthew \"strager\" Glazar" Date: Sat, 21 Sep 2024 12:56:17 -0400 Subject: [PATCH] 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. --- .../vscode/vscode-diag-reporter.h | 20 ++++++-------- src/quick-lint-js/c-api-diag-reporter.cpp | 10 ++----- src/quick-lint-js/c-api-diag-reporter.h | 1 - .../cli/emacs-lisp-diag-reporter.cpp | 14 ++++------ .../cli/emacs-lisp-diag-reporter.h | 1 - src/quick-lint-js/cli/text-diag-reporter.cpp | 22 +++++++--------- src/quick-lint-js/cli/text-diag-reporter.h | 1 - .../cli/vim-qflist-json-diag-reporter.cpp | 26 ++++++++----------- .../cli/vim-qflist-json-diag-reporter.h | 1 - src/quick-lint-js/diag/diag-reporter.h | 4 --- .../diag/reported-diag-statistics.h | 14 ++++------ src/quick-lint-js/lsp/lsp-diag-reporter.cpp | 18 +++++-------- src/quick-lint-js/lsp/lsp-diag-reporter.h | 1 - test/quick-lint-js/failing-diag-reporter.cpp | 8 ++---- test/quick-lint-js/failing-diag-reporter.h | 1 - test/test-translation.cpp | 8 ++---- 16 files changed, 51 insertions(+), 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 index 30b36c8f1..502ff8bf5 100644 --- 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 index 6c61647f5..6bc9fdd38 100644 --- 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::set_translator(Translator t) { template void C_API_Diag_Reporter::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 -void C_API_Diag_Reporter::report_impl(Diag_Type type, - void *diag) { - C_API_Diag_Formatter formatter(this); - formatter.format(get_diagnostic_info(type), diag); -} - template const Diagnostic *C_API_Diag_Reporter::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 index 66f6bebd2..59f0ac6cd 100644 --- 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 index e85d40315..d6fb66279 100644 --- 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 index c85b9c3f3..b7264acfd 100644 --- 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 index 8d27b561d..604e7095f 100644 --- 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 index 0032be994..f1198db7c 100644 --- 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 index d0c88167c..3585eda18 100644 --- 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 index e0499a72c..ff37d6c50 100644 --- 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 index 64b33af91..05f839cb9 100644 --- a/src/quick-lint-js/diag/diag-reporter.h +++ b/src/quick-lint-js/diag/diag-reporter.h @@ -21,9 +21,6 @@ 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 { @@ -31,7 +28,6 @@ class Null_Diag_Reporter : public Diag_Reporter { 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 index 83f90f3d8..95893d073 100644 --- 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 index 92f853612..740f93072 100644 --- 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 index 3cc9d66bb..2190b8859 100644 --- 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 index e5343848d..457ea22ae 100644 --- 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 index 0cf6590a7..7707bb6df 100644 --- 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 index a62819605..da8c82658 100644 --- 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 messages_; Translator translator_;