@Identity and @Fluent inferred from a single return path #31

@Identity and @fluent inferred from a single return path

Both properties promise something about every call: @Identity that the call returns its first argument, @Fluent that it returns its receiver. Consumers rely on that. They alias the result away, replacing x = m(p, ..) by p and x = o.m(..) by o. Both are currently inferred when only some return path has the property.

1. @Identity: one path returns the parameter, another a fresh object

import java.util.ArrayList;
import java.util.List;

class X {
    private List<String> process(List<String> indentation, List<String> prev) {
        List<String> indentation0 = indentation;
        int eolIndex = prev.lastIndexOf("\n");
        if (eolIndex < 0) { return indentation0; }
        indentation0 = new ArrayList<>(prev.subList(eolIndex + 1, prev.size()));
        return indentation0;
    }
}

After the modification analysis, process.isIdentity() is true. The same happens when the parameter itself is reassigned: if (..) return p; p = takeWhile(..); return p;.

TypeModIndyAnalyzerImpl.doIdentityAnalysis requires one link rv ≡/← p0 (anyMatch) and requires that all links from rv's primary go to p0 (allMatch). The second return value is a fresh object, which adds no link at all, so it can't violate the allMatch. This is the vacuous truth that the comment above that code describes for faces-only summaries, here per return path.

2. @fluent: any link to this is enough

class X {
    interface Context { }
    interface RT {
        default RT solve(Context context) { return this; }
    }
    static class W implements RT {
        private final boolean ext;
        private final RT bound;
        W(boolean ext, RT bound) { this.ext = ext; this.bound = bound; }
        boolean isExtends() { return ext; }
        boolean isSuper() { return !ext && bound != null; }
        RT getBound() {
            if (bound == null) { throw new IllegalStateException(); }
            return bound;
        }
        static W extendsBound(RT t) { return new W(true, t); }
        static W superBound(RT t) { return new W(false, t); }
        @Override
        public RT solve(Context context) {
            if (isExtends() || isSuper()) {
                RT boundResolved = getBound().solve(context);
                if (isExtends()) { return extendsBound(boundResolved); } else { return superBound(boundResolved); }
            }
            return this;
        }
    }
}

W.solve gets isFluent() == true, but on one of its paths it returns a new object (extendsBound(..) / superBound(..)). doFluentAnalysis only asks anyMatch(link -> link.to() instanceof This ...).

Related: RT.solve, the interface default return this, is correctly fluent as a body. But a virtual call rt.solve(c) resolves to it statically, while the overrides (like W.solve above) are not fluent. Should a method with non-fluent overrides be reported as @Fluent?

3. @fluent on a method returning boolean

In a full analysis of javaparser (javaparser-core 3.25.4-SNAPSHOT, 537 primary types), com.github.javaparser.ast.Node.remove(Node) ends up with isFluent() == true. It returns boolean:

public boolean remove(Node node) {
    if (node == null) { return false; }
    if (comment != null) { if (node == comment) { removeComment(); return true; } }
    return false;
}

removeComment() is genuinely fluent (return setComment(null), and setComment returns this), but its result is discarded. I couldn't reduce this one. A small copy of Node, with setComment, removeComment, remove(Node), remove() and an overriding subclass, gets isFluent() == false. So something in the full hierarchy (javaparser has many generated overrides of remove(Node)) brings it about. Regardless of the cause, a method whose return type is primitive or void can't be fluent; that check alone would rule it out.

Seen with maddi-mod 0.9.5-alpha.11. The two methods are unchanged on devel.

Impact

A consumer that aliases @Identity / @Fluent results makes the caller's code read the argument or the receiver, where the program uses a different object. In the boolean case, a boolean result became a Node. Downstream code noticed that only because the boolean then reached &&.