I'm reluctant to make this change. The whole point of the {single,multi}checker.Main functions is to do the entire job of the command-line tool. If part of that job is to support coverage, then unitchecker should write the coverage report. Why should some command-line tools get that feature and not others?

It would also be a pernicious API change: it breaks people's programs without breaking the build.

If part of that job is to support coverage, then unitchecker should write the coverage report. Why should some command-line tools get that feature and not others?

Perhaps I gave the wrong impression in my original post. I don't intend to use this to add extra features or work to my built tool. The use case is @rogpeppe's https://godoc.org/github.com/rogpeppe/go-internal/testscript, which is a port of cmd/go/script_test.go to be portable to other tools.

To be able to add one's main function as a command in the script, a func() int is supplied. The tests work if the function exits directly, but then extra testscript features like test coverage don't work. In this scenario, adding a coverage flag to singlechecker or unitchecker wouldn't be the same, as then testscript would have to add special treatment of analyzers.

And of course, I could be filing the same issue over and over again for other testing features that I'll probably need down the line, like cpu and memory profiles. I just need to be able to run my analyzer within a test, without it insisting on exiting the entire program.

You could argue that testscript requiring main-like funcs that return exit codes is asking too much, but in practice, practically all libraries out there can be tuned to not exit directly, such as flag.ContinueOnError or testing itself, which I linked to earlier. singlechecker is the only outlier I've found so far.

It would also be a pernicious API change: it breaks people's programs without breaking the build.

Yes, agreed. This is why I was hoping that very few developers would have started using the API already, meaning it would be easier to coordinate a breaking change.

Another option would be to add parallel functions like RunMain, which would be like Main but returning an integer. Then you'd have something like func Main(t T) { os.Exit(RunMain(t)) } to not break existing programs.

FWIW I have a convention of using Main1 for main functions that return an exit code rather than exiting. shrug

In general I think that any Go package entry point should avoid calling os.Exit, even if it's intended to be a top level entry. There are always going to be use cases that one might not have thought about, and calling os.Exit shuts them all down hard.

The caller is free to run code before calling Main, so why not afterwards too?

The os.Exit at the end of singlechecker/multichecker is far from the only point at which it might terminate the process. These packages were designed for one job alone, and that is to make it trivial to implement a standalone tool that wraps one or many analyzers. As such, these packages are entitled to all the usual rights of a main package.

If you need an importable library with no global effects (on os.Args, flags.CommandLine, os.Exit, signals, whatever) much more invasive changes are required. We could certainly make them, but it is not as simple is not calling os.Exit, and if we're going to make them, they should be exposed using a different API than a main function.

To clarify, we don't need the API to have zero side effects. These testscript main-like commands are run in their own separate process, so it's fine to use globals and such. But exiting is different, as it makes certain features like coverage and cpu profiles impossible.

In other words, it's fine if singlechecker has the usual rights of a main package. Like Roger said, we just need to be able to run code after Main has finished.

Are there any other places besides singlechecker and unitchecker where os.Exit might be called? As far as I can see, there's only a handful of pretty simple direct calls to os.Exit. I presume there will be others like flag's default to exit on errors, but that can be changed with a few lines of code as well.

Every log.Fatal is an os.Exit, remember. I suppose in your scenario, for coverage, you only care about exits after the main work is done, not those that occur during setup.

This is not the first time we've encountered the problem of trying to measure coverage of a standalone (non-test) executable, and it seems we're fixing the symptom but not the cause. What if there were a "cover" package in the standard library that exposed a function to flush out the coverage report, if applicable? Applications could call it prior to their final exit. It's still imperfect because you have to remember to call flush before each exit, but at least the API would then resemble the existing ones for cpu/memory/trace.

What if there were a "cover" package in the standard library that exposed a function to flush out the coverage report,

That would be great! I had to go to some rather unsavoury lengths to do that in the testscript package and I'd love to be able to remove those hacks.

The companion to that would be for a running test binary to be able to request that additional coverage information be added. I did succeed in doing that but I definitely invoked Hyrum as I did so.

I've been meaning to raise an issue, but I didn't think it would be accepted so haven't done so yet. This conversation is a good reference point for it.

Happy to close this issue if the "cover" package and API gets its own issue :)

Enjoy: #31007

It's pretty bare, but I'm sure we can edit the issue into shape

Thanks @matloob!