-
Notifications
You must be signed in to change notification settings - Fork 305
Description
Is your feature request related to a problem? Please describe.
In some cases using immutable values can be a good style. For example, I often use type Config struct{...}
this way, to have both (a variant of) dysfunctional options and ensure there are no references to config (or it values) outside of object using that config:
package main
import "maps"
type Config struct {
b bool // required
i int // optional
m map[int]bool // optional
}
func NewConfig(b bool) Config {
return Config{b: b, i: 42}
}
func (c Config) WithI(i int) Config {
c.i = i
return c
}
func (c Config) WithM(m map[int]bool) Config {
c.m = maps.Clone(m) // clone referental values
return c
}
type Thing struct {
cfg Config // or *Config, both are safe choice here
}
func NewThing(cfg Config) *Thing {
return &Thing{cfg: cfg}
}
This result in clean and safe code, but revive thinks it's bad:
config.go:16:2: modifies-value-receiver: suspicious assignment to a by-value method receiver (revive)
c.i = i
^
config.go:21:2: modifies-value-receiver: suspicious assignment to a by-value method receiver (revive)
c.m = maps.Clone(m) // clone referental values
^
Describe the solution you'd like
I'd like to make it possible to not trigger modifies-value-receiver
rule in case method returns value of receiver type (or a pointer to receiver type). Examples:
func (v T) same(...) T {...}
func (v T) same_multi(...) (T, error) {...}
func (v T) sameref(...) *T {...}
func (v T) sameref_multi(...) (*T, error) {...}
The reason to support references in returned value is: both NewConfig()
and WithX()
methods may return *Config
without actually changing whole pattern. This is because copy will anyway happens when it will be used as non-reference argument (as a receiver for WithX()
or as an argument for NewThing()
. Using reference may make sense, e.g. if Config
is huge, or just to make NewConfig()
looks more consistent (New
usually means return reference).
I think it's safe to make modifies-value-receiver
work this way by default, but if you unsure it can be configurable.
Describe alternatives you've considered
Adding //nolint:revive
or disabling modifies-value-receiver
rule. First is annoying, second is unsafe.
Additional context
N/A