Allow the checker to run vet (#1673)

Add a go_checker attribute that causes the generated checker
binary to run 'go tool vet'.

Just like in the official Go build toolchain, only a subset of
vet checks are run. Any vet findings will fail compilation. To avoid
breaking users, vet is disabled by default.

While there:
- Fix issues with the importer used to load analysis packages.
- Silence type-checking errors generated by the checker binary. It will
  now fail silently if type-checking errors occur.
- Refactor the checker binary and unit tests to improve readability.

BUILD.bazel

@@ -16,7 +16,6 @@ builders(
     visibility = ["//visibility:public"],
 )
 
-# TODO(samueltan): add checks.
 go_checker(
     name = "default_checker",
     visibility = ["//visibility:public"],

go/checks.rst

@@ -8,6 +8,7 @@ Go build-time code analysis
 .. _GoLibrary: providers.rst#GoLibrary
 .. _GoSource: providers.rst#GoSource
 .. _GoArchive: providers.rst#GoArchive
+.. _vet: https://golang.org/cmd/vet/
 
 .. role:: param(kbd)
 .. role:: type(emphasis)
@@ -19,11 +20,9 @@ Please do not rely on it for production use, but feel free to use it and file
 issues.
 
 rules_go allows you to define custom source-code-level checks that are executed
-alongside the Go compiler. These checks print error messages by default, but can
-be configured to also fail compilation. Checks can be used to catch bug and
-anti-patterns early in the development process.
-
-**TODO**: make vet run by default.
+alongside the Go compiler. These checks will print error messages and fail
+compilation if they find issues in source code. Checks can be used to catch
+bugs and coding anti-patterns early in the development process.
 
 .. contents:: :depth: 2
 
@@ -73,6 +72,9 @@ syntax trees (ASTs) and type information for that package. For example:
       return &analysis.Result{Findings: findings}, nil
     }
 
+Any findings returned by the check will fail compilation. Do not emit findings
+unless they are severe enough to warrant interrupting the compiler.
+
 Each check must be written as a `go_tool_library`_ rule. This rule
 is identical to `go_library`_ but avoids a bootstrapping problem, which
 we will explain later. For example:
@@ -213,6 +215,27 @@ This label referencing this configuration file must be provided as the
         visibility = ["//visibility:public"],
     )
 
+Running vet
+~~~~~~~~~~~
+
+You can choose to run the `vet`_ tool alongside the Go compiler and custom
+checks by setting the ``vet`` attribute of your `go_checker`_ rule:
+
+.. code:: bzl
+
+    go_checker(
+        name = "my_checker",
+        vet = True,
+        visibility = ["//visibility:public"],
+    )
+
+`vet`_ will print error messages and fail compilation if it finds any issues in
+the source code being built. Just like in the upstream Go build toolchain, only
+a subset of `vet`_ checks which are 100% accurate will be run.
+
+In the above example, `vet`_ will run alone. It can also run alongside custom
+checks given by the ``deps`` attribute.
+
 API
 ---
 
@@ -243,6 +266,10 @@ Attributes
 +----------------------------+-----------------------------+---------------------------------------+
 | JSON configuration file that configures one or more of the checks in `deps`.                     |
 +----------------------------+-----------------------------+---------------------------------------+
+| :param:`vet`               | :type:`bool`                | :value:`False`                        |
++----------------------------+-----------------------------+---------------------------------------+
+| Whether to run the `vet` tool.                                                                   |
++----------------------------+-----------------------------+---------------------------------------+
 
 Example
 ^^^^^^^

go/private/actions/compile.bzl

@@ -67,6 +67,7 @@ def emit_compile(
     builder_args.add_all(archives, before_each = "-importmap", map_each = _importmap)
     builder_args.add_all(archives, before_each = "-archivefile", map_each = _archivefile)
     builder_args.add_all(["-o", out_lib])
+    builder_args.add_all(["-stdlib", go.toolchain.sdk.root_file.dirname+"/pkg/"+go.toolchain.sdk.goos+"_"+go.toolchain.sdk.goarch])
     builder_args.add_all(["-package_list", go.package_list])
     if testfilter:
         builder_args.add_all(["-testfilter", testfilter])

go/private/rules/builders.bzl

@@ -33,6 +33,7 @@ def _builders_impl(ctx):
                 ctx.executable._pack,
                 ctx.executable._link,
                 ctx.executable._cgo,
+                ctx.executable._checker_generator,
                 ctx.executable._test_generator,
                 ctx.executable._cover,
             ]),

go/private/rules/checker.bzl

@@ -33,12 +33,14 @@ def _go_checker_impl(ctx):
     go = go_context(ctx)
     checker_main = go.declare_file(go, "checker_main.go")
     checker_args = ctx.actions.args()
-    checker_args.add(["-output", checker_main])
+    checker_args.add("-output", checker_main)
     check_archives = [get_archive(dep) for dep in ctx.attr.deps]
     check_importpaths = [archive.data.importpath for archive in check_archives]
-    checker_args.add(check_importpaths, before_each = "-check_importpath")
+    checker_args.add_all(check_importpaths, before_each = "-check_importpath")
+    if ctx.attr.vet:
+        checker_args.add("-vet")
     if ctx.file.config:
-        checker_args.add(["-config", ctx.file.config])
+        checker_args.add("-config", ctx.file.config.path)
     ctx.actions.run(
         outputs = [checker_main],
         mnemonic = "GoGenChecker",
@@ -81,11 +83,13 @@ go_checker = go_rule(
     attrs = {
         "deps": attr.label_list(
             providers = [GoArchive],
-            # TODO(samueltan): make this attribute mandatory.
         ),
         "config": attr.label(
             allow_single_file = True,
         ),
+        "vet": attr.bool(
+            default = False,
+        ),
         "_analysis": attr.label(
             default = "@io_bazel_rules_go//go/tools/analysis:analysis",
         ),

go/tools/builders/BUILD.bazel

@@ -37,6 +37,7 @@ go_tool_binary(
         "env.go",
         "filter.go",
         "flags.go",
+        "vet.go",
     ],
     visibility = ["//visibility:public"],
 )
@@ -71,6 +72,7 @@ go_source(
     srcs = [
         "checker_main.go",
         "flags.go",
+        "vet.go",
     ],
     deps = [
         "@io_bazel_rules_go//go/tools/analysis:analysis",