Skip to content
This repository was archived by the owner on Apr 10, 2019. It is now read-only.

Conversation

@thaJeztah
Copy link
Contributor

Adding Go 1.11 to travis to catch possible issues; I can update this PR when newer betas are released

/cc @dnephin @kolyshkin

@thaJeztah
Copy link
Contributor Author

thaJeztah commented Jul 4, 2018

Alright, looks like there are some failures;

--- FAIL: TestVetShadow (1.46s)
    support.go:51: 
        	Error Trace:	support.go:51
        	            				vet_shadow_test.go:36
        	Error:      	Not equal: 
        	            	expected: regressiontests.Issues{regressiontests.Issue{Linter:"vetshadow", Severity:"warning", Path:"test.go", Line:7, Col:3, Message:"foo declared and not used"}}
        	            	actual  : regressiontests.Issues{regressiontests.Issue{Linter:"vetshadow", Severity:"warning", Path:"test.go", Line:7, Col:3, Message:"foo declared and not used"}, regressiontests.Issue{Linter:"vetshadow", Severity:"warning", Path:"test.go", Line:7, Col:3, Message:"foo declared but not used"}}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,3 +1,4 @@
        	            	-(regressiontests.Issues) (len=1) {
        	            	- (regressiontests.Issue) test.go:7:3:warning: foo declared and not used (vetshadow)
        	            	+(regressiontests.Issues) (len=2) {
        	            	+ (regressiontests.Issue) test.go:7:3:warning: foo declared and not used (vetshadow),
        	            	+ (regressiontests.Issue) test.go:7:3:warning: foo declared but not used (vetshadow)
        	            	 }
        	Test:       	TestVetShadow

edit: TestVet is fixed (was due to different output message)

Details
--- FAIL: TestVet (5.74s)
    vet_test.go:43: 
        	Error Trace:	vet_test.go:43
        	Error:      	Not equal: 
        	            	expected: regressiontests.Issues{regressiontests.Issue{Linter:"vet", Severity:"error", Path:"file.go", Line:7, Col:0, Message:"Printf format %d reads arg #1, but call has only 0 args"}, regressiontests.Issue{Linter:"vet", Severity:"error", Path:"file.go", Line:7, Col:0, Message:"unreachable code"}, regressiontests.Issue{Linter:"vet", Severity:"error", Path:"file_test.go", Line:5, Col:0, Message:"unreachable code"}, regressiontests.Issue{Linter:"vet", Severity:"error", Path:"sub/file.go", Line:7, Col:0, Message:"Printf format %d reads arg #1, but call has only 0 args"}, regressiontests.Issue{Linter:"vet", Severity:"error", Path:"sub/file.go", Line:7, Col:0, Message:"unreachable code"}}
        	            	actual  : regressiontests.Issues{regressiontests.Issue{Linter:"vet", Severity:"error", Path:"file.go", Line:7, Col:0, Message:"Printf format %d reads arg #1, but call has 0 args"}, regressiontests.Issue{Linter:"vet", Severity:"error", Path:"file.go", Line:7, Col:0, Message:"unreachable code"}, regressiontests.Issue{Linter:"vet", Severity:"error", Path:"file_test.go", Line:5, Col:0, Message:"unreachable code"}, regressiontests.Issue{Linter:"vet", Severity:"error", Path:"sub/file.go", Line:7, Col:0, Message:"Printf format %d reads arg #1, but call has 0 args"}, regressiontests.Issue{Linter:"vet", Severity:"error", Path:"sub/file.go", Line:7, Col:0, Message:"unreachable code"}}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,6 +1,6 @@
        	            	 (regressiontests.Issues) (len=5) {
        	            	- (regressiontests.Issue) file.go:7::error: Printf format %d reads arg #1, but call has only 0 args (vet),
        	            	+ (regressiontests.Issue) file.go:7::error: Printf format %d reads arg #1, but call has 0 args (vet),
        	            	  (regressiontests.Issue) file.go:7::error: unreachable code (vet),
        	            	  (regressiontests.Issue) file_test.go:5::error: unreachable code (vet),
        	            	- (regressiontests.Issue) sub/file.go:7::error: Printf format %d reads arg #1, but call has only 0 args (vet),
        	            	+ (regressiontests.Issue) sub/file.go:7::error: Printf format %d reads arg #1, but call has 0 args (vet),
        	            	  (regressiontests.Issue) sub/file.go:7::error: unreachable code (vet)
        	Test:       	TestVet
--- FAIL: TestGas (1.51s)
    gas_test.go:23: 
        	Error Trace:	gas_test.go:23
        	Error:      	Not equal: 
        	            	expected: regressiontests.Issues{regressiontests.Issue{Linter:"gas", Severity:"warning", Path:"file.go", Line:3, Col:0, Message:"Errors unhandled.,LOW,HIGH"}, regressiontests.Issue{Linter:"gas", Severity:"warning", Path:"sub/file.go", Line:3, Col:0, Message:"Errors unhandled.,LOW,HIGH"}}
        	            	actual  : regressiontests.Issues{}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,4 +1,2 @@
        	            	-(regressiontests.Issues) (len=2) {
        	            	- (regressiontests.Issue) file.go:3::warning: Errors unhandled.,LOW,HIGH (gas),
        	            	- (regressiontests.Issue) sub/file.go:3::warning: Errors unhandled.,LOW,HIGH (gas)
        	            	+(regressiontests.Issues) {
        	            	 }
        	Test:       	TestGas
--- PASS: TestDupl (1.10s)
FAIL
FAIL	github.com/alecthomas/gometalinter/regressiontests	34.163s

TestGas looks to be failing because the output is different on go 1.11, and go vet prints messages twice in the output;

output on 1.11
Stderr: 
DEBUG: [Jul  4 23:13:15.501] setenv PATH="/go/bin:/usr/local/go/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
DEBUG: [Jul  4 23:13:15.527] setenv GOROOT="/usr/local/go"
DEBUG: [Jul  4 23:13:15.527] Current environment:
DEBUG: [Jul  4 23:13:15.527] PATH="/go/bin:/usr/local/go/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
DEBUG: [Jul  4 23:13:15.527] GOPATH="/go"
DEBUG: [Jul  4 23:13:15.527] GOBIN=""
DEBUG: [Jul  4 23:13:15.527] GOROOT="/usr/local/go"
DEBUG: [Jul  4 23:13:15.530] linting path .
DEBUG: [Jul  4 23:13:15.539] [vetshadow.1]: executing /usr/local/go/bin/go vet --shadow .
DEBUG: [Jul  4 23:13:15.763] [vetshadow.1]: warning: /usr/local/go/bin/go returned exit status 2: # github.com/alecthomas/gometalinter/regressiontests/gometalinter-655212277
./test.go:7:3: foo declared and not used
# github.com/alecthomas/gometalinter/regressiontests/gometalinter-655212277
./test.go:7:3: foo declared but not used
vet: typecheck failures

DEBUG: [Jul  4 23:13:15.763] [vetshadow.1]: vetshadow hits 2: ^(?:vet:.*?\.go:\s+(?P<path>.*?\.go):(?P<line>\d+):(?P<col>\d+):\s*(?P<message>.*))|((?P<path>.*?\.go):(?P<line>\d+):(?P<col>\d+):\s*(?P<message>.*))|(?:(?P<path>.*?\.go):(?P<line>\d+):\s*(?P<message>.*))$
DEBUG: [Jul  4 23:13:15.765] nolint: parsing test.go for directives
DEBUG: [Jul  4 23:13:15.767] [vetshadow.1]: vetshadow linter took 227.61925ms
DEBUG: [Jul  4 23:13:15.767] nolint: parsing test.go took 2.383726ms
DEBUG: [Jul  4 23:13:15.768] total elapsed time 240.62228ms

Output:
[
  {"linter":"vetshadow","severity":"warning","path":"test.go","line":7,"col":3,"message":"foo declared and not used"},
  {"linter":"vetshadow","severity":"warning","path":"test.go","line":7,"col":3,"message":"foo declared but not used"}
]

Difference between 1.10 and 1.11:

1.10

DEBUG: [Jul  4 23:19:47.618] [vetshadow.1]: executing /usr/local/bin/go vet --shadow .
DEBUG: [Jul  4 23:19:47.761] [vetshadow.1]: warning: /usr/local/bin/go returned exit status 2: # github.com/alecthomas/gometalinter/regressiontests/gometalinter-179702599
./test.go:7:3: foo declared and not used

1.11

DEBUG: [Jul  4 23:13:15.539] [vetshadow.1]: executing /usr/local/go/bin/go vet --shadow .
DEBUG: [Jul  4 23:13:15.763] [vetshadow.1]: warning: /usr/local/go/bin/go returned exit status 2: # github.com/alecthomas/gometalinter/regressiontests/gometalinter-655212277
./test.go:7:3: foo declared and not used
# github.com/alecthomas/gometalinter/regressiontests/gometalinter-655212277
./test.go:7:3: foo declared but not used
vet: typecheck failures

@thaJeztah
Copy link
Contributor Author

Ok, so definitely something weird with go vet;

test.go:

package test

type MyStruct struct {}
func test(mystructs []*MyStruct) *MyStruct {
	var foo *MyStruct
	for _, mystruct := range mystructs {
		foo := mystruct
	}
	return foo
}

Go 1.10.3:

 /usr/local/go/bin/go vet --shadow .
# _/foo
./test.go:7:3: foo declared and not used

Go 1.11beta1:

/usr/local/go/bin/go vet --shadow .
# _/foo
./test.go:7:3: foo declared and not used
# _/foo
./test.go:7:3: foo declared but not used
vet: typecheck failures

@thaJeztah
Copy link
Contributor Author

Opened an issue upstream golang/go#26222

@thaJeztah
Copy link
Contributor Author

my issue was marked as a duplicate of golang/go#26125 (which is labeled "release blocker", so should be fixed before Go 1.11 final)

@thaJeztah
Copy link
Contributor Author

Updated to Go 1.11beta2; TestGas is still failing;

=== CONT  TestDupl
--- FAIL: TestGas (1.13s)
    gas_test.go:23: 
        	Error Trace:	gas_test.go:23
        	Error:      	Not equal: 
        	            	expected: regressiontests.Issues{regressiontests.Issue{Linter:"gas", Severity:"warning", Path:"file.go", Line:3, Col:0, Message:"Errors unhandled.,LOW,HIGH"}, regressiontests.Issue{Linter:"gas", Severity:"warning", Path:"sub/file.go", Line:3, Col:0, Message:"Errors unhandled.,LOW,HIGH"}}
        	            	actual  : regressiontests.Issues{}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,4 +1,2 @@
        	            	-(regressiontests.Issues) (len=2) {
        	            	- (regressiontests.Issue) file.go:3::warning: Errors unhandled.,LOW,HIGH (gas),
        	            	- (regressiontests.Issue) sub/file.go:3::warning: Errors unhandled.,LOW,HIGH (gas)
        	            	+(regressiontests.Issues) {
        	            	 }
        	Test:       	TestGas
--- PASS: TestDupl (0.97s)
FAIL

@kolyshkin
Copy link

Updated to Go 1.11beta2; TestGas is still failing.

This seems to be the problem with a particular linter, gas, which is not part of gometalinter per se.

It looks like gas was renamed to gosec, and its current git tip version (https://github.com/securego/gosec/tree/2785f7aaf8c5236da2412f85e76351123b21b350) correctly detects the error missed by an earlier version. But gometalinter needs some love to switch to this new version, there's an issue for that: #505.

@thaJeztah
Copy link
Contributor Author

@kolyshkin thanks! I arrived at the same conclusion yesterday, and found that PR for the rename; haven't tried yet, but I could temporarily cherry-pick that PR here to check if the latest version resolved the problem

@thaJeztah
Copy link
Contributor Author

cherry-picked #505 into this PR to see if that resolves the issue

@thaJeztah
Copy link
Contributor Author

Looks good! All green with that patch applied

@kolyshkin
Copy link

I tested manually before, it was working. Heck, I almost created a dupe of #505...

@thaJeztah thaJeztah changed the title [do not merge] add go 1.11beta1 [do not merge] add go 1.11beta2 Aug 1, 2018
@thaJeztah thaJeztah changed the title [do not merge] add go 1.11beta2 [do not merge] add go 1.11beta3 Aug 13, 2018
@thaJeztah
Copy link
Contributor Author

Updated to 1.11beta3

@kolyshkin
Copy link

go 1.11rc1 is out (no docker images as of yet)

@wayneashleyberry
Copy link

images have been updated to rc1

$ cat Dockerfile
FROM golang:1.11-rc
RUN go version
$ docker build .
Sending build context to Docker daemon  2.048kB
Step 1/2 : FROM golang:1.11-rc
1.11-rc: Pulling from library/golang
55cbf04beb70: Pull complete
1607093a898c: Pull complete
9a8ea045c926: Pull complete
d4eee24d4dac: Pull complete
9c35c9787a2f: Pull complete
a586ab591509: Pull complete
fb94d9d3eaaf: Pull complete
Digest: sha256:848d7c6e26a6ca99bdf50ae952769998f4ffd458328e2f4089ba8e784b09ffd7
Status: Downloaded newer image for golang:1.11-rc
 ---> 798d75db8922
Step 2/2 : RUN go version
 ---> Running in 1a480348c540
go version go1.11rc1 linux/amd64
Removing intermediate container 1a480348c540
 ---> cfa17a33c4fd
Successfully built cfa17a33c4fd

@thaJeztah thaJeztah changed the title [do not merge] add go 1.11beta3 [do not merge] add go 1.11rc1 Aug 15, 2018
@thaJeztah
Copy link
Contributor Author

Thanks for the ping! Updated the PR to 1.11rc1

@robinjoseph08
Copy link

The official 1.11.0 image has also been released if you want to update it to that.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah changed the title [do not merge] add go 1.11rc1 Add go 1.11.0 Aug 28, 2018
go:
- 1.9.x
- 1.10.x
- 1.11
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be changed to 1.11.x once the first patch release for Go 1.11 is out (erm, unless I remember incorrectly, but I seem to recall .x didn't work if there's no patch release yet)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.11.x seems to work well for several of my repos, FYI.

There were some problems with Go 1.10, since YAML and some other code parsed 1.10 a a number (1.1), so perhaps that is what you are recalling?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember that 1.10 is a JSON number, which is identical to 1.1 -- for an actual 1.10 you want "1.10" to force it to a string!

@thaJeztah
Copy link
Contributor Author

@robinjoseph08 thanks for the ping! I was waiting for those images to arrive indeed, but forgot to update Yesterday 😅

PR is updated, and should be ready for review now

@alecthomas alecthomas merged commit 9460458 into alecthomas:master Aug 28, 2018
@alecthomas
Copy link
Owner

Thanks!

@thaJeztah thaJeztah deleted the bump_go_1.11 branch August 28, 2018 10:52
@thaJeztah
Copy link
Contributor Author

thaJeztah commented Sep 1, 2018 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants