refactor: handling of imports in pretty printer

[pvojtechovsky]

This PR implements #2624

[pvojtechovsky]

Hallelujah! It passed all tests :-)

So, I started to split this PR to smaller parts. Please see, review and merge other PRs, which comes from this one.

[pvojtechovsky]

@monperrus, I see you gave it a lot of time. Thank you for having look!
I will have a look at your changes and add comments here, so we can discuss and reach common understanding.

C1) You removed CtElement#print(boolean), whose purpose was to print Spoon model in form in which it really actually is - without any further modifications of model. Are you sure Spoon should not offer that possibility?

C2) I like new prettyprint() method, which is kind of higher level API from my point of view and which is needed too.

[monperrus]

Hi Pavel,

Yes, we're almost ready to merge. You did a fantastic job.

C1) You removed CtElement#print(boolean), whose purpose was to print Spoon model in form in which it really actually is - without any further modifications of model. Are you sure Spoon should not offer that possibility?

This is the goal of toString, toString is a debug method. However, you're right, it still hides some implicit elements such as this.. We can change this easily so as to show all implicit elements but: 1) we would have to change tons of assertions 2) we would break backward compatibility and break many tests in client code. So, I prefer to stay as is.

C2) I like new prettyprint() method, which is kind of higher level API from my point of view and which is needed too.

thanks :-) it's still experimental though (we sometimes create a fake compilation unit on the fly)

[pvojtechovsky]

it still hides some implicit elements such as this.

I agree with you, we should keep it hidden. So no change is needed.

[pvojtechovsky]

The status of this PR after commit 2dd73cd
is OK for me. I have no more comments.

[monperrus]

Let's cool it down a little bit. @nharrand I guess you would be the final one to merge.

[monperrus]

Big kudos to Pavel @pvojtechovsky
Spoon loves you ❤️❤️!

[seintur]

Typo: relvant -> relevant

[monperrus]

thanks @seintur but it's already merged, would you edit the file directly on Github?

[monperrus]

As expected we break most client code:

• https://ci.inria.fr/sos/job/npefix/954/display/redirect
• https://ci.inria.fr/sos/job/nopol/595/display/redirect
• Regression with latest dependencies STAMP-project/AssertFixer#42
• https://ci.inria.fr/sos/job/gumtree-spoon-ast-diff/

[monperrus]

It seems that the reason is writing fully-qualified java.lang.*.

To solve this, I propose to never write java.lang.* in toString().

WDYT?

[nharrand]

So, you would like to always write import for classes part of the standard library? (like everything prefixed with java.lang or java.util ?) Or just java.lang?

[monperrus]

I'm talking about `toString` which does not involve imports. I want that toString never writes `java.lang`.

[nharrand]

My bad for imports. Still why just java.lang and not all classes of the standard library?

[monperrus]

to be backward-compatible and get the client builds passing again.

[nharrand]

Ok my bad, after some clarification, java.lang is the only package imported by default, so it makes sense to simplify its fully qualified name all the time.
SGTM.

[seintur]

And broke also https://ci.inria.fr/sos/job/juliac/1056/

Don't know for sure this is only about java.lang.* stuff. Still investigating the issue...

[monperrus]

Started to analyze. We have three kinds of regressions:

• R1 before toString was dependent on autoimport mode
• R2 some changes in autoimport mode
• R3 some changes in full-qualified autoimport mode

I'm on it.