ldez / tagliatelle

A linter that handles struct tags.
Apache License 2.0
47 stars 9 forks source link

tagliatelle: issue with invalid struct #11

Closed ccoVeille closed 2 years ago

ccoVeille commented 2 years ago

Tagliatelle is unable to detect issue with invalid tags

// MessedUpTags is to validate structtag validation is done
// without it, the tool could let think everything is ok, while it's not.
type MessedUpTags struct {
    // an invalid tag listed in the rule is supported.
    Bad string `json:"bad` // want "struct field tag `json:\"bad` not compatible with reflect.StructTag.Get: bad syntax for struct tag value"

    // an invalid tag not in the rule is supported.
    Whatever string `foo:whatever` // want "struct field tag `foo:whatever` not compatible with reflect.StructTag.Get: bad syntax for struct tag value"

    // a invalid tag supported by the rule, is not hidden by another broken tag
    Mixed string `json:"mixed" foo:mixed` // want "struct field tag `json:\"mixed\" foo:mixed` not compatible with reflect.StructTag.Get: bad syntax for struct tag value"
}
ccoVeille commented 2 years ago

I worked on a fix, I'm about to publish.

ldez commented 2 years ago

Hello,

the goal of tagliatelle is not to validate the syntax of struct tags, there are already existing linters to manage that.

ccoVeille commented 2 years ago

I agree with you, but it failed to validate an issue with the following code (example)

type Foo struct {
Mixed `json:"mixe" yaml:"mixed` // here the ending quote is missing
}

For me this, this is a legitimate use cases, in this example json:"mixe" is wrong, if you consider the purpose of tagliatelle.

tagliatelle may not return information about what the problem is, but I think it should at least report, it cannot analyze the tag.

Because otherwise, it reports there is no problem at all with such a structure

ldez commented 2 years ago

In your example, tagliatelle will find the problem with JSON tag.

json(camel): got 'mixe' want 'mixed'
type Mixed struct{}

type Foo struct {
    Mixed `json:"mixe" yaml:"mixed`
}

Note: in this case, the tag yaml is useless because YAML parser use json tag by default.


But in the following example, the struct tag will be "ignored":

type Mixed struct{}

type Foo struct {
    Mixed `json:"mixe`
}
ldez commented 2 years ago

but I think it should at least report, it cannot analyze the tag.

The reflect API doesn't allow us to see a difference between a missing tag or a malformed tag.

ldez commented 2 years ago

FYI, this linter has been created mainly to be used inside golangci-lint.

In golangci-lint, you can enable the go vet pass called structtag: https://golangci-lint.run/usage/linters/#govet

ccoVeille commented 2 years ago

Yes, I use it locally via golang-ci, also tagliatelle.

It's just that I installed tagliatelle directly, and failed to detect something, while doing my tests.

I agree with you and the need to keep your linter simple, so I will close this MR, and the issue, but I will open a new MR to document tagliatelle is not about validating struct, and that structtag is the perfect candidate to do that.