leighmcculloch/gochecknoglobals

Accept global var when only one single Go file in the package containing the `func main()`

olibre opened this issue · 4 comments

I have an app having the following layout:

├── cmd
│   └── myprj
│       └── main.go       # package main
└── pkg
    └── myprj
        ├── featureA.go   # package myprj
        └── featureB.go   # package myprj

The file main.go looks like the following:

[...]

//nolint:gochecknoglobals
var (
	url     = flag.String("url", "", "Host and port")
	id      = flag.String("id", "", "Identifier")
	token   = flag.String("token", "", "Authentication Token")
	table   = flag.String("table", "", "Table name")
	key     = flag.String("key", "", "Key name")
	record  = flag.String("record", "", "Record name")
	symbol  = flag.String("symbol", "", "Symbol ID")
	topic   = flag.String("topic", "", "Topic ID")
	start   = flag.String("start", "", "Start time")
	stop    = flag.String("stop", "", "Start time")
	limit   = flag.Uint("limit", 0, "Max")
	columns = flag.String("columns", "", "Column names")
    [...]
    // more flags
)

func main() {
	flag.Parse()
	log.SetLevel(log.DebugLevel)
	log.Infof("config: -url     = %v", *url)
	log.Infof("config: -id      = %v", *id)
	log.Infof("config: -token   = %v", *token)
	log.Infof("config: -table   = %v", *table)
	log.Infof("config: -key     = %v", *key)
	log.Infof("config: -record  = %v", *record)
	log.Infof("config: -symbol  = %v", *symbol)
	log.Infof("config: -topic   = %v", *topic)
	log.Infof("config: -start   = %v", *start)
	log.Infof("config: -stop    = %v", *stop)
	log.Infof("config: -limit   = %v", *limit)
	log.Infof("config: -columns = %v", *columns)
    [...]
    // more flags

    [...]

    // processing: calling functions in the package myprj
}

I want to remove the line //nolint:gochecknoglobals.

I could copy the global variables witin the main() body, but the number of lines becomes too high.

However, I think in such cases, global variable is acceptable:

  1. the package containing the function main() is composed of one signgle Go file.
  2. the signgle Go file contains very few functions (in such cases, the stuff is in the pkg folder).

There is no issue with global variable in a tiny main.go file like that.

I agree in small applications with few lines of code many best practices don't have the same gains and maybe aren't worth the effort. It's really up to you to weigh the trade-offs of using this checker or any best practice in your development of an application.

If you're suggesting the checker should intelligently make the decision of when global variables are allowed based on program size, I don't think the checker should try and make that decision for you. It would be difficult, complex, and confusing to make it work, and to use the tool if it behaved like that.

For example, if the tool didn't error about global variables in single file main packages you might write code in a single main.go file build, release and everything would appear fine. Then a colleague arrives to contribute and adds one more file, resulting in global variable errors. It would be unexpected and difficult to deal with.

I could copy the global variables within the main() body, but the number of lines becomes to high.

I would expect the number of lines to be the same, but I don't know the full context of how your application is structured. Could you share an example of how they would be high?

Thank you for your answer. 👍

When I copy the variables within the function main() the linter funlen says:

Function 'main' is too long (68 > 60)

I agree with funlen, the function main() becomes too long to be read comfortably. 😕

Fighting two linters is rough! You could restructure your code to setup all the flags in another function and return a pointer t o a struct value that contains fields for each flag, then pass that pointer around. When the flags are parsed its fields would contain the values. I think you could keep your functions smaller than 60 by doing this.

Thanks @leighmcculloch for the advice.
Since my team wants to keep the simplicity of the Go spirit, we had three options:

  1. put all flag operations in the main() and move the rest of the main() in a library functions.
  2. put all flag operations in the main() and split the rest in smaller functions.
  3. Disable gochecknoglobals

In the end, we disabled gochecknoglobals (and some other linters) in our CI.
Sometimes simplicity means freedom for my teammates.