From bd7fbccc635af297db7b36f6c81d0e7db7921cca Mon Sep 17 00:00:00 2001 From: Son Luong Ngoc Date: Mon, 1 Nov 2021 01:32:25 +0100 Subject: [PATCH] cover: use Z instead of underscore for variable name separator (#2995) During the invocation of `bazel coverage`, rules_go will rewrite the source fils using `go tool cover -var ...` so that the variable can be extracted for coverage statistic after test execution. For more information on this, review the relevant Official Go Team's blog post (1) The variable used by rules_go is following the `Cover_snake_case` naming convention, which is natural to starlark and python, but not Golang. For this reason, some static analysis could get triggered by the usage of snake case naming variable while running over coverage source code. The correct solution for this problem should be to make rules_go's static analysis framework `nogo` to NOT run on the generated source code and instead only run on the original source code. However, replacing underscore separators with character `Z` is a relatively cheap fix that would let us unblock static analysis run during `bazel coverage` for now. See discussion in (2) for more details. (1): https://go.dev/blog/cover (2): https://github.com/bazelbuild/rules_go/pull/2984 --- go/private/actions/cover.bzl | 3 ++- go/tools/builders/compilepkg.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go/private/actions/cover.bzl b/go/private/actions/cover.bzl index 2e00dd8168..2b162cd000 100644 --- a/go/private/actions/cover.bzl +++ b/go/private/actions/cover.bzl @@ -44,7 +44,8 @@ def emit_cover(go, source): _, pkgpath = effective_importpath_pkgpath(source.library) srcname = pkgpath + "/" + orig.basename if pkgpath else orig.path - cover_var = "Cover_%s_%s" % (_sanitize(pkgpath), _sanitize(src.basename[:-3])) + _cover_var = "Cover_%s_%s" % (_sanitize(pkgpath), _sanitize(src.basename[:-3])) + cover_var = _cover_var.replace("_", "Z") out = go.declare_file(go, path = "Cover_%s" % _sanitize(src.basename[:-3]), ext = ".cover.go") covered_src_map.pop(src, None) covered_src_map[out] = orig diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index c8896d5b10..60ea5857d9 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -251,6 +251,7 @@ func compileArchive( stem = stem[:len(stem)-len(ext)] } coverVar := fmt.Sprintf("Cover_%s_%d_%s", sanitizePathForIdentifier(importPath), i, sanitizePathForIdentifier(stem)) + coverVar = strings.ReplaceAll(coverVar, "_", "Z") coverSrc := filepath.Join(workDir, fmt.Sprintf("cover_%d.go", i)) if err := instrumentForCoverage(goenv, origSrc, srcName, coverVar, coverMode, coverSrc); err != nil { return err