refactor(classloader): reduces to the maximum the use of classloaders

[monperrus]

should simplify debug when Maven classloaders, JDT classloaders, IDE classloaders
interplay.

[seintur]

Isn't it misleading to say "with class loader " Thread.currentThread().getContextClassLoader()? This is not this one that is in cause in here, but rather the URLClassLoader with class paths. Shouldn't you better say "in class paths ...".

[tdurieux]

@monperrus Are you aware that this PR will broke Nopol and NPEFix?

[monperrus]

Then a test case is missing because nothing is broken currently.

[tdurieux]

How does a not merged pull request can currently broke two external projects?
Nopol and NPEFix use getClassloader(), no need of test the projects will not compile.

[seintur]

I'm a bit concerned about removing this method that I'm using. The idea is that when you embedded this Spoon as a third-party framework in another framework, you need a simple and direct way to instruct Spoon the class loader that you want to use and not bother about some complicated stuff with class paths and thread context class loader. So I would vote against removing this method.

[monperrus]

Hi Lionel,
Since there is no related test case, I don't see yet why this method is
essential and does not overlap with the sourceClassPath option.

Could you propose a test case showing the behavior you rely on?

[seintur]

Because not all class loader are sourceClassPath oriented. You can build a customer class loader that does at all rely on class paths. Building a test case for that is obvious: I want to be able to call setCallItWhateverYouWant(myCustomClassLoader) where myCustomClassLoader is an instance of class that extends ClassLoader (not URLClassLoader, neither something which is in relation with class paths).

[monperrus]

I understand your use case.

Bbut what the Spoon code does is then simply

Thread.currentThread().setContextClassLoader(myCustomClassLoader);

so to me, it looks more meaningful that you call this directly instead of an implicit, poorly documented and untested:

*.getEnvironment().setInputClassLoader(myCustomClassLoader)

Am I missing something?

--Martin

[seintur]

I agree that Thread.currentThread().setContextClassLoader(myCustomClassLoader) does the job. I just a bit concern about the fact that this "contract" (i.e. what to do when one wants to instruct Spoon to use a custom, non-classpath oriented, classloader) should be documented somewhere. setInputClassloader() was the easy obvious place for that: the developer that is embedding Spoon in her/his framework directly and easily bumps into the method when using code completion for instance. Now I have the feeling that this will be fuzzier to find this information. But when one knows... Just my €0.02.

[monperrus]

Basically, the contract with the Spoon-specific classloader is twofold:

1. getTypeDeclaration uses it to build shadow classes

2. JDT uses it it to compile.

With a classpath-based classloader (aka URLClassLoader) as supported by
addSourceClassPath/getClassLoader it works.

With a custom classloader, only 2) works easily. 1) is impossible
because JDT only accepts classpaths options.

So the only way to suport 1) and 2) with a custom class loader is to do
with Thread.currentThread().setContextClassLoader.

That said, we can do this in Spoon (+ documentation + test). But we
would still have only one method:

setClassLoader (instead of the current two setClassLoader and
setInputClassLoader)

OK for you ?

--Martin

[seintur]

Dear all,

I managed to get rid of setInputClassLoader in my projects that use Spoon. I no longer need to transmit any class loader to Spoon. So as far as I am concerned, this is ok for me to deprecate/remove this method.

Lionel.

[pvojtechovsky]

just one note to the class loader usage in spoon. The class loader is set thread locally and therefore influences all the code which is called in that thread later after setInputClassLoader is called. It can be quite dangerous, in some environments with pools of threads, etc. I do not know if it is the case how spoon will be used.
The design problem is that you can be never sure what classloader you are actually using. May be some method somewhere inside asks different classloader?
I just wanted to suggest to set class loaded always localy in scope of run of some functions

spoon.runWithClassLoader(()->{
//  only code  in scope of this function will use that class loader. 
//When we leave the function, then it should use origin class loader again.
})