Provide option to indent formal parameters 2 spaces instead of 4 (wrap).

This affects formal parameters in modules, interfaces, classes. Default remains wrapping +4 spaces. issues #40 PiperOrigin-RevId: 330979059
chipsalliance · Sep 10, 2020 · 16d54aa · 16d54aa
1 parent b933354
commit 16d54aa
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 2 deletions.
diff --git a/verilog/formatting/format_style.h b/verilog/formatting/format_style.h
@@ -62,6 +62,9 @@ struct FormatStyle : public verible::BasicFormatStyle {
   // Internal tests assume these are forced to kAlign.
   AlignmentPolicy module_net_variable_alignment = AlignmentPolicy::kAlign;
 
+  // Control indentation amount for formal parameter declarations.
+  IndentationStyle formal_parameters_indentation = IndentationStyle::kWrap;
+
   // Control how formal parameters in modules/interfaces/classes are formatted.
   // Internal tests assume these are forced to kAlign.
   AlignmentPolicy formal_parameters_alignment = AlignmentPolicy::kAlign;
@@ -92,6 +95,12 @@ struct FormatStyle : public verible::BasicFormatStyle {
                : indentation_spaces;
   }
 
+  int FormalParametersIndentation() const {
+    return formal_parameters_indentation == IndentationStyle::kWrap
+               ? wrap_spaces
+               : indentation_spaces;
+  }
+
   void ApplyToAllAlignmentPolicies(AlignmentPolicy policy) {
     port_declarations_alignment = policy;
     named_parameter_alignment = policy;

diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc
@@ -55,6 +55,7 @@ namespace {
 
 using absl::StatusCode;
 using verible::AlignmentPolicy;
+using verible::IndentationStyle;
 using verible::LineNumberSet;
 
 // Tests that clean output passes.
@@ -7525,7 +7526,7 @@ TEST(FormatterEndToEndTest, ModulePortDeclarationsIndentNotWrap) {
   style.indentation_spaces = 2;
   style.wrap_spaces = 4;
   // Indent 2 spaces instead of wrapping 4 spaces.
-  style.port_declarations_indentation = verible::IndentationStyle::kIndent;
+  style.port_declarations_indentation = IndentationStyle::kIndent;
   for (const auto& test_case : kTestCases) {
     VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>";
     std::ostringstream stream;
@@ -7536,6 +7537,78 @@ TEST(FormatterEndToEndTest, ModulePortDeclarationsIndentNotWrap) {
     EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
   }
 }
+
+TEST(FormatterEndToEndTest, FormalParametersIndentNotWrap) {
+  static constexpr FormatterTestCase kTestCases[] = {
+      {"", ""},
+      {"\n", "\n"},
+      {"\n\n", "\n\n"},
+      {"module  m  ;\t\n"
+       "  endmodule\n",
+       "module m;\n"
+       "endmodule\n"},
+      {"module  m #(   ) ;\n"  // empty parameters
+       "  endmodule\n",
+       "module m #();\n"
+       "endmodule\n"},
+      {// single parameter example
+       "module  m   #( int W = 2)\t;\n"
+       "  endmodule\n",
+       "module m #(\n"
+       "  int W = 2\n"  // indented 2 spaces
+       ");\n"
+       "endmodule\n"},
+      {// module with two parameters
+       "module  m   #(\n"
+       "int W = 2,\n"
+       "int L = 4\n"
+       ")\t;\n"
+       "  endmodule\n",
+       "module m #(\n"
+       "  int W = 2,\n"  // indented 2 spaces
+       "  int L = 4\n"
+       ");\n"
+       "endmodule\n"},
+      {// interface with two parameters
+       "interface  m   #(\n"
+       "int W = 2,\n"
+       "int L = 4\n"
+       ")\t;\n"
+       "  endinterface\n",
+       "interface m #(\n"
+       "  int W = 2,\n"  // indented 2 spaces
+       "  int L = 4\n"
+       ");\n"
+       "endinterface\n"},
+      {// class with two parameters
+       "class  c   #(\n"
+       "int W = 2,\n"
+       "int L = 4\n"
+       ")\t;\n"
+       "  endclass\n",
+       "class c #(\n"
+       "  int W = 2,\n"  // indented 2 spaces
+       "  int L = 4\n"
+       ");\n"
+       "endclass\n"},
+  };
+  FormatStyle style;
+  style.column_limit = 40;
+  style.indentation_spaces = 2;
+  style.wrap_spaces = 4;
+  // Indent 2 spaces instead of wrapping 4 spaces.
+  style.formal_parameters_indentation = IndentationStyle::kIndent;
+  for (const auto& test_case : kTestCases) {
+    VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>";
+    std::ostringstream stream;
+    const auto status =
+        FormatVerilog(test_case.input, "<filename>", style, stream);
+    // Require these test cases to be valid.
+    EXPECT_OK(status) << status.message();
+    EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+  }
+}
+
 struct SelectLinesTestCase {
   absl::string_view input;
   LineNumberSet lines;  // explicit set of lines to enable formatting

diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc
@@ -1047,7 +1047,8 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
     }
     case NodeEnum::kFormalParameterList: {
       // Do not further indent preprocessor clauses.
-      const int indent = suppress_indentation ? 0 : style_.wrap_spaces;
+      const int indent =
+          suppress_indentation ? 0 : style_.FormalParametersIndentation();
       if (Context().IsInside(NodeEnum::kClassHeader) ||
           // kModuleHeader covers interfaces and programs
           Context().IsInside(NodeEnum::kModuleHeader)) {

diff --git a/verilog/tools/formatter/verilog_format.cc b/verilog/tools/formatter/verilog_format.cc
@@ -123,8 +123,11 @@ ABSL_FLAG(int, max_search_states, 100000,
 // These flags exist in the short term to disable formatting of some regions.
 // Do not expect to be able to use these in the long term, once they find
 // a better home in a configuration struct.
+
 ABSL_FLAG(IndentationStyle, port_declarations_indentation,
           IndentationStyle::kWrap, "Indent port declarations: {indent,wrap}");
+ABSL_FLAG(IndentationStyle, formal_parameters_indentation,
+          IndentationStyle::kWrap, "Indent formal parameters: {indent,wrap}");
 
 // For most of the following in this group, kInferUserIntent is a reasonable
 // default behavior because it allows for user-control with minimal invasiveness
@@ -211,6 +214,8 @@ static bool formatOneFile(absl::string_view filename,
     // various indentation control
     format_style.port_declarations_indentation =
         absl::GetFlag(FLAGS_port_declarations_indentation);
+    format_style.formal_parameters_indentation =
+        absl::GetFlag(FLAGS_formal_parameters_indentation);
 
     // various alignment control
     format_style.port_declarations_alignment =