Skip to content

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

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

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

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

thinkerou opened this issue Nov 14, 2018 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@thinkerou
Copy link

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
Copy link
Member

mvdan 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 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 14, 2018
@mvdan mvdan self-assigned this Nov 14, 2018
@thinkerou
Copy link
Author

@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
Copy link
Contributor

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

@thinkerou
Copy link
Author

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
Copy link
Member

mvdan 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
Copy link
Contributor

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

@thinkerou
Copy link
Author

@mvdan got it, thanks!

gopherbot pushed a commit that referenced this issue Nov 19, 2018
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
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>
@golang golang locked and limited conversation to collaborators Nov 19, 2019
@rsc rsc unassigned mvdan Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants