[java] fixes not-null instrumentation for type-use annotation targets (IDEA-200052)

This commit is contained in:
Roman Shevchenko
2018-11-30 19:36:51 +01:00
parent 7e94efb24a
commit f0b214d65c
7 changed files with 94 additions and 37 deletions
@@ -4,8 +4,6 @@ package com.intellij.compiler.notNullVerification;
import com.intellij.compiler.instrumentation.FailSafeClassReader;
import com.intellij.compiler.instrumentation.FailSafeMethodVisitor;
import org.jetbrains.org.objectweb.asm.*;
import org.jetbrains.org.objectweb.asm.signature.SignatureReader;
import org.jetbrains.org.objectweb.asm.signature.SignatureVisitor;
import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
@@ -33,7 +31,6 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode
private final Set<String> myNotNullAnnotations = new HashSet<String>();
private boolean myEnum;
private boolean myInner;
private boolean myEnclosed;
private NotNullVerifyingInstrumenter(ClassVisitor classVisitor, ClassReader reader, String[] notNullAnnotations) {
super(Opcodes.API_VERSION, classVisitor);
@@ -107,12 +104,6 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode
}
}
@Override
public void visitOuterClass(String owner, String name, String desc) {
super.visitOuterClass(owner, name, desc);
myEnclosed = true;
}
private static class NotNullState {
String message;
String exceptionType;
@@ -150,9 +141,6 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode
final boolean isStatic = isStatic(access);
final Type[] args = Type.getArgumentTypes(desc);
boolean hasOuterClassParameter = myEnclosed && myInner && "<init>".equals(name);
// see http://forge.ow2.org/tracker/?aid=307392&group_id=23&atid=100023&func=detail
final int syntheticCount = signature == null ? 0 : hasOuterClassParameter ? 1 : Math.max(0, args.length - getSignatureParameterCount(signature));
final int paramAnnotationOffset = !"<init>".equals(name) ? 0 : myEnum ? 2 : myInner ? 1 : 0;
final Type returnType = Type.getReturnType(desc);
@@ -189,7 +177,7 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode
return checkNotNullMethod(desc, base);
}
if (ref.getSort() == TypeReference.METHOD_FORMAL_PARAMETER) {
return checkNotNullParameter(ref.getFormalParameterIndex() + syntheticCount, desc, base);
return checkNotNullParameter(ref.getFormalParameterIndex() + paramAnnotationOffset, desc, base);
}
return base;
}
@@ -253,7 +241,7 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode
String descrPattern = state.getNullParamMessage(paramName);
String[] args = state.message != null
? EMPTY_STRING_ARRAY
: new String[]{paramName != null ? paramName : String.valueOf(param - syntheticCount), myClassName, name};
: new String[]{paramName != null ? paramName : String.valueOf(param - paramAnnotationOffset), myClassName, name};
reportError(state.exceptionType, end, descrPattern, args);
}
}
@@ -303,18 +291,6 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode
return (access & ACC_STATIC) != 0;
}
private static int getSignatureParameterCount(String signature) {
final int[] count = {0};
new SignatureReader(signature).accept(new SignatureVisitor(Opcodes.ASM6) {
@Override
public SignatureVisitor visitParameterType() {
count[0]++;
return super.visitParameterType();
}
});
return count[0];
}
private static boolean isReferenceType(Type type) {
return type.getSort() == Type.OBJECT || type.getSort() == Type.ARRAY;
}
@@ -6,6 +6,10 @@ class GroovyInnerClass {
new Inner(null, "")
}
static void fail() {
new Inner(new GroovyInnerClass(), "", null)
}
private class Inner {
Inner(String s1, @NotNull String s2) { }
}
@@ -5,7 +5,11 @@ public class NonStaticInnerClass {
new Inner(null, "");
}
public static void fail() {
new NonStaticInnerClass().new Inner("", null);
}
public class Inner {
public Inner(String s1, @NotNull String s2) { }
}
}
}
@@ -0,0 +1,10 @@
package org.jetbrains.annotations;
import java.lang.annotation.*;
@Retention(RetentionPolicy.CLASS)
@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE})
public @interface NotNull {
String value() default "";
Class<? extends Exception> exception() default Exception.class;
}
@@ -0,0 +1,10 @@
package org.jetbrains.annotations;
import java.lang.annotation.*;
@Retention(RetentionPolicy.CLASS)
@Target({ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE, ElementType.TYPE_USE})
public @interface NotNull {
String value() default "";
Class<? extends Exception> exception() default Exception.class;
}
@@ -0,0 +1,10 @@
package org.jetbrains.annotations;
import java.lang.annotation.*;
@Retention(RetentionPolicy.CLASS)
@Target({ElementType.TYPE_USE})
public @interface NotNull {
String value() default "";
Class<? extends Exception> exception() default Exception.class;
}
@@ -6,8 +6,8 @@ import com.intellij.codeInsight.AnnotationUtil;
import com.intellij.compiler.instrumentation.FailSafeClassReader;
import com.intellij.compiler.notNullVerification.NotNullVerifyingInstrumenter;
import com.intellij.openapi.util.io.FileUtil;
import com.intellij.openapi.util.io.IoTestUtil;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.project.IntelliJProjectConfiguration;
import com.intellij.testFramework.IdeaTestUtil;
import com.intellij.testFramework.PlatformTestUtil;
import com.intellij.testFramework.rules.TempDirectory;
@@ -16,12 +16,18 @@ import com.intellij.util.ExceptionUtil;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.org.objectweb.asm.ClassWriter;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExternalResource;
import org.junit.rules.TestName;
import org.junit.runner.Description;
import org.junit.runners.model.Statement;
import java.io.File;
import java.io.IOException;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Member;
@@ -35,7 +41,42 @@ import static org.junit.Assert.*;
/**
* @author yole
*/
public class NotNullVerifyingInstrumenterTest {
public abstract class NotNullVerifyingInstrumenterTest {
@Retention(RetentionPolicy.RUNTIME)
private @interface TestDirectory { String value(); }
@TestDirectory("members")
public static class MembersTargetTest extends NotNullVerifyingInstrumenterTest { }
@TestDirectory("types")
public static class TypesTargetTest extends NotNullVerifyingInstrumenterTest { }
@TestDirectory("mixed")
public static class MixedTargetTest extends NotNullVerifyingInstrumenterTest { }
private static final String TEST_DATA_PATH = "/compiler/notNullVerification/";
private static class AnnotationCompiler extends ExternalResource {
private File classes;
@Override
public Statement apply(Statement base, Description description) {
Class<?> testClass = description.getTestClass();
TestDirectory annotation = testClass.getAnnotation(TestDirectory.class);
if (annotation == null) throw new IllegalArgumentException(testClass + " misses @TestDirectory annotation");
File source = new File(JavaTestUtil.getJavaTestDataPath() + TEST_DATA_PATH + annotation.value() + "/NotNull.java");
if (!source.isFile()) throw new IllegalArgumentException("Cannot find annotation file at " + source);
classes = IoTestUtil.createTestDir("test-notNullInstrumenter-" + annotation.value());
IdeaTestUtil.compileFile(source, classes);
return super.apply(base, description);
}
@Override
protected void after() {
IoTestUtil.delete(classes);
}
}
@ClassRule public static final AnnotationCompiler annotation = new AnnotationCompiler();
@Rule public TempDirectory tempDir = new TempDirectory();
@Rule public TestName testName = new TestName();
@@ -160,14 +201,18 @@ public class NotNullVerifyingInstrumenterTest {
@Test
public void testNonStaticInnerClass() throws Exception {
Class aClass = prepareTest();
assertNotNull(aClass.newInstance());
Class<?> testClass = prepareTest();
assertNotNull(testClass.newInstance());
verifyCallThrowsException(
"Argument 1 for @NotNull parameter of NonStaticInnerClass$Inner.<init> must not be null", null, testClass.getMethod("fail"));
}
@Test
public void testGroovyInnerClass() throws Exception {
Class aClass = prepareTest();
assertNotNull(aClass.newInstance());
Class<?> testClass = prepareTest();
assertNotNull(testClass.newInstance());
verifyCallThrowsException(
"Argument for @NotNull parameter 's2' of GroovyInnerClass$Inner.<init> must not be null", null, testClass.getMethod("fail"));
}
@Test
@@ -285,12 +330,10 @@ public class NotNullVerifyingInstrumenterTest {
}
private Class<?> prepareTest(boolean withDebugInfo, String... notNullAnnotations) throws IOException {
String testDir = JavaTestUtil.getJavaTestDataPath() + "/compiler/notNullVerification/";
String testName = PlatformTestUtil.getTestName(this.testName.getMethodName(), false);
File testFile = IdeaTestUtil.findSourceFile(testDir + testName);
File testFile = IdeaTestUtil.findSourceFile((JavaTestUtil.getJavaTestDataPath() + TEST_DATA_PATH) + testName);
File classesDir = tempDir.newFolder("output");
List<String> libRoots = IntelliJProjectConfiguration.getProjectLibraryClassesRootPaths("jetbrains-annotations-java5");
List<String> args = ContainerUtil.newArrayList("-cp", StringUtil.join(libRoots, File.pathSeparator));
List<String> args = ContainerUtil.newArrayList("-cp", annotation.classes.getPath());
if (withDebugInfo) args.add("-g");
IdeaTestUtil.compileFile(testFile, classesDir, ArrayUtil.toStringArray(args));