From eed7f584d39a967a5f4642fd806dbf985846e699 Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Mon, 24 Sep 2012 18:35:31 +0200 Subject: [PATCH] Instrumenter cleanup + couple more tests --- .../NotNullVerifyingInstrumenter.java | 75 ++++++++++--------- .../notNullVerification/ConstructorParam.java | 6 ++ .../notNullVerification/SimpleParam.java | 6 ++ .../NotNullVerifyingInstrumenterTest.java | 30 +++++++- 4 files changed, 77 insertions(+), 40 deletions(-) create mode 100644 java/java-tests/testData/compiler/notNullVerification/ConstructorParam.java create mode 100644 java/java-tests/testData/compiler/notNullVerification/SimpleParam.java diff --git a/java/compiler/instrumentation-util/src/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenter.java b/java/compiler/instrumentation-util/src/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenter.java index 9de8ce8f84a3..72fbc554a8ca 100644 --- a/java/compiler/instrumentation-util/src/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenter.java +++ b/java/compiler/instrumentation-util/src/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenter.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2009 JetBrains s.r.o. + * Copyright 2000-2012 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,19 +18,27 @@ package com.intellij.compiler.notNullVerification; import org.jetbrains.asm4.*; import java.util.ArrayList; +import java.util.List; /** * @author ven - * @noinspection HardCodedStringLiteral */ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcodes { - private boolean myIsModification = false; - private String myClassName; - public static final String NOT_NULL = "org/jetbrains/annotations/NotNull"; - public static final String NOT_NULL_ANNO = "L"+ NOT_NULL + ";"; - public static final String IAE_CLASS_NAME = "java/lang/IllegalArgumentException"; - public static final String ISE_CLASS_NAME = "java/lang/IllegalStateException"; + private static final String NOT_NULL_CLASS_NAME = "org/jetbrains/annotations/NotNull"; + private static final String NOT_NULL_TYPE = "L"+ NOT_NULL_CLASS_NAME + ";"; + private static final String SYNTHETIC_CLASS_NAME = "java/lang/Synthetic"; + private static final String SYNTHETIC_TYPE = "L" + SYNTHETIC_CLASS_NAME + ";"; + private static final String IAE_CLASS_NAME = "java/lang/IllegalArgumentException"; + private static final String ISE_CLASS_NAME = "java/lang/IllegalStateException"; + private static final String STRING_CLASS_NAME = "java/lang/String"; private static final String CONSTRUCTOR_NAME = ""; + private static final String EXCEPTION_INIT_SIGNATURE = "(L" + STRING_CLASS_NAME + ";)V"; + + private static final String NULL_ARG_MESSAGE = "Argument %d for @NotNull parameter of %s.%s must not be null"; + private static final String NULL_RESULT_MESSAGE = "@NotNull method %s.%s must not return null"; + + private String myClassName; + private boolean myIsModification = false; public NotNullVerifyingInstrumenter(final ClassVisitor classVisitor) { super(Opcodes.ASM4, classVisitor); @@ -40,27 +48,22 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode return myIsModification; } - public void visit(final int version, - final int access, - final String name, - final String signature, - final String superName, - final String[] interfaces) { + @Override + public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { super.visit(version, access, name, signature, superName, interfaces); myClassName = name; } - public MethodVisitor visitMethod(final int access, final String name, final String desc, final String signature, final String[] exceptions) { + @Override + public MethodVisitor visitMethod(final int access, final String name, String desc, String signature, String[] exceptions) { final Type[] args = Type.getArgumentTypes(desc); final Type returnType = Type.getReturnType(desc); - MethodVisitor v = cv.visitMethod(access, name, desc, signature, exceptions); + final MethodVisitor v = cv.visitMethod(access, name, desc, signature, exceptions); return new MethodVisitor(Opcodes.ASM4, v) { - private final ArrayList myNotNullParams = new ArrayList(); + private final List myNotNullParams = new ArrayList(); private int mySyntheticCount = 0; private boolean myIsNotNull = false; - //private boolean myIsUnmodifiable = false; - //public Label myWrapLabel; private Label myStartGeneratedCodeLabel; public AnnotationVisitor visitParameterAnnotation( @@ -69,35 +72,35 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode final boolean visible) { AnnotationVisitor av; av = mv.visitParameterAnnotation(parameter, anno, visible); - if (isReferenceType(args[parameter]) && anno.equals(NOT_NULL_ANNO)) { + if (isReferenceType(args[parameter]) && anno.equals(NOT_NULL_TYPE)) { 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 + else if (anno.equals(SYNTHETIC_TYPE)) { + // see http://forge.ow2.org/tracker/?aid=307392&group_id=23&atid=100023&func=detail mySyntheticCount++; } return av; } + @Override public AnnotationVisitor visitAnnotation(String anno, boolean isRuntime) { final AnnotationVisitor av = mv.visitAnnotation(anno, isRuntime); if (isReferenceType(returnType) && - anno.equals(NOT_NULL_ANNO)) { + anno.equals(NOT_NULL_TYPE)) { myIsNotNull = true; } return av; } + @Override public void visitCode() { if (myNotNullParams.size() > 0) { myStartGeneratedCodeLabel = new Label(); mv.visitLabel(myStartGeneratedCodeLabel); } - for (int p = 0; p < myNotNullParams.size(); ++p) { + for (Integer param : myNotNullParams) { int var = ((access & ACC_STATIC) == 0) ? 1 : 0; - int param = ((Integer)myNotNullParams.get(p)).intValue(); for (int i = 0; i < param; ++i) { var += args[i].getSize(); } @@ -106,26 +109,26 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode Label end = new Label(); mv.visitJumpInsn(IFNONNULL, end); - generateThrow(IAE_CLASS_NAME, "Argument " + (param - mySyntheticCount) + " for @NotNull parameter of " + myClassName + "." + name + " must not be null", end); + generateThrow(IAE_CLASS_NAME, String.format(NULL_ARG_MESSAGE, param - mySyntheticCount, myClassName, name), end); } } - public void visitLocalVariable(final String name, final String desc, final String signature, final Label start, final Label end, - final int index) { + @Override + public void visitLocalVariable(String name, String desc, String signature, Label start, Label end, int index) { final boolean isStatic = (access & ACC_STATIC) != 0; final boolean isParameter = isStatic ? index < args.length : index <= args.length; - mv.visitLocalVariable(name, desc, signature, (isParameter && myStartGeneratedCodeLabel != null) ? myStartGeneratedCodeLabel : start, end, index); + final Label label = (isParameter && myStartGeneratedCodeLabel != null) ? myStartGeneratedCodeLabel : start; + mv.visitLocalVariable(name, desc, signature, label, end, index); } + @Override public void visitInsn(int opcode) { if (opcode == ARETURN) { if (myIsNotNull) { mv.visitInsn(DUP); - /*generateConditionalThrow("@NotNull method " + myClassName + "." + name + " must not return null", - "java/lang/IllegalStateException");*/ final Label skipLabel = new Label(); mv.visitJumpInsn(IFNONNULL, skipLabel); - generateThrow(ISE_CLASS_NAME, "@NotNull method " + myClassName + "." + name + " must not return null", skipLabel); + generateThrow(ISE_CLASS_NAME, String.format(NULL_RESULT_MESSAGE, myClassName, name), skipLabel); } } @@ -133,23 +136,23 @@ public class NotNullVerifyingInstrumenter extends ClassVisitor implements Opcode } private void generateThrow(final String exceptionClass, final String descr, final Label end) { - String exceptionParamClass = "(Ljava/lang/String;)V"; mv.visitTypeInsn(NEW, exceptionClass); mv.visitInsn(DUP); mv.visitLdcInsn(descr); - mv.visitMethodInsn(INVOKESPECIAL, exceptionClass, CONSTRUCTOR_NAME, exceptionParamClass); + mv.visitMethodInsn(INVOKESPECIAL, exceptionClass, CONSTRUCTOR_NAME, EXCEPTION_INIT_SIGNATURE); mv.visitInsn(ATHROW); mv.visitLabel(end); myIsModification = true; } + @Override public void visitMaxs(final int maxStack, final int maxLocals) { try { super.visitMaxs(maxStack, maxLocals); } catch (ArrayIndexOutOfBoundsException e) { - throw new ArrayIndexOutOfBoundsException("maxs processing failed for method " + name + ": " + e.getMessage()); + throw new ArrayIndexOutOfBoundsException("Maximums processing failed for " + myClassName + "." + name + ": " + e.getMessage()); } } }; diff --git a/java/java-tests/testData/compiler/notNullVerification/ConstructorParam.java b/java/java-tests/testData/compiler/notNullVerification/ConstructorParam.java new file mode 100644 index 000000000000..d99acbd1c254 --- /dev/null +++ b/java/java-tests/testData/compiler/notNullVerification/ConstructorParam.java @@ -0,0 +1,6 @@ +import org.jetbrains.annotations.NotNull; + +public class ConstructorParam { + public ConstructorParam(@NotNull Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/compiler/notNullVerification/SimpleParam.java b/java/java-tests/testData/compiler/notNullVerification/SimpleParam.java new file mode 100644 index 000000000000..b21f96e3989e --- /dev/null +++ b/java/java-tests/testData/compiler/notNullVerification/SimpleParam.java @@ -0,0 +1,6 @@ +import org.jetbrains.annotations.NotNull; + +public class SimpleParam { + public void test(@NotNull Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenterTest.java b/java/java-tests/testSrc/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenterTest.java index b0dd097cf245..4ba78cfd9b5b 100644 --- a/java/java-tests/testSrc/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenterTest.java +++ b/java/java-tests/testSrc/com/intellij/compiler/notNullVerification/NotNullVerifyingInstrumenterTest.java @@ -24,19 +24,22 @@ 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.jetbrains.annotations.Nullable; import org.jetbrains.asm4.ClassReader; import org.jetbrains.asm4.ClassWriter; import java.io.File; import java.io.IOException; +import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Member; import java.lang.reflect.Method; /** * @author yole */ public class NotNullVerifyingInstrumenterTest extends UsefulTestCase { - private final boolean myJava6 = SystemInfo.isJavaVersionAtLeast("1.6"); + private boolean myJava6; private IdeaProjectTestFixture myFixture; @SuppressWarnings({"JUnitTestCaseWithNonTrivialConstructors"}) @@ -51,6 +54,7 @@ public class NotNullVerifyingInstrumenterTest extends UsefulTestCase { final TestFixtureBuilder testFixtureBuilder = fixtureFactory.createLightFixtureBuilder(); myFixture = testFixtureBuilder.getFixture(); myFixture.setUp(); + myJava6 = SystemInfo.isJavaVersionAtLeast("1.6"); } @Override @@ -73,6 +77,19 @@ public class NotNullVerifyingInstrumenterTest extends UsefulTestCase { verifyCallThrowsException("@NotNull method MultipleReturns.test must not return null", instance, method, 1); } + public void testSimpleParam() throws Exception { + Class testClass = prepareTest(); + Object instance = testClass.newInstance(); + Method method = testClass.getMethod("test", Object.class); + verifyCallThrowsException("Argument 0 for @NotNull parameter of SimpleParam.test must not be null", instance, method, (Object)null); + } + + public void testConstructorParam() throws Exception { + Class testClass = prepareTest(); + Constructor method = testClass.getConstructor(Object.class); + verifyCallThrowsException("Argument 0 for @NotNull parameter of ConstructorParam. must not be null", null, method, (Object)null); + } + public void testEnumConstructor() throws Exception { Class testClass = prepareTest(); Object field = testClass.getField("Value"); @@ -95,14 +112,19 @@ public class NotNullVerifyingInstrumenterTest extends UsefulTestCase { assertNotNull(aClass.newInstance()); } - private static void verifyCallThrowsException(final String expectedError, final Object instance, final Method method, final Object... args) throws IllegalAccessException { + private static void verifyCallThrowsException(String expectedError, @Nullable Object instance, Member member, @Nullable Object... args) throws Exception { String exceptionText = null; try { - method.invoke(instance, args); + if (member instanceof Constructor) { + ((Constructor)member).newInstance(args); + } + else { + ((Method)member).invoke(instance, args); + } } catch(InvocationTargetException ex) { Throwable cause = ex.getCause(); - if (cause instanceof IllegalStateException) { + if (cause instanceof IllegalStateException || cause instanceof IllegalArgumentException) { exceptionText = cause.getMessage(); } }