cmd/vet: Show warning when StructTags not space-separated

[cjwebb]

If StructTags are separated via tabs rather than spaces, they will not work... but go vet will not issue any warnings.

Example code (Github does not appear to render tabs and spaces differently though):

package main

import (
    "encoding/json"
    "fmt"
)

// the StructTag contains tabs, and does not recognise the json keyval
type TabsStruct struct {
    Foo string `xml:"foo1,attr" json:"foo1"`
}

// this StructTag contains only spaces, and works perfectly
type StringsStruct struct {
    Foo string `xml:"foo1,attr" json:"foo1"`
}

func main() {

    // both should print out {"foo1":"hello"}, but only the StringsStruct example does

    thing, _ := json.Marshal(TabsStruct{"hello"})
    fmt.Println(string(thing))

    thing2, _ := json.Marshal(StringsStruct{"hello"})
    fmt.Println(string(thing2))
}

Playground example: https://play.golang.org/p/DAvrKtIBKI

This issue was requested by @robpike: https://twitter.com/rob_pike/status/551313510991265792

[gfrey]

The fix for this issue (https://github.com/golang/tools/commit/682ca03389b762630846eff46e0d91d9307a2e63) introduced a new one. Something like

field type `tag:"foo bar"`

isn't working any more, due to the extra space in the quoted value string. At least in my interpretation of the docs (speaking of quoted string literals) this should be valid.

[zimmski]

Bugged me too, I made a patch for this https://github.com/zimmski/tools/commit/4b820846b5c4a083d782755ceb8e9e1dbbb51d49 and I will try to bring it upstream.

[zimmski]

I forgot to post the CL here https://go-review.googlesource.com/#/c/2974/

[nigeltao]

Please review https://go-review.googlesource.com/3952 for the field type tag:"foo bar" problem.

[zimmski]

@robpike: I do not get it. Why does https://go-review.googlesource.com/3952 get reviewed and https://go-review.googlesource.com/#/c/2974/ does not? Is there some kind of rule I am missing?

[dsymonds]

On 5 February 2015 at 19:28, Markus Zimmermann notifications@github.com wrote:

@robpike https://github.com/robpike: I do not get it. Why does https://go-review.googlesource.com/3952 get reviewed and https://go-review.googlesource.com/#/c/2974/ does not? Is there some kind of rule I am missing?

You never mailed 2974 for review.

[zimmski]

I used git codereview mail which worked for my other CLs.

[dsymonds]

On 5 February 2015 at 19:38, Markus Zimmermann notifications@github.com wrote:

I used git codereview mail which worked for my other CLs.

I don't know what else to tell you, then. I can see that there is no sign of mail being sent on your CL in gerrit. Consider filing a bug for that.

[zimmski]

@dsymonds: Sorry to be a burden here, but what is the sign for mail? Because the CL is in the category "Outgoing reviews" under "My Changes" so I do not know the indication that something is missing. And what should I do with it now? Besides some test cases and the control character handling it is the same.

[ianlancetaylor]

As far as I can tell 2974 was sent out. I see it here: https://groups.google.com/forum/#!searchin/golang-codereviews/2974/golang-codereviews/hVZNIPvazNY/YISaX9QxKbkJ .

I think the main issue is that no reviewer was assigned and nobody happened to pick it up. For 3952 Nigel assigned Rob as the reviewer when he sent it out, so Rob got it in direct mail, and reviewed it quickly.

That is, CL 2974 would have been reviewed at some point, but 3952 was reviewed quickly because it was sent directly to the reviewer.

[dsymonds]

If I visit https://go-review.googlesource.com/#/c/2974/, the first thing in the History section is "Uploaded patch set 1.". That's why I thought it hadn't been mailed. But it looks like that's the standard text, so I expect Ian's hypothesis is the reason.

[nigeltao]

@zimmski, Ian's hypothesis sounds plausible to me.

From my point of view, I didn't realize that this bug existed (and hence that your code change was proposed) until I'd actually written the patch. (I came across this bug independently). Once I had my change (to the golang.org/x/tools repo), and the matching change to the main Go repo, it seemed faster to send both of my changes out instead of reviewing yours, especially as your new code differed from the StructTag.Get code in the standard reflect package and I'd prefer them to be similar.

Apologies for having your proposal fall through the cracks.

[nigeltao]

Separately, this issue was closed by https://github.com/golang/tools/commit/682ca03389b762630846eff46e0d91d9307a2e63