Skip to content

Commit

Permalink
Replace config.IsTestEnvironment with a configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
yuxincs committed Aug 4, 2023
1 parent b19870d commit 4db2508
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 8 deletions.
9 changes: 9 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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")
Expand All @@ -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, ",")
}
Expand Down
4 changes: 0 additions & 4 deletions config/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions nilaway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 18 additions & 1 deletion nilaway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"os"
"testing"

"github.com/stretchr/testify/require"
"go.uber.org/nilaway/config"
"golang.org/x/tools/go/analysis/analysistest"
)
Expand Down Expand Up @@ -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",
}
Expand Down
9 changes: 9 additions & 0 deletions testdata/src/prettyprint/main.go
Original file line number Diff line number Diff line change
@@ -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"
}

0 comments on commit 4db2508

Please sign in to comment.