Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

golang linting outputs fake positives for test files #1358

Closed
sebiwi opened this issue Feb 18, 2018 · 3 comments
Closed

golang linting outputs fake positives for test files #1358

sebiwi opened this issue Feb 18, 2018 · 3 comments

Comments

@sebiwi
Copy link

sebiwi commented Feb 18, 2018

Information

VIM version

VIM - Vi IMproved 8.0 (2016 Sep 12, compiled Feb 13 2018 12:01:41)
macOS version

macOS High Sierra 10.13.1 (17B1003

:ALEInfo

 Current Filetype: go
Available Linters: ['go build', 'gofmt', 'golint', 'gometalinter', 'gosimple', 'gotype', 'go vet', 'staticcheck']
  Enabled Linters: ['gofmt', 'golint', 'go vet']
 Linter Variables:

 Global Variables:

let g:ale_cache_executable_check_failures = 0
let g:ale_change_sign_column_color = 0
let g:ale_command_wrapper = ''
let g:ale_completion_delay = 100
let g:ale_completion_enabled = 0
let g:ale_completion_max_suggestions = 50
let g:ale_echo_cursor = 1
let g:ale_echo_msg_error_str = 'Error'
let g:ale_echo_msg_format = '%code: %%s'
let g:ale_echo_msg_info_str = 'Info'
let g:ale_echo_msg_warning_str = 'Warning'
let g:ale_enabled = 1
let g:ale_fix_on_save = 0
let g:ale_fixers = {}
let g:ale_history_enabled = 1
let g:ale_history_log_output = 1
let g:ale_keep_list_window_open = 0
let g:ale_lint_delay = 200
let g:ale_lint_on_enter = 1
let g:ale_lint_on_filetype_changed = 1
let g:ale_lint_on_save = 1
let g:ale_lint_on_text_changed = 'always'
let g:ale_lint_on_insert_leave = 0
let g:ale_linter_aliases = {}
let g:ale_linters = {}
let g:ale_linters_explicit = 0
let g:ale_list_window_size = 10
let g:ale_loclist_msg_format = '%code: %%s'
let g:ale_max_buffer_history_size = 20
let g:ale_max_signs = -1
let g:ale_maximum_file_size = 0
let g:ale_open_list = 0
let g:ale_pattern_options = {}
let g:ale_pattern_options_enabled = 0
let g:ale_set_balloons = 0
let g:ale_set_highlights = 1
let g:ale_set_loclist = 1
let g:ale_set_quickfix = 0
let g:ale_set_signs = 1
let g:ale_sign_column_always = 0
let g:ale_sign_error = '>>'
let g:ale_sign_info = '--'
let g:ale_sign_offset = 1000000
let g:ale_sign_style_error = '>>'
let g:ale_sign_style_warning = '--'
let g:ale_sign_warning = '--'
let g:ale_statusline_format = ['%d error(s)', '%d warning(s)', 'OK']
let g:ale_type_map = {}
let g:ale_warn_about_trailing_blank_lines = 1
let g:ale_warn_about_trailing_whitespace = 1
  Command History:

(started) ['/bin/bash', '-c', 'gofmt -e ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/12/gigasecond_test.go''']
(started) ['/bin/bash', '-c', 'golint ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/13/gigasecond_test.go''']
(started) ['/bin/bash', '-c', 'go vet ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/14/gigasecond_test.go''']
(finished - exit code 0) ['/bin/bash', '-c', 'gofmt -e ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/15/gigasecond_test.go''']

<<<NO OUTPUT RETURNED>>>

(finished - exit code 0) ['/bin/bash', '-c', 'golint ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/16/gigasecond_test.go''']

<<<NO OUTPUT RETURNED>>>

(finished - exit code 2) ['/bin/bash', '-c', 'go vet ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/17/gigasecond_test.go''']

<<<OUTPUT STARTS>>>
# command-line-arguments
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/17/gigasecond_test.go:14:18: undefined: AddGigaSecond
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/17/gigasecond_test.go:28:18: undefined: AddGigaSecond
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/17/gigasecond_test.go:42:18: undefined: AddGigaSecond
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/17/gigasecond_test.go:56:18: undefined: AddGigaSecond
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/17/gigasecond_test.go:70:18: undefined: AddGigaSecond
<<<OUTPUT ENDS>>>

(finished - exit code 0) ['/bin/bash', '-c', 'gofmt -e ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/21/gigasecond_test.go''']

<<<NO OUTPUT RETURNED>>>

(finished - exit code 0) ['/bin/bash', '-c', 'golint ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/22/gigasecond_test.go''']

<<<NO OUTPUT RETURNED>>>

(finished - exit code 2) ['/bin/bash', '-c', 'go vet ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/23/gigasecond_test.go''']

<<<OUTPUT STARTS>>>
# command-line-arguments
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/23/gigasecond_test.go:14:18: undefined: AddGigaSecond
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/23/gigasecond_test.go:28:18: undefined: AddGigaSecond
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/23/gigasecond_test.go:42:18: undefined: AddGigaSecond
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/23/gigasecond_test.go:56:18: undefined: AddGigaSecond
/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/23/gigasecond_test.go:70:18: undefined: AddGigaSecond
<<<OUTPUT ENDS>>>

What went wrong

I'm getting go vet warnings on my test file as if my function weren't defined:

undefined: AddGigaSecond

But the function is clearly present in the file. Not in the test file, but in the file containing the implementation of the function
.
As with most go commands, go vet should typically operate on a package. I'm under the impression that each lint test is executed in a separate temporary directory for each file:

(finished - exit code 2) ['/bin/bash', '-c', 'go vet ''/var/folders/6y/ns4_3ggs175bl0pvg0ydpw6r0000gn/T/vZlpfFX/23/gigasecond_test.go''']

Which is what seems to trigger the alert.

Reproducing the bug

Steps for repeating the bug:

  1. Install go, gofmt, and golint. I'm using go go1.10 darwin/amd64.
  2. Activate gofmt, golint and go vet as linters
  3. Write a simple golang function in a file
// gigasecond.go
package main

import (
	"time"
)

const gigaSecond = time.Duration(1e9) * time.Second

func AddGigaSecond(date time.Time) time.Time {
	return date.Add(gigaSecond)
}
  1. Write a simple golang test file with a test that tests the implementation of the function:
package main

import (
	"testing"
	"time"
)

func TestAddGigaSecondDateOnly(t *testing.T) {
	// Given
	inputTime, _ := time.Parse("2006-01-02", "2011-04-25")
	expectedTime, _ := time.Parse("2006-01-02T15:04:05", "2043-01-01T01:46:40")

	// When
	returnedTime := AddGigaSecond(inputTime)

	// Then
	if returnedTime != expectedTime {
		t.Errorf("The returned time (%s) was different from the expected time (%s)", returnedTime.String(), expectedTime.String())
	}
}

You should be able to see the syntax error at line 14 of the previous file, on the returnedTime := AddGigaSecond(inputTime), specifying that AddGigaSecond is undefined.

It's slightly weird, I'm pretty sure this worked fine before.

Thanks in advance!

@ooesili
Copy link

ooesili commented Feb 19, 2018

Looks like an issue with Go itself. I opened an issue here golang/go#23916 with what I think is the underlying problem.

As an easy workaround, we could switch to using go tool vet instead of go vet, according to this https://golang.org/cl/74750.

For now "go tool vet" will continue to cope without type information, but the eventual plan is for "go tool vet" to query the go command for what it needs, and also to be able to query alternate build systems like bazel. But that's future work.

However, without type information, some vet checks might be skipped, according to golang/go#16086 (comment). This is actually already happening, which you can see by running go vet against a single file with the -v flag (1.9.4 silently hid this error, while 1.10 treats this as a failure). This is running against some example code I linked to in the first issue I mentioned.

$ go tool vet -v vetissue_test.go
vetissue_test.go:6:6: undeclared name: value
Checking file vetissue_test.go
$ echo $?
0

Considering that, I think changing the vet linter to run go vet against the current package would be the best course of action, although it might require more work (I'm not really familiar with the ALE codebase).

TL:DR

Switching from go vet to go tool vet will fix this issue. As an improvement, we could switch to running go vet against the current file's package, which could give us better results.

@ooesili
Copy link

ooesili commented Feb 19, 2018

Looks like somebody already had my idea #1356.

@sebiwi
Copy link
Author

sebiwi commented Feb 19, 2018

Hah, great!

I guess I'll just wait until that gets merged. Thanks for the quick reply!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants