From 4db2508880b04390ccfa1760a317a807f3824741 Mon Sep 17 00:00:00 2001 From: Yuxin Wang Date: Thu, 3 Aug 2023 23:00:18 -0400 Subject: [PATCH] Replace config.IsTestEnvironment with a configuration --- config/config.go | 9 +++++++++ config/const.go | 4 ---- nilaway.go | 6 +++--- nilaway_test.go | 19 ++++++++++++++++++- testdata/src/prettyprint/main.go | 9 +++++++++ 5 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 testdata/src/prettyprint/main.go diff --git a/config/config.go b/config/config.go index 77395e10..6372f3b3 100644 --- a/config/config.go +++ b/config/config.go @@ -27,6 +27,8 @@ import ( // Config is the struct that stores the user-configurable options for NilAway. type Config struct { + // PrettyPrint indicates whether the error messages should be pretty printed. + PrettyPrint bool // includePkgs is the list of packages to analyze. includePkgs []string // excludePkgs is the list of packages to exclude from analysis. Exclude list takes @@ -111,6 +113,8 @@ var Analyzer = &analysis.Analyzer{ } const ( + // PrettyPrintFlag is the flag for pretty printing the error messages. + PrettyPrintFlag = "pretty-print" // IncludePkgsFlag is the flag name for include package prefixes. IncludePkgsFlag = "include-pkgs" // ExcludePkgsFlag is the flag name for exclude package prefixes. @@ -125,6 +129,7 @@ func newFlagSet() flag.FlagSet { // We do not keep the returned pointer to the flags because we will not use them directly here. // Instead, we will use the flags through the analyzer's Flags field later. + _ = fs.Bool(PrettyPrintFlag, true, "Pretty print the error messages") _ = fs.String(IncludePkgsFlag, "", "Comma-separated list of packages to analyze") _ = fs.String(ExcludePkgsFlag, "", "Comma-separated list of packages to exclude from analysis") _ = fs.String(ExcludeFileDocStringsFlag, "", "Comma-separated list of docstrings to exclude from analysis") @@ -135,12 +140,16 @@ func newFlagSet() flag.FlagSet { func run(pass *analysis.Pass) (any, error) { // Set up default values for the config. conf := &Config{ + PrettyPrint: true, // If the user does not provide an include list, we give an empty package prefix to catch // all packages. includePkgs: []string{""}, } // Override default values if the user provides flags. + if prettyPrint, ok := pass.Analyzer.Flags.Lookup(PrettyPrintFlag).Value.(flag.Getter).Get().(bool); ok { + conf.PrettyPrint = prettyPrint + } if include, ok := pass.Analyzer.Flags.Lookup(IncludePkgsFlag).Value.(flag.Getter).Get().(string); ok && include != "" { conf.includePkgs = strings.Split(include, ",") } diff --git a/config/const.go b/config/const.go index 92493aae..7fd8a21a 100644 --- a/config/const.go +++ b/config/const.go @@ -7,10 +7,6 @@ import ( // This file hosts non-user-configurable parameters --- these are for development and testing purposes only. -// IsTestEnvironment is a global flag that should be to set true if running the test suite, false otherwise. This flag is used -// to disable the pretty printing used in error reporting. -var IsTestEnvironment = false - // StableRoundLimit is the number of rounds in backpropagation algorithm after which, if there is no change // in the collected triggers, the algorithm halts. It is possible to carefully craft known false negative for any value // of StableRoundLimit (check test loopflow.go/longRotNilLoop). Setting this value too low may result in false negatives diff --git a/nilaway.go b/nilaway.go index f833f18c..87fff730 100644 --- a/nilaway.go +++ b/nilaway.go @@ -33,15 +33,15 @@ var Analyzer = &analysis.Analyzer{ Doc: _doc, Run: run, FactTypes: []analysis.Fact{}, - Requires: []*analysis.Analyzer{accumulation.Analyzer}, + Requires: []*analysis.Analyzer{config.Analyzer, accumulation.Analyzer}, } // nilable(result 0) func run(pass *analysis.Pass) (interface{}, error) { + conf := pass.ResultOf[config.Analyzer].(*config.Config) deferredErrors := pass.ResultOf[accumulation.Analyzer].([]analysis.Diagnostic) for _, e := range deferredErrors { - // Pretty print the error messages in console when not in test environments. - if !config.IsTestEnvironment { + if conf.PrettyPrint { e.Message = util.PrettyPrintErrorMessage(e.Message) } pass.Report(e) diff --git a/nilaway_test.go b/nilaway_test.go index 3c59f11f..5be43216 100644 --- a/nilaway_test.go +++ b/nilaway_test.go @@ -19,6 +19,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/require" "go.uber.org/nilaway/config" "golang.org/x/tools/go/analysis/analysistest" ) @@ -216,9 +217,25 @@ func TestGenerics(t *testing.T) { analysistest.Run(t, testdata, Analyzer, "go.uber.org/generics") } +func TestPrettyPrint(t *testing.T) { //nolint:paralleltest + // We specifically do not set this test to be parallel such that this test is run separately + // from the parallel tests. This makes it possible to set the pretty-print flag to true for + // testing and false for the other tests. + err := config.Analyzer.Flags.Set(config.PrettyPrintFlag, "true") + require.NoError(t, err) + defer func() { + err := config.Analyzer.Flags.Set(config.PrettyPrintFlag, "false") + require.NoError(t, err) + }() + + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "prettyprint") +} + func TestMain(m *testing.M) { - config.IsTestEnvironment = true flags := map[string]string{ + // Pretty print should be turned off for easier error message matching in test files. + config.PrettyPrintFlag: "false", config.ExcludeFileDocStringsFlag: "@generated,Code generated by", config.ExcludePkgsFlag: "ignoredpkg1,ignoredpkg2", } diff --git a/testdata/src/prettyprint/main.go b/testdata/src/prettyprint/main.go new file mode 100644 index 00000000..75130821 --- /dev/null +++ b/testdata/src/prettyprint/main.go @@ -0,0 +1,9 @@ +// Package prettyprint is meant to check if our pretty-print flag has effect. +package prettyprint + +func main() { + var a *int + // Ensure that the ASCII escape code is in the want strings (such that the errors are pretty + // printed). + print(*a) //want "\u001B" +}