gordonklaus / ineffassign

Detect ineffectual assignments in Go code.
MIT License
390 stars 22 forks source link

False positive: string concatenation #60

Closed ocket8888 closed 3 years ago

ocket8888 commented 3 years ago

Because ineffassign doesn't consider types in its checks, it makes the mistake of assuming that the + operator is associative. This is not the case for all types - most notably strings.

Here's an example of a small program that does an assignment that is not ineffectual, but does get flagged as such by ineffassign:

package main

import (
    "fmt"
)

const prefix = `prefix `

func main() {
    prefixMe := `a string to prefix`
    // the below line is flagged
    prefixMe = prefix + prefixMe
    fmt.Println(prefixMe)
}

(playground)

gordonklaus commented 3 years ago

The line you indicate is not flagged for me. Can you check again that the code you posted is as intended?

ocket8888 commented 3 years ago

hmm... that's actually a MCVE of an issue I was seeing in a much larger file. I'm actually having trouble reproducing it, but the code that does exhibit the problem is here: https://github.com/ocket8888/trafficcontrol/blob/to/restructure-roles/traffic_ops/traffic_ops_golang/user/user.go#L158-L163 - both of those assignments to query in the if/else block are marked as "ineffectual".

I don't understand how it's different from my example. Certainly doesn't seem to truly be ineffectual.

gordonklaus commented 3 years ago

In the code you linked, query is assigned and then never used after the if/else block. Perhaps on line 165 you meant to write query instead of readQuery?

ocket8888 commented 3 years ago

... yes, yes I did.

I guess I was just confused what this linter actually does; the README just says "Detect ineffectual assignments in Go code.". I thought that meant things like a = a and a = a + 1 which should be a += 1 or something.

False false positive.

gordonklaus commented 3 years ago

Thanks for the feedback, I updated the README now in 0589229737b250c0aa92942483f6e8e58c6affaa.