INRIA / spoon

Spoon is a metaprogramming library to analyze and transform Java source code. :spoon: is made with :heart:, :beers: and :sparkles:. It parses source files to build a well-designed AST with powerful analysis and transformation API.
http://spoon.gforge.inria.fr/
Other
1.75k stars 351 forks source link

NPE when missing return type in a method and pretty-printing it. #2915

Open pzaragoza93 opened 5 years ago

pzaragoza93 commented 5 years ago

Bonjour,

Je voudrais utiliser Spoon pour faire analyser des dépendances entre deux classes, créer une interface pour l'une d'entre elles et passer par l'interface pour la manipuler. L'analyse avec l'outil Spoon se passe sans aucun soucis, or lorsque j'essaie de faire ma transformation je rencontre un problème. Je n'utilise pas de Processors ni de Template pour cette transformation, voici mon code:

I would like to use Spoon to analyse dependencies between two classes (A and B), create an interface that represents class A and use the interface (IA) in class B instead of referencing A directly. The analysis works without problems, however when I try to transform my project I have a problem. I am not using any Processors or Templates in my code as of now to transform, here is my code:

// Creation d'une interface pour représenter une classe nommé invokedClass
CtInterface commonInterface = new CtInterfaceImpl();
commonInterface.setSimpleName("I"+invokedClass.getSimpleName());

// Creation des methodes de l'interface à partir des méthodes de la classe.
List<CtMethod> methods = invokedClass.getElements(new TypeFilter<CtMethod>(CtMethod.class));
// Parcours des méthodes pour en extraire leurs signatures pour l'ajouter à l'interface.
for(CtMethod method : methods) {
    CtMethod m = new CtMethodImpl(); // méthode pour l'interface
    m.setSimpleName(method.getSimpleName()); // on copie le nom de la méthode
    // On parcours les paramètres pour les ajouter à la nouvelle méthode
    for(CtParameter param : params) {
        CtParameter p = new CtParameterImpl();
        p.setSimpleName(param.getSimpleName());
        p.setType(param.getType()); // ligne qui casse
        m.addParameter(p);
    }
commonInterface.addMethod(m);
}
// commonInterface est ajouté au model au même niveau que invokedClass dans l'AST

Lors de la construction de mon sous arbre et l'ajout au modèle il n'y a aucun soucis. Mais lorsque je souhaite utiliser le prettyprint() de Launcher je rencontre l'erreur suivante:

I do not encounter any problems while adding to the AST, but when I use the prettyprint() of Launcher I have the following problem.

Exception in thread "main" spoon.SpoonException: java.lang.NullPointerException at spoon.Launcher.prettyprint(Launcher.java:791) at migration.Main.main(Main.java:68) Caused by: java.lang.NullPointerException at spoon.reflect.factory.ExecutableFactory.createReferenceInternal(ExecutableFactory.java:113) at spoon.reflect.factory.ExecutableFactory.createReference(ExecutableFactory.java:99) at spoon.support.reflect.declaration.CtExecutableImpl.getReference(CtExecutableImpl.java:191) at spoon.reflect.visitor.ImportScannerImpl.addClassImport(ImportScannerImpl.java:398) ...

C'est la ligne suivante qui créer cet effet mais je n'arrive pas à savoir pourquoi: The following line creates this error and I cannot see why:

p.setType(param.getType());

Je pense que c'est lié au CtTypeReference qui lie les classes entre elles mais je ne trouve pas de documentation pour les créer proprement. Pouvez vous m'indiquer où est-ce que je pourrais me renseigner pour avoir plus d'informations, ou m'aider à mieux comprendre mon erreur?

I think it is linked to the creation of the CtTypeReference that links the classes between each other ut I cannot find the documentation to do this properly. Can someone show me where I can find more information on how to properly perform this action, or help me understand my error?

J'ai déjà lu votre papier “Spoon: A Library for Implementing Analyses and Transformations of Java Source Code” ainsi que les codes que l'on trouve sur votre site mais je n'arrive toujours pas à résoudre ce problème.

I have already read the paper on “Spoon: A Library for Implementing Analyses and Transformations of Java Source Code” as well as the sample code found on the main site but I still cannot find a solution

Merci.

After writing this up, I now realize that it might be helpful to write this in english as well (for the potential contributor or someone having the same problem as me). A quick summary of my problem is that I would like to create an Interface from the AST of a Class navigating through its nodes. I am able to succesfully do this but when I want to copy/clone/create a CtTypReference for the parameter of my new method (of my new interface), I have the previously-stated error. I believe I am incorrectly using CtTypeReference but I do not know how to do so correctly... Any help would be appreciated, thank you.

surli commented 5 years ago

Hi @greenpanther93

we mostly speak in english indeed here, especially since some contributors are not french and do not speak french :) So regarding your error, I think it's related to the way you build the new elements: basically you should always use the factories and never call directly a new CtSomething() in Spoon. The main factory is available through the Launcher and allow you to create any kind of object: launcher.getFactory().createParameter().

Now if it's not fixing your issue, it might be a bug in Spoon, so we'd need some more info like a minimal snippet of code on which you're replicating the bug.

pzaragoza93 commented 5 years ago

I've update my issue with the english translation.

Here is a Processor that is called for each CtClass. For testing purposes I added a condition so that it only goes through the ContentProvider Class which is also provided below

public class ForeignInstanceProcessor extends AbstractProcessor<CtClass> {

    public ForeignInstanceProcessor() {
        super();
    }

    public void process(CtClass element) {

        if(element.getSimpleName().equals("ContentProvider"))
            testProcess(element);

    }

    private void testProcess(CtClass element) {
        System.out.println("process called for " + element.getSimpleName());
        // getting the parent of the package to place the interface in the same package
        CtPackage invokingCtPackage = (CtPackage) element.getParent();
        // creating the Interface with the correct Factory
        CtInterface commonInterface = getFactory().createInterface();

        // naming the new interface
        commonInterface.setSimpleName("I"+element.getSimpleName());

        // Retrieving all the methods in the CtClass of ContentProvider and looping through them
        List<CtMethod> methods = element.getElements(new TypeFilter<CtMethod>(CtMethod.class));
        for(CtMethod method : methods) {
            // creating the new method for the interface
            CtMethod m = getFactory().createMethod();
            // setting the same name
            m.setSimpleName(method.getSimpleName());
            // getting all the CtParameters (in this case for the push method: Content content)
            List<CtParameter> params = method.getElements(new TypeFilter<CtParameter>(CtParameter.class));
            for(CtParameter param : params) {
                // Creating the new parameter
                CtParameter p = getFactory().createParameter();
                // Setting the same parameter name
                p.setSimpleName(param.getSimpleName());
                // Creating the reference 
                CtTypeReference reference = getFactory().createReference(param.getType().getQualifiedName());
                // setting the CtTypeReference for the parameter
                p.setType(reference); // <- this is the line that breaks while printing
//              p.setType(param.getType());
//              p.setType(param.getType().clone());
                m.addParameter(p);
            }
            commonInterface.addMethod(m);
        }

        invokingCtPackage.addType(commonInterface);
    }

}

input:

public class ContentProvider {
    private ContentProvider instance;

    public Stack<Content> contentStack;

    public ContentProvider getInstance() {
        return instance;
    }

    public Content pop() {
        return contentStack.pop();
    }

    public void push(Content content) {
        contentStack.push(content);
    }
}

output (when the line that breaks is commented out):

interface IContentProvider {
     getInstance();

     pop();

     push( content); // here the parameter is added without the CtTypeReference
}

All of which produce the same error upon printing:

Exception in thread "main" spoon.SpoonException: java.lang.NullPointerException at spoon.Launcher.prettyprint(Launcher.java:791) at migration.Main.main(Main.java:68) Caused by: java.lang.NullPointerException at spoon.reflect.factory.ExecutableFactory.createReferenceInternal(ExecutableFactory.java:113) at spoon.reflect.factory.ExecutableFactory.createReference(ExecutableFactory.java:99)

If you need more info, or a better snippet of code, feel free to ask and I'll try and provide a better example.

nharrand commented 5 years ago

Hi @greenpanther93 I'am having troubles replicating your issue.

When I run your code like this: (with spoon 7.3.0)

Launcher launcher = new Launcher();
launcher.addInputResource("path/to/ContentProvider.java");
launcher.setSourceOutputDirectory("path/to/outputDIr");
launcher.buildModel();
launcher.addProcessor(new ForeignInstanceProcessor());
launcher.process();
launcher.prettyprint();

it produces

interface IContentProvider {
     getInstance();

     pop();

     push(Content content);
}

What does your main contains? What version of spoon do you use?

Note: I think you also forgot to set the return type of your interface methods. You can dot it by inserting m.setType(method.getType());. and obtain:

interface IContentProvider {
    ContentProvider getInstance();
    Content pop();
    void push(Content content);
}
surli commented 5 years ago

@nharrand might be related to a no/fullclasspath mode. Have you tried with both config?

pzaragoza93 commented 5 years ago
MavenLauncher launcher = new MavenLauncher(monolith_filepath, MavenLauncher.SOURCE_TYPE.ALL_SOURCE);
Environment environment = launcher.getEnvironment();
environment.setCommentEnabled(true);
environment.setAutoImports(true);
launcher.buildModel(); // creates model of project

launcher.addProcessor(new ForeignInstanceProcessor());
launcher.process();

/** Print output java classes **/
launcher.setSourceOutputDirectory(output_blank_filepath); // destination of output
        launcher.prettyprint();

console log:

process called for ContentProvider
Exception in thread "main" spoon.SpoonException: java.lang.NullPointerException
    at spoon.Launcher.prettyprint(Launcher.java:791)
    at migration.Main.main(Main.java:68)
Caused by: java.lang.NullPointerException
    at spoon.reflect.factory.ExecutableFactory.createReferenceInternal(ExecutableFactory.java:113)
    at spoon.reflect.factory.ExecutableFactory.createReference(ExecutableFactory.java:99)
    at spoon.support.reflect.declaration.CtExecutableImpl.getReference(CtExecutableImpl.java:191)

I am using Spoon 7.3.0

Yes I didn't include m.setType(method.getType());, upon doing so, I get:

interface IContentProvider {
    ContentProvider getInstance();

    Content pop();

    void push(Content content);
}

Which is what I was looking looking for! So it seems (since I don't know the inner workings of Spoon yet) adding parameters starts off the printing of the return type as well(?) however if there are no parameters then the print does not check if there is a return type either and prints the method as is. I was wrong in assuming the return was optional (which makes sense of course, but it was not clear when it printed out the method without parameters and without a return type).

Anyway, this fixes my problem. Thank you very much for taking the time to test this out.

surli commented 5 years ago

I'm reopening the issue: we shouldn't get a NPE in that case but a more clear error.

nharrand commented 5 years ago

Ok I managed to replicate the crash. (Thanks @greenpanther93 for the replication steps.)

So, the pretty printer crashes because one method ("push") has getType() that returns null. Now, do we want to let it crash, but with a cleaner exception, or do we want to ignore it? WDYT @surli ?

surli commented 5 years ago

Now, do we want to let it crash, but with a cleaner exception, or do we want to ignore it?

I don't think it makes sense for the standard pretty-printer to output a class without the return type of the methods. So I'd make it crash with a nice explanation.

monperrus commented 4 years ago

is the bug still present in 8.2.0?