mgechev / revive

🔥 ~6x faster, stricter, configurable, extensible, and beautiful drop-in replacement for golint
https://revive.run
MIT License
4.82k stars 282 forks source link

modifies-value-receiver: miss modification by `++`/`--` #1109

Open powerman opened 4 days ago

powerman commented 4 days ago

Describe the bug modifies-value-receiver does not trigger on ++ and --.

To Reproduce Steps to reproduce the behavior:

  1. I updated revive go install github.com/mgechev/revive@latest
  2. I run it with the following flags & configuration file:
$ nl -ba main.go
     1  package main
     2  
     3  import "fmt"
     4  
     5  type Data struct {
     6      i int
     7  }
     8  
     9  func (d Data) Inc() {
    10      d.i++
    11  }
    12  
    13  func (d Data) IncBy(n int) {
    14      d.i += n
    15  }
    16  
    17  func (d Data) Dec() {
    18      d.i--
    19  }
    20  
    21  func (d Data) DecBy(n int) {
    22      d.i -= n
    23  }
    24  
    25  func main() {
    26      var d Data
    27      d.Inc()
    28      d.IncBy(10)
    29      d.Dec()
    30      d.DecBy(5)
    31      fmt.Println(d)
    32  }
$ revive
main.go:14:2: suspicious assignment to a by-value method receiver
main.go:22:2: suspicious assignment to a by-value method receiver
$
[rule.modifies-value-receiver]

Expected behavior

main.go:10:2: suspicious assignment to a by-value method receiver
main.go:14:2: suspicious assignment to a by-value method receiver
main.go:18:2: suspicious assignment to a by-value method receiver
main.go:22:2: suspicious assignment to a by-value method receiver

Desktop (please complete the following information):

ccoVeille commented 4 days ago

For records, it was detected here https://github.com/mgechev/revive/issues/1066#issuecomment-2466227459

ccoVeille commented 4 days ago

I'm curious if all assignments operators are supported

https://www.w3schools.com/go/go_assignment_operators.php

Including the non common one such as &=,|=, << and co

I checked the code, I think it's OK

but, maybe it is worth adding the unit tests

chavacava commented 4 days ago

Hi @powerman @ccoVeille Thank you for reporting this. I'll add support for increment, decrement and assignment ops to the rule