Resolves #3081: add analyzer_flags to nogo config

go/nogo.rst

@@ -195,13 +195,23 @@ contain the following key-value pairs:
 | in both ``only_files`` and ``exclude_files``, the analyzer will not emit diagnostics for that    |
 | file.                                                                                            |
 +----------------------------+---------------------------------------------------------------------+
+| ``"analyzer_flags"``       | :type:`dictionary, string to string`                                |
++----------------------------+---------------------------------------------------------------------+
+| Passes on a set of flags as defined by the Go ``flag`` package to the analyzer via the           |
+| ``analysis.Analyzer.Flags`` field. Its keys are the flag names *without* a ``-`` prefix, and its |
+| values are the flag values. nogo will exit with an error upon receiving flags not recognized by  |
+| the analyzer or upon receiving ill-formatted flag values as defined by the corresponding         |
+| ``flag.Value`` specified by the analyzer.                                                        |
++----------------------------+---------------------------------------------------------------------+
 
 Example
 ^^^^^^^
 
 The following configuration file configures the analyzers named ``importunsafe``
 and ``unsafedom``. Since the ``loopclosure`` analyzer is not explicitly
 configured, it will emit diagnostics for all Go files built by Bazel.
+``unsafedom`` will receive a flag equivalent to ``-block-unescaped-html=false``
+on a command line driver.
 
 .. code:: json
 
@@ -218,7 +228,10 @@ configured, it will emit diagnostics for all Go files built by Bazel.
         },
         "exclude_files": {
           "src/(third_party|vendor)/.*": "enforce DOM safety requirements only on first-party code"
-        }
+        },
+        "analyzer_flags": {
+            "block-unescaped-html": "false",
+        },
       }
     }
 

go/tools/builders/generate_nogo_main.go

@@ -54,6 +54,13 @@ var analyzers = []*analysis.Analyzer{
 var configs = map[string]config{
 {{- range $name, $config := .Configs}}
 	{{printf "%q" $name}}: config{
+		{{- if $config.AnalyzerFlags }}
+		analyzerFlags: map[string]string {
+			{{- range $flagKey, $flagValue := $config.AnalyzerFlags}}
+			{{printf "%q: %q" $flagKey $flagValue}},
+			{{- end}}
+		},
+		{{- end -}}
 		{{- if $config.OnlyFiles}}
 		onlyFiles: []*regexp.Regexp{
 			{{- range $path, $comment := $config.OnlyFiles}}
@@ -171,8 +178,9 @@ func buildConfig(path string) (Configs, error) {
 		}
 		configs[name] = Config{
 			// Description is currently unused.
-			OnlyFiles:    config.OnlyFiles,
-			ExcludeFiles: config.ExcludeFiles,
+			OnlyFiles:     config.OnlyFiles,
+			ExcludeFiles:  config.ExcludeFiles,
+			AnalyzerFlags: config.AnalyzerFlags,
 		}
 	}
 	return configs, nil
@@ -181,7 +189,8 @@ func buildConfig(path string) (Configs, error) {
 type Configs map[string]Config
 
 type Config struct {
-	Description  string
-	OnlyFiles    map[string]string `json:"only_files"`
-	ExcludeFiles map[string]string `json:"exclude_files"`
+	Description   string
+	OnlyFiles     map[string]string `json:"only_files"`
+	ExcludeFiles  map[string]string `json:"exclude_files"`
+	AnalyzerFlags map[string]string `json:"analyzer_flags"`
 }

go/tools/builders/nogo_main.go

@@ -172,6 +172,21 @@ func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFil
 
 	roots := make([]*action, 0, len(analyzers))
 	for _, a := range analyzers {
+		if cfg, ok := configs[a.Name]; ok {
+			for flagKey, flagVal := range cfg.analyzerFlags {
+				if err := a.Flags.Set(flagKey, flagVal); err != nil {
+					if strings.HasPrefix(flagKey, "-") {
+						return "", nil, fmt.Errorf(
+							"%s: flag should not begin with '-': %s", a.Name, flagKey)
+					}
+					if flag := a.Flags.Lookup(flagKey); flag == nil {
+						return "", nil, fmt.Errorf("%s: unrecognized flag: %s", a.Name, flagKey)
+					}
+					return "", nil, fmt.Errorf(
+						"%s: invalid value for flag: %s=%s", a.Name, flagKey, flagVal)
+				}
+			}
+		}
 		roots = append(roots, visit(a))
 	}
 
@@ -457,6 +472,11 @@ type config struct {
 	// excludeFiles is a list of regular expressions that match files that an
 	// analyzer will not emit diagnostics for.
 	excludeFiles []*regexp.Regexp
+
+	// analyzerFlags is a map of flag names to flag values which will be passed
+	// to Analyzer.Flags. Note that no leading '-' should be present in a flag
+	// name
+	analyzerFlags map[string]string
 }
 
 // importer is an implementation of go/types.Importer that imports type

tests/core/nogo/custom/flags/BUILD.bazel

@@ -0,0 +1,6 @@
+load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test")
+
+go_bazel_test(
+    name = "flags_test",
+    srcs = ["flags_test.go"],
+)

tests/core/nogo/custom/flags/README.rst

@@ -0,0 +1,19 @@
+Custom nogo analyzer flags
+=====================
+
+.. _nogo: /go/nogo.rst
+.. _go_library: /docs/go/core/rules.md#_go_library
+
+Tests to ensure that custom `nogo`_ analyzers that consume flags can be
+supplied those flags via nono config.
+
+.. contents::
+
+flags_test
+-----------
+Verifies that a simple custom analyzer's behavior can be modified by setting
+its analyzer flags in the nogo driver, and that these flags can be provided to
+the driver via the nogo config `analyzer_flags` field. Also checks that
+invalid flags as defined by the `flag` package cause the driver to immediately
+return an error.
+

tests/core/nogo/custom/flags/flags_test.go

@@ -0,0 +1,262 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// 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.
+
+package flags_test
+
+import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"regexp"
+	"testing"
+
+	"github.com/bazelbuild/rules_go/go/tools/bazel_testing"
+)
+
+const origConfig = `# config = "",`
+
+func TestMain(m *testing.M) {
+	bazel_testing.TestMain(m, bazel_testing.Args{
+		Nogo: "@//:nogo",
+		Main: `
+-- BUILD.bazel --
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo")
+
+nogo(
+    name = "nogo",
+    deps = [
+        ":flagger",
+    ],
+    # config = "",
+    visibility = ["//visibility:public"],
+)
+
+go_library(
+    name = "flagger",
+    srcs = ["flagger.go"],
+    importpath = "flaggeranalyzer",
+    deps = [
+        "@org_golang_x_tools//go/analysis",
+    ],
+    visibility = ["//visibility:public"],
+)
+
+go_library(
+    name = "some_file",
+    srcs = ["some_file.go"],
+    importpath = "somefile",
+    deps = [":dep"],
+)
+
+go_library(
+    name = "dep",
+    srcs = ["dep.go"],
+    importpath = "dep",
+)
+
+-- flagger.go --
+// flagger crashes when three flags are set in the config or else it no-ops
+package flagger
+
+import (
+        "errors"
+
+        "golang.org/x/tools/go/analysis"
+)
+
+var (
+        boolSwitch   bool
+        stringSwitch string
+        intSwitch    int
+)
+
+var Analyzer = &analysis.Analyzer{
+        Name: "flagger",
+        Run:  run,
+        Doc:  "Dummy analyzer that crashes when all its flags are set correctly",
+}
+
+func init() {
+        Analyzer.Flags.BoolVar(&boolSwitch, "bool-switch", false, "Bool must be set to true to run")
+        Analyzer.Flags.StringVar(&stringSwitch, "string-switch", "no", "String must be set to \"yes\" to run")
+        Analyzer.Flags.IntVar(&intSwitch, "int-switch", 0, "Int must be set to 1 to run")
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+        if !boolSwitch {
+                return nil, nil
+        }
+        if stringSwitch != "yes" {
+                return nil, nil
+        }
+        if intSwitch != 1 {
+                return nil, nil
+        }
+        return nil, errors.New("all switches were set -> fail")
+}
+
+-- all_flags_set.json --
+{
+  "flagger": {
+    "description": "this will crash on every file",
+    "analyzer_flags": {
+        "bool-switch": "true",
+        "int-switch": "1",
+        "string-switch": "yes"
+    }
+  }
+}
+
+-- two_flags_set.json --
+{
+  "flagger": {
+    "description": "this will succeed on every file",
+    "analyzer_flags": {
+        "bool-switch": "true",
+        "int-switch": "1"
+    }
+  }
+}
+
+-- invalid_int.json --
+{
+  "flagger": {
+    "description": "this will crash immediately due to an invalid int flag",
+    "analyzer_flags": {
+        "int-switch": "one",
+        "string-switch": "yes"
+    }
+  }
+}
+
+-- nonexistent_flag.json --
+{
+  "flagger": {
+    "description": "this will crash immediately due to a nonexistent flag",
+    "analyzer_flags": {
+        "int-switch": "1",
+        "bool-switch": "true",
+        "string-switch": "yes",
+        "description": "This is a good analyzer"
+    }
+  }
+}
+
+-- hyphenated_flag.json --
+{
+  "flagger": {
+    "description": "this will crash immediately due to a hyphenated flag",
+    "analyzer_flags": {
+      "-int-switch": "1"
+    }
+  }
+}
+
+-- some_file.go --
+// package somefile contains a file and has a dep
+package somefile
+
+import "dep"
+
+func Baz() int {
+	dep.D()
+	return 1
+}
+
+-- dep.go --
+package dep
+
+func D() {
+}
+
+`,
+	})
+}
+
+func Test(t *testing.T) {
+	for _, test := range []struct {
+		desc, config       string
+		wantSuccess        bool
+		includes, excludes []string
+	}{
+		{
+			desc:        "config_flags_triggering_error",
+			wantSuccess: false,
+			config:      "all_flags_set.json",
+			includes:    []string{"all switches were set -> fail"},
+		}, {
+			desc:        "config_flags_triggering_success",
+			wantSuccess: true,
+			config:      "two_flags_set.json",
+		}, {
+			desc:        "invalid_int_triggering_error",
+			wantSuccess: false,
+			config:      "invalid_int.json",
+			includes:    []string{"flagger: invalid value for flag: int-switch=one"},
+		}, {
+			desc:        "nonexistent_flag_triggering_error",
+			wantSuccess: false,
+			config:      "nonexistent_flag.json",
+			includes:    []string{"flagger: unrecognized flag: description"},
+		}, {
+			desc:        "hyphenated_flag_triggering_error",
+			wantSuccess: false,
+			config:      "hyphenated_flag.json",
+			includes:    []string{"flagger: flag should not begin with '-': -int-switch"},
+		},
+	} {
+		t.Run(test.desc, func(t *testing.T) {
+			if test.config != "" {
+				customConfig := fmt.Sprintf("config = %q,", test.config)
+				if err := replaceInFile("BUILD.bazel", origConfig, customConfig); err != nil {
+					t.Fatal(err)
+				}
+				defer replaceInFile("BUILD.bazel", customConfig, origConfig)
+			}
+
+			cmd := bazel_testing.BazelCmd("build", "//:some_file")
+			stderr := &bytes.Buffer{}
+			cmd.Stderr = stderr
+			if err := cmd.Run(); err == nil && !test.wantSuccess {
+				t.Fatal("unexpected success")
+			} else if err != nil && test.wantSuccess {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			for _, pattern := range test.includes {
+				if matched, err := regexp.Match(pattern, stderr.Bytes()); err != nil {
+					t.Fatal(err)
+				} else if !matched {
+					t.Errorf("got output:\n %s\n which does not contain pattern: %s", string(stderr.Bytes()), pattern)
+				}
+			}
+			for _, pattern := range test.excludes {
+				if matched, err := regexp.Match(pattern, stderr.Bytes()); err != nil {
+					t.Fatal(err)
+				} else if matched {
+					t.Errorf("output contained pattern: %s", pattern)
+				}
+			}
+		})
+	}
+}
+
+func replaceInFile(path, old, new string) error {
+	data, err := ioutil.ReadFile(path)
+	if err != nil {
+		return err
+	}
+	data = bytes.ReplaceAll(data, []byte(old), []byte(new))
+	return ioutil.WriteFile(path, data, 0666)
+}