go.tools/go/loader: Don't print errors, only collect them.

go.tools/go/loader: Don't print errors, only collect them.

[adonovan, 2014-06-19 18:55:55]

The default behaviour has always been to print errors to standard error,
and this is sensible for most applications.   If your application wants to
suppress them, then add this line:

   conf.TypeChecker.Error = func(error){}

to your configuration, before calling conf.Load().

[gmk, 2014-06-19 19:05:53]

The default behavior in go/types isn't to print them, so why should go/loader? 
Whatever it has always been is not relevant.  I don't agree that printing is the
sensible for most applications; for most applications this is spam.  The
sensible behavior for any library is to never print anything unless explicitly
requested to do so.

[adonovan, 2014-06-19 19:18:23]

On 2014/06/19 19:05:53, gmk wrote:
> The default behavior in go/types isn't to print them, so why should go/loader?


The default behaviour in go/types is to abort type checking and return the first
error from (*Checker).Files; it does not silently accumulate them, as this would
create a trap of forgetting to report them.


> Whatever it has always been is not relevant.  

It is certainly relevant to all existing clients, since you are changing their
behaviour.


> I don't agree that printing is the sensible for most applications; for most
applications this is spam.

That has not been my experience with command-line tools built atop go/loader.

The only application for which I have wanted to suppress this behaviour is
godoc, since it reports type errors via its HTTP interface.
It uses the one-line trick I mentioned before, which works nicely.


> The sensible behavior for any library is to never print anything unless
explicitly
> requested to do so.

Usually, but with error reporting, you have many choices:
- fail the entire operation at the first error (go/types' default)
- force the user to supply an error handler
- accumulate errors and hope the user remembers to look at them
- print them by default, but permit other strategies.

The advantage of the last one is that it provides the most information with the
least configuration, and fails safe.  (It is also quite handy when a program
panics, since the code to print the accumulated errors may not have been reached
yet, but the default error handler will have already printed them.)

[gmk, 2014-06-19 19:46:12]

go/loader doesn't silently accumulate errors unless you explicitly set
AllowErrors=true.

Of course it is not desirable to change existing behavior, but at this point I
think it is more important to get the right behavior.  The docs make it clear
that this package is unstable.

I can understand that printing errors might be a sensible default for
command-line tools, but we shouldn't assume that all clients are like that. 
Even if I was writing a command-line tool, I would rather have the
responsibility of printing errors at the appropriate time and place than have it
done for me.

Does anything in the standard library print its errors by default?  It's
paranoid to worry that users will forget to check for errors.  This is Go - The
Language In Which We Check For Errors.  :-)