go-simpler / sloglint

🪵 Ensure consistent code style when using log/slog
https://go-simpler.org/sloglint
Mozilla Public License 2.0
75 stars 5 forks source link

Problems when using slog function assignments or their returns #35

Closed nikpivkin closed 2 months ago

nikpivkin commented 2 months ago

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:
```yaml
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:
```yaml
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 commented 2 months ago

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

tmzane commented 2 months ago
  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!