move ViolationFixer to common

common/analysis/BUILD

@@ -93,6 +93,10 @@ cc_library(
     deps = [
         ":lint_rule_status",
         ":rdformat",
+        "//common/strings:diff",
+        "//common/util:file_util",
+        "//common/util:user_interaction",
+        "@com_google_absl//absl/status",
         "@com_google_absl//absl/strings",
         "@jsonhpp//:jsonhpp",
     ]

common/analysis/violation_handler.cc

@@ -14,12 +14,39 @@
 
 #include "common/analysis/violation_handler.h"
 
+#include "absl/status/status.h"
 #include "common/analysis/rdformat.h"
+#include "common/strings/diff.h"
+#include "common/util/file_util.h"
+#include "common/util/user_interaction.h"
 #include "nlohmann/json.hpp"
 
 namespace verible {
 namespace {
 
+void PrintFix(std::ostream& stream, absl::string_view text,
+                     const verible::AutoFix& fix) {
+  std::string after = fix.Apply(text);
+  verible::LineDiffs diff(text, after);
+
+  verible::LineDiffsToUnifiedDiff(stream, diff, 1);
+}
+
+void PrintFixAlternatives(std::ostream& stream, absl::string_view text,
+                                 const std::vector<verible::AutoFix>& fixes) {
+  const bool print_alternative_number = fixes.size() > 1;
+  for (size_t i = 0; i < fixes.size(); ++i) {
+    if (print_alternative_number) {
+      stream << verible::term::inverse(absl::StrCat(
+          "[ ", (i + 1), ". Alternative ", fixes[i].Description(), " ]\n"));
+    } else {
+      stream << verible::term::inverse(
+          absl::StrCat("[ ", fixes[i].Description(), " ]\n"));
+    }
+    PrintFix(stream, text, fixes[i]);
+  }
+}
+
 void SuggestionFromEdit(const verible::ReplacementEdit& edit,
                         const verible::LineColumn& line_column_start,
                         const verible::LineColumn& line_column_end,
@@ -53,6 +80,193 @@ void ViolationPrinter::HandleViolations(
   }
 }
 
+void ViolationFixer::CommitFixes(absl::string_view source_content,
+                                 absl::string_view source_path,
+                                 const verible::AutoFix& fix) const {
+  if (fix.Edits().empty()) {
+    return;
+  }
+  std::string fixed_content = fix.Apply(source_content);
+
+  if (patch_stream_) {
+    verible::LineDiffs diff(source_content, fixed_content);
+    verible::LineDiffsToUnifiedDiff(*patch_stream_, diff, 1, source_path);
+  } else {
+    const absl::Status write_status =
+        verible::file::SetContents(source_path, fixed_content);
+    if (!write_status.ok()) {
+      LOG(ERROR) << "Failed to write fixes to file '" << source_path
+                 << "': " << write_status.ToString();
+      return;
+    }
+  }
+}
+
+void ViolationFixer::HandleViolations(
+    const std::set<LintViolationWithStatus>& violations, absl::string_view base,
+    absl::string_view path) {
+  verible::AutoFix fix;
+  verible::LintStatusFormatter formatter(base);
+  for (auto violation : violations) {
+    HandleViolation(*violation.violation, base, path, violation.status->url,
+                    violation.status->lint_rule_name, formatter, &fix);
+  }
+
+  CommitFixes(base, path, fix);
+}
+
+void ViolationFixer::HandleViolation(
+    const verible::LintViolation& violation, absl::string_view base,
+    absl::string_view path, absl::string_view url, absl::string_view rule_name,
+    const verible::LintStatusFormatter& formatter, verible::AutoFix* fix) {
+  std::stringstream violation_message;
+  formatter.FormatViolation(&violation_message, violation, base, path, url,
+                            rule_name);
+  (*message_stream_) << violation_message.str() << std::endl;
+
+  if (violation.autofixes.empty()) {
+    return;
+  }
+
+  static std::string_view previous_fix_conflict =
+      "The fix conflicts with "
+      "previously applied fixes, rejecting.\n";
+
+  Answer answer;
+  for (bool first_round = true; /**/; first_round = false) {
+    if (ultimate_answer_.choice != AnswerChoice::kUnknown) {
+      answer = ultimate_answer_;
+    } else if (auto found = rule_answers_.find(rule_name);
+               found != rule_answers_.end()) {
+      answer = found->second;
+      // If the ApplyAll specifies alternative not available here, use first.
+      if (answer.alternative >= violation.autofixes.size()) {
+        answer.alternative = 0;
+      }
+    } else {
+      if (is_interactive_ && first_round) {  // Show the user what is available.
+        PrintFixAlternatives(*message_stream_, base, violation.autofixes);
+      }
+      answer = answer_chooser_(violation, rule_name);
+    }
+
+    switch (answer.choice) {
+      case AnswerChoice::kApplyAll:
+        ultimate_answer_ = {AnswerChoice::kApply};
+        [[fallthrough]];
+      case AnswerChoice::kApplyAllForRule:
+        rule_answers_[rule_name] = {AnswerChoice::kApply, answer.alternative};
+        [[fallthrough]];
+      case AnswerChoice::kApply:  // Apply fix chosen in the alternatative
+        if (answer.alternative >= violation.autofixes.size())
+          continue;  // ask again.
+        if (!fix->AddEdits(violation.autofixes[answer.alternative].Edits())) {
+          *message_stream_ << previous_fix_conflict;
+        }
+        break;
+      case AnswerChoice::kRejectAll:
+        ultimate_answer_ = {AnswerChoice::kReject};
+        [[fallthrough]];
+      case AnswerChoice::kRejectAllForRule:
+        rule_answers_[rule_name] = {AnswerChoice::kReject};
+        [[fallthrough]];
+      case AnswerChoice::kReject:
+        return;
+
+      case AnswerChoice::kPrintFix:
+        PrintFixAlternatives(*message_stream_, base, violation.autofixes);
+        continue;
+      case AnswerChoice::kPrintAppliedFixes:
+        PrintFix(*message_stream_, base, *fix);
+        continue;
+
+      default:
+        continue;
+    }
+
+    break;
+  }
+}
+
+ViolationFixer::Answer ViolationFixer::InteractiveAnswerChooser(
+    const verible::LintViolation& violation, absl::string_view rule_name) {
+  static absl::string_view fixed_help_message =
+      "n - reject fix\n"
+      "a - apply this and all remaining fixes for violations of this rule\n"
+      "d - reject this and all remaining fixes for violations of this rule\n"
+      "A - apply this and all remaining fixes\n"
+      "D - reject this and all remaining fixes\n"
+      "p - show fix\n"
+      "P - show fixes applied in this file so far\n"
+      "? - print this help and prompt again\n";
+
+  const size_t fix_count = violation.autofixes.size();
+  std::string help_message;
+  std::string alternative_list;  // Show alternatives in the short-menu.
+  if (fix_count > 1) {
+    help_message =
+        absl::StrCat("y - apply first fix\n[1-", fix_count,
+                     "] - apply given alternative\n", fixed_help_message);
+    for (size_t i = 0; i < fix_count; ++i)
+      absl::StrAppend(&alternative_list, (i + 1), ",");
+  } else {
+    help_message = absl::StrCat("y - apply fix\n", fixed_help_message);
+  }
+
+  for (;;) {
+    const char c = verible::ReadCharFromUser(
+        std::cin, std::cerr, verible::IsInteractiveTerminalSession(),
+        verible::term::bold("Autofix is available. Apply? [" +
+                            alternative_list + "y,n,a,d,A,D,p,P,?] "));
+
+    // Single character digit chooses the available alternative.
+    if (c >= '1' && c <= '9' &&
+        c < static_cast<char>('1' + violation.autofixes.size())) {
+      return {AnswerChoice::kApply, static_cast<size_t>(c - '1')};
+    }
+
+    switch (c) {
+      case 'y':
+        return {AnswerChoice::kApply, 0};
+
+        // TODO(hzeller): Should we provide a way to choose 'all for rule'
+        // including an alternative ? Maybe with a two-letter response
+        // such as 1a, 2a, 3a ? Current assumption of interaction is
+        // single character.
+      case 'a':
+        return {AnswerChoice::kApplyAllForRule};
+      case 'A':
+        return {AnswerChoice::kApplyAll};  // No alternatives
+      case 'n':
+        return {AnswerChoice::kReject};
+      case 'd':
+        return {AnswerChoice::kRejectAllForRule};
+      case 'D':
+        return {AnswerChoice::kRejectAll};
+
+      case '\0':
+        // EOF: received when too few "answers" have been piped to stdin.
+        std::cerr << "Received EOF while there are questions left. "
+                  << "Rejecting all remaining fixes." << std::endl;
+        return {AnswerChoice::kRejectAll};
+
+      case 'p':
+        return {AnswerChoice::kPrintFix};
+      case 'P':
+        return {AnswerChoice::kPrintAppliedFixes};
+
+      case '\n':
+        continue;
+
+      case '?':
+      default:
+        std::cerr << help_message << std::endl;
+        continue;
+    }
+  }
+}
+
+
 void RDJsonPrinter::HandleViolations(
     const std::set<LintViolationWithStatus>& violations, absl::string_view base,
     absl::string_view path) {

common/analysis/violation_handler.h

@@ -15,6 +15,8 @@
 #ifndef VERIBLE_COMMON_ANALYSIS_VIOLATION_HANDLER_H_
 #define VERIBLE_COMMON_ANALYSIS_VIOLATION_HANDLER_H_
 
+#include <functional>
+#include <map>
 #include <ostream>
 #include <set>
 
@@ -56,6 +58,94 @@ class ViolationPrinter : public ViolationHandler {
   verible::LintStatusFormatter* formatter_;
 };
 
+// ViolationHandler that prints all violations and gives an option to fix those
+// that have autofixes available.
+//
+// By default, when violation has an autofix available, ViolationFixer asks an
+// user what to do. The answers can be provided by AnswerChooser callback passed
+// to the constructor as the answer_chooser parameter. The callback is called
+// once for each fixable violation with a current violation object and a
+// violated rule name as arguments, and must return one of the values from
+// AnswerChoice enum.
+//
+// When the constructor's patch_stream parameter is not null, the fixes are
+// written to specified stream in unified diff format. Otherwise the fixes are
+// applied directly to the source file.
+//
+// The HandleLintRuleStatuses method can be called multiple times with statuses
+// generated from different files. The state of answers like "apply all for
+// rule" or "apply all" is kept between the calls.
+class ViolationFixer : public verible::ViolationHandler {
+ public:
+  enum class AnswerChoice {
+    kUnknown,
+    kApply,              // apply fix
+    kReject,             // reject fix
+    kApplyAllForRule,    // apply this and all remaining fixes for violations
+                         // of this rule
+    kRejectAllForRule,   // reject this and all remaining fixes for violations
+                         // of this rule
+    kApplyAll,           // apply this and all remaining fixes
+    kRejectAll,          // reject this and all remaining fixes
+    kPrintFix,           // show fix
+    kPrintAppliedFixes,  // show fixes applied so far
+  };
+
+  struct Answer {
+    AnswerChoice choice;
+    // If there are multiple alternatives for fixes available, this is
+    // the one chosen. By default the first one.
+    size_t alternative = 0;
+  };
+
+  using AnswerChooser =
+      std::function<Answer(const verible::LintViolation&, absl::string_view)>;
+
+  // Violation fixer with user-chosen answer chooser.
+  ViolationFixer(std::ostream* message_stream, std::ostream* patch_stream,
+                 const AnswerChooser& answer_chooser)
+      : ViolationFixer(message_stream, patch_stream, answer_chooser, false) {}
+
+  // Violation fixer with interactive answer choice.
+  ViolationFixer(std::ostream* message_stream, std::ostream* patch_stream)
+      : ViolationFixer(message_stream, patch_stream, InteractiveAnswerChooser,
+                       true) {}
+
+  void HandleViolations(
+      const std::set<verible::LintViolationWithStatus>& violations,
+      absl::string_view base, absl::string_view path) final;
+
+ private:
+  ViolationFixer(std::ostream* message_stream, std::ostream* patch_stream,
+                 const AnswerChooser& answer_chooser, bool is_interactive)
+      : message_stream_(message_stream),
+        patch_stream_(patch_stream),
+        answer_chooser_(answer_chooser),
+        is_interactive_(is_interactive),
+        ultimate_answer_({AnswerChoice::kUnknown, 0}) {}
+
+  void HandleViolation(const verible::LintViolation& violation,
+                       absl::string_view base, absl::string_view path,
+                       absl::string_view url, absl::string_view rule_name,
+                       const verible::LintStatusFormatter& formatter,
+                       verible::AutoFix* fix);
+
+  static Answer InteractiveAnswerChooser(
+      const verible::LintViolation& violation, absl::string_view rule_name);
+
+  void CommitFixes(absl::string_view source_content,
+                   absl::string_view source_path,
+                   const verible::AutoFix& fix) const;
+
+  std::ostream* const message_stream_;
+  std::ostream* const patch_stream_;
+  const AnswerChooser answer_chooser_;
+  const bool is_interactive_;
+
+  Answer ultimate_answer_;
+  std::map<absl::string_view, Answer> rule_answers_;
+};
+
 // ViolationHandler that prints all violations in Reviewdog Diagnostic Format
 class RDJsonPrinter : public ViolationHandler {
  public:

verilog/analysis/BUILD

@@ -229,13 +229,11 @@ cc_library(
         "//common/analysis:token_stream_linter",
         "//common/analysis:violation_handler",
         "//common/strings:line_column_map",
-        "//common/strings:diff",
         "//common/text:concrete_syntax_tree",
         "//common/text:text_structure",
         "//common/text:token_info",
         "//common/util:file_util",
         "//common/util:logging",
-        "//common/util:user_interaction",
         "//verilog/parser:verilog_token_classifications",
         "//verilog/parser:verilog_token_enum",
         "@com_google_absl//absl/flags:flag",