Skip to content

x/tools/go/analysis/singlechecker: don't use os.Exit in Main #30219

Closed
@mvdan

Description

@mvdan

Right now, the func is used like:

func main() { singlechecker.Main(foo.Analyzer) }

I propose to instead have it return the exit code integer, changing the usage to:

func main() { os.Exit(singlechecker.Main(foo.Analyzer)) }

The reasoning is that calling os.Exit directly in libraries should generally be avoided, even in those APIs which are meant to be used in func main directly. For example, see https://golang.org/pkg/testing/#M.Run and https://godoc.org/golang.org/x/tools/go/packages#PrintErrors.

In particular, using os.Exit means that it's impossible for the developer to run any code after the checker has run. The only option would be to stick the code at the very end of the checker's Run function, but that's not the same. And it would likely pollute the non-main package with code from the main package.

My use case is that I want to write a coverage profile to disk after executing main function. This happens for many main packages, so the tooling does not care about go/analysis or singlechecker. So the tool requires that users write their main function as follows, to be able to call main1 and do the extra work afterward:

func main() { os.Exit(main1()) }

func main1() int {
     // actual implementation
}

I think changing the signature is reasonable, and now is the right time to make the change. There are very few users of go/analysis outside of x/tools, and the API is still experimental.

My biggest worry is that all main packages doing func main() { singlechecker.Main(foo.Analyzer) } would suddenly stop properly exiting with non-zero status codes when an error happens. However, we can communicate this API change properly via the golang-tools group and Hangouts calls, and like I said above, I presume almost noone has shipped code using singlechecker yet.

/cc @alandonovan @ianthehat @matloob @rogpeppe

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeNeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions