Auto Import not working: Output not showing simple names

[shuchi03]

I am trying to change the type of the field from org.apache.log4j.Logger to org.apache.logging.log4j.Logger. I am also creating an assignment expression to set that in the field.

The code for this is -

public void process(CtField element) {
    final CtTypeReference<Logger> loggerRef = getFactory().Code().createCtTypeReference(Logger.class);
    final CtField<Logger> loggerField = getFactory()
                .createCtField("LOGGER", loggerRef, null, ModifierKind.PRIVATE, ModifierKind.FINAL, ModifierKind.STATIC);
    CtExpression assignment = element.getAssignment();
    CtTypeAccess<Object> typeAccess = getFactory().createTypeAccess(getFactory().Code().createCtTypeReference(LogManager.class));
    ((CtInvocation<?>) assignment).setTarget(typeAccess);
    loggerField.setAssignment(assignment);
    element.replace(loggerField);
}

I have set the autoimport as true for the launcher and the code for running the processor is -

Launcher launcher = new Launcher();
launcher.addInputResource("./src/main/java/");
launcher.setSourceOutputDirectory("./src/main/java/");
launcher.getEnvironment().setAutoImports(true);
launcher.getEnvironment().setPrettyPrinterCreator(() -> new SniperJavaPrettyPrinter(launcher.getEnvironment()));
launcher.buildModel();

But my output is still getting created in this way -

private static final org.apache.logging.log4j.Logger LOGGER = org.apache.logging.log4j.LogManager.getLogger(spoon.process.classes.InstrumentUtils.class);

The simple names are not being used. Also, org.apache.log4j.Logger import is still there in the file.

Is there something I am missing in my code?

[slarse]

Hi @shuchi03,

Auto-import is not compatible with the sniper printer. Essentially, if you set a custom pretty-printer with Environment.setPrettyPrinterCreator(), all of the settings you've entered regarding pretty printers are thrown out the window, at least when using the sniper printer as it sets its own settings.

That's not very intuitive and something that has been bothering be for some time. It would be good to at least issue a warning when incompatible settings are entered. It's a bit unfortunate that incompatible settings can be entered in the first place, but that'

[slarse]

The problem with using the auto-import functionality from Spoon with the sniper printer is that it might change already existing qualified names. In Sorald, we've rolled our own solution where only newly added references are auto-imported, see here:

https://github.com/SpoonLabs/sorald/blob/6452f48125050f7f94e9e76d9cb1390fa25845f9/src/main/java/sorald/Repair.java#L288-L302

And here: https://github.com/SpoonLabs/sorald/blob/master/src/main/java/sorald/SelectiveForceImport.java

Maybe we should add this into Spoon and make auto-import mode compatible with the sniper printer. @monperrus wdyt?

[slarse]

@shuchi03 Feel free to just copy the pretty-printing code from Sorald and see if that solves your problem.

[shuchi03]

Thank you for the response.

I used the below code from Sorald -

private static void setPrettyPrinter(Environment env, CtModel model) {
        Supplier<? extends DefaultJavaPrettyPrinter> basePrinterCreator = createSniperPrinter(env);
        Supplier<PrettyPrinter> configuredPrinterCreator = applyCommonPrinterOptions(basePrinterCreator, model);
        env.setPrettyPrinterCreator(configuredPrinterCreator);
    }

    private static Supplier<PrettyPrinter> applyCommonPrinterOptions(Supplier<? extends DefaultJavaPrettyPrinter> prettyPrinterCreator, CtModel model) {
        Collection<CtTypeReference<?>> existingReferences = model.getElements(e -> true);
        List<Processor<CtElement>> preprocessors = List.of(new SelectiveForceImport(existingReferences),
                new ImportConflictDetector(),
                new ImportCleaner().setImportComparator(new DefaultImportComparator()));
        return () -> {
            DefaultJavaPrettyPrinter printer = prettyPrinterCreator.get();
            printer.setIgnoreImplicit(false);
            printer.setPreprocessors(preprocessors);
            return printer;
        };
    }

But I am still getting a fully qualified name for one of the fields -

private static final org.apache.logging.log4j.Logger LOGGER = LogManager.getLogger(InstrumentUtils.class);

I think this might be happening because it is not removing this import - import org.apache.log4j.Logger; due to which we get two definitions for Logger (the one which I am setting and the one that is already there). Is there a way to remove the unused import as well?

Also, I noticed that this import - import java.util.function.IntFunction; is being added as an unused import to the class, even though it was not there before. I noticed that when I changed from sniper printer to default java printer and used auto imports, this import was still added.

[slarse]

I think this might be happening because it is not removing this import - import org.apache.log4j.Logger;

That's likely the case. Try adding the following as the second processor in the preprocessors list:

new ImportCleaner().setCanAddImports(false)

I don't remember why we did not include that in Sorald, so there might be some problem with it that I'm just not remembering. But that might also have been specific to our use case.

Also, I noticed that this import - import java.util.function.IntFunction;

Are you running in classpath mode or noclasspath? Auto-import can sometimes get funky in noclasspath as it's hard for Spoon to resolve references. Without seeing the actual code being analyzed, it's hard to make anything but guesses, but most likely it's an incorrectly resolved reference somewhere.

On a side note, if you don't mind the odd preexisting qualified reference being replaced with an unqualified reference and an import, then you might as well just use Spoon's built-in processors instead of the SelectiveForceImport from Sorald. If that's the case, just replace SelectiveForceImport with ForceImport (the latter being a Spoon built-in).

[shuchi03]

Thank you for the response.

I added the second processor as follows -

List<Processor<CtElement>> preprocessors = List.of(new ForceImportProcessor(),
new ImportCleaner().setCanAddImports(false),
new ImportCleaner().setImportComparator(new DefaultImportComparator()));

This helped to remove the fully qualified names, but I get this error now -

Are you running in classpath mode or noclasspath? Auto-import can sometimes get funky in noclasspath as it's hard for Spoon to resolve references. Without seeing the actual code being analyzed, it's hard to make anything but guesses, but most likely it's an incorrectly resolved reference somewhere.

I am running the code in classpath mode and I see that even for the code I copied from Sorald (as shown below), an unused import gets added (import java.util.function.Consumer;)

import java.util.Collection;
import java.util.IdentityHashMap;

import spoon.reflect.path.CtRole;
import spoon.reflect.reference.CtTypeReference;
import spoon.reflect.visitor.ForceImportProcessor;
import spoon.reflect.visitor.LexicalScope;

public class SelectiveForceImport extends ForceImportProcessor {
    // use identity rather than equality to identify existing references to avoid mistaking clones
    // for originals
    private final IdentityHashMap<CtTypeReference<?>, Boolean> excludedReferences;

    /**
     * @param referencesToIgnore
     *            A collection of references to ignore when force-importing.
     */
    public SelectiveForceImport(Collection<CtTypeReference<?>> referencesToIgnore) {
        excludedReferences = new IdentityHashMap<>();
        referencesToIgnore.forEach(ref -> excludedReferences.put(ref, true));
    }

    @Override
    protected void handleTypeReference(CtTypeReference<?> reference, LexicalScope nameScope, CtRole role) {
        if (!excludedReferences.containsKey(reference)) {
            super.handleTypeReference(reference, nameScope, role);
        }
    }
}

[SirYwell]

To me, that sounds like imports are added for functional interfaces even when they are used as lambda expression (and likely method references too?). Does that match your processed/printed code?

[shuchi03]

Actually, the imports which are being added might be used in a superclass and it does not have relevance in my processed code. Also, I am only refactoring a logger initialization line in one class, but these additional imports are coming in other classes as well.

[slarse]

To me, that sounds like imports are added for functional interfaces even when they are used as lambda expression (and likely method references too?). Does that match your processed/printed code?

I tried this; it happens. Bug!

[slarse]

@shuchi03 I just fixed the bug with importing of functional interfaces, could you try out the latest version of Spoon and see if your problems persist?

[shuchi03]

Hi @slarse, I used the latest version of spoon which 9.1.0-beta-22. I am really sorry but I am still getting the same issue. So for example as I have mentioned above (copied from sorald) -

package spoon.process.classes;

import java.util.Collection;
import java.util.IdentityHashMap;

import spoon.reflect.path.CtRole;
import spoon.reflect.reference.CtTypeReference;
import spoon.reflect.visitor.ForceImportProcessor;
import spoon.reflect.visitor.LexicalScope;

public class SelectiveForceImport extends ForceImportProcessor {
    private final IdentityHashMap<CtTypeReference<?>, Boolean> excludedReferences;
    public SelectiveForceImport(Collection<CtTypeReference<?>> referencesToIgnore) {
        excludedReferences = new IdentityHashMap<>();
        referencesToIgnore.forEach(ref -> excludedReferences.put(ref, true));
    }

    @Override
    protected void handleTypeReference(CtTypeReference<?> reference, LexicalScope nameScope, CtRole role) {
        if (!excludedReferences.containsKey(reference)) {
            super.handleTypeReference(reference, nameScope, role);
        }
    }
}

the functional interface (import java.util.function.Consumer;) is getting imported.

[slarse]

Hi @shuchi03,

The beta version will not have the update until next week (after the weekend beta release), you'll need to use the latest snapshot. I wasn't very clear on that point. Of course, feel free to wait until the next beta release and try it then if you don't feel like trying the snapshot.

[shuchi03]

I tried the snapshot version which you have linked here - 9.1.0-SNAPSHOT, but I was still getting the same issue. So I will wait till next week for the updated beta version. Thank you.

[shuchi03]

Hi @slarse , I used the latest version of spoon which is 9.1.0-beta-23 and the issue was resolved in the file copied from sorald. But, for the other files, this issue is coming up. For example, for this file -

package spoon.process.classes;

import java.util.List;

import javax.persistence.criteria.Expression;
import javax.persistence.criteria.Path;
import javax.persistence.criteria.Predicate;

public class Restrictions {
    public static PredicateBuilder eq(String propertyName, Object value) {
        return (cb, p) -> cb.equal(getExpression(propertyName, p), value);
    }

    @SuppressWarnings("unchecked")
    public static PredicateBuilder eq(String propertyName, boolean value) {
        if (value) {
            return (cb, p) -> cb.isTrue(getExpression(propertyName, p));
        }
        return (cb, p) -> cb.isFalse(getExpression(propertyName, p));
    }

    public static PredicateBuilder eqOrIsNull(String propertyName, Object value) {
        return value == null ? isNull(propertyName) : eq(propertyName, value);
    }

    public static PredicateBuilder ne(String propertyName, Object value) {
        return (cb, p) -> cb.notEqual(getExpression(propertyName, p), value);
    }

    @SuppressWarnings("unchecked")
    public static PredicateBuilder like(String propertyName, String value) {
        return (cb, p) -> cb.like(getExpression(propertyName, p), value);
    }

    @SuppressWarnings("unchecked")
    public static <Y extends Comparable<? super Y>> PredicateBuilder gt(String propertyName, Y value) {
        return (cb, p) -> cb.greaterThan(getExpression(propertyName, p), value);
    }

    @SuppressWarnings("unchecked")
    public static <Y extends Comparable<? super Y>> PredicateBuilder lt(String propertyName, Y value) {
        return (cb, p) -> cb.lessThan(getExpression(propertyName, p), value);
    }

    @SuppressWarnings("unchecked")
    public static <Y extends Comparable<? super Y>> PredicateBuilder ge(String propertyName, Y value) {
        return (cb, p) -> cb.greaterThanOrEqualTo(getExpression(propertyName, p), value);
    }

    @SuppressWarnings("unchecked")
    public static <Y extends Comparable<? super Y>> PredicateBuilder le(String propertyName, Y value) {
        return (cb, p) -> cb.lessThanOrEqualTo(getExpression(propertyName, p), value);
    }

    @SuppressWarnings("unchecked")
    public static <Y extends Comparable<? super Y>> PredicateBuilder between(String propertyName, Y low, Y high) {
        return (cb, p) -> cb.between(((Expression<? extends Y>) (getExpression(propertyName, p))), low, high);
    }

    public static PredicateBuilder in(String propertyName, Object... values) {
        return (cb, p) -> getExpression(propertyName, p).in(values);
    }

    public static PredicateBuilder isNull(String propertyName) {
        return (cb, p) -> cb.isNull(getExpression(propertyName, p));
    }

    public static PredicateBuilder isNotNull(String propertyName) {
        return (cb, p) -> cb.isNotNull(getExpression(propertyName, p));
    }

    @SuppressWarnings("unchecked")
    public static PredicateBuilder isNotEmpty(String propertyName) {
        return (cb, p) -> cb.isNotEmpty(getExpression(propertyName, p));
    }

    public static PredicateBuilder isFalse(String propertyName) {
        return eq(propertyName, Boolean.FALSE);
    }

    public static PredicateBuilder isTrue(String propertyName) {
        return eq(propertyName, Boolean.FALSE);
    }

    public static PredicateBuilder and(List<PredicateBuilder> predicates) {
        return and(predicates.toArray(PredicateBuilder[]::new));
    }

    public static PredicateBuilder and(PredicateBuilder... predicates) {
        return (cb, p) -> {
            Predicate[] restrictions = new Predicate[predicates.length];
            for (int i = 0; i < predicates.length; i++) {
                restrictions[i] = predicates[i].build(cb, p);
            }
            return cb.and(restrictions);
        };
    }

    public static PredicateBuilder or(List<PredicateBuilder> predicates) {
        return or(predicates.toArray(PredicateBuilder[]::new));
    }

    public static PredicateBuilder or(PredicateBuilder... predicates) {
        return (cb, p) -> {
            Predicate[] restrictions = new Predicate[predicates.length];
            for (int i = 0; i < predicates.length; i++) {
                restrictions[i] = predicates[i].build(cb, p);
            }
            return cb.or(restrictions);
        };
    }

    public static PredicateBuilder not(PredicateBuilder expression) {
        return (cb, p) -> cb.not(expression.build(cb, p));
    }

    @SuppressWarnings("rawtypes")
    private static Expression getExpression(String propertyName, Path p) {
        int separatorIndex = propertyName.indexOf('.');
        if (separatorIndex == (-1)) {
            return p.get(propertyName);
        }
        String alias = propertyName.substring(0, separatorIndex);
        String property = propertyName.substring(separatorIndex + 1);
        return getExpression(property, p.get(alias));
    }

    @SuppressWarnings("unchecked")
    public static PredicateBuilder contains(String collectionPropertyName, Object value) {
        return (cb, p) -> cb.isMember(value, getExpression(collectionPropertyName, p));
    }
}

the functional interface (import java.util.function.Consumer;) is still getting imported.

[slarse]

@shuchi03 I suppose I'll have to take another look at that, perhaps my solution wasn't as generalizable as I thought.