test for thisAccess like this.field and not SomeClass.this.field

[pvojtechovsky]

I guess the issue #826 created by @leventov is about the problem, which is reproducable by 2 new tests of this PR

The problem, which I feel too is that in the source class like

package spoon.test.fieldaccesses.testclasses;
public class Pizza {
	int size;
	void setSize(int size) {
		this.size = size;
	}
}

The spoon produces field access this.size in these two wrong forms

if factory.getEnvironment().setAutoImports(true) then it is

Pizza.this.size = size

if factory.getEnvironment().setAutoImports(false) then it is

spoon.test.fieldaccesses.testclasses.Pizza.this.size = size

But I would prefer to print this.size in all cases.

[pvojtechovsky]

And I have added another test, then one which should automatically use thisAccess even if the field was implicit before the refactoring.

Idea of the test. Let's have method addSize in class like:

public class Pizza {
    int size;
    void addSize(int plus) {
        size = size + plus;
    }
}

The field size uses implicit access - OK.
But then if I apply refactoring - rename parameter "plus" to "size" ... and spoon prints invalid result

    void addSize(int size) {
        size = size + size;
    }

but it should produce `this.size = this.size + size;'

So I agree with @leventov, that printer should not care about CtFieldAccess.isImplicit, but should print optimized value.
... I see that it will be not simple task, because printer needs to have correct information about context where field access is printed.

[tdurieux]

Hi, I wrote a possible solution here: tdurieux@52d75a7
I don't know if I handled every case (probably no).

[pvojtechovsky]

I tried your solution on my project and it produce compilation errors in following situation

public abstract class A {
  public void  methodOnA() {}
}
public class B extends A {
  public void methodOnB() {
    methodOnA();
  }
}

because it produced

public void methodOnB() {
  A.this.methodOnA();
}

[pvojtechovsky]

Anyway thank You for investigating this problem! I will need solution for that probably next year - in the case, when all pass and I let spoon refactor sources of my project. More I understand spoon more I see how much work is in it! ... and how complicated task it is to analyze/refactor sources.

[tdurieux]

Yes I know, it does not work at all. It's much more complex that anticipated.

[pvojtechovsky]

Looking at AccessibleVariablesFinder, I always wonder how powerful are these visitors. I am not get used to use visitors in my work. I wonder that it is quite fast too, even if it looks for variable for each this access...

If I would have time to do it, I would try opposite approach. To make PrintingContext more clever then it is now. The PrintingContext is called (push/pop) with each CtElement. So it knows the context the best. I guess the algorithm, which would resolve accessible variables, should be based on the PrintingContext and helper data collected there. But it is just my feeling, I have not tried it yet or thought about that deeper.

[tdurieux]

I guess the algorithm, which would resolve accessible variables, should be based on the PrintingContext and helper data collected there. But it is just my feeling, I have not tried it yet or thought about that deeper.

It does not work when you extends classes from external libraries.

Example

// class from external library
public External() {
  int a = 0;
}


public class Parent () {
  int a = 0;
  private class Child() extends External {
    public void m () {
       Parent.this.a++; // with your technique the qualified name will be removed and java will access a from External not from Parent
    }
  }
}

I update my code, now I succeed to compile Spoon tdurieux@64a6bcb

[pvojtechovsky]

It is really a challenge :-)
... I have never wrote such code, but I see that spoon must handle all such tricks...

[tdurieux]

my code is not compatible with the no-classpath mode.
Nevertheless is only a POC, but seams possible when you have a valid classpath.

[pvojtechovsky]

I just checked the ClassNotFoundExceptions and the problem is not in the class path. I am not using noClassPath mode.

That new algorithm fails on each nested class like

package a.b.c;
public class A {
protected static class B {}
}

because it tries to load class with name "B", but it should load class "a.b.c.A$B"

[tdurieux]

If I have some time I will try to fix to fix.

By the way I updated my branch and now it supports invocation.

[tdurieux]

I just checked the ClassNotFoundExceptions and the problem is not in the class path. I am not using noClassPath mode.

I did not succeed to reproduce your issue. The qualified name of your example is correct.

[pvojtechovsky]

I have committed here the test which reproduces the ClassNotFoundException on invalid qualified name of nested type.

Run the FieldAccessTest.testFieldAccessDeclaredInADefaultClass() in debug and put breakpoint into
CtTypeReferenceImpl.isSubtypeOf ...
Launcher.LOGGER.error("cannot determine runtime types for

The type.getQualifiedName() is "Fails", but should be spoon.test.fieldaccesses.testclasses.internal.Foo$Fails

[tdurieux]

Ok, thanks.

With my current code I cannot reproduce the bug because I remove the isSubtypeOf call from the jdttreebuilder.
Maybe the qualified name was incorrect because the spoon ast was not complete.
I will investigate.

[monperrus]

fixed by #1031?

[pvojtechovsky]

Yes, #1031 is fixing this issue

[monperrus]

closed by #1031