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

cmd/vet: incorrect error on a MarshalXML method #28792

Closed
thinkerou opened this issue Nov 14, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@thinkerou
Copy link

commented Nov 14, 2018

What version of Go are you using (go version)?

$ go version
go version devel +6d620fc42e Wed Nov 14 00:22:40 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

No, only go master branch.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

My code:

// MarshalXML allows type H to be used with xml.Marshal.
func (h H) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
	start.Name = xml.Name{
		Space: "",
		Local: "map",
	}
	if err := e.EncodeToken(start); err != nil {
		return err
	}
	for key, value := range h {
		elem := xml.StartElement{
			Name: xml.Name{Space: "", Local: key},
			Attr: []xml.Attr{},
		}
		if err := e.EncodeElement(value, elem); err != nil {
			return err
		}
	}

	return e.EncodeToken(xml.EndElement{Name: start.Name})
}

What did you expect to see?

correctly run on go master.

What did you see instead?

15.41s$ make vet
go vet github.com/gin-gonic/gin github.com/gin-gonic/gin/binding github.com/gin-gonic/gin/ginS github.com/gin-gonic/gin/internal/json github.com/gin-gonic/gin/render
# github.com/gin-gonic/gin
./utils.go:56: method MarshalXML(e *encoding/xml.Encoder, start encoding/xml.StartElement) error should have signature MarshalXML(*xml.Encoder, xml.StartElement) error
# github.com/gin-gonic/gin/render
render/render_test.go:221: method MarshalXML(e *encoding/xml.Encoder, start encoding/xml.StartElement) error should have signature MarshalXML(*xml.Encoder, xml.StartElement) error
make: *** [vet] Error 2
The command "make vet" exited with 2.
@mvdan

This comment has been minimized.

Copy link
Member

commented Nov 14, 2018

A CL of mine touching this code was merged yesterday, so that's probably the cause. Will look into it in a bit.

@mvdan mvdan added this to the Go1.12 milestone Nov 14, 2018

@mvdan mvdan self-assigned this Nov 14, 2018

@thinkerou

This comment has been minimized.

Copy link
Author

commented Nov 14, 2018

@mvdan thanks your reply, waiting.

@mvdan mvdan changed the title cmd/go: vet: should have signature MarshalXML(*xml.Encoder, xml.StartElement) error cmd/vet: incorrect error on a MarshalXML method Nov 16, 2018

@gopherbot

This comment has been minimized.

Copy link

commented Nov 16, 2018

Change https://golang.org/cl/149977 mentions this issue: go/analysis: use TypeString when matching types

@thinkerou

This comment has been minimized.

Copy link
Author

commented Nov 19, 2018

hi @mvdan still have the below error info:

go vet github.com/gin-gonic/gin github.com/gin-gonic/gin/binding github.com/gin-gonic/gin/ginS github.com/gin-gonic/gin/internal/json github.com/gin-gonic/gin/render
# github.com/gin-gonic/gin
./utils.go:56:12: method MarshalXML(e *xml.Encoder, start xml.StartElement) error should have signature MarshalXML(*xml.Encoder, xml.StartElement) error
# github.com/gin-gonic/gin/render
render/render_test.go:221:17: method MarshalXML(e *xml.Encoder, start xml.StartElement) error should have signature MarshalXML(*xml.Encoder, xml.StartElement) error
make: *** [vet] Error 2
@mvdan

This comment has been minimized.

Copy link
Member

commented Nov 19, 2018

This is because vet now lives in the tools repository, and is vendored in the main repository. We need to update that vendored copy.

@gopherbot

This comment has been minimized.

Copy link

commented Nov 19, 2018

Change https://golang.org/cl/150161 mentions this issue: cmd/vendor: update to golang.org/x/tools@139d099f

@thinkerou

This comment has been minimized.

Copy link
Author

commented Nov 19, 2018

@mvdan got it, thanks!

gopherbot pushed a commit that referenced this issue Nov 19, 2018

cmd/vendor: update to golang.org/x/tools@139d099f
Mainly to pull the fix for the regression in #28792.

Change-Id: If71ae783fd9a9e3935186b49fdf501ba098235a2
Reviewed-on: https://go-review.googlesource.com/c/150161
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>

bradfitz pushed a commit that referenced this issue Nov 21, 2018

cmd/vendor: update to golang.org/x/tools@139d099f
Mainly to pull the fix for the regression in #28792.

Change-Id: If71ae783fd9a9e3935186b49fdf501ba098235a2
Reviewed-on: https://go-review.googlesource.com/c/150161
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.