Golang go cmd syntax checks

[ghthor]

I've made some updates to the go syntax checker via the go cmd.

It initially checks via go fmt and if there are any errors it returns those. If go fmt passes then it updates the buffer by filtering through gofmt since go fmt will update formatting on disk but not in the buffer.

If the go file is a test file then go build will not end up checking it so we check for this and instead use go test -c which at this time will generate a test executable in the package directory named packagename.test and there isn't an option to avoid creating this file but this is a quick and easy way to do syntax checking on all *.go files. It could be removed after SyntasticMake is called but I don't see any problem with leaving it lying around as it should be ignored by the DVCS anyway and go clean will remove it.

There are some problems with using go build & go test if you are editing a file that isn't in the current directory since they operate on the current directory. Is there a simple way to cd to the buffers directory run lmake and then cd back to the original directory?

[rbrown]

expand("%:p:h") will give you the name of the directory the current file is in.

If you need to change directory then you need to do something like

let oldcd=getcwd()
exec 'lcd ' . fnameescape(expand("%:p:h"))

let makeout=SyntasticMake({ 'makeprg': makeprg, 'errorformat': errorformat })

exec 'lcd ' . fnameescape(oldcd)

return makeout

[ghthor]

Perfect. Thanks!
tehehe let makeout

[scrooloose]

Hey im a bit confused.

Is go fmt a style checker? Will it catch syntax errors? If not, should the syntax checkers (go build and go test) be run first and then go fmt afterwards if no syntax errors are detected?

Also, filtering the buffer through go fmt is outside the scope of syntastic. Our goal is to point problems out to the user, but not be intrusive - and filtering the buffer is pretty intrusive :)

Remove the filtering and ill pull.

[ghthor]

Go doesn't exactly need a style checker because it has 1 universal formatting style that is applied by the automated formatting tool go fmt. Anyone writing go code wants their code to be go fmt'd. It is also a syntax checker that will catch many simple syntax errors and executes much quicker then go build and go test because it doesn't compile the entire package/command/test it only parses and reformats the the single source file.

package main

func main() {
    s := TwoStrings{
        "a member",
        someStr
    }
}

In the snippet above go fmt would complain that a comma is missing after someStr but it wouldn't perform the analysis to determine someStr is an undeclared variable and TwoStrings is an undefined type. That analysis is left to the compiler commands(go build and go test) which can identify that variable someStr is undeclared and the type TwoStrings struct is undefined.

go fmt is intrusive but this intrusion is desired by the go developer :)

[c14n]

I understand your concerns about intrusiveness. But: Most go vimmers (vim gophers?) have an autocommand like this in their .vimrc:

autocmd FileType go autocmd BufWritePre <buffer> Fmt

where :Fmt is povided by a ftplugin and calls something like %!gofmt on the buffer.

As ghthor explained above, it is beneficial to use gofmt (not go fmt, which is an alias for gofmt -l -w)for inital syntax checking, since it only reads the current file. go build and go test compile the whole package, and maybe even packages the package of the current file depends on.

Calling gofmt twice seems wasteful. Would introducing an option variable be acceptable?

"============================================================================
"File:        go.vim
"Description: Check go syntax using 'go build'
"Maintainer:  Kamil Kisiel <kamil@kamilkisiel.net>
"License:     This program is free software. It comes without any warranty,
"             to the extent permitted by applicable law. You can redistribute
"             it and/or modify it under the terms of the Do What The Fuck You
"             Want To Public License, Version 2, as published by Sam Hocevar.
"             See http://sam.zoy.org/wtfpl/COPYING for more details.
"
" Use 'let g:syntastic_go_checker_option_gofmt_write=1' to save gofmt output.
"============================================================================
function! SyntaxCheckers_go_GetLocList()

    if !exists("g:syntastic_go_checker_option_gofmt_write")
        let g:syntastic_go_checker_option_gofmt_write = 0
    endif

    if g:syntastic_go_checker_option_gofmt_write == 0
        " Calling 'gofmt %' does not change the file or the buffer.
        let makeprg = 'gofmt %'
    else
        " Calling 'gofmt -w %' reformats the file on disk if no errors are detected.
        let makeprg = 'gofmt -w %'
    endif

    let errorformat = '%f:%l:%c: %m,%-G%.%#'
    let errors = SyntasticMake({ 'makeprg': makeprg, 'errorformat': errorformat, 'defaults': {'type': 'e'} })

    if !empty(errors)
        return errors
    endif

    if g:syntastic_go_checker_option_gofmt_write == 1
        " cat seems to be robust. edit kills syntax highlighting.
        let view = winsaveview()
        silent %!cat %
        call winrestview(view)
    endif

    " Files that end on '_test.go' are not compiled by 'go build'. They are
    " test files and thus only compiled by 'go test'. Consequently, test files
    " need to be checked with 'go test' while for the sake of lazyness, all
    " other go files should be checked with 'go build'.
    if match(expand('%'), '_test.go$') == 1
        let makeprg = 'go test -o /dev/null'
    else
        let makeprg = 'go build -o /dev/null'
    endif

    let errorformat = '%f:%l:%c:%m,%E%f:%l:%m,%C%m,%-G#%.%#'

    " 'go build' and 'go test' should be run from the directory of the file.
    let popd = getcwd()
    let pushd = expand('%:p:h')
    exec 'lcd '.fnameescape(pushd)

    let errorformat = '%f:%l:%c:%m,%E%f:%l:%m,%C%m,%-G#%.%#'
    let errors = SyntasticMake({ 'makeprg': makeprg, 'errorformat': errorformat })

    " Restore the current directory.
    exec 'lcd '.fnameescape(popd)

    return errors
