Ineffassign does not appear to detect when a map is written, but never read from

[jvmatl]

In the code below, (which is obviously simplified from a larger context) map m is created, then an optional code path accidentally shadows the map, adds to the (wrong) map, and then the original map is accessed.

There are probably other lint-like tools which might warn about the unintentionally shadowed variable, but in this context I'm wondering if it would be possible to detect that ineffectual assignments were made to a map. (i.e. the inner map was created, then items were added to the map, then the whole map went out of scope, without anybody ever attempting to access a member of the map or to even check then number of items in the map.) It's effectively a /dev/null map.

Maybe this is out of scope for ineffassign, but I figured I'd mention it in case anybody had ideas on how to tackle the issue.

func MinimalRepro(option bool) int {                                                                                                                                                                            
        var m map[int]struct{}                                                                                                                                                                                  

        if option {                                                                                                                                                                                               
                m := make(map[int]struct{}) // BUG -- intended to be =, not :=                                                                                                                                  
                m[1] = struct{}{}                                                                                                                                                                               
                // m is never read from within this block, and len(m) is never analyzed, so all the assignments to the map are ineffectual                                                                      
        }                                                                                                                                                                                                       

        return len(m)                                                                                                                                                                                           
} 

[gordonklaus]

I think this should be quite easily solvable.

Currently, we only treat ast.Idents on the left hand side of an ast.AssignStmt as assignments to those identifiers. This could be expanded to include ast.IndexExprs.

Do you want to take a crack at it?

[jvmatl]

Possibly? TBH, it took me a long while to work out how to make my last MR, which was a much simpler change (I think?). I'll try to set aside a day in the next few weeks to read enough to remember how the code works so I can take a stab at it. :)

Side question: what's the best way to get feedback on partial progress? e.g. can i make an MR (which you will not accept, of course) just to make sure we agree on the new unit tests? (assuming test first development here)

[gordonklaus]

It's up to you. I don't mind doing it, but you're welcome to, if you'd like the opportunity.

Yep, just create a PR and I can give feedback on it and we can go from there.