search

dgryski / semgrep-go

Go rules for semgrep and go-ruleguard
MIT License
457 stars 37 forks source link

semgrep 0.56 badnilguard weirdness #35

Closed rubensayshi closed 2 years ago

rubensayshi commented 3 years ago

as of semgrep 0.56 the bad-nil-guard pattern check seems to check both ways and as a result flags good nil guards as well: 

severity:error rule:tmp.semgrep-go.bad-nil-guard: Bad nil guard
194:        } else if equity != nil && equity.LastPrice != nil {
rubensayshi commented 3 years ago

I'm not very familiar with semgrep's workings, but maybe this is related: https://github.com/returntocorp/semgrep/issues/3198 ?

disconnect3d commented 3 years ago

I believe this behavior was there even before Semgrep 0.56 since the rule just checks for any X.Y access after an if: https://github.com/dgryski/semgrep-go/blob/master/badnilguard.yml#L6

rubensayshi commented 3 years ago 
pattern: $X != nil || <... $X.$F ...>

that's after an != nil || only, not after != nil &&

dgryski commented 3 years ago

This is https://github.com/returntocorp/semgrep/issues/3399 which will be fixed in the next semgrep release.

rubensayshi commented 3 years ago

awesome! sorry I couldn't be off my help than reporting it

dgryski commented 2 years ago

This has been fixed for a while now.