Provide option to indent named parameters/ports 2 spaces instead of wrap +4. fixes #40 PiperOrigin-RevId: 330980195
diff --git a/verilog/formatting/format_style.h b/verilog/formatting/format_style.h index 8d0e3ae..f68821d 100644 --- a/verilog/formatting/format_style.h +++ b/verilog/formatting/format_style.h
@@ -41,19 +41,16 @@ // formatted. Internal tests assume these are forced to kAlign. AlignmentPolicy port_declarations_alignment = AlignmentPolicy::kAlign; - // If true, format module instantiations, else leave those regions - // unformatted. A user may wish to disable this in favor of their own aligned - // formatting. The default value here is true so that compact formatting can - // continue to be covered in integration tests. This flag is prone to change - // or be removed in the future. - // TODO(b/164286027): align module instance named parameters - // and promote the control for this into AlignmentPolicy. - bool format_module_instantiations = true; + // Control indentation amount for named parameter assignments. + IndentationStyle named_parameter_indentation = IndentationStyle::kWrap; // Control how named parameters (e.g. in module instances) are formatted. // For internal testing purposes, this is default to kAlign. AlignmentPolicy named_parameter_alignment = AlignmentPolicy::kAlign; + // Control indentation amount for named port connections. + IndentationStyle named_port_indentation = IndentationStyle::kWrap; + // Control how named ports (e.g. in module instances) are formatted. // Internal tests assume these are forced to kAlign. AlignmentPolicy named_port_alignment = AlignmentPolicy::kAlign; @@ -101,6 +98,18 @@ : indentation_spaces; } + int NamedParameterIndentation() const { + return named_parameter_indentation == IndentationStyle::kWrap + ? wrap_spaces + : indentation_spaces; + } + + int NamedPortIndentation() const { + return named_port_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 index a1f53af..f4ecec9 100644 --- a/verilog/formatting/formatter_test.cc +++ b/verilog/formatting/formatter_test.cc
@@ -7538,6 +7538,54 @@ } } +TEST(FormatterEndToEndTest, NamedPortConnectionsIndentNotWrap) { + static constexpr FormatterTestCase kTestCases[] = { + {"", ""}, + {"\n", "\n"}, + {"\n\n", "\n\n"}, + {"module m ;\t\n" + " endmodule\n", + "module m;\n" + "endmodule\n"}, + {"module m( ) ;\n" + " endmodule\n", + "module m ();\n" // empty ports formatted compactly + "endmodule\n"}, + {// single port example + "module m ;\n" + "foo bar( .clk( clk ) )\t;\n" + " endmodule\n", + "module m;\n" + " foo bar (.clk(clk));\n" + "endmodule\n"}, + {// two port example + "module m ;\n" + "foo bar( .clk2( clk ),.data (data) )\t;\n" + " endmodule\n", + "module m;\n" + " foo bar (\n" + " .clk2(clk),\n" // indent only +2 spaces + " .data(data)\n" + " );\n" + "endmodule\n"}, + }; + FormatStyle style; + style.column_limit = 40; + style.indentation_spaces = 2; + style.wrap_spaces = 4; + // Indent 2 spaces instead of wrapping 4 spaces. + style.named_port_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; + } +} + TEST(FormatterEndToEndTest, FormalParametersIndentNotWrap) { static constexpr FormatterTestCase kTestCases[] = { {"", ""}, @@ -7609,6 +7657,81 @@ } } +TEST(FormatterEndToEndTest, NamedParametersIndentNotWrap) { + 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"}, + {"module m ;\t\n" + " foo #()bar();\n" + " endmodule\n", + "module m;\n" + " foo #() bar ();\n" + "endmodule\n"}, + {// one named parameter + "module m ;\n" + "foo #(.W(1)) bar();\n" + " endmodule\n", + "module m;\n" + " foo #(.W(1)) bar ();\n" + "endmodule\n"}, + {// two named parameters + "module m ;\n" + "foo #(.W(1), .L(2)) bar();\n" + " endmodule\n", + "module m;\n" + " foo #(\n" + " .W(1),\n" // indent +2 spaces only + " .L(2)\n" + " ) bar ();\n" + "endmodule\n"}, + {// class data member with two parameters + "class c ;\n" + " foo_pkg::bar_t#(\n" + ".W(2),.L(4)" + ") baz;\n" + " endclass\n", + "class c;\n" + " foo_pkg::bar_t #(\n" + " .W(2),\n" // indent +2 spaces only + " .L(4)\n" + " ) baz;\n" + "endclass\n"}, + {// typedef with two parameters + "typedef \n" + " foo_pkg::bar_t #(" + ".W(2),.L(4)" + ") baz;\n", + "typedef foo_pkg::bar_t#(\n" + " .W(2),\n" // indent +2 spaces only + " .L(4)\n" + ") baz;\n"}, + }; + FormatStyle style; + style.column_limit = 40; + style.indentation_spaces = 2; + style.wrap_spaces = 4; + // Indent 2 spaces instead of wrapping 4 spaces. + style.named_parameter_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 index 1ced6e1..b6df5a0 100644 --- a/verilog/formatting/tree_unwrapper.cc +++ b/verilog/formatting/tree_unwrapper.cc
@@ -1015,14 +1015,16 @@ // module instantiations (which look like data declarations) want to // expand one parameter/port per line. case NodeEnum::kActualParameterByNameList: { - const int indent = suppress_indentation ? 0 : style_.wrap_spaces; + const int indent = + suppress_indentation ? 0 : style_.NamedParameterIndentation(); VisitIndentedSection(node, indent, PartitionPolicyEnum::kTabularAlignment); break; } case NodeEnum::kPortActualList: // covers named and positional ports { - const int indent = suppress_indentation ? 0 : style_.wrap_spaces; + const int indent = + suppress_indentation ? 0 : style_.NamedPortIndentation(); const auto policy = Context().IsInside(NodeEnum::kDataDeclaration) ? PartitionPolicyEnum::kTabularAlignment : PartitionPolicyEnum::kFitOnLineElseExpand;
diff --git a/verilog/tools/formatter/verilog_format.cc b/verilog/tools/formatter/verilog_format.cc index 28f87e6..188c4a4 100644 --- a/verilog/tools/formatter/verilog_format.cc +++ b/verilog/tools/formatter/verilog_format.cc
@@ -124,10 +124,16 @@ // Do not expect to be able to use these in the long term, once they find // a better home in a configuration struct. +// "indent" means 2 spaces, "wrap" means 4 spaces. 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}"); +ABSL_FLAG(IndentationStyle, named_parameter_indentation, + IndentationStyle::kWrap, + "Indent named parameter assignments: {indent,wrap}"); +ABSL_FLAG(IndentationStyle, named_port_indentation, IndentationStyle::kWrap, + "Indent named port connections: {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