Resource Leak Checker: Add a check to avoid mapping multiple keys to a ternary expression for tempVarToTree #5137
Conversation
Nargeshdb
changed the title
| @@ -231,7 +231,9 @@ protected ResourceLeakAnalysis createFlowAnalysis() { | |||
| * @param tree the tree of the expression the tempvar represents | |||
| */ | |||
| /* package-private */ void addTempVar(LocalVariableNode tmpVar, Tree tree) { | |||
| tempVarToTree.put(tmpVar, tree); | |||
| if (!tempVarToTree.containsValue(tree)) { | |||
There was a problem hiding this comment.
I'm a little concerned about this change. If addTempVar() is being called, the caller has already created a new tmpVar for the tree. It seems problematic if tmpVar gets used elsewhere but doesn't get added to the tempVarToTree mapping. What do you think of this concern?
There was a problem hiding this comment.
I don't think it makes any problem because each node has to have a unique tmpVar which is defined in MustCallAnnotatedTypeFactory. But the problem here is that in ResourceLeakTransfer in visitTernaryExpression we don't check if there exist a tmpVar for the node like this.
There was a problem hiding this comment.
Ok, assuming the tests pass, I am ok with this change for now. I'll finish reviewing once CI passes (I think there is a formatting error)
msridhar
changed the title
…a ternary expression for tempVarToTree (typetools#5137)
Fixes #5136