Adding inspection for unnecessary value store before return #653
Conversation
* Now checking for a case where a value is stored in a block, and then
immediately returned in the next line.
* Tests for both implicit and explicit scope returns.
| case defn @ ValDef(_, assignmentName, _, _) | ||
| if maybeLastExprName.contains(assignmentName.toString()) => | ||
| context.warn(defn.pos, self) | ||
| case _ => stmts.foreach(inspect) |
There was a problem hiding this comment.
I have a suspicion that this will blow up for mutually-recursive functions, but maybe the traversal algorithm already handles this? If not, any suggestions on how to approach this? I guess I could use an accumulator to keep track of visited locations, but that seems naive.
There was a problem hiding this comment.
mutually-recursive functions,
Do you mean:
def a(): Int = {
val x = b()
x
}
def b(): Int = {
val x = a()
x
}
or something else?
I have a suspicion [...] how to approach this?
Can you write down a UT for this case?
I am still not sure what's the suspected code.
There was a problem hiding this comment.
Added a unit test, looks like it isn't a problem (which makes sense, since this is just a syntactic check and the linter doesn't actually execute the code).
There was a problem hiding this comment.
Yes. We don't follow recursion (unless your inspection code chooses to, but even if it did it could only do within a single class / compilation unit)
immediately returned in the next line.