wrapper: add support for Tricium clang-tidy

This adds a special `WITH_TIDY` mode that's specifically for use with
Tricium. Crucially, this has us dump diagnostics as YAML, and stash a
fair amount of information about each clang-tidy invocation in the same
place where we dump YAML.

These bits are intended to be used by the script added in
I54ecc88d38faa4bfd502d632d3fd5c74734dabc0.

BUG=chromium:1035951
TEST=Ran on all platform2 packages `emerge`able on amd64-generic.

Change-Id: I63ef06dc6ddc016ebb6ba0c4a0cea8320fef7415
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2245785
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>

5 files changed, 215 insertions, 34 deletions

diff --git a/compiler_wrapper/clang_tidy_flag.go b/compiler_wrapper/clang_tidy_flag.go
index d8bf3cb2..edbbb412 100644
--- a/compiler_wrapper/clang_tidy_flag.go
+++ b/compiler_wrapper/clang_tidy_flag.go
@@ -5,15 +5,26 @@
 package main
 
 import (
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 )
 
-func processClangTidyFlags(builder *commandBuilder) (cSrcFile string, useClangTidy bool) {
+type useTidyMode int
+
+const (
+	tidyModeNone useTidyMode = iota
+	tidyModeAll
+	tidyModeTricium
+)
+
+func processClangTidyFlags(builder *commandBuilder) (cSrcFile string, mode useTidyMode) {
 	withTidy, _ := builder.env.getenv("WITH_TIDY")
 	if withTidy == "" {
-		return "", false
+		return "", tidyModeNone
 	}
 	srcFileSuffixes := []string{
 		".c",
@@ -24,48 +35,138 @@ func processClangTidyFlags(builder *commandBuilder) (cSrcFile string, useClangTi
 		".c++",
 	}
 	cSrcFile = ""
+	srcSuffix := ""
 	lastArg := ""
 	for _, arg := range builder.args {
-		if hasAtLeastOneSuffix(arg.value, srcFileSuffixes) && lastArg != "-o" {
-			cSrcFile = arg.value
+		if lastArg != "-o" {
+			for _, suffix := range srcFileSuffixes {
+				if strings.HasSuffix(arg.value, suffix) {
+					srcSuffix = suffix
+					cSrcFile = arg.value
+					break
+				}
+			}
 		}
 		lastArg = arg.value
 	}
-	useClangTidy = cSrcFile != ""
-	return cSrcFile, useClangTidy
-}
 
-func runClangTidy(env env, clangCmd *command, cSrcFile string) error {
-	defaultTidyChecks := strings.Join([]string{
-		"*",
-		"-bugprone-narrowing-conversions",
-		"-cppcoreguidelines-*",
-		"-fuchsia-*",
-		"-google-readability*",
-		"-google-runtime-references",
-		"-hicpp-*",
-		"-llvm-*",
-		"-misc-non-private-member-variables-in-classes",
-		"-misc-unused-parameters",
-		"-modernize-*",
-		"-readability-*",
-	}, ",")
+	if cSrcFile == "" {
+		return "", tidyModeNone
+	}
 
+	if withTidy == "tricium" {
+		// Files generated from protobufs can result in _many_ clang-tidy complaints, and aren't
+		// worth linting in general. Don't.
+		if strings.HasSuffix(cSrcFile, ".pb"+srcSuffix) {
+			mode = tidyModeNone
+		} else {
+			mode = tidyModeTricium
+		}
+	} else {
+		mode = tidyModeAll
+	}
+	return cSrcFile, mode
+}
+
+func calcClangTidyInvocation(env env, clangCmd *command, cSrcFile string, tidyFlags ...string) (*command, error) {
 	resourceDir, err := getClangResourceDir(env, clangCmd.Path)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	clangTidyPath := filepath.Join(filepath.Dir(clangCmd.Path), "clang-tidy")
-	clangTidyCmd := &command{
-		Path: clangTidyPath,
-		Args: append([]string{
-			"-checks=" + defaultTidyChecks,
-			cSrcFile,
-			"--",
-			"-resource-dir=" + resourceDir,
-		}, clangCmd.Args...),
+	args := append([]string{}, tidyFlags...)
+	args = append(args, cSrcFile, "--", "-resource-dir="+resourceDir)
+	args = append(args, clangCmd.Args...)
+	return &command{
+		Path:       clangTidyPath,
+		Args:       args,
 		EnvUpdates: clangCmd.EnvUpdates,
+	}, nil
+}
+
+func runClangTidyForTricium(env env, clangCmd *command, cSrcFile, fixesDir string) error {
+	if err := os.MkdirAll(fixesDir, 0777); err != nil {
+		return fmt.Errorf("creating fixes directory at %q: %v", fixesDir, err)
+	}
+
+	f, err := ioutil.TempFile(fixesDir, "lints-")
+	if err != nil {
+		return fmt.Errorf("making tempfile for tidy: %v", err)
+	}
+	f.Close()
+
+	// `f` is an 'anchor'; it ensures we won't create a similarly-named file in the future.
+	// Hence, we can't delete it.
+	fixesFilePath := f.Name() + ".yaml"
+	fixesMetadataPath := f.Name() + ".json"
+
+	// FIXME(gbiv): Remove `-checks=*` when testing is complete; we should defer to .clang-tidy
+	// files, which are both more expressive and more approachable than `-checks=*`.
+	clangTidyCmd, err := calcClangTidyInvocation(env, clangCmd, cSrcFile, "-checks=*", "--export-fixes="+fixesFilePath)
+	if err != nil {
+		return fmt.Errorf("calculating tidy invocation: %v", err)
+	}
+
+	stdstreams := &strings.Builder{}
+	// Note: We pass nil as stdin as we checked before that the compiler
+	// was invoked with a source file argument.
+	exitCode, err := wrapSubprocessErrorWithSourceLoc(clangTidyCmd,
+		env.run(clangTidyCmd, nil, stdstreams, stdstreams))
+	if err != nil {
+		return err
+	}
+
+	type metadata struct {
+		Args       []string `json:"args"`
+		Executable string   `json:"executable"`
+		ExitCode   int      `json:"exit_code"`
+		LintTarget string   `json:"lint_target"`
+		Stdstreams string   `json:"stdstreams"`
+		Wd         string   `json:"wd"`
+	}
+
+	f, err = os.Create(fixesMetadataPath)
+	if err != nil {
+		return fmt.Errorf("creating fixes metadata: %v", err)
+	}
+
+	meta := &metadata{
+		Args:       clangTidyCmd.Args,
+		Executable: clangTidyCmd.Path,
+		ExitCode:   exitCode,
+		LintTarget: cSrcFile,
+		Stdstreams: stdstreams.String(),
+		Wd:         env.getwd(),
+	}
+	if err := json.NewEncoder(f).Encode(meta); err != nil {
+		return fmt.Errorf("writing fixes metadata: %v", err)
+	}
+
+	if err := f.Close(); err != nil {
+		return fmt.Errorf("finalizing fixes metadata: %v", err)
+	}
+	return nil
+}
+
+func runClangTidy(env env, clangCmd *command, cSrcFile string) error {
+	clangTidyCmd, err := calcClangTidyInvocation(env, clangCmd, cSrcFile, "-checks="+
+		strings.Join([]string{
+			"*",
+			"-bugprone-narrowing-conversions",
+			"-cppcoreguidelines-*",
+			"-fuchsia-*",
+			"-google-readability*",
+			"-google-runtime-references",
+			"-hicpp-*",
+			"-llvm-*",
+			"-misc-non-private-member-variables-in-classes",
+			"-misc-unused-parameters",
+			"-modernize-*",
+			"-readability-*",
+		}, ","))
+	if err != nil {
+		return fmt.Errorf("calculating clang-tidy invocation: %v", err)
 	}
 
 	// Note: We pass nil as stdin as we checked before that the compiler
diff --git a/compiler_wrapper/clang_tidy_flag_test.go b/compiler_wrapper/clang_tidy_flag_test.go
index baf5219e..d45d1bdd 100644
--- a/compiler_wrapper/clang_tidy_flag_test.go
+++ b/compiler_wrapper/clang_tidy_flag_test.go
@@ -273,6 +273,66 @@ func TestPartiallyOmitGomaWithClangTidy(t *testing.T) {
 	})
 }
 
+func TestTriciumClangTidyIsProperlyDetectedFromEnv(t *testing.T) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
+		ctx.env = []string{"WITH_TIDY=tricium"}
+		ctx.cmdMock = func(cmd *command, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
+			switch ctx.cmdCount {
+			case 1:
+				if err := verifyPath(cmd, "usr/bin/clang"); err != nil {
+					t.Error(err)
+				}
+				return nil
+			case 2:
+				if err := verifyPath(cmd, "usr/bin/clang-tidy"); err != nil {
+					return err
+				}
+
+				hasFixesFile := false
+				for _, arg := range cmd.Args {
+					if path := strings.TrimPrefix(arg, "--export-fixes="); path != arg {
+						hasFixesFile = true
+						if !strings.HasPrefix(path, ctx.cfg.triciumNitsDir+"/") {
+							t.Errorf("fixes file was %q; expected it to be in %q", path, ctx.cfg.triciumNitsDir)
+						}
+						break
+					}
+				}
+
+				if !hasFixesFile {
+					t.Error("no fixes file was provided to a tricium invocation")
+				}
+
+				return nil
+			default:
+				return nil
+			}
+		}
+		cmd := ctx.must(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(clangX86_64, mainCc)))
+		if ctx.cmdCount != 3 {
+			t.Errorf("expected 3 calls. Got: %d", ctx.cmdCount)
+		}
+		if err := verifyPath(cmd, "usr/bin/clang"); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
+func TestTriciumClangTidySkipsProtobufFiles(t *testing.T) {
+	withClangTidyTestContext(t, func(ctx *testContext) {
+		ctx.env = []string{"WITH_TIDY=tricium"}
+		cmd := ctx.must(callCompiler(ctx, ctx.cfg,
+			ctx.newCommand(clangX86_64, mainCc+".pb.cc")))
+		if ctx.cmdCount != 1 {
+			t.Errorf("expected tricium clang-tidy to not execute on a protobuf file")
+		}
+		if err := verifyPath(cmd, "usr/bin/clang"); err != nil {
+			t.Error(err)
+		}
+	})
+}
+
 func withClangTidyTestContext(t *testing.T, work func(ctx *testContext)) {
 	withTestContext(t, func(ctx *testContext) {
 		ctx.env = []string{"WITH_TIDY=1"}
diff --git a/compiler_wrapper/compiler_wrapper.go b/compiler_wrapper/compiler_wrapper.go
index 4c3db14a..14de210a 100644
--- a/compiler_wrapper/compiler_wrapper.go
+++ b/compiler_wrapper/compiler_wrapper.go
@@ -90,16 +90,29 @@ func callCompilerInternal(env env, cfg *config, inputCmd *command) (exitCode int
 			return 0, newErrorwithSourceLocf("unsupported compiler: %s", mainBuilder.target.compiler)
 		}
 	} else if mainBuilder.target.compilerType == clangType {
-		cSrcFile, useClangTidy := processClangTidyFlags(mainBuilder)
+		cSrcFile, tidyMode := processClangTidyFlags(mainBuilder)
 		err := prepareClangCommand(mainBuilder)
 		if err != nil {
 			return 0, err
 		}
 		allowCCache := true
-		if useClangTidy {
+		if tidyMode != tidyModeNone {
 			allowCCache = false
 			clangCmdWithoutGomaAndCCache := mainBuilder.build()
-			if err := runClangTidy(env, clangCmdWithoutGomaAndCCache, cSrcFile); err != nil {
+			var err error
+			switch tidyMode {
+			case tidyModeTricium:
+				if cfg.triciumNitsDir == "" {
+					return 0, newErrorwithSourceLocf("tricium linting was requested, but no nits directory is configured")
+				}
+				err = runClangTidyForTricium(env, clangCmdWithoutGomaAndCCache, cSrcFile, cfg.triciumNitsDir)
+			case tidyModeAll:
+				err = runClangTidy(env, clangCmdWithoutGomaAndCCache, cSrcFile)
+			default:
+				panic(fmt.Sprintf("Unknown tidy mode: %v", tidyMode))
+			}
+
+			if err != nil {
 				return 0, err
 			}
 		}
diff --git a/compiler_wrapper/config.go b/compiler_wrapper/config.go
index a122b5a2..3c3668df 100644
--- a/compiler_wrapper/config.go
+++ b/compiler_wrapper/config.go
@@ -29,6 +29,8 @@ type config struct {
 	rootRelPath string
 	// Directory to store errors that were prevented with -Wno-error.
 	newWarningsDir string
+	// Directory to store nits in when using `WITH_TIDY=tricium`.
+	triciumNitsDir string
 	// Version. Only used for printing via -print-cmd.
 	version string
 }
@@ -137,6 +139,7 @@ var crosHardenedConfig = &config{
 		"-Wno-implicit-int-float-conversion",
 	},
 	newWarningsDir: "/tmp/fatal_clang_warnings",
+	triciumNitsDir: "/tmp/linting_output/clang-tidy",
 }
 
 // Flags to be added to non-hardened toolchain.
@@ -166,6 +169,7 @@ var crosNonHardenedConfig = &config{
 		"-Wno-implicit-int-float-conversion",
 	},
 	newWarningsDir: "/tmp/fatal_clang_warnings",
+	triciumNitsDir: "/tmp/linting_output/clang-tidy",
 }
 
 // Flags to be added to host toolchain.
@@ -200,6 +204,7 @@ var crosHostConfig = &config{
 		"-Wno-implicit-int-float-conversion",
 	},
 	newWarningsDir: "/tmp/fatal_clang_warnings",
+	triciumNitsDir: "/tmp/linting_output/clang-tidy",
 }
 
 var androidConfig = &config{
@@ -211,4 +216,5 @@ var androidConfig = &config{
 	clangFlags:       []string{},
 	clangPostFlags:   []string{},
 	newWarningsDir:   "",
+	triciumNitsDir:   "",
 }
diff --git a/compiler_wrapper/testutil_test.go b/compiler_wrapper/testutil_test.go
index 5d8ef920..d23a8434 100644
--- a/compiler_wrapper/testutil_test.go
+++ b/compiler_wrapper/testutil_test.go
@@ -141,6 +141,7 @@ func (ctx *testContext) mustFail(exitCode int) string {
 func (ctx *testContext) updateConfig(cfg *config) {
 	*ctx.cfg = *cfg
 	ctx.cfg.newWarningsDir = filepath.Join(ctx.tempDir, "fatal_clang_warnings")
+	ctx.cfg.triciumNitsDir = filepath.Join(ctx.tempDir, "tricium_nits")
 }
 
 func (ctx *testContext) newCommand(path string, args ...string) *command {