fix @NotNull instrumentation for enum constructor parameters with ASM 3.3 (IDEA-56943...

diff --git a/java/compiler/notNull/src/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenter.java b/java/compiler/notNull/src/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenter.java
index d6bcc68bbe76c64f2699b895c55f0865037e8b8c..7990ab4d73f1492df88a318aa41cdbf08bd0aa33 100644 (file)
--- a/java/compiler/notNull/src/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenter.java
+++ b/java/compiler/notNull/src/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenter.java
@@ -93,15 +93,13 @@ public class NotNullVerifyingInstrumenter extends ClassAdapter implements Opcode
         av = mv.visitParameterAnnotation(parameter,
                                          anno,
                                          visible);
-        if (isReferenceType(args[parameter])) {
-          if (anno.equals(NOT_NULL_ANNO)) {
-            myNotNullParams.add(new Integer(parameter));
-          }
-          else if (anno.equals("Ljava/lang/Synthetic;")) {
-            // See asm r1278 for what we do this,
-            // http://forge.objectweb.org/tracker/index.php?func=detail&aid=307392&group_id=23&atid=100023
-            mySyntheticCount++;
-          }
+        if (isReferenceType(args[parameter]) && anno.equals(NOT_NULL_ANNO)) {
+          myNotNullParams.add(new Integer(parameter));
+        }
+        else if (anno.equals("Ljava/lang/Synthetic;")) {
+          // See asm r1278 for what we do this,
+          // http://forge.objectweb.org/tracker/index.php?func=detail&aid=307392&group_id=23&atid=100023
+          mySyntheticCount++;
         }
         return av;
       }
@@ -197,9 +195,6 @@ public class NotNullVerifyingInstrumenter extends ClassAdapter implements Opcode
   private int getStartParameterIndex(final String name) {
     int result = 0;
     if (CONSTRUCTOR_NAME.equals(name)) {
-      if (mySuperName.equals(ENUM_CLASS_NAME)) {
-        result += 2;
-      }
       if (myIsNotStaticInner) {
         result += 1;
       }

diff --git a/java/java-tests/java-tests.iml b/java/java-tests/java-tests.iml
index d18da953d9a76bcfa3363c82252184398265e82d..643d97843f308f6f01933cab95be09ee814e15d2 100644 (file)
--- a/java/java-tests/java-tests.iml
+++ b/java/java-tests/java-tests.iml
@@ -15,6 +15,9 @@
     <orderEntry type="module" module-name="platform-api" />
     <orderEntry type="library" name="Velocity" level="project" />
     <orderEntry type="module" module-name="java-i18n" />
+    <orderEntry type="module" module-name="compiler-impl" />
+    <orderEntry type="library" name="asm" level="project" />
+    <orderEntry type="module" module-name="notNull" />
   </component>
 </module>
 

diff --git a/java/java-tests/testData/compiler/notNullVerification/EnumConstructor.java b/java/java-tests/testData/compiler/notNullVerification/EnumConstructor.java
new file mode 100644 (file)
index 0000000..0c78423
--- /dev/null
+++ b/java/java-tests/testData/compiler/notNullVerification/EnumConstructor.java
@@ -0,0 +1,11 @@
+import org.jetbrains.annotations.NotNull;
+
+public enum EnumConstructor {
+    Value("label");
+
+    private final String label;
+
+    EnumConstructor(@NotNull String label) {
+        this.label = label;
+    }
+}

diff --git a/java/java-tests/testData/compiler/notNullVerification/MultipleReturns.java b/java/java-tests/testData/compiler/notNullVerification/MultipleReturns.java
new file mode 100644 (file)
index 0000000..61446a6
--- /dev/null
+++ b/java/java-tests/testData/compiler/notNullVerification/MultipleReturns.java
@@ -0,0 +1,9 @@
+import org.jetbrains.annotations.NotNull;
+
+public class MultipleReturns {
+  @NotNull public Object test(int i) {
+    if (i == 0) return null;
+    if (i == 1) return null;
+    return null;
+  }
+}
\ No newline at end of file

diff --git a/java/java-tests/testData/compiler/notNullVerification/SimpleReturn.java b/java/java-tests/testData/compiler/notNullVerification/SimpleReturn.java
new file mode 100644 (file)
index 0000000..1a85ad9
--- /dev/null
+++ b/java/java-tests/testData/compiler/notNullVerification/SimpleReturn.java
@@ -0,0 +1,7 @@
+import org.jetbrains.annotations.NotNull;
+
+public class SimpleReturn {
+  @NotNull public Object test() {
+    return null;
+  }
+}
\ No newline at end of file

diff --git a/java/java-tests/testData/compiler/notNullVerification/annotations.jar b/java/java-tests/testData/compiler/notNullVerification/annotations.jar
new file mode 100644 (file)
index 0000000..45860d8
Binary files /dev/null and b/java/java-tests/testData/compiler/notNullVerification/annotations.jar differ

diff --git a/java/java-tests/testSrc/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenterTest.java b/java/java-tests/testSrc/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenterTest.java
new file mode 100644 (file)
index 0000000..12f5e0a
--- /dev/null
+++ b/java/java-tests/testSrc/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenterTest.java
@@ -0,0 +1,119 @@
+package com.intellij.compiler.notNullVerification;
+
+import com.intellij.JavaTestUtil;
+import com.intellij.compiler.PsiClassWriter;
+import com.intellij.openapi.util.io.FileUtil;
+import com.intellij.testFramework.IdeaTestCase;
+import com.intellij.testFramework.UsefulTestCase;
+import com.intellij.testFramework.fixtures.IdeaProjectTestFixture;
+import com.intellij.testFramework.fixtures.JavaTestFixtureFactory;
+import com.intellij.testFramework.fixtures.TestFixtureBuilder;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassWriter;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+/**
+ * @author yole
+ */
+public class NotNullVerifyingInstrumenterTest extends UsefulTestCase {
+  private IdeaProjectTestFixture myFixture;
+
+  @SuppressWarnings({"JUnitTestCaseWithNonTrivialConstructors"})
+  public NotNullVerifyingInstrumenterTest() {
+    IdeaTestCase.initPlatformPrefix();
+  }
+
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+    final JavaTestFixtureFactory fixtureFactory = JavaTestFixtureFactory.getFixtureFactory();
+    final TestFixtureBuilder<IdeaProjectTestFixture> testFixtureBuilder = fixtureFactory.createLightFixtureBuilder();
+    myFixture = testFixtureBuilder.getFixture();
+    myFixture.setUp();
+  }
+
+  @Override
+  protected void tearDown() throws Exception {
+    myFixture.tearDown();
+    super.tearDown();
+  }
+
+  public void testSimpleReturn() throws Exception {
+    Class testClass = prepareTest();
+    Object instance = testClass.newInstance();
+    Method method = testClass.getMethod("test");
+    verifyCallThrowsException("@NotNull method SimpleReturn.test must not return null", instance, method);
+  }
+
+  public void testMultipleReturns() throws Exception {
+    Class testClass = prepareTest();
+    Object instance = testClass.newInstance();
+    Method method = testClass.getMethod("test", int.class);
+    verifyCallThrowsException("@NotNull method MultipleReturns.test must not return null", instance, method, 1);
+  }
+
+  public void testEnumConstructor() throws Exception {
+    Class testClass = prepareTest();
+    Object field = testClass.getField("Value");
+    assertNotNull(field);
+  }
+
+  private static void verifyCallThrowsException(final String expectedError, final Object instance, final Method method, final Object... args) throws IllegalAccessException {
+    String exceptionText = null;
+    try {
+      method.invoke(instance, args);
+    }
+    catch(InvocationTargetException ex) {
+      Throwable cause = ex.getCause();
+      if (cause instanceof IllegalStateException) {
+        exceptionText = cause.getMessage();
+      }
+    }
+    assertEquals(expectedError, exceptionText);
+  }
+
+  private Class prepareTest() throws IOException {
+    String base = JavaTestUtil.getJavaTestDataPath() + "/compiler/notNullVerification/";
+    String path = base + getTestName(false);
+    String javaPath = path + ".java";
+    String classPath = path + ".class";
+    try {
+      com.sun.tools.javac.Main.compile(new String[] { "-classpath", base+"annotations.jar", javaPath } );
+      FileInputStream stream = new FileInputStream(classPath);
+      byte[] content = FileUtil.adaptiveLoadBytes(stream);
+      stream.close();
+
+      ClassReader reader = new ClassReader(content, 0, content.length);
+      ClassWriter writer = new PsiClassWriter(myFixture.getProject(), false);
+      final NotNullVerifyingInstrumenter instrumenter = new NotNullVerifyingInstrumenter(writer);
+      reader.accept(instrumenter, 0);
+      assertTrue(instrumenter.isModification());
+
+      MyClassLoader classLoader = new MyClassLoader(getClass().getClassLoader());
+      byte[] instrumented = writer.toByteArray();
+      return classLoader.doDefineClass(getTestName(false), instrumented);
+    }
+    finally {
+      FileUtil.delete(new File(classPath));
+    }
+  }
+
+  private static class MyClassLoader extends ClassLoader {
+    public MyClassLoader(ClassLoader parent) {
+      super(parent);
+    }
+
+    public Class doDefineClass(String name, byte[] data) {
+      return defineClass(name, data, 0, data.length);
+    }
+
+    public Class<?> loadClass(String name) throws ClassNotFoundException {
+      return super.loadClass(name);
+    }
+  }
+}