Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
the1derer opened this issue Jun 5, 2019 · 12 comments

Comments

@the1derer
Copy link
Member

the1derer commented Jun 5, 2019

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
Copy link
Member Author

/cc @mernst

@the1derer the1derer changed the title AUF(most probably insert-annotations-to-source) transferring/inserting non-present @SideEffectFree AFU(most probably insert-annotations-to-source) transferring/inserting non-present @SideEffectFree Jun 8, 2019
@mernst mernst changed the title AFU(most probably insert-annotations-to-source) transferring/inserting non-present @SideEffectFree insert-annotations-to-source inserting non-present @SideEffectFree on a field Jun 11, 2019
@mernst
Copy link
Member

mernst commented Jun 11, 2019

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 the1derer changed the title insert-annotations-to-source inserting non-present @SideEffectFree on a field insert-annotations-to-source inserting non-present modifier and @SideEffectFree at the wrong location Jun 12, 2019
@the1derer the1derer changed the title insert-annotations-to-source inserting non-present modifier and @SideEffectFree at the wrong location insert-annotations-to-source inserting @SideEffectFree at the wrong location Jun 12, 2019
@the1derer
Copy link
Member Author

the1derer commented Jun 12, 2019

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.

@the1derer the1derer changed the title insert-annotations-to-source inserting @SideEffectFree at the wrong location insert-annotations-to-source inserting Annotations on Constructor at the field where it was non-present Jun 12, 2019
@the1derer the1derer changed the title insert-annotations-to-source inserting Annotations on Constructor at the field where it was non-present insert-annotations-to-source inserting annotations from Constructor at the field where it was non-present Jun 12, 2019
@mernst
Copy link
Member

mernst commented Jun 12, 2019

This is a very useful minimization! Thanks.

@the1derer
Copy link
Member Author

the1derer commented Jun 12, 2019

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

@mernst
Copy link
Member

mernst commented Jun 12, 2019

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
Copy link
Member Author

Thank you very much for fixing the bug.

@the1derer the1derer reopened this Jun 14, 2019
@the1derer
Copy link
Member Author

the1derer commented Jun 14, 2019

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
Copy link
Member Author

/cc @mernst

@mernst mernst closed this as completed in ccf7fd0 Jun 16, 2019
@the1derer the1derer reopened this Jul 27, 2019
@the1derer
Copy link
Member Author

@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() {}

}

mernst added a commit to mernst/annotation-tools that referenced this issue Jul 28, 2019
@mernst mernst closed this as completed in 95c57d6 Jul 28, 2019
@the1derer the1derer reopened this Jul 28, 2019
@the1derer
Copy link
Member Author

@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
Copy link
Member

mernst commented Sep 10, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants