Issue #16 new

instanceof incorrectly evaluates to true

Ernie Rael
created an issue

When I started writing this it was a "why doesn't this work", but further analysis indicated that instanceof returns true when it shouldn't.

See "BUG:" in "First attempt" Goal is to selectively tranform xxx.getBoolean() into xxx() {{{ if(A.p_f1.getBoolean()) { stuff; } if(A.p_f2.getBoolean()) { stuff; } }}} into {{{ if(A.p_f1()) { stuff; } if(A.p_f2.getBoolean()) { stuff; } }}} NOTE: Only A.p_f1 gets changed, not A.p_f2 Access from within class A, e.g. "p_f1.getBoolean()" is not changed.

== First attempt == {{{ t.A.$opt.getBoolean() :: $opt instanceof t.Option => A.$opt() :: matchName($opt) ;;

public boolean matchName(Variable v) {
    String name =;
    return "p_f1".equals(name);

}}} The query/fix patterns work ok, but custom matchName condition doesn't work; in "" getSingleVariable(v) returns null. Although "ctx.getVariableNames().get(v.variableName)" does return what I'm looking for.

Before I made the second attempt, I was wondering about putting
something like

{{{ public String patternVariableValue(Variable v) }}} in class Context.

=== BUG with instance of === the "$opt instanceof t.Option" seems to always return true. In t.A I put {{{ class X { boolean x; boolean getBoolean() { return x; } } static X x; }}} and in some other file {{{ if(A.x.getBoolean())... }}} and the pattern matched, so instanceof must have been true.

I gues matching little pieces with jackpot is just a bad idea.

== Second attmpt == {{{ $opt.getBoolean() :: $opt instanceof t.Option && matchName($opt) => $opt() ;; }}} This works, but does a little too much; accesses from within class A (without the A.p_f1) get changed. This is OK, since I can just hg revert class A after the change.

Something like

{{{ context.getMatchedText(Variable v) }}} or a way to get the file in which the match occured would provide the needed info here. (Though I see how that might be called a hack).

Comments (3)

  1. Ernie Rael reporter

    Taking a fresh look...

    I think I can do context.parent($v) in a loop until I get to a CLASS; then presumably I can get the class name. Should be able to create custom condition isClass($v, "class-name")

  2. Jan Lahoda repo owner

    (I assume that t.A.p_f1 is a static field.) For the simple rewrite, this should work:


    It of course would change the code everywhere, including t.A. I have seen similar problems many times, so some kind of support to skip some files/classes/packages is clearly needed.

    As a hack, this might work to some extent:

    t.A.p_f1.getBoolean() :: matches("t.A.$opt.getBoolean()")

    But, this is not only ugly and a big hack, but also will only rewrite patterns where p_f1 is referred to through the class name, regardless of where this occurs. So it might rewrite occurrences in class t.A, if they would be in form A.p_f1, and not write occurrences elsewhere if p_f1 would be accessed without explicit reference to A (e.g. statically imported, or accessible through from superclass).

  3. Ernie Rael reporter

    about: support to skip some files/classes/packages is clearly needed

    Something like builtin conditions


    That return the class/package of where the rule is matching might do the trick. Then you could do something like

        $ :: !className().equals("com.mypackage.MyClass")

    I guess they are not conditions since they return strings, but... BTW, for the packageName() I have a case where I only want to make the change in a given package.

    With the debugger in context, I tried doing parent($var) in a "loop" but it stalled at elementKind METHOD. I couldn't get to CLASS. The idea was getting a "context.clazz()" and "context.package()". I don't know if I was doing something wrong or if the parse tree just didn't have the info.

  4. Log in to comment