These are chat archives for JnRouvignac/AutoRefactor

3rd
Oct 2016
Luis Cruz
@luiscruz
Oct 03 2016 14:12
Hi Jean-Noël! First question of the day:
I’ve moved on to a new refactoring rule: recycle. This is a refactoring to check whether structures like query cursors are being properly closed when they are no longer necessary.
example:
Cursor cursor = db.query("TABLE_TRIPS",
  new String[]{"KEY_TRIP_ID"},
  "ROUTE_ID=?",
  new String[]{Long.toString(route_id)},
  null, null, null);
cursor.close(); // this is mandatory
Sometimes developers forget to call close()
Luis Cruz
@luiscruz
Oct 03 2016 14:18
For now, I’m just trying to add a cursor.close()statement after the query.
r.replace(node.getName(), b.simpleName("querby"));
MethodInvocation closeInvocation = b.getAST().newMethodInvocation();
closeInvocation.setName(b.simpleName("close"));
VariableDeclarationFragment variableDeclarationFragment = (VariableDeclarationFragment) ASTNodes.getParent(node, ASTNode.VARIABLE_DECLARATION_FRAGMENT);
SimpleName cursorExpression = variableDeclarationFragment.getName();
closeInvocation.setExpression(b.copy(cursorExpression));
ExpressionStatement expressionStatement = b.getAST().newExpressionStatement(closeInvocation);
Statement cursorAssignmentExpressionStatement = (Statement) ASTNodes.getParent(node, ASTNode.VARIABLE_DECLARATION_STATEMENT);
r.insertAfter(expressionStatement, cursorAssignmentExpressionStatement);
where node is the db.query()method invocation
Luis Cruz
@luiscruz
Oct 03 2016 14:24
nevermind.
I found the issue
:D
it is a silly one, don’t bother trying to figure it out :D

So, here’s the thing I have to do now:
After having the cursor returned by the query() invocation, I have to check whether a the cursor is being closed.
I need to check whether close() is being called before the end of the block or before it is assigned to something else.
I’ll start by checking the closeuntil the end of the block. But later I’ll need to stop the search when the cursorvariable is assigned with other object.
Jean-Noël Rouvignac
@JnRouvignac
Oct 03 2016 14:35
This is a lot more tricky if any control flow statement is at play
You would need a CFG
Not yet implemented
Hi BTW :)
Luis Cruz
@luiscruz
Oct 03 2016 14:35
Hi :D
Jean-Noël Rouvignac
@JnRouvignac
Oct 03 2016 14:36
The CFG will make you confident you go through all possible pathe
Luis Cruz
@luiscruz
Oct 03 2016 14:36
yea, you’re right
Jean-Noël Rouvignac
@JnRouvignac
Oct 03 2016 14:36
I suggest you implement the easy path
You handle the case when no control flow statement is involved
Luis Cruz
@luiscruz
Oct 03 2016 14:37
ok, sounds good
Jean-Noël Rouvignac
@JnRouvignac
Oct 03 2016 14:38
Otherwise you bail out
And do nothing
Luis Cruz
@luiscruz
Oct 03 2016 14:46
So, I could check whether exists cursor.close(), without checking if there are control flow statements. If it doesn’t exist, it will create a cursor.close(). If it exists, it does not matter if it is inside a control flow statement or not — we do nothing.
Jean-Noël Rouvignac
@JnRouvignac
Oct 03 2016 14:47
+1
Luis Cruz
@luiscruz
Oct 03 2016 18:16
I’m having trouble getting the right position to insert the close()statement.
So far, I am inserting right after calling query()as you can see here
However, now I would like to insert it right after the last variable access. I’m done for today byt do you have any idea?
Here’s what I’ve done as a first attempt, with no luck:
    public class LastCursorChecker extends ASTVisitor {
        public Statement lastCursorStatement;
        private SimpleName cursorSimpleName;

        public LastCursorChecker(SimpleName cursorSimpleName) {
            super();
            this.lastCursorStatement = null;
            this.cursorSimpleName = cursorSimpleName;
        }

        @Override
        public boolean visit(SimpleName node) {
            if(node.equals(cursorSimpleName)){
                ASTNode newLastCursorNode = node.getParent();
//                if(node != null){
//                    throw new IllegalArgumentException(""+node.getParent());                    
//                }
                while(newLastCursorNode!= null &&
                        newLastCursorNode.getNodeType() != EXPRESSION_STATEMENT &&
                        newLastCursorNode.getNodeType() != WHILE_STATEMENT &&
                        newLastCursorNode.getNodeType() != VARIABLE_DECLARATION_STATEMENT
                        )
                    newLastCursorNode = newLastCursorNode.getParent();
                this.lastCursorStatement = (Statement) newLastCursorNode;
            }
            return VISIT_SUBTREE;
        }
    }
Note that I did not use any return DO_NOT_VISIT_SUBTREEhere because I want to make sure I get into the last statement.
(this has no rush. I’ll only get back to this tomorrow ;) )
Jean-Noël Rouvignac
@JnRouvignac
Oct 03 2016 20:17
Use ASTHelper.isSameLocalVariable()
It does not matter whether you VISIT_SUBTREE or DO_NOT_VISIT_SUBTREE here: a SimpleName is a leaf in the AST :)
What is your
problem exactly?
Are you having difficulties finding the last use of the variable?