Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace config.IsTestEnvironment with a configuration #10

Merged
merged 1 commit into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
Comment on lines +221 to +223
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also documented here: https://pkg.go.dev/testing#T.Parallel

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"
}
Loading