Inconsistent formatting of parameterized types

[rfdonnelly]

Test case

Command:

verible-v0.0-2037-g4cccc6b2/bin/verible-verilog-format parameterized_types.svh

File: parameterized_types.svh

module a_module (
    a_type#(a_literal) a_port
);
  a_type#(a_literal) a_variable;

  function a_function(a_type#(a_literal) a_parameter);
    a_type#(a_literal) a_variable;
  endfunction
endmodule

Actual output

module a_module (
    a_type#(a_literal) a_port
);
  a_type #(a_literal) a_variable;

  function a_function(a_type#(a_literal) a_parameter);
    a_type #(a_literal) a_variable;
  endfunction
endmodule

Note that module port declarations and function port declarations don't have a space between a_type and the parameter_value_assignment (#(a_literal)). While the a_variable declarations do have a space.

Expected or suggested output

module a_module (
    a_type#(a_literal) a_port
);
  a_type#(a_literal) a_variable;

  function a_function(a_type#(a_literal) a_parameter);
    a_type#(a_literal) a_variable;
  endfunction
endmodule

Here's a diff from actual to expected:

--- actual
+++ expected
@@ -1,10 +1,10 @@
 module a_module (
     a_type#(a_literal) a_port
 );
-  a_type #(a_literal) a_variable;
+  a_type#(a_literal) a_variable;

   function a_function(a_type#(a_literal) a_parameter);
-    a_type #(a_literal) a_variable;
+    a_type#(a_literal) a_variable;
   endfunction
 endmodule

[rfdonnelly]

I've narrowed down this behavior to this section of the code: https://github.com/chipsalliance/verible/blob/8e8cc27d934f8f8bab13877cb457b47b35f0220c/verilog/formatting/token_annotator.cc#L500-L513

Here is a naïve fix that gives me the output I expected.

diff --git a/verilog/formatting/token_annotator.cc b/verilog/formatting/token_annotator.cc
index e53fd44f..106ff879 100644
--- a/verilog/formatting/token_annotator.cc
+++ b/verilog/formatting/token_annotator.cc
@@ -508,7 +508,7 @@ static WithReason<int> SpacesRequiredBetween(
             {})) {
       return {0, "No space before # when direct parent is kUnqualifiedId."};
     } else {
-      return {1, "Spaces before # in most other contexts."};
+      return {0, "Spaces before # in most other contexts."};
     }
   }

However, it also removes the space for type definitions which is undesired. For example, this naive fix changes module a_module #(... to module a_module#(.

A better fix is to modify the conditional. But I don't know enough about the code yet to make an informed change here.