ucr-riple / NullAwayAnnotator

A tool to help adapting code bases to NullAway type system.
MIT License
13 stars 6 forks source link

Injector misses enums constants while other anonymous classes exists #124

Closed nimakarimipour closed 1 year ago

nimakarimipour commented 1 year ago

Describe the bug According to java language specifics, Enum constants are first members of enums. In javaparser structure, enum constants are stored after all other members of enum which does not conform to how javac assigns flat names. We should first prioritize locating enums constants over other anonymous classes. As shown in the example below, enum constants are numbered before other existing anonymous classes, but in javaparser data structures, in the list of child nodes, enum constants are the last elements.

This will cause injector to annotate a wrong method e.g. EC2#bar(), instead of Anonymous#bar() or miss the annotation.

public enum Main {

    EC1 { --------------------------------//Main$1.
        @Override
        public String bar() {
            return "EC1 IMPL";
        }
    },

    EC2 { --------------------------------//Main$2.
        @Override
        public String bar() {
            return "EC2 IMPL";
        }
    };

    A a = new A() {-----------------------//Main$3.
        @Override
        public String bar() {
            return null;
        }
    };

    public final void foo() {
        System.out.println("TEST");
    }

    public abstract String bar();
}

interface A {
    String bar();
}
msridhar commented 1 year ago

I think I understand what is going on here at a high level, but not the code example. Where is Anonymous#bar()? What is the input to the injector that would cause the wrong method to be annotated?

nimakarimipour commented 1 year ago

Anonymous#bar() refers to the bar() declaration in the expression used to initialize A a (Main#3).

In javaparser, direct child nodes of Main are (which are instances of BodyDeclaration):

  1. anonymous class used to initialize A a.
  2. enum constant EC1
  3. enum constant EC2

Our current implementation, locates all nodes with the same parent path and then returns the node at the requested index. For Main#1, all three nodes above have the same parent (Main#) therefore the anonymous class will be selected. But in javac, flatnames are assigned to enum constants prior to any other defined anonymous classes, therefore Main#1 refers to EC1.

The main issue with this bug is that a node of type BodyDeclaration at the wrong index will be located by the injector. In most cases annotator fails to inject any annotation for the requested change (since it is unlikely that the overridden method in the anonymous class and in enum constants have an identical signature), but if they actually do have identical signature, a wrong annotation will be injected.