[#IDEADEV-12974] @NotNull/@Nullable annotation give false run-time exception with inner class

IDEA: Development

@NotNull/@Nullable annotation give false run-time exception with inner class

Created: 02 Jan 07 16:39 Updated: 04 Jan 07 14:36

Component/s:

None

Fix Version/s:

Demetra 6.0.3

Original Estimate:

Unknown

Remaining Estimate:

Unknown

Time Spent:

Unknown

Build:	6,141
Fixed in build:	6,625
Severity:	High

Description

« Hide

The code below is perfectly correct and gives no warnings in Idea. However, at run-time it throws a java.lang.IllegalArgumentException:
"Argument 1 for @NotNull parameter of tests/Outer$Inner.<init> must not be null".
This is presumably because the inner class is not static and so is passed as an extra argument which is not taken into account at the annotation weaving stage - making class Inner be static resolves the problem, but this is not always an option.

package tests;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public class Outer {
	public static void main (String[] args) {
		Inner i = new Outer ().createIt ();
	}

	Inner createIt () {
		return new Inner (null, "Test");
	}

	public class Inner {
		@Nullable
		private final String s1;
		@NotNull
		private final String s2;

		public Inner (@Nullable String s1, @NotNull String s2) {
			this.s1 = s1;
			this.s2 = s2;
		}
	}
}

All

Comments

Work Log

Change History

Sort Order:

[ Permlink | « Hide ]

Eugene Vigdorchik - 03 Jan 07 16:26

This is a bug in compiler generating wrong parameter annotations. I currently don't see a way to fix this. We should know somehow, that it is an inner class and that it is not static. None of this information is written to class file.

[ Permlink | « Hide ]

Sascha Weinreuter - 03 Jan 07 21:08

It's probably debatable whether this is a compiler bug. I tested it with the Eclipse compiler and it reports the same indices for parameter annotations. Anyway, here's how I think this could be fixed. The key is the ASM-method "visitInnerClass". See the following rough fragment from my IntelliLang-implementation (which should be sufficiently similar to the @NotNull one):

class Instrumenter extends ClassAdapter {
    boolean myIsNonStaticInnerClass;

    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 void visitInnerClass(String name, String outerName, String innerName, int access) {
        super.visitInnerClass(name, outerName, innerName, access);
        if (myClassName.equals(name)) {
            myIsNonStaticInnerClass = (access & ACC_STATIC) == 0;
        }
    }

    ...
}

class InstrumentationAdapter extends MethodAdapter {
    Instrumenter myInstrumenter;

    public void visitCode() {
        for (PatternValue parameter : myParameterPatterns) {
            int j;
            if ((myAccess & Opcodes.ACC_STATIC) == 0) {
                // special case: ctor of non-static inner classes (see IDEA-10889)
                if ("<init>".equals(myMethodName)) {
                    j = (myInstrumenter.myIsNonStaticInnerClass) ?
                            1 + myArgTypes[0].getSize() // skip first (synthetic) "Outer.this" parameter
                            : 1;
                } else {
                    j = 1;
                }
            } else {
                j = 0;
            }

            for (int l = 0; l < parameter.index; l++) {
                j += myArgTypes[l].getSize();
            }

            ...
        }        
    }
}

At least it seems to work for all kinds of weird inner-class constructs

[ Show » ]

Sascha Weinreuter - 03 Jan 07 21:08 It's probably debatable whether this is a compiler bug. I tested it with the Eclipse compiler and it reports the same indices for parameter annotations. Anyway, here's how I think this could be fixed. The key is the ASM-method "visitInnerClass". See the following rough fragment from my IntelliLang-implementation (which should be sufficiently similar to the @NotNull one):

class Instrumenter extends ClassAdapter {
    boolean myIsNonStaticInnerClass;

    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 void visitInnerClass(String name, String outerName, String innerName, int access) {
        super.visitInnerClass(name, outerName, innerName, access);
        if (myClassName.equals(name)) {
            myIsNonStaticInnerClass = (access & ACC_STATIC) == 0;
        }
    }

    ...
}

class InstrumentationAdapter extends MethodAdapter {
    Instrumenter myInstrumenter;

    public void visitCode() {
        for (PatternValue parameter : myParameterPatterns) {
            int j;
            if ((myAccess & Opcodes.ACC_STATIC) == 0) {
                // special case: ctor of non-static inner classes (see IDEA-10889)
                if ("<init>".equals(myMethodName)) {
                    j = (myInstrumenter.myIsNonStaticInnerClass) ?
                            1 + myArgTypes[0].getSize() // skip first (synthetic) "Outer.this" parameter
                            : 1;
                } else {
                    j = 1;
                }
            } else {
                j = 0;
            }

            for (int l = 0; l < parameter.index; l++) {
                j += myArgTypes[l].getSize();
            }

            ...
        }        
    }
}

At least it seems to work for all kinds of weird inner-class constructs

[ Permlink | « Hide ]

Eugene Vigdorchik - 03 Jan 07 23:02

Oops, I missed that InnerClasses attribute is written to both outer and inner class files. That would certainly fix it, thank you, Sascha.