endfunction

By the way: silent %!cat % seems ugly. Is there a vim way to do this? Using edit killed my syntax highlighting.

[ghthor]

The only reason to use gofmt % versus go fmt % is that gofmt % allows the option g:syntastic_go_checker_option_gofmt_write to exist where go fmt % can only write out the file.

I used the go fmt % because the checker file implied that it used the go tool exclusively for the syntax checking. If breaking this implication is ok lets use gofmt and enable the write out option. I enjoy the idea of the gofmt rewriting the file being an option!

@ChristophMartin If you submit this change as a pull request to https://github.com/ghthor/syntastic/tree/golang-gocmd It will be reflected in this pull request or if you like I can commit it as myself crediting you.

[c14n]

TL;DR: Don't pull. Go support broken as of now, this pull request is not the right way to solve it.

Rereading this thread and the commit history, I realized we've been approaching this the wrong way, mainly because of the confusion about what gofmt respectively go fmt are supposed to do. While it is true that the original purpose of gofmt - sic, since it existed before the go tool - was in fact to avoid tedious discussions about style by automatically enforcing the One True Style:tm:, its side effect of recognizing syntax errors is useful for syntastic, and a combined approach of using gofmt first, then one of go build or go test is more useful than an approach that only uses either gofmt or one of the build tools.

Consider the following broken go code:

// ==> bar.go <==
package foo

func Bar() {
                fmt.Println("Fmt package imported.") // Compile: fmt package not imported
}

// ==> foo.go <==
package foo

func Foo(n int) (int, bool) {
    return n ok // Syntax: missing comma between return values; Compile: ok undefined
}

func foo(n int, ok bool) (int, bool) {
    return n ok // Syntax: missing comma between return values
}

// ==> baz.go <==
package foo

func Baz() {
    Bar( // Syntax: missing close paren
}

Now, both gofmt and go build will complain, albeit in a different manner:

// gofmt bar.go
package foo

func Bar() {
    fmt.Println("Fmt package imported.") // fmt package not imported
}

// gofmt baz.go
baz.go:5:1: expected operand, found '}'

// gofmt foo.go
foo.go:4:11: expected ';', found 'IDENT' ok
foo.go:8:11: expected ';', found 'IDENT' ok
foo.go:9:3: expected '}', found 'EOF'

// gofmt -l bar.go
bar.go

// gofmt -l bar.go 1>/dev/null

// gofmt -l baz.go 1>/dev/null
baz.go:5:1: expected operand, found '}'

// gofmt -l foo.go 1>/dev/null
foo.go:4:11: expected ';', found 'IDENT' ok
foo.go:8:11: expected ';', found 'IDENT' ok
foo.go:9:3: expected '}', found 'EOF'

// go build
./baz.go:5: syntax error: unexpected }

As evident from the output, invoking gofmt -l % has the following effects:

• Syntax errors are detected. -> gofmt -l is suitable for syntax checking.
• If syntax errors are detected, just the errors are printed to STDERR.
• The -l flag does not print the reformatted code to STDOUT. -> Less to parse for syntastic!
• The absence of the -w flag means the file itself is not changed. The ugly indentation of bar.go is retained.
• Only the syntax for the current file is checked.
• Both the line and the column of the syntax error are returned.
  • Useful in foo.go, less useful in baz.go, but go build in a package with just baz.go complains about line 5 instead of line 4 too.

Invoking go build - used in the standard syntax checker as of date! - however is problematic, since it aborts after a source file that contains errors has been parsed. This forces the dev into a broken workflow:

If I'm editing foo.go, which contains syntax errors, and SyntasticMake is called on BufWritePost, it will complain about baz.go. I open baz.go to fix the errors, :w, and only then does syntastic tell me, that foo.go was broken all along.

This is not to say that go build should not be used for syntax checking after gofmt -l, since it provides useful error messages about undefined variables, mismatched argument/return value counts and so on. Furthermore, calling it after gofmt -l passes notifies the developer about problems in other files belonging to the same package, which means that he gets to see problems before leaving vim to go build his package.

As of now, my initial motivation to introduce an option to write gofmt's changes to the file are a moot point anyway, since we're calling gofmt twice anyways: Once for syntax checking, and once for updating the buffer. Adding a note to syntax_checkers/go/gofmt.vim like Does not reformat source code. would have been cleaner.

I apologize for making a right mess out of this. I'm about to make amends.

[ghthor]

I don't think there is a clean way to incorporate gofmt reformatting and syntax checking without using it twice or reading from the file because syntastic is based on BufWritePost as opposed to BufWritePre.

autocmd FileType go autocmd BufWritePre <buffer> Fmt

The reasons for my initial inclusion of go fmt into the checker with cde11c2 instead of using the autocmd above are

1. The Fmt function works terribly with folding when there are errors. It resets all folding in the open buffer.
2. If there is a syntax error that is caught by gofmt syntastic only finds out about it by calling the heavyweight go [build|test] which are fast, but a magnitude or more slower then gofmt.
3. For syntastic to find errors early with gofmt then it needs to the be the one invoking it.This is t

The functionality I want.

1. To catch errors quickly with gofmt
2. To catch all remaining errors with the slower go [build|test]
3. To filter the file through gofmt fixing the formatting

This pull request fulfills my wants and with the introduction of a variable to determine gofmt writing out formatting changes addresses with scrooloose's concern.

I'm interested to see what you come up with as an alternative. But in defense of what we've made here my go workflow has never been more productive.

[ghthor]

Use #393. I'll find another way to incorporate the gofmt reformatting into my workflow.

[c14n]

Does this work for you? https://gist.github.com/3959117 Seems to respect basic folds.