[germainelong]

The comment claim that every branch is accounted for and yet few functions below you can see this is certainly not the case. They should either have fixed it first and then make such comment or shouldn't make such comment at all. Otherwise this looks a bit cringey.

[geofft]

It says "(exception: simple error checks for a client API call)" and that seems accurate to me.

In particular Go (like C) has no built-in exception "throwing" / unwinding support, so for any function call where you want to pass an error onto the caller, you need to do something like

    result, err := try_to_get_a_result()
    if err != nil {
        return nil, err
    }

See also https://blog.golang.org/error-handling-and-go . As far as I can tell, all of the if-statements without else-clauses are doing just this.

(It would be nice in theory if there were better language support for making this lexically obvious but they're in a language where that's not doable.)

[germainelong]

Is someone not familiar with the code competent enough to decide what is a "simple error check" and not a bug? This is very weak as they suggest that even the branches that would result in no-op are accounted for. So if someone introduce a code with a branch that is unaccounted for that automatically means the code is either faulty or is a "simple error check". With something supposedly trying to be a space shuttle worthy code the lack of definition of "simple error check" is very worrying. Would I want this code to fly me to the moon? I'd be wary.

[geofft]

I think you can syntactically state that anything where the check is on the second return value (which is, by convention, the error return) is a "simple error check", and their rule for if statements is always for things that come from a first return value.

For instance, this would not be a simple error check:

    server, err := find_current_server()
    if server != nil {
        ...
    }

because if find_current_server() believes that it's a non-exceptional case that there might be no server at all (i.e., it might return nil, nil instead of nil and an error), then you absolutely want to handle that case.

[germainelong]

What if the second returned variable is not err, but the code using it assumes it is? (the code breaks the convention) This will not be accounted for. This means with that in mind a lot more discipline must be used to analyse the code that is being used in that module.

[geofft]

I think that is highly unusual in Go (but someone who actively uses Go would have to correct me).

Besides, this syntactic rule is not implemented by code but by the author and reviewer, who should know what the function returns. (And shouldn't name it "err", then.) They're doing the best they can in a language without syntactic support for what they want, I think.

[AlexCoventry]

Go pretty much assumes decent editor support, which should inform you of the type of err as you use it. It's not perfect, but it hasn't been a problem for me in practice, so far.

[slumos]

This. It’s the second function in the file.