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

Problems when using slog function assignments or their returns #35

Closed
nikpivkin opened this issue Apr 11, 2024 · 2 comments
Closed

Problems when using slog function assignments or their returns #35

nikpivkin opened this issue Apr 11, 2024 · 2 comments

Comments

@nikpivkin
Copy link

nikpivkin commented Apr 11, 2024

Hi, thanks for this linter! I have encountered some problems while using it.

  1. Panic when using an attribute assigned to a variable (or returned from a function) when using the slog.Attrconstructor:
    main.go:
package main

import "log/slog"

var MyInt = slog.Int

func main() {
	slog.Info("Hello, World!", MyInt("foo-foo", 123))
}

config:

linters:
  enable:
    - sloglint

linters-settings:
  sloglint:
    key-naming-case: snake
  1. No violation is detected when using the custom slog.Attr constructor:

main.go:

package main

import "log/slog"

func UserId(value int) slog.Attr { return slog.Int("user-id", value) }

func main() {
	slog.Info("a user has logged in", slog.Int("user-id", 42)) // Detected
	slog.Info("a user has logged in", UserId(42))
}

config:

linters:
  enable:
    - sloglint

linters-settings:
  sloglint:
    key-naming-case: snake
  1. The violation is not detected when using the logging function of the assigned variable. This looks like the second case, but I had a quick look at the code and it affects a different part, so I thought I'd mention it.
    main.go
package main

import (
	"fmt"
	"log/slog"
)

var MyInfo = slog.Info

func main() {
	slog.Info(fmt.Sprintf("a user with id %d has logged in", 42)) // Detected
	MyInfo(fmt.Sprintf("a user with id %d has logged in", 42))
}

config:

linters:
  enable:
    - sloglint

linters-settings:
  sloglint:
    static-msg: true
  1. The LogAttrs function (and method) are not detected.

main.go:

package main

import (
	"context"
	"fmt"
	"log/slog"
)

func main() {
	slog.Info(fmt.Sprintf("a user with id %d has logged in", 42)) // Detected
	slog.LogAttrs(context.TODO(), slog.LevelInfo, fmt.Sprintf("a user with id %d has logged in", 42), slog.String("id", "42"))
}

config:

linters:
  enable:
    - sloglint

linters-settings:
  sloglint:
    static-msg: true
@tmzane
Copy link
Member

tmzane commented Apr 11, 2024

Hi, thank you for the report. I will take a look.

@tmzane
Copy link
Member

tmzane commented Apr 12, 2024

  1. Panic when using an attribute assigned to a variable (or returned from a function) when using the slog.Attr constructor.

Fixed in #36.

  1. The violation is not detected when using the logging function of the assigned variable.

This is by design. Currently, we only analyze static function calls (i.e. those that are not wrapped in a variable). I'm not sure it is even possible to detect non-static calls. Do you have a use case for those?

  1. No violation is detected when using the custom slog.Attr constructor.
  2. The LogAttrs function (and method) are not detected.

I created separate issues for these. Thanks!

@tmzane tmzane closed this as completed Apr 15, 2024
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