Merge pull request #2162 from sconwayaus/module_file_name_rule_autofix

Module file name rule autofix

common/analysis/linter_test_utils.h

@@ -123,14 +123,15 @@ struct AutoFixInOut {
 // Expects test.code to be accepted by AnalyzerType.
 template <class AnalyzerType, class RuleType>
 void RunLintAutoFixCase(const AutoFixInOut &test,
-                        const LintRuleGenerator<RuleType> &make_rule) {
+                        const LintRuleGenerator<RuleType> &make_rule,
+                        absl::string_view filename = "") {
   // All linters start by parsing to yield a TextStructure.
-  AnalyzerType analyzer(test.code, "");
+  AnalyzerType analyzer(test.code, filename);
   absl::Status unused_parser_status = analyzer.Analyze();
 
   // Instantiate a linter that runs a single rule to analyze text.
   LintRunner<RuleType> lint_runner(make_rule());
-  const LintRuleStatus rule_status = lint_runner.Run(analyzer.Data(), "");
+  const LintRuleStatus rule_status = lint_runner.Run(analyzer.Data(), filename);
   const auto &violations(rule_status.violations);
 
   CHECK_EQ(violations.size(), 1) << "TODO: apply multi-violation fixes";
@@ -144,7 +145,8 @@ void RunLintAutoFixCase(const AutoFixInOut &test,
 
 template <class AnalyzerType, class RuleClass>
 void RunApplyFixCases(std::initializer_list<AutoFixInOut> tests,
-                      absl::string_view configuration = "") {
+                      absl::string_view configuration = "",
+                      absl::string_view filename = "") {
   using rule_type = typename RuleClass::rule_type;
   auto rule_generator = [&configuration]() -> std::unique_ptr<rule_type> {
     std::unique_ptr<rule_type> instance(new RuleClass());
@@ -153,7 +155,7 @@ void RunApplyFixCases(std::initializer_list<AutoFixInOut> tests,
     return instance;
   };
   for (const auto &test : tests) {
-    RunLintAutoFixCase<AnalyzerType, rule_type>(test, rule_generator);
+    RunLintAutoFixCase<AnalyzerType, rule_type>(test, rule_generator, filename);
   }
 }
 

verilog/analysis/checkers/BUILD

@@ -430,6 +430,7 @@ cc_library(
         "//common/analysis:lint-rule-status",
         "//common/analysis:syntax-tree-search",
         "//common/analysis:text-structure-lint-rule",
+        "//common/text:concrete-syntax-leaf",
         "//common/text:config-utils",
         "//common/text:symbol",
         "//common/text:text-structure",

verilog/analysis/checkers/module_filename_rule.cc

@@ -27,6 +27,7 @@
 #include "absl/strings/string_view.h"
 #include "common/analysis/lint_rule_status.h"
 #include "common/analysis/syntax_tree_search.h"
+#include "common/text/concrete_syntax_leaf.h"
 #include "common/text/config_utils.h"
 #include "common/text/symbol.h"
 #include "common/text/text_structure.h"
@@ -121,11 +122,27 @@ void ModuleFilenameRule::Lint(const TextStructureView &text_structure,
   }
 
   // Only report a violation on the last module declaration.
-  const auto *last_module_id = GetModuleName(*module_cleaned.back().match);
+  const verible::Symbol &last_module = *module_cleaned.back().match;
+  const auto *last_module_id = GetModuleName(last_module);
   if (!last_module_id) LOG(ERROR) << "Couldn't extract module name";
   if (last_module_id) {
-    violations_.insert(verible::LintViolation(
-        last_module_id->get(), absl::StrCat(kMessage, "\"", unitname, "\"")));
+    const std::string autofix_msg =
+        absl::StrCat("Rename module to '", unitname, "' to match filename");
+
+    auto autofix =
+        verible::AutoFix(autofix_msg, {last_module_id->get(), unitname});
+
+    const verible::SyntaxTreeLeaf *module_end_label =
+        GetModuleEndLabel(last_module);
+    if (module_end_label) {
+      autofix.AddEdits({{module_end_label->get(), unitname}});
+    }
+
+    verible::LintViolation violation = verible::LintViolation(
+        last_module_id->get(), absl::StrCat(kMessage, "\"", unitname, "\""),
+        {autofix});
+
+    violations_.insert(violation);
   }
 }
 

verilog/analysis/checkers/module_filename_rule_test.cc

@@ -29,6 +29,7 @@ namespace analysis {
 namespace {
 
 using verible::LintTestCase;
+using verible::RunApplyFixCases;
 using verible::RunConfiguredLintTestCases;
 using verible::RunLintTestCases;
 
@@ -180,6 +181,74 @@ TEST(ModuleFilenameRuleTest, NoModuleMatchesFilenameRelPath) {
   RunLintTestCases<VerilogAnalyzer, ModuleFilenameRule>(kTestCases, filename);
 }
 
+TEST(ModuleFilenameRuleTest, AutoFixModuleFilenameRule) {
+  const std::initializer_list<verible::AutoFixInOut> kTestCases = {
+      {"module a;\n\nendmodule", "module r;\n\nendmodule"},
+      {"module some_name1;\n\nendmodule", "module r;\n\nendmodule"},
+      {"module some_name2();\n\nendmodule", "module r();\n\nendmodule"},
+      {"module some_name3#()();\n\nendmodule", "module r#()();\n\nendmodule"},
+      {"module a;\n\nendmodule : a", "module r;\n\nendmodule : r"},
+      {"module some_name1;\n\nendmodule: some_name1",
+       "module r;\n\nendmodule: r"},
+      {"module some_name2();\n\nendmodule :some_name2",
+       "module r();\n\nendmodule :r"},
+      {"module some_name3#()();\n\nendmodule:some_name3",
+       "module r#()();\n\nendmodule:r"},
+
+  };
+  const std::string filename = "path/to/r.sv";
+  RunApplyFixCases<VerilogAnalyzer, ModuleFilenameRule>(kTestCases, "",
+                                                        filename);
+}
+
+TEST(ModuleFilenameRuleTest, AutoFixModuleFilenameRuleWithDashes) {
+  const std::initializer_list<verible::AutoFixInOut> kTestCases = {
+      {"module a;\n\nendmodule", "module file_with_dashes;\n\nendmodule"},
+      {"module some_name1;\n\nendmodule",
+       "module file_with_dashes;\n\nendmodule"},
+      {"module some_name2();\n\nendmodule",
+       "module file_with_dashes();\n\nendmodule"},
+      {"module some_name3#()();\n\nendmodule",
+       "module file_with_dashes#()();\n\nendmodule"},
+      {"module a;\n\nendmodule : a",
+       "module file_with_dashes;\n\nendmodule : file_with_dashes"},
+      {"module some_name1;\n\nendmodule :some_name1",
+       "module file_with_dashes;\n\nendmodule :file_with_dashes"},
+      {"module some_name2();\n\nendmodule: some_name2",
+       "module file_with_dashes();\n\nendmodule: file_with_dashes"},
+      {"module some_name3#()();\n\nendmodule:some_name3",
+       "module file_with_dashes#()();\n\nendmodule:file_with_dashes"},
+  };
+  const std::string filename = "path/to/file-with-dashes.sv";
+  RunApplyFixCases<VerilogAnalyzer, ModuleFilenameRule>(
+      kTestCases, "allow-dash-for-underscore:on", filename);
+}
+
+TEST(ModuleFilenameRuleTest, AutoFixModuleFilenameRuleWithUnderscore) {
+  const std::initializer_list<verible::AutoFixInOut> kTestCases = {
+      {"module a;\n\nendmodule", "module file_no_dashes;\n\nendmodule"},
+      {"module some_name1;\n\nendmodule",
+       "module file_no_dashes;\n\nendmodule"},
+      {"module some_name2();\n\nendmodule",
+       "module file_no_dashes();\n\nendmodule"},
+      {"module some_name3#()();\n\nendmodule",
+       "module file_no_dashes#()();\n\nendmodule"},
+      {"module a;\n\nendmodule : a",
+       "module file_no_dashes;\n\nendmodule : file_no_dashes"},
+      {"module some_name1;\n\nendmodule :some_name1",
+       "module file_no_dashes;\n\nendmodule :file_no_dashes"},
+      {"module some_name2();\n\nendmodule: some_name2",
+       "module file_no_dashes();\n\nendmodule: file_no_dashes"},
+      {"module some_name3#()();\n\nendmodule:some_name3",
+       "module file_no_dashes#()();\n\nendmodule:file_no_dashes"},
+  };
+  const std::string filename = "path/to/file_no_dashes.sv";
+  RunApplyFixCases<VerilogAnalyzer, ModuleFilenameRule>(
+      kTestCases, "allow-dash-for-underscore:off", filename);
+  RunApplyFixCases<VerilogAnalyzer, ModuleFilenameRule>(
+      kTestCases, "allow-dash-for-underscore:on", filename);
+}
+
 }  // namespace
 }  // namespace analysis
 }  // namespace verilog