RFC: build/analysis: add commentwrap
This adds a Go analyzer which limits the length of comment lines to 80
characters.
Rationale:
Monogon currently follows gofmt style. Gofmt in itself is already quite
opinionated, but one thing it explicitly does not check for is maximum
line length.
This implements a limit for the maximum length of a comment line in Go
source within Monogon. It explicitly does not limit code line length, as
these can be handled much more easily by soft reflows.
The tool used, github.com/corverroos/commentwrap, will now be
automatically ran by our nogo pass, and prevent any line of commnets
within Go to be longer than 80 characters, with the exception of:
- cgo/generate directives
- TODOs
- indented comments (eg. sample code or long URLs)
Downsides:
1. We have to reformat the entire codebase. CR/67 does this.
2. We end up with a bulk Git commit that will pollute Git history. A
followup CR attempts to resolve this by using Git's ignoreRevsFile
functionality.
3. There's currently no integration with IntelliJ and no way to
automatically reformat code. If this RFC gets approved, a follow up
CR will be created that adds integration/automation to make this
easier to work against.
Open questions:
1. Is 80 characters the right limit? I, personally, quite like it, but
am willing to compromise on line length.
Change-Id: I063d64596ca5ef038a8426c6b9f806b65c18451e
Reviewed-on: https://review.monogon.dev/c/monogon/+/66
Reviewed-by: Leopold Schabel <leo@nexantic.com>
diff --git a/build/analysis/BUILD.bazel b/build/analysis/BUILD.bazel
index edaefbe..621e30b 100644
--- a/build/analysis/BUILD.bazel
+++ b/build/analysis/BUILD.bazel
@@ -33,5 +33,13 @@
name = "nogo",
config = "nogo_config.json",
visibility = ["//visibility:public"],
- deps = govet,
+ deps = govet + [
+ # This analyzer ensures that all comment lines are <= 80 characters long
+ # in Go source. This is in line with general practices around the Go
+ # community, where code lines can be as long as needed (and is expected
+ # to be soft-reflowable by text editors), but comments are kept at a
+ # 'standard' 80 characters long, as prose within comment blocks does not
+ # soft-reflow well.
+ "@com_github_corverroos_commentwrap//:go_tool_library",
+ ],
)
diff --git a/build/analysis/nogo_config.json b/build/analysis/nogo_config.json
index c13baaf..ae01f0a 100644
--- a/build/analysis/nogo_config.json
+++ b/build/analysis/nogo_config.json
@@ -88,5 +88,11 @@
"external/com_github_google_gvisor/": "third_party",
"external/com_github_sbezverk_nfproxy/": "third_party"
}
+ },
+ "commentwrap": {
+ "exclude_files": {
+ "dev_source_monogon/": "temporary until fixup CR",
+ "external/": "third_party"
+ }
}
}
diff --git a/build/fietsje/main.go b/build/fietsje/main.go
index 1345644..50055ab 100644
--- a/build/fietsje/main.go
+++ b/build/fietsje/main.go
@@ -135,6 +135,18 @@
// goimports
p.collectOverride("golang.org/x/tools", "v0.0.0-20201215171152-6307297f4651")
+ // commentwrap is used as a nogo analyzer to stick to a maximum line
+ // length for comments.
+ // We have to patch both it and its only direct dependency to add generated
+ // go_tool_library targets. This is needed as Gazelle doesn't generate them,
+ // because they're a temporary solution to a problem that shouldn't exist soon:
+ // https://github.com/bazelbuild/rules_go/issues/2374
+ p.collect("github.com/corverroos/commentwrap", "2926638be44ce0c6c0ee2471e9b5ad9473c984cd",
+ patches("commentwrap-tool-library.patch"),
+ ).with(patches("reflow-tool-library.patch")).use(
+ "github.com/muesli/reflow",
+ )
+
// First generate the repositories starlark rule into memory. This is because rendering will lock all unlocked
// dependencies, which might take a while. If a use were to interrupt it now, they would end up with an incomplete
// repositories.bzl and would have to restore from git.
diff --git a/third_party/go/patches/commentwrap-tool-library.patch b/third_party/go/patches/commentwrap-tool-library.patch
new file mode 100644
index 0000000..a5bf2cb
--- /dev/null
+++ b/third_party/go/patches/commentwrap-tool-library.patch
@@ -0,0 +1,38 @@
+Copyright 2021 The Monogon Project Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+This adds a go_tool_library target to github.com/corverroos/commentwrap. This can
+be removed when go_tool_library gets replaced with transitions:
+https://github.com/bazelbuild/rules_go/issues/2374
+
+diff -ur a/BUILD.bazel b/BUILD.bazel
+--- a/BUILD.bazel 2021-05-20 13:21:49.582041541 +0200
++++ b/BUILD.bazel 2021-05-21 14:25:51.550052340 +0200
+@@ -1,4 +1,15 @@
+-load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
++load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "go_tool_library")
++
++go_tool_library(
++ name = "go_tool_library",
++ srcs = ["commentwrap.go"],
++ importpath = "github.com/corverroos/commentwrap",
++ visibility = ["//visibility:public"],
++ deps = [
++ "@com_github_muesli_reflow//:go_tool_library",
++ "@org_golang_x_tools//go/analysis:go_tool_library",
++ ],
++)
+
+ go_library(
+ name = "go_default_library",
diff --git a/third_party/go/patches/reflow-tool-library.patch b/third_party/go/patches/reflow-tool-library.patch
new file mode 100644
index 0000000..dcee286
--- /dev/null
+++ b/third_party/go/patches/reflow-tool-library.patch
@@ -0,0 +1,37 @@
+Copyright 2021 The Monogon Project Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+This adds a go_tool_library target to github.com/muesli/reflow. This can
+be removed when go_tool_library gets replaced with transition:
+https://github.com/bazelbuild/rules_go/issues/2374
+
+diff -ur a/BUILD.bazel b/BUILD.bazel
+--- a/BUILD.bazel 2021-05-21 14:36:39.411306726 +0200
++++ b/BUILD.bazel 2021-05-21 14:37:02.602460127 +0200
+@@ -1,4 +1,14 @@
+-load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
++load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "go_tool_library")
++
++go_tool_library(
++ name = "go_tool_library",
++ srcs = [
++ "ansibuffer.go",
++ "reflow.go",
++ ],
++ importpath = "github.com/muesli/reflow",
++ visibility = ["//visibility:public"],
++)
+
+ go_library(
+ name = "go_default_library",
diff --git a/third_party/go/repositories.bzl b/third_party/go/repositories.bzl
index f5fd825..bdb7601 100644
--- a/third_party/go/repositories.bzl
+++ b/third_party/go/repositories.bzl
@@ -475,6 +475,20 @@
],
)
go_repository(
+ name = "com_github_corverroos_commentwrap",
+ importpath = "github.com/corverroos/commentwrap",
+ version = "v0.0.0-20191204065359-2926638be44c",
+ sum = "h1:toeMwwechJKH0iwOoGJLZK6x42Ba9si+816KxqmgFc8=",
+ patches = [
+ "//third_party/go/patches:commentwrap-tool-library.patch",
+ ],
+ patch_args = ["-p1"],
+ build_extra_args = [
+ "-go_naming_convention=go_default_library",
+ "-go_naming_convention_external=go_default_library",
+ ],
+ )
+ go_repository(
name = "com_github_cosiner_argv",
importpath = "github.com/cosiner/argv",
version = "v0.0.0-20170225145430-13bacc38a0a5",
@@ -1571,6 +1585,20 @@
],
)
go_repository(
+ name = "com_github_muesli_reflow",
+ importpath = "github.com/muesli/reflow",
+ version = "v0.0.0-20191128061954-86f094cbed14",
+ sum = "h1:99aDTygRy9yEwggATz+ZLrDFRsjRog5BqbAfsr47Ztw=",
+ patches = [
+ "//third_party/go/patches:reflow-tool-library.patch",
+ ],
+ patch_args = ["-p1"],
+ build_extra_args = [
+ "-go_naming_convention=go_default_library",
+ "-go_naming_convention_external=go_default_library",
+ ],
+ )
+ go_repository(
name = "com_github_munnerz_goautoneg",
importpath = "github.com/munnerz/goautoneg",
version = "v0.0.0-20191010083416-a7dc8b61c822",
diff --git a/third_party/go/shelf.pb.text b/third_party/go/shelf.pb.text
index 64fa40e..f519385 100644
--- a/third_party/go/shelf.pb.text
+++ b/third_party/go/shelf.pb.text
@@ -629,6 +629,13 @@
semver: "v0.0.0-20180928190104-399ea9e2e55f"
>
entry: <
+ import_path: "github.com/corverroos/commentwrap"
+ version: "2926638be44ce0c6c0ee2471e9b5ad9473c984cd"
+ bazel_name: "com_github_corverroos_commentwrap"
+ sum: "h1:toeMwwechJKH0iwOoGJLZK6x42Ba9si+816KxqmgFc8="
+ semver: "v0.0.0-20191204065359-2926638be44c"
+>
+entry: <
import_path: "github.com/cosiner/argv"
version: "v0.0.0-20170225145430-13bacc38a0a5"
bazel_name: "com_github_cosiner_argv"
@@ -1847,6 +1854,13 @@
semver: "v0.0.0-20200520151820-abd8a0e76976"
>
entry: <
+ import_path: "github.com/muesli/reflow"
+ version: "v0.0.0-20191128061954-86f094cbed14"
+ bazel_name: "com_github_muesli_reflow"
+ sum: "h1:99aDTygRy9yEwggATz+ZLrDFRsjRog5BqbAfsr47Ztw="
+ semver: "v0.0.0-20191128061954-86f094cbed14"
+>
+entry: <
import_path: "github.com/munnerz/goautoneg"
version: "v0.0.0-20191010083416-a7dc8b61c822"
bazel_name: "com_github_munnerz_goautoneg"