Generated gRPC comments are merged with prior closing brace

[MichaelUrman]

In several places this results in the comment for the next section becoming a trailing comment on the prior syntax. For example in gen/grpc/*/client/client.go, diffed against the output from an embarassingly old version of goa:

@@ -20,27 +20,23 @@ import (
 type Client struct {
        grpccli some_servicepb.SomeServiceClient
        opts    []grpc.CallOption
-}
-
-// NewClient instantiates gRPC client for all the SomeService service servers.
+} // NewClient instantiates gRPC client for all the SomeService service servers.

I believe this is due to missing newlines at the end of the template files. (The http client_init.go.tpl ends in 7d 0a; the grpc client_init.go.tpl ends in 7d.)

% xxd http/codegen/templates/client_init.go.tpl | tail -n 3
00000310: 0909 636f 6e66 6967 7572 6572 3a20 6366  ..configurer: cf
00000320: 6e2c 0a09 097b 7b2d 2065 6e64 207d 7d0a  n,...{{- end }}.
00000330: 097d 0a7d 0a                             .}.}.

% xxd grpc/codegen/templates/client_init.go.tpl | tail -n 3
00000110: 6365 496e 6974 207d 7d28 6363 292c 0a09  ceInit }}(cc),..
00000120: 096f 7074 733a 206f 7074 732c 0a09 7d0a  .opts: opts,..}.
00000130: 7d

I'm unclear why the current templates have a mix of present and missing final newlines. I suspect they should at least be more similar between http and grpc, if not just always have them. Alternately, perhaps readTemplate should ensure a trailing newline.

# (Note that the root codegen also has several templates without newlines.)
% find http grpc -name \*.tpl -print0 | xargs -0 -L1 bash -c 'test $(xxd -p -s -1 "$0") != 0a && echo "$0: no newline"'
http/codegen/templates/path_init.go.tpl: no newline
http/codegen/templates/request_init.go.tpl: no newline
http/codegen/templates/partial/client_type_conversion.go.tpl: no newline
http/codegen/templates/partial/query_slice_conversion.go.tpl: no newline
http/codegen/templates/partial/response.go.tpl: no newline
http/codegen/templates/partial/header_conversion.go.tpl: no newline
http/codegen/templates/partial/websocket_upgrade.go.tpl: no newline
http/codegen/templates/partial/single_response.go.tpl: no newline
http/codegen/templates/partial/query_type_conversion.go.tpl: no newline
http/codegen/templates/partial/slice_item_conversion.go.tpl: no newline
http/codegen/templates/partial/element_slice_conversion.go.tpl: no newline
http/codegen/templates/partial/client_map_conversion.go.tpl: no newline
http/codegen/templates/partial/query_map_conversion.go.tpl: no newline
http/codegen/templates/partial/path_conversion.go.tpl: no newline
http/codegen/templates/response_decoder.go.tpl: no newline
http/codegen/templates/path.go.tpl: no newline
http/codegen/templates/server_start.go.tpl: no newline
http/codegen/templates/append_fs.go.tpl: no newline
http/codegen/templates/cli_end.go.tpl: no newline
grpc/codegen/templates/request_encoder.go.tpl: no newline
grpc/codegen/templates/client_init.go.tpl: no newline
grpc/codegen/templates/do_grpc_cli.go.tpl: no newline
grpc/codegen/templates/partial/convert_string_to_type.go.tpl: no newline
grpc/codegen/templates/partial/convert_type_to_string.go.tpl: no newline
grpc/codegen/templates/client_struct.go.tpl: no newline
grpc/codegen/templates/response_decoder.go.tpl: no newline
grpc/codegen/templates/remote_method_builder.go.tpl: no newline

[raphael]

Makes sense! would you be able to create a PR to fix the missing newline? Unfortunately there's no simple rule one can apply as to whether a template should or should not end with a newline as it depends on its usage. Also Goa will apply go fmt to the generated code so extra newlines get removed.

[MichaelUrman]

Unfortunately there's no simple rule one can apply as to whether a template should or should not end with a newline as it depends on its usage.

This is what prevented me from creating a PR.

Is there a short summary version of the rule that I could use to identify this, or would it be a matter of trying to find where each template is used and assessing it?

As a matter of mechanics, I would propose rules like this to increase the uniformity of the files and the visibility of exceptions:

• The .go.tpl file (better yet all source files) should end in a newline. Always. Ideally add formatting rules, actions, and/or tests for this.
• If a template's content should not end in a newline, use a trailing comment tag to implement and explain this.
  (e.g. {{- /* strip whitespace; template a must appear on the same line as template b */ -}} as the final line.)

This recommendation can be tweaked if, say, the last line needs to include a trailing space.

Also Goa will apply go fmt to the generated code so extra newlines get removed.

Right, go fmt should handle the difference between one, two, or twenty trailing newlines. But zero acts very differently. In the case I highlighted, this changes the semantics of the comment from the following template.

[raphael]

I like the proposal! Historically we were trying to have the generated code formatted correctly independently of go fmt as a good practice. This ended up being almost impossible because of the different ways the same template can end up being used. As a result though some older tests do not run go fmt and instead ensure that the output is formatted as expected independently. I mention this because one strategy could be "it's OK to have extra newlines since go fmt will fix it" but that will require updating a number of tests. Not saying we go that route, we could also choose to be more "precise" and follow the idea you are suggesting of adding a comment when no newline should be added - there might be places where that's required. Just providing additional context...