Add error handling, tests, docs

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 ``flags`` 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/nogo_main.go

@@ -174,7 +174,17 @@ func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFil
 	for _, a := range analyzers {
 		if cfg, ok := configs[a.Name]; ok {
 			for flagKey, flagVal := range cfg.analyzerFlags {
-				a.Flags.Set(flagKey, flagVal)
+				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))
@@ -463,7 +473,9 @@ type config struct {
 	// analyzer will not emit diagnostics for.
 	excludeFiles []*regexp.Regexp
 
-	// analyzerFlags is a map of
+	// 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
 }
 

tests/core/nogo/custom/custom_test.go

@@ -39,7 +39,6 @@ nogo(
         ":foofuncname",
         ":importfmt",
         ":visibility",
-        ":flagger",
     ],
     # config = "",
     visibility = ["//visibility:public"],
@@ -72,16 +71,6 @@ go_library(
     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 = "has_errors",
     srcs = ["has_errors.go"],
@@ -291,48 +280,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
--- flagger.go --
-// flagger crashes when three flags are set in the config or else it no-ops
-package flagger
-
-import (
-        "errors"
-        "fmt"
-
-        "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 requires some flags to be set true in order to pass",
-}
-
-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")
-}
-
 -- config.json --
 {
   "importfmt": {
@@ -350,36 +297,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
   }
 }
 
--- flagger_config.json --
-{
-  "importfmt": {
-    "only_files": {
-      "no_errors\\.go": ""
-    }
-  },
-  "foofuncname": {
-    "only_files": {
-      "no_errors\\.go": ""
-    }
-  },
-  "flagger": {
-    "description": "this will crash on the specified file",
-    "only_files": {
-      "has_errors\\.go": ""
-    },
-    "analyzer_flags": {
-        "bool-switch": "true",
-        "int-switch": "1",
-        "string-switch": "yes"
-    }
-  },
-  "visibility": {
-    "only_files": {
-      "no_errors\\.go": ""
-    }
-  }
-}
-
 -- has_errors.go --
 package haserrors
 
@@ -510,12 +427,6 @@ func Test(t *testing.T) {
 				// note the cross platform regex :)
 				`.*[\\/]cgo[\\/]examplepkg[\\/]pure_src_with_err_calling_native.go:.*function must not be named Foo \(foofuncname\)`,
 			},
-		}, {
-			desc:        "config_flags_triggering_error",
-			target:      "//:has_errors",
-			wantSuccess: false,
-			config:      "flagger_config.json",
-			includes:    []string{"all switches were set -> fail"},
 		}, {
 			desc:        "no_errors",
 			target:      "//:no_errors",

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,16 @@
+Custom nogo analyzers
+=====================
+
+.. _nogo: /go/nogo.rst
+.. _go_library: /docs/go/core/rules.md#_go_library
+
+Tests to ensure that custom `nogo`_ analyzers run and detect errors.
+
+.. contents::
+
+custom_test
+-----------
+Verifies that custom analyzers print errors and fail a `go_library`_ build when
+a configuration file is not provided, and that analyzers with the same package
+name do not conflict. Also checks that custom analyzers can be configured to
+apply only to certain file paths using a custom configuration file.