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

Possible false positive #98

Closed
davidnewhall opened this issue Jul 21, 2024 · 3 comments
Closed

Possible false positive #98

davidnewhall opened this issue Jul 21, 2024 · 3 comments

Comments

@davidnewhall
Copy link

I might be missing something, but this looks like a false positive to me. Is there some way I can fix this? Thanks!

Output

[david@dns-mm2]14:00:42|/Volumes/Storage/david/go/src/github.com/Notifiarr/notifiarr$ golangci-lint run ./...
pkg/configfile/config.go:110:26: the given struct should be annotated with the `toml` tag (musttag)
	if _, err := dec.Decode(&newConfig); err != nil {
	                        ^
pkg/website/clientinfo/clientinfo.go:179:49: the given struct should be annotated with the `json` tag (musttag)
	if err = json.Unmarshal(body.Details.Response, &clientInfo); err != nil {
	                                               ^
[david@dns-mm2]14:01:01|/Volumes/Storage/david/go/src/github.com/Notifiarr/notifiarr$

Same thing in a screenshot:

Screenshot 2024-07-21 at 2 04 31 PM

Code

The links go directly to the affected lines on GitHub.

// CopyConfig returns a copy of the configuration data.
// Useful for writing a config file with different values than what's running.
func (c *Config) CopyConfig() (*Config, error) {
	var newConfig Config

	buf := bytes.Buffer{}
	if err := Template.Execute(&buf, c); err != nil {
		return nil, fmt.Errorf("encoding config into toml for copying (this is a bug!): %w", err)
	}

	dec := toml.NewDecoder(&buf)
	if _, err := dec.Decode(&newConfig); err != nil {   // <<--- LINE 110 - ERROR HERE
		var parseErr toml.ParseError
		if errors.As(err, &parseErr) {
			return nil, fmt.Errorf("decoding config from toml for copying (this is a bug!): %w: %s",
				err, parseErr.ErrorWithUsage())
		}

		return nil, fmt.Errorf("decoding config from toml for copying: %w", err)
	}

	return &newConfig, nil
}
// SaveClientInfo returns an error if the API key is wrong. Caches and returns client info otherwise.
func (c *Config) SaveClientInfo(ctx context.Context, startup bool) (*ClientInfo, error) {
	body, err := c.GetData(&website.Request{
		Route:      website.ClientRoute,
		Event:      website.EventStart,
		Payload:    c.Info(ctx, startup),
		LogPayload: true,
	})
	if err != nil {
		return nil, fmt.Errorf("sending client info: %w", err)
	}

	clientInfo := ClientInfo{}
	if err = json.Unmarshal(body.Details.Response, &clientInfo); err != nil { // <<--- LINE 179 - ERROR HERE
		return &clientInfo, fmt.Errorf("parsing response: %w, %s", err, string(body.Details.Response))
	}

	// Only set this if there was no error.
	data.Save("clientInfo", &clientInfo)

	return &clientInfo, nil
}
@tmzane
Copy link
Member

tmzane commented Jul 27, 2024

Hey, thanks for the detailed report.

From config.go:

I think it complains about LogConfig.AppName not having the toml tag.

From clientinfo.go:

And here the problem is with MySQLConfig fields missing the json tag.

Am I missing something here? Please let me know.

I can see why it might be hard to get the exact reason with many struct layers without field name hints, sorry for the inconvenience. We're about to add a verbose mode in #89, which will print the names of the untagged fields.

@davidnewhall
Copy link
Author

Thank you for the detailed analysis. I've could swear I've seen musttag complain directly with the line number for the struct missing tags. This behavior was unexpected. I'll add the missing tags you found and report back if this goes away.

+1 to hints or struct/struct member names.

@davidnewhall
Copy link
Author

Indeed you found the problem and fixing it makes the lint warning go away. Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants