dominikh / go-tools

Staticcheck - The advanced Go linter
https://staticcheck.dev
MIT License
6.16k stars 375 forks source link

Quick fix 1002 suggests use of tagged switch statement inappropriately #1613

Open seh opened 4 hours ago

seh commented 4 hours ago

Consider the following code, which I admit could use strings.Cut instead:

// Variable "c" is of type string.
// We wish to split it on an embedded space character, if any.
switch i := strings.IndexByte(c, ' '); {
case i == -1:
    return c, "", nil
case i == 0:
    return "", "", errors.New("value starts with a space character")
default:
    return c[:i], c[i+1:], nil
}

My LSP implementation—gopls—reports the following:

could use tagged switch on i [default]

It then offers a quick fix: "Replace with tagged switch". If I accept that suggestion, though, the resulting code is invalid, since I lose access to the "i" variable used in the return statement in the default case.

// Note the extra "i" here.
switch i i := strings.IndexByte(c, ' '); {
case -1:
    return c, "", nil
case 0:
    return "", "", errors.New("value starts with a space character")
default:
    // We can no longer use "i" here.
    return c[:i], c[i+1:], nil
}

Note also the strange insertion of an extra "i" in the initial simple statement.

It seems that there are three things wrong here:

I don't know whether any of these are to blame on Emacs's lsp-mode that's making use of gopls.

dominikh commented 2 hours ago

I reckon the intended rewrite is switch i := strings.IndexByte(c, ' '); i { which AFAICT would be correct. I'll have to check tomorrow if we emit a broken quickfix, or if this is an instance of https://github.com/golang/go/issues/63930.