typetools / annotation-tools

Tools for type annotations in Java
https://checkerframework.org/annotation-file-utilities/
MIT License
38 stars 34 forks source link

`insert-annotations-to-source` inserting annotations from Constructor at the field where it was non-present #216

Open the1derer opened 5 years ago

the1derer commented 5 years ago

insert-annotation-to-source is inserting annotation that was present on Constructors to field where is was non-present. "Step to reproduce" uses @SideEffectFree but using @RequiresNonNull or @Deprecated instead of @SideEffectFree also yields the same result i.e. Annotation at the wrong place. This means this bug is not isolated to just @SideEffectFree

To reproduce:

Test.java(Annotated)

import org.checkerframework.dataflow.qual.SideEffectFree;
class Test {

  Object value;

  @SideEffectFree
  public Test(){

  }
}
export CLASSPATH=checker-qual.jar
javac Test.java
extract-annotations Test.class

Test.jaif

package org.checkerframework.dataflow.qual:
annotation @SideEffectFree: @java.lang.annotation.Retention(value=RUNTIME) @java.lang.annotation.Target(value={METHOD,CONSTRUCTOR})

package :
class Test:

    field value:

    method <init>()V: @org.checkerframework.dataflow.qual.SideEffectFree
        return:

Test.java(Unannotated)

class Test {

  Object value;

  public Test(){

  }
}
insert-annotations-to-source -i=true Test.jaif Test.java

Result

import org.checkerframework.dataflow.qual.SideEffectFree;
class Test {

  Object @SideEffectFree value;

  public Test(){

  }
}

Note:- This problem is only caused if @SideEffectFree is on Constructor. That is if there is @SideEffectFree on method there will be no error.

Example: (There will be no error in this case)

import org.checkerframework.dataflow.qual.SideEffectFree;
class Test {

  Object value;

  @SideEffectFree
  public void test(){

  }
}
the1derer commented 5 years ago

/cc @mernst

mernst commented 5 years ago

Could you please minimize the test case? That is, please find a smaller .java file and a smaller .jaif file that reproduce the error. That will assist me (or you!) in debugging the problem. Thanks!

the1derer commented 5 years ago

Could you please minimize the test case?

Done Thanks for the feedback to create the smaller Test-Case because of this While creating test case I was able to find the bug more precisely. Sorry for changing the title this many times this was the result of more experimentation on bugs and making the title more appropriate to the bug report.

Thanks.

mernst commented 5 years ago

This is a very useful minimization! Thanks.

the1derer commented 5 years ago

Thanks for the feedback. I will start to work on this immediately.

mernst commented 5 years ago

I saw your message after I had started to work on the issue (because I didn't want you to get blocked). I believe it is fixed now.

the1derer commented 5 years ago

Thank you very much for fixing the bug.

the1derer commented 5 years ago

This is till present in field that are initialized at definition.

To reproduce:

Test.java(Annotated)

import org.checkerframework.dataflow.qual.SideEffectFree;
class Test {

  Object value;
  Object value2= new Object();

  @SideEffectFree
  public Test(){

  }
}
export CLASSPATH=checker-qual.jar
javac Test.java
extract-annotations Test.class

Test.jaif

package org.checkerframework.dataflow.qual:
annotation @SideEffectFree: @java.lang.annotation.Retention(value=RUNTIME) @java.lang.annotation.Target(value={METHOD,CONSTRUCTOR})

package :
class Test:

    field value:

    field value2:

    method <init>()V: @org.checkerframework.dataflow.qual.SideEffectFree
        return:

Test.java(Unannotated)

class Test {

  Object value;
  Object value2= new Object();

  public Test(){

  }
}
insert-annotations-to-source -i=true Test.jaif Test.java

Result

import org.checkerframework.dataflow.qual.SideEffectFree;
class Test {

  Object value;
  Object value2= new @SideEffectFree Object();

  public Test(){

  }
}
the1derer commented 5 years ago

/cc @mernst

the1derer commented 4 years ago

@mernst This is still present for the following:

To reproduce:

Test.java(Annotated)

import org.checkerframework.dataflow.qual.SideEffectFree;

public class Test {

 Test t = this;

@SideEffectFree
public Test() {}

}
export CLASSPATH=checker-qual.jar
javac Test.java
extract-annotations Test.class

Test.jaif

package org.checkerframework.dataflow.qual:
annotation @SideEffectFree: @java.lang.annotation.Retention(value=RUNTIME) @java.lang.annotation.Target(value={METHOD,CONSTRUCTOR})

package :
class Test:

    field t:

    method <init>()V: @org.checkerframework.dataflow.qual.SideEffectFree
        return:

Test.java(Unannotated)

public class Test {

 Test t = this;

public Test() {}

}
insert-annotations-to-source -i=true Test.jaif Test.java

Result

import org.checkerframework.dataflow.qual.SideEffectFree;
public class Test {

 Test t = @SideEffectFree this;

public Test() {}

}
the1derer commented 4 years ago

@mernst New full Test-Case

To reproduce:

Test.java(Annotated)

import org.checkerframework.dataflow.qual.SideEffectFree;
public class Test {

  Object[] stackTrace = new Object[0];

  Object value;
  Object value2= new Object();

  Test t = this;

  @SideEffectFree
  public Test() {
  }

}
export CLASSPATH=checker-qual.jar
javac Test.java
extract-annotations Test.class

Test.jaif

package org.checkerframework.dataflow.qual:
annotation @SideEffectFree: @java.lang.annotation.Retention(value=RUNTIME) @java.lang.annotation.Target(value={METHOD,CONSTRUCTOR})

package :
class Test:

    field stackTrace:

    field value:

    field value2:

    field t:

    method <init>()V: @org.checkerframework.dataflow.qual.SideEffectFree
        return:

Test.java(Unannotated)

public class Test {

  Object[] stackTrace = new Object [0];

  Object value;
  Object value2= new Object();

  Test t = this;

  public Test() {
  }

}
insert-annotations-to-source -i=true Test.jaif Test.java

Result

import org.checkerframework.dataflow.qual.SideEffectFree;
public class Test {

  Object[] stackTrace = new Object @SideEffectFree [0];

  Object value;
  Object value2= new Object();

  Test t = this;

  public Test() {
  }

}
mernst commented 4 years ago

This last variant is hard to fix. The others could be worked around, but this exposes the fundamentally bad design of having a set of Criteria objects rather than a path.