From 47f40bc8d30e614b881ca524f1f3b44c16cf20d3 Mon Sep 17 00:00:00 2001 From: sebthom Date: Sat, 5 Oct 2024 15:14:58 +0200 Subject: [PATCH] feat: analyze bytecode of methods to determine return type nullability --- eea-generator/pom.xml | 11 +- .../no_npe/eea_generator/EEAFile.java | 15 +- .../no_npe/eea_generator/EEAGenerator.java | 200 +++++---- .../internal/BytecodeAnalyzer.java | 424 ++++++++++++++++++ .../internal/ClassGraphUtils.java | 39 +- .../no_npe/eea_generator/EEAFileTest.java | 9 + .../eea_generator/EEAGeneratorTest.java | 59 +++ .../internal/BytecodeAnalyzerTest.java | 285 ++++++++++++ .../eea_generator/EEAFileTest$TestEntity.eea | 8 +- 9 files changed, 946 insertions(+), 104 deletions(-) create mode 100644 eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzer.java create mode 100644 eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzerTest.java diff --git a/eea-generator/pom.xml b/eea-generator/pom.xml index a75200cf15..315b7b85a9 100644 --- a/eea-generator/pom.xml +++ b/eea-generator/pom.xml @@ -5,7 +5,9 @@ SPDX-FileContributor: Sebastian Thomschke (https://sebthom.de), Vegard IT GmbH ( SPDX-License-Identifier: EPL-2.0 SPDX-ArtifactOfProjectHomePage: https://github.com/vegardit/no-npe --> - + 4.0.0 @@ -53,7 +55,12 @@ SPDX-ArtifactOfProjectHomePage: https://github.com/vegardit/no-npe io.github.classgraph classgraph - 4.8.176 + 4.8.177 + + + org.ow2.asm + asm-util + 9.7 diff --git a/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/EEAFile.java b/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/EEAFile.java index b2a7d08b92..3eb96adebf 100644 --- a/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/EEAFile.java +++ b/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/EEAFile.java @@ -17,7 +17,6 @@ import java.nio.file.StandardOpenOption; import java.util.ArrayDeque; import java.util.ArrayList; -import java.util.Collection; import java.util.Deque; import java.util.HashSet; import java.util.List; @@ -25,6 +24,7 @@ import java.util.Set; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jdt.internal.compiler.classfmt.ExternalAnnotationProvider; @@ -107,19 +107,19 @@ public void applyAnnotationsAndCommentsFrom(final ClassMember applyFrom, final b } // apply name comment - if (overrideOnConflict || !name.hasComment()) { + if (overrideOnConflict && applyFrom.name.hasComment() || !name.hasComment()) { name.comment = applyFrom.name.comment; } // apply original signature comment - if (overrideOnConflict || !originalSignature.hasComment()) { + if (overrideOnConflict && applyFrom.originalSignature.hasComment() || !originalSignature.hasComment()) { originalSignature.comment = applyFrom.originalSignature.comment; } if (overrideOnConflict || !hasNullAnnotations()) { - if (applyFrom.hasNullAnnotations()) { + if (applyFrom.hasNullAnnotations() || applyFrom.annotatedSignature.comment.contains(MARKER_KEEP)) { annotatedSignature = applyFrom.annotatedSignature.clone(); - } else if (overrideOnConflict || !annotatedSignature.hasComment()) { + } else if (overrideOnConflict && applyFrom.annotatedSignature.hasComment() || !annotatedSignature.hasComment()) { annotatedSignature.comment = applyFrom.annotatedSignature.comment; } } @@ -195,6 +195,7 @@ public String toString(final boolean omitComment) { public static final String MARKER_KEEP = "@Keep"; public static final String MARKER_OVERRIDES = "@Overrides"; public static final String MARKER_INHERITED = "@Inherited"; + public static final String MARKER_POLY_NULL = "@PolyNull"; /** * Used to match the 0/1 null annotation of types generic type variables, which is especially tricky @@ -484,8 +485,8 @@ public boolean exists(final Path rootPath) { return Files.exists(rootPath.resolve(relativePath)); } - public Collection getClassMembers() { - return members; + public Stream getClassMembers() { + return members.stream(); } private void renderLine(final List lines, final Object... newLineContent) { diff --git a/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/EEAGenerator.java b/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/EEAGenerator.java index 93cfa03b5d..37a5195be9 100644 --- a/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/EEAGenerator.java +++ b/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/EEAGenerator.java @@ -38,6 +38,10 @@ import com.vegardit.no_npe.eea_generator.EEAFile.ClassMember; import com.vegardit.no_npe.eea_generator.EEAFile.SaveOption; import com.vegardit.no_npe.eea_generator.EEAFile.ValueWithComment; +import com.vegardit.no_npe.eea_generator.internal.BytecodeAnalyzer; +import com.vegardit.no_npe.eea_generator.internal.BytecodeAnalyzer.Nullability; +import com.vegardit.no_npe.eea_generator.internal.ClassGraphUtils; +import com.vegardit.no_npe.eea_generator.internal.ClassGraphUtils.MethodReturnKind; import com.vegardit.no_npe.eea_generator.internal.Props; import io.github.classgraph.ClassGraph; @@ -218,7 +222,7 @@ public static void main(final String... args) throws Exception { } protected static ValueWithComment computeAnnotatedSignature(final EEAFile.ClassMember member, final ClassInfo classInfo, - final ClassMemberInfo memberInfo) { + final ClassMemberInfo memberInfo, final BytecodeAnalyzer bytecodeAnalyzer) { final var templates = new ArrayList(); if (isThrowable(classInfo)) { @@ -238,73 +242,97 @@ protected static ValueWithComment computeAnnotatedSignature(final EEAFile.ClassM if (memberInfo instanceof MethodInfo) { final MethodInfo methodInfo = (MethodInfo) memberInfo; - /* - * mark the return value of builder methods as @NonNull. - */ - if (classInfo.getName().endsWith("Builder") // - && !methodInfo.isStatic() // non-static - && methodInfo.isPublic() // - && methodInfo.getTypeDescriptor().getResultType() instanceof ClassRefTypeSignature // - && (methodInfo.getName().equals("build") && methodInfo.getParameterInfo().length == 0 // - || Objects.equals(((ClassRefTypeSignature) methodInfo.getTypeDescriptor().getResultType()).getClassInfo(), classInfo))) - // (...)Lcom/example/MyBuilder -> (...)L1com/example/MyBuilder; - return new ValueWithComment(insert(member.originalSignature.value, member.originalSignature.value.lastIndexOf(")") + 2, "1"), - ""); - - /* - * mark the parameter of Comparable#compareTo(Object) as @NonNull. - */ - if (classInfo.implementsInterface("java.lang.Comparable") // - && !methodInfo.isStatic() // non-static - && member.originalSignature.value.endsWith(")I") // returns Integer - && methodInfo.isPublic() // - && methodInfo.getParameterInfo().length == 1 // only 1 parameter - && methodInfo.getParameterInfo()[0].getTypeDescriptor() instanceof ClassRefTypeSignature) - // (Lcom/example/Entity;)I -> (L1com/example/Entity;)I - return new ValueWithComment(insert(member.originalSignature.value, 2, "1"), ""); - - /* - * mark the parameter of single-parameter void methods as @NonNull, - * if the class name matches "*Listener" and the parameter type name matches "*Event" - */ - if (classInfo.isInterface() // - && classInfo.getName().endsWith("Listener") // - && !methodInfo.isStatic() // non-static - && member.originalSignature.value.endsWith(")V") // returns void - && methodInfo.getParameterInfo().length == 1 // only 1 parameter - && methodInfo.getParameterInfo()[0].getTypeDescriptor().toString().endsWith("Event")) - // (Ljava/lang/String;)V -> (L1java/lang/String;)V - return new ValueWithComment(insert(member.originalSignature.value, 2, "1"), ""); - - /* - * mark the parameter of single-parameter methods as @NonNull - * with signature matching: void (add|remove)*Listener(*Listener) - */ - if (!methodInfo.isStatic() // non-static - && (methodInfo.getName().startsWith("add") || methodInfo.getName().startsWith("remove")) // - && methodInfo.getName().endsWith("Listener") // - && member.originalSignature.value.endsWith(")V") // returns void - && methodInfo.getParameterInfo().length == 1 // only 1 parameter - && methodInfo.getParameterInfo()[0].getTypeDescriptor().toString().endsWith("Listener")) - return new ValueWithComment( // - member.originalSignature.value.startsWith("(") // - // (Lcom/example/MyListener;)V -> (L1com/example/MyListener;)V - // (TT;)V -> (T1T;)V - ? insert(member.originalSignature.value, 2, "1") // - // (TT;)V --> <1T::Lcom/example/MyListener;>(TT;)V - : insert(member.originalSignature.value, 1, "1"), // - ""); - - if (hasObjectReturnType(member)) { // returns non-void - if (hasNullableAnnotation(methodInfo.getAnnotationInfo())) - // ()Ljava/lang/String -> ()L0java/lang/String; - return new ValueWithComment(insert(member.originalSignature.value, member.originalSignature.value.lastIndexOf(")") + 2, "0"), - ""); + final var returnKind = ClassGraphUtils.getMethodReturnKind(methodInfo); + if (returnKind == MethodReturnKind.ARRAY || returnKind == MethodReturnKind.OBJECT) { - if (hasNonNullAnnotation(methodInfo.getAnnotationInfo())) + final var returnTypeNullability = bytecodeAnalyzer.determineMethodReturnTypeNullability(methodInfo); + /* + * mark the return value of a method as nullable if the byte code analysis of the method body determines it returns null values + * or the method is annotated with a known nullable annotation. + */ + if (returnTypeNullability == Nullability.DEFINITLY_NULL // + || hasNullableAnnotation(methodInfo.getAnnotationInfo())) + // ()Ljava/lang/String -> ()L0java/lang/String; + return new ValueWithComment(insert(member.originalSignature.value, member.originalSignature.value.lastIndexOf(")") + 2, + "0")); + + /* + * mark method as @PolyNull if it only returns null if it was invoked with a null method argument + */ + if (returnTypeNullability == Nullability.POLY_NULL) + // ()Ljava/lang/String -> ()Ljava/lang/String; # @PolyNull + return new ValueWithComment(member.originalSignature.value, "# " + EEAFile.MARKER_POLY_NULL); + + /* + * mark the return value of a method as non-null if the method is annotated with a non-null annotation + * or has a method name starting with "create". + */ + if (returnTypeNullability == Nullability.NEVER_NULL // + || hasNonNullAnnotation(methodInfo.getAnnotationInfo()) // + || methodInfo.getName().startsWith("create")) // ()Ljava/lang/String -> ()L1java/lang/String; - return new ValueWithComment(insert(member.originalSignature.value, member.originalSignature.value.lastIndexOf(")") + 2, "1"), - ""); + // create...(...)LLcom/example/Entity -> create...(...)L1Lcom/example/Entity; + return new ValueWithComment(insert(member.originalSignature.value, member.originalSignature.value.lastIndexOf(")") + 2, + "1")); + + /* + * mark the return value of builder methods as @NonNull. + */ + if (classInfo.getName().endsWith("Builder") // + && !methodInfo.isStatic() // non-static + && methodInfo.isPublic() // + && methodInfo.getTypeDescriptor().getResultType() instanceof ClassRefTypeSignature // + && (methodInfo.getName().equals("build") && methodInfo.getParameterInfo().length == 0 // + || Objects.equals(((ClassRefTypeSignature) methodInfo.getTypeDescriptor().getResultType()).getClassInfo(), + classInfo))) + // (...)Lcom/example/MyBuilder -> (...)L1com/example/MyBuilder; + return new ValueWithComment(insert(member.originalSignature.value, member.originalSignature.value.lastIndexOf(")") + 2, + "1")); + + } else { + + /* + * mark the parameter of Comparable#compareTo(Object) as @NonNull. + */ + if (classInfo.implementsInterface("java.lang.Comparable") // + && !methodInfo.isStatic() // non-static + && member.originalSignature.value.endsWith(")I") // returns Integer + && methodInfo.isPublic() // + && methodInfo.getParameterInfo().length == 1 // only 1 parameter + && methodInfo.getParameterInfo()[0].getTypeDescriptor() instanceof ClassRefTypeSignature) + // (Lcom/example/Entity;)I -> (L1com/example/Entity;)I + return new ValueWithComment(insert(member.originalSignature.value, 2, "1")); + + /* + * mark the parameter of single-parameter void methods as @NonNull, + * if the class name matches "*Listener" and the parameter type name matches "*Event" + */ + if (classInfo.isInterface() // + && classInfo.getName().endsWith("Listener") // + && !methodInfo.isStatic() // non-static + && member.originalSignature.value.endsWith(")V") // returns void + && methodInfo.getParameterInfo().length == 1 // only 1 parameter + && methodInfo.getParameterInfo()[0].getTypeDescriptor().toString().endsWith("Event")) + // (Ljava/lang/String;)V -> (L1java/lang/String;)V + return new ValueWithComment(insert(member.originalSignature.value, 2, "1")); + + /* + * mark the parameter of single-parameter methods as @NonNull + * with signature matching: void (add|remove)*Listener(*Listener) + */ + if (!methodInfo.isStatic() // non-static + && (methodInfo.getName().startsWith("add") || methodInfo.getName().startsWith("remove")) // + && methodInfo.getName().endsWith("Listener") // + && member.originalSignature.value.endsWith(")V") // returns void + && methodInfo.getParameterInfo().length == 1 // only 1 parameter + && methodInfo.getParameterInfo()[0].getTypeDescriptor().toString().endsWith("Listener")) + return new ValueWithComment( // + member.originalSignature.value.startsWith("(") // + // (Lcom/example/MyListener;)V -> (L1com/example/MyListener;)V + // (TT;)V -> (T1T;)V + ? insert(member.originalSignature.value, 2, "1") // + // (TT;)V --> <1T::Lcom/example/MyListener;>(TT;)V + : insert(member.originalSignature.value, 1, "1")); } } @@ -324,14 +352,6 @@ protected static ValueWithComment computeAnnotatedSignature(final EEAFile.ClassM return new ValueWithComment(member.originalSignature.value); } - protected static boolean hasObjectReturnType(final EEAFile.ClassMember member) { - final String sig = member.originalSignature.value; - // object return type: (Ljava/lang/String;)Ljava/lang/String; or (Ljava/lang/String;)TT; - // void return type: (Ljava/lang/String;)V - // primitive return type: (Ljava/lang/String;)B - return sig.charAt(sig.length() - 2) != ')'; - } - protected static EEAFile computeEEAFile(final ClassInfo classInfo) { LOG.log(Level.DEBUG, "Scanning class [{0}]...", classInfo.getName()); @@ -390,6 +410,8 @@ protected static EEAFile computeEEAFile(final ClassInfo classInfo) { } eeaFile.addEmptyLine(); + final var bytecodeAnalyzer = new BytecodeAnalyzer(classInfo); + // static fields for (final FieldInfo f : getStaticFields(fields)) { if (classInfo.isEnum()) { @@ -400,28 +422,28 @@ protected static EEAFile computeEEAFile(final ClassInfo classInfo) { } final var member = eeaFile.addMember(f.getName(), f.getTypeSignatureOrTypeDescriptorStr()); // CHECKSTYLE:IGNORE .* - member.annotatedSignature = computeAnnotatedSignature(member, classInfo, f); + member.annotatedSignature = computeAnnotatedSignature(member, classInfo, f, bytecodeAnalyzer); } eeaFile.addEmptyLine(); // static methods for (final MethodInfo m : getStaticMethods(methods)) { final var member = eeaFile.addMember(m.getName(), m.getTypeSignatureOrTypeDescriptorStr()); - member.annotatedSignature = computeAnnotatedSignature(member, classInfo, m); + member.annotatedSignature = computeAnnotatedSignature(member, classInfo, m, bytecodeAnalyzer); } eeaFile.addEmptyLine(); // instance fields for (final FieldInfo f : getInstanceFields(fields)) { final var member = eeaFile.addMember(f.getName(), f.getTypeSignatureOrTypeDescriptorStr()); // CHECKSTYLE:IGNORE .* - member.annotatedSignature = computeAnnotatedSignature(member, classInfo, f); + member.annotatedSignature = computeAnnotatedSignature(member, classInfo, f, bytecodeAnalyzer); } eeaFile.addEmptyLine(); // instance methods for (final MethodInfo m : getInstanceMethods(methods)) { final var member = eeaFile.addMember(m.getName(), m.getTypeSignatureOrTypeDescriptorStr()); // CHECKSTYLE:IGNORE .* - member.annotatedSignature = computeAnnotatedSignature(member, classInfo, m); + member.annotatedSignature = computeAnnotatedSignature(member, classInfo, m, bytecodeAnalyzer); } return eeaFile; } @@ -568,19 +590,17 @@ public static long generateEEAFiles(final Config cfg) throws IOException { superClasses.add(OBJECT_CLASS_INFO); final var interfaces = classInfo.getInterfaces(); - for (final ClassMember member : eeaFile.getClassMembers()) { + eeaFile.getClassMembers().forEach(member -> { switch (member.getType()) { case CONSTRUCTOR: - continue; // exclude constructors + return; // exclude constructors case FIELD: - if (isStaticField(classInfo, member.name.value)) { - continue; // exclude static fields - } + if (isStaticField(classInfo, member.name.value)) + return; // exclude static fields break; case METHOD: - if (isStaticMethod(classInfo, member.name.value, member.originalSignature.value)) { - continue; // exclude static methods - } + if (isStaticMethod(classInfo, member.name.value, member.originalSignature.value)) + return; // exclude static methods break; } @@ -672,7 +692,7 @@ public static long generateEEAFiles(final Config cfg) throws IOException { } } } - } + }); }); } @@ -810,16 +830,16 @@ public static long validateEEAFiles(final Config config) throws IOException { final var parsedEEAFile = EEAFile.load(path); // ensure the EEA file does not contain declarations of non-existing fields/methods - for (final ClassMember parsedMember : parsedEEAFile.getClassMembers()) { + parsedEEAFile.getClassMembers().forEach(parsedMember -> { if (computedEEAFile.findMatchingClassMember(parsedMember) == null) { - final var candidates = computedEEAFile.getClassMembers().stream() // + final var candidates = computedEEAFile.getClassMembers() // .filter(m -> m.name.equals(parsedMember.name)) // .map(m -> m.name + "\n" + " " + m.originalSignature) // .collect(Collectors.joining("\n")); throw new IllegalStateException("Unknown member declaration found in [" + path + "]:\n" + parsedMember + (candidates .length() > 0 ? "\nPotential candidates: \n" + candidates : "")); } - } + }); }); LOG.log(Level.INFO, "{0} EEA file(s) of package [{1}] validated.", count, packageName); totalValidations += count; diff --git a/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzer.java b/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzer.java new file mode 100644 index 0000000000..157bf6e899 --- /dev/null +++ b/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzer.java @@ -0,0 +1,424 @@ +/* + * SPDX-FileCopyrightText: © Vegard IT GmbH (https://vegardit.com) and contributors. + * SPDX-License-Identifier: EPL-2.0 + */ +package com.vegardit.no_npe.eea_generator.internal; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.ConstantDynamic; +import org.objectweb.asm.Handle; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; +import org.objectweb.asm.util.Textifier; + +import io.github.classgraph.ClassInfo; +import io.github.classgraph.MethodInfo; + +/** + * @author Sebastian Thomschke (https://sebthom.de), Vegard IT GmbH (https://vegardit.com) + */ +public class BytecodeAnalyzer { + + private static final boolean DEBUG = false; + + public enum Nullability { + /** a method never returns null */ + NEVER_NULL, + + /** at least one code branch definitely returns null */ + DEFINITLY_NULL, + + /** a method may or may not return null */ + UNKNOWN, + + /** a method only returns null when an argument is null */ + POLY_NULL + } + + static class Instruction { + final int opcode; + + Instruction(final int opcode) { + this.opcode = opcode; + } + + @Override + public int hashCode() { + return Objects.hash(opcode); + } + + @Override + public boolean equals(final @Nullable Object obj) { + if (this == obj) + return true; + if (obj == null || getClass() != obj.getClass()) + return false; + final Instruction other = (Instruction) obj; + return opcode == other.opcode; + } + + @Override + @SuppressWarnings("null") + public String toString() { + return Textifier.OPCODES[opcode]; + } + } + + static final class VarInstruction extends Instruction { + final int varIndex; + + VarInstruction(final int opcode, final int varIndex) { + super(opcode); + this.varIndex = varIndex; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + Objects.hash(varIndex); + return result; + } + + @Override + public boolean equals(final @Nullable Object obj) { + if (this == obj) + return true; + if (obj == null || getClass() != obj.getClass()) + return false; + final VarInstruction other = (VarInstruction) obj; + return opcode == other.opcode && varIndex == other.varIndex; + } + + @Override + public String toString() { + return Textifier.OPCODES[opcode] + " " + varIndex; + } + } + + private final ClassReader classReader; + + public BytecodeAnalyzer(final ClassInfo classInfo) { + try (var classFileResource = classInfo.getResource()) { + if (classFileResource == null) + throw new IOException("Class resource not found: " + classInfo); + try (var is = classFileResource.open()) { + classReader = new ClassReader(is); + } + } catch (final IOException ex) { + throw new UncheckedIOException("Failed to read class resource: " + classInfo, ex); + } + } + + /** + * Analyzes bytecode to determine the nullability of method return types. + */ + public Nullability determineMethodReturnTypeNullability(final MethodInfo methodInfo) { + switch (ClassGraphUtils.getMethodReturnKind(methodInfo)) { + case PRIMITIVE: + case VOID: + return Nullability.NEVER_NULL; + default: + // continue analysis for object return types + } + + if (methodInfo.isAbstract()) + return Nullability.UNKNOWN; + + final String methodName = methodInfo.getName(); + final String methodDescriptor = methodInfo.getTypeDescriptorStr(); + final int methodArgumentCount = methodInfo.getParameterInfo().length; + + final var returnNullabilities = new ArrayList(); + + if (DEBUG) { + System.out.println("==========================="); + System.out.println(methodName + methodDescriptor); + } + classReader.accept(new ClassVisitor(Opcodes.ASM9) { + @Override + @NonNullByDefault({}) + public MethodVisitor visitMethod(final int access, final String name, final String descriptor, final String signature, + final String[] exceptions) { + if (!name.equals(methodName) || !descriptor.equals(methodDescriptor)) + return super.visitMethod(access, name, descriptor, signature, exceptions); + + return new MethodVisitor(Opcodes.ASM9) { + /** tracks the nullability of operands on the stack */ + private final Deque operandStackNullability = new ArrayDeque<>(); + private final Map localVariableNullability = new HashMap<>(); + private final List instructions = new ArrayList<>(); + + private void debugLogVisit(final String msg) { + if (DEBUG) { + System.out.println(" | " + operandStackNullability); + System.out.println(" > " + msg); + } + } + + private boolean isKnownNonNullMethod(final String clazz, final String methodName, final String descriptor) { + // CHECKSTYLE:IGNORE .* FOR NEXT 6 LINES + return methodName.equals("") // + || methodName.equals("toString") && descriptor.equals("()Ljava/lang/String;") // + || clazz.equals("java/lang/String") && methodName.equals("valueOf") && descriptor.equals( + "(Ljava/lang/Object;)Ljava/lang/String;") // + || clazz.equals("java/lang/StringBuilder") && (methodName.startsWith("append") || methodName.startsWith("insert")) // + || clazz.equals("java/lang/invoke/StringConcatFactory") && methodName.startsWith("makeConcat"); + } + + private boolean isMethodArgument(final int varIndex) { + final int firstLocalVariableIndex = (access & Opcodes.ACC_STATIC) == 0 ? 1 : 0; + final int argIndex = varIndex - firstLocalVariableIndex; + return argIndex >= 0 && argIndex < methodArgumentCount; + } + + @Override + public void visitFieldInsn(final int opcode, final String owner, final String name, final String descriptor) { + debugLogVisit("visitFieldInsn " + Textifier.OPCODES[opcode]); + instructions.add(new Instruction(opcode)); + switch (opcode) { + case Opcodes.GETSTATIC: + case Opcodes.GETFIELD: + // pushing a field value onto the stack; nullability unknown + operandStackNullability.push(Nullability.UNKNOWN); + break; + case Opcodes.PUTSTATIC: + case Opcodes.PUTFIELD: + if (!operandStackNullability.isEmpty()) { + operandStackNullability.pop(); + } + break; + default: + operandStackNullability.push(Nullability.UNKNOWN); + break; + } + } + + @Override + public void visitJumpInsn(final int opcode, final Label label) { + debugLogVisit("visitJumpInsn " + Textifier.OPCODES[opcode] + " " + label); + instructions.add(new Instruction(opcode)); + } + + @Override + public void visitInsn(final int opcode) { + debugLogVisit("visitInsn " + Textifier.OPCODES[opcode]); + instructions.add(new Instruction(opcode)); + switch (opcode) { + case Opcodes.ACONST_NULL: + operandStackNullability.push(Nullability.DEFINITLY_NULL); + break; + case Opcodes.ARETURN: + if (operandStackNullability.isEmpty()) { + // stack underflow; treat as possibly null + returnNullabilities.add(Nullability.UNKNOWN); + } else { + + /* handle: + * if (arg1 == null) + * return null; + * + * which translates to: + * [ALOAD 0, IFNONNULL, ACONST_NULL, ARETURN] + * + * or: + * if (arg1 == null) + * return arg1; + * + * which translates to: + * [ALOAD 0, IFNONNULL, ALOAD 0, ARETURN] + */ + final var count = instructions.size(); + if (count > 3 // + && instructions.get(count - 3).opcode == Opcodes.IFNONNULL // + && instructions.get(count - 4).opcode == Opcodes.ALOAD // + && isMethodArgument(((VarInstruction) instructions.get(count - 4)).varIndex) // + && (instructions.get(count - 2).opcode == Opcodes.ACONST_NULL // + || instructions.get(count - 2).equals(instructions.get(count - 4)))) { + returnNullabilities.add(Nullability.POLY_NULL); + } else { + final Nullability returnValue = operandStackNullability.pop(); + returnNullabilities.add(returnValue); + } + } + // clear the operand stack after a return + operandStackNullability.clear(); + instructions.clear(); + break; + case Opcodes.DUP: + if (operandStackNullability.isEmpty()) { + // stack underflow; treat as possibly null + operandStackNullability.push(Nullability.UNKNOWN); + } else { + final Nullability top = operandStackNullability.peek(); + operandStackNullability.push(top); + } + break; + case Opcodes.POP: + if (!operandStackNullability.isEmpty()) { + operandStackNullability.pop(); + } + break; + default: + // for other instructions, assume they might alter the stack + // for simplicity, we reset the stack to UNKNOWN + operandStackNullability.clear(); + operandStackNullability.push(Nullability.UNKNOWN); + break; + } + } + + @Override + public void visitLdcInsn(final Object constant) { + debugLogVisit("visitLdcInsn " + constant + " " + constant.getClass()); + instructions.add(new Instruction(Opcodes.LDC)); + if (constant instanceof Integer || constant instanceof Float // + || constant instanceof Long || constant instanceof Double // + || constant instanceof String // + || constant instanceof Type || constant instanceof Handle) { + // primitive constants, string constants, class literals, method handles are non-null + operandStackNullability.push(Nullability.NEVER_NULL); + } else if (constant instanceof ConstantDynamic) { + // ConstantDynamic may resolve to null, so treat it as possibly null + operandStackNullability.push(Nullability.UNKNOWN); + } else { + // handle other unexpected types conservatively as possibly null + operandStackNullability.push(Nullability.UNKNOWN); + } + } + + @Override + public void visitIntInsn(final int opcode, final int operand) { + debugLogVisit("visitIntInsn " + Textifier.OPCODES[opcode] + " " + operand); + instructions.add(new Instruction(opcode)); + if (opcode == Opcodes.NEWARRAY) { + operandStackNullability.push(Nullability.NEVER_NULL); + } + } + + @Override + public void visitInvokeDynamicInsn(final String name, final String descriptor, final Handle bootstrapMethodHandle, + final Object... bootstrapMethodArgs) { + visitMethodInsn(Opcodes.INVOKEDYNAMIC, bootstrapMethodHandle.getOwner(), name, descriptor, bootstrapMethodHandle + .isInterface()); + } + + @Override + public void visitMethodInsn(final int opcode, final String owner, final String name, final String descriptor, + final boolean isInterface) { + debugLogVisit("visitMethodInsn " + Textifier.OPCODES[opcode] + " " + owner + "." + name + descriptor); + instructions.add(new Instruction(opcode)); + + // pop arguments off the stack + final Type methodType = Type.getMethodType(descriptor); + for (int i = 0, argCount = methodType.getArgumentTypes().length; i < argCount; i++) { + if (!operandStackNullability.isEmpty()) { + operandStackNullability.pop(); + } + } + + if (opcode != Opcodes.INVOKESTATIC) { + // pop 'this' reference + if (!operandStackNullability.isEmpty()) { + operandStackNullability.pop(); + } + } + + // push the return value onto the stack + switch (methodType.getReturnType().getSort()) { + case Type.VOID: + break; + case Type.OBJECT: + case Type.ARRAY: + if (isKnownNonNullMethod(owner, name, descriptor)) { + operandStackNullability.push(Nullability.NEVER_NULL); + } else { + // reference type; nullability unknown + operandStackNullability.push(Nullability.UNKNOWN); + } + break; + default: + // primitive type; definitely non-null + operandStackNullability.push(Nullability.NEVER_NULL); + } + } + + @Override + public void visitTypeInsn(final int opcode, final String type) { + debugLogVisit("visitTypeInsn " + Textifier.OPCODES[opcode] + " " + type); + instructions.add(new Instruction(opcode)); + + switch (opcode) { + case Opcodes.NEW: + case Opcodes.ANEWARRAY: + operandStackNullability.push(Nullability.NEVER_NULL); + break; + case Opcodes.CHECKCAST: + case Opcodes.INSTANCEOF: + break; + default: + operandStackNullability.push(Nullability.UNKNOWN); + } + } + + @Override + public void visitVarInsn(final int opcode, final int varIndex) { + debugLogVisit("visitVarInsn " + Textifier.OPCODES[opcode] + " " + varIndex); + instructions.add(new VarInstruction(opcode, varIndex)); + + switch (opcode) { + case Opcodes.ALOAD: + // loading a reference from a local variable + final Nullability varNullability = localVariableNullability.getOrDefault(varIndex, Nullability.UNKNOWN); + operandStackNullability.push(varNullability); + break; + case Opcodes.ASTORE: + // storing a value into a local variable + if (!operandStackNullability.isEmpty()) { // CHECKSTYLE:IGNORE .* + final Nullability valueNullability = operandStackNullability.pop(); + localVariableNullability.put(varIndex, valueNullability); + } else { + // stack underflow; assume possibly null + localVariableNullability.put(varIndex, Nullability.UNKNOWN); + } + break; + default: + operandStackNullability.push(Nullability.UNKNOWN); + break; + } + } + }; + } + }, 0); + + if (returnNullabilities.isEmpty()) + // no return statements returning objects found; nullability unknown + return Nullability.NEVER_NULL; + + if (returnNullabilities.contains(Nullability.DEFINITLY_NULL)) + return Nullability.DEFINITLY_NULL; + + if (returnNullabilities.contains(Nullability.UNKNOWN)) + return Nullability.UNKNOWN; + + if (returnNullabilities.contains(Nullability.POLY_NULL)) + return Nullability.POLY_NULL; + + return Nullability.NEVER_NULL; + } +} diff --git a/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/internal/ClassGraphUtils.java b/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/internal/ClassGraphUtils.java index 54855d928e..1573854748 100644 --- a/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/internal/ClassGraphUtils.java +++ b/eea-generator/src/main/java/com/vegardit/no_npe/eea_generator/internal/ClassGraphUtils.java @@ -10,17 +10,28 @@ import java.util.TreeSet; import io.github.classgraph.AnnotationInfoList; +import io.github.classgraph.ArrayTypeSignature; +import io.github.classgraph.BaseTypeSignature; import io.github.classgraph.ClassInfo; +import io.github.classgraph.ClassRefTypeSignature; import io.github.classgraph.FieldInfo; import io.github.classgraph.FieldInfoList; import io.github.classgraph.MethodInfo; import io.github.classgraph.MethodInfoList; +import io.github.classgraph.TypeSignature; /** * @author Sebastian Thomschke (https://sebthom.de), Vegard IT GmbH (https://vegardit.com) */ public final class ClassGraphUtils { + public enum MethodReturnKind { + ARRAY, + PRIMITIVE, + OBJECT, + VOID + } + private static final Set NULLABLE_ANNOTATIONS = Set.of( // "android.annotation.Nullable", // "android.support.annotation.Nullable", // @@ -160,6 +171,26 @@ public static SortedSet getInstanceMethods(final MethodInfoList meth return getFilteredAndSortedMethods(methods, false); } + /** + * Determines the return kind of the given method. + * This method distinguishes between methods that return objects, arrays, primitive types, or void. + * + * @param methodInfo the method whose return type is to be checked + * @return MethodReturnKind representing if the method returns an object, array, primitive, or void + */ + public static MethodReturnKind getMethodReturnKind(final MethodInfo methodInfo) { + final TypeSignature returnType = methodInfo.getTypeDescriptor().getResultType(); + if (returnType == null) + return MethodReturnKind.VOID; + if (returnType instanceof BaseTypeSignature) + return MethodReturnKind.PRIMITIVE; + if (returnType instanceof ArrayTypeSignature) + return MethodReturnKind.ARRAY; + if (returnType instanceof ClassRefTypeSignature) + return MethodReturnKind.OBJECT; + throw new IllegalStateException("Unknown method return kind: " + returnType); + } + public static SortedSet getStaticFields(final FieldInfoList fields) { return getFilteredAndSortedFields(fields, true); } @@ -176,14 +207,14 @@ public static boolean hasNullableAnnotation(final AnnotationInfoList annos) { return annos.stream().anyMatch(a -> NULLABLE_ANNOTATIONS.contains(a.getName())); } - public static boolean hasSuperclass(final ClassInfo classInfo, final String superClassName) { - return !classInfo.getSuperclasses().filter(c -> c.getName().equals(superClassName)).isEmpty(); - } - public static boolean hasPackageVisibility(final ClassInfo classInfo) { return !classInfo.isPublic() && !classInfo.isPrivate() && !classInfo.isProtected(); } + public static boolean hasSuperclass(final ClassInfo classInfo, final String superClassName) { + return !classInfo.getSuperclasses().filter(c -> c.getName().equals(superClassName)).isEmpty(); + } + public static boolean isStaticField(final ClassInfo classInfo, final String fieldName) { final var fieldInfo = classInfo.getDeclaredFieldInfo(fieldName); if (fieldInfo == null) diff --git a/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/EEAFileTest.java b/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/EEAFileTest.java index ee483cd383..2555a3d07f 100644 --- a/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/EEAFileTest.java +++ b/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/EEAFileTest.java @@ -13,6 +13,7 @@ import java.nio.file.Path; import java.util.Set; +import org.eclipse.jdt.annotation.Nullable; import org.junit.jupiter.api.Test; import com.vegardit.no_npe.eea_generator.EEAFile.SaveOption; @@ -31,6 +32,14 @@ public static final class TestEntity { public TestEntity(final String name) { // CHECKSTYLE:IGNORE RedundantModifier this.name = name; } + + public @Nullable String keepTest1() { + return null; + } + + public String keepTest2() { + return name; + } } public static final class TestEntity2 { diff --git a/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/EEAGeneratorTest.java b/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/EEAGeneratorTest.java index 5124c87d40..ee6d3541a3 100644 --- a/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/EEAGeneratorTest.java +++ b/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/EEAGeneratorTest.java @@ -8,9 +8,13 @@ import java.io.IOException; import java.nio.file.Path; +import java.util.List; +import java.util.Set; import org.junit.jupiter.api.Test; +import com.vegardit.no_npe.eea_generator.EEAFile.SaveOption; + /** * @author Sebastian Thomschke (https://sebthom.de), Vegard IT GmbH (https://vegardit.com) */ @@ -41,6 +45,61 @@ void testVadilateInvalidEEAFiles() { + "] no found on classpath."); } + @Test + void testAnnotationMerge() throws IOException { + final var eeaFiles = EEAGenerator.computeEEAFiles("com.vegardit.no_npe.eea_generator", classInfo -> classInfo.getName().equals( + "com.vegardit.no_npe.eea_generator.EEAFileTest$TestEntity")); + assertThat(eeaFiles).hasSize(1); + final var computedEeaFile = eeaFiles.values().iterator().next(); + final var existingEeaFile = EEAFile.load(Path.of( + "src/test/resources/valid/com/vegardit/no_npe/eea_generator/EEAFileTest$TestEntity.eea")); + + assertThat(computedEeaFile.renderFileContent(Set.of(SaveOption.OMIT_MEMBERS_WITHOUT_ANNOTATED_SIGNATURE))).isEqualTo(List.of( + "class com/vegardit/no_npe/eea_generator/EEAFileTest$TestEntity", // + "", // + "STATIC_STRING", // + " Ljava/lang/String;", // + " L1java/lang/String;", // + "", // + "keepTest1", // + " ()Ljava/lang/String;", // + " ()L0java/lang/String;" // + )); + + final var keepTestMethodComputed = computedEeaFile.getClassMembers().filter(m -> m.name.value.equals("keepTest1")).findFirst().get(); + final var keepTestMethodExisting = existingEeaFile.getClassMembers().filter(m -> m.name.value.equals("keepTest1")).findFirst().get(); + assertThat(keepTestMethodComputed.annotatedSignature.value).contains("L0java/lang/String"); + assertThat(keepTestMethodExisting.annotatedSignature.value).contains("Ljava/lang/String"); + assertThat(keepTestMethodExisting.annotatedSignature.comment).contains("@Keep"); + assertThat(keepTestMethodComputed.annotatedSignature.value).isNotEqualTo(keepTestMethodExisting.annotatedSignature.value); + assertThat(keepTestMethodComputed.annotatedSignature.comment).isNotEqualTo(keepTestMethodExisting.annotatedSignature.comment); + + computedEeaFile.applyAnnotationsAndCommentsFrom(existingEeaFile, true, false); + final var keepTestMethodComputedUpdated = computedEeaFile.getClassMembers().filter(m -> m.name.value.equals("keepTest1")).findFirst() + .get(); + assertThat(keepTestMethodComputedUpdated.annotatedSignature.value).contains("Ljava/lang/String"); + assertThat(keepTestMethodComputedUpdated.annotatedSignature.comment).contains("@Keep"); + + assertThat(computedEeaFile.renderFileContent(Set.of(SaveOption.OMIT_MEMBERS_WITHOUT_ANNOTATED_SIGNATURE))).isEqualTo(List.of( + "class com/vegardit/no_npe/eea_generator/EEAFileTest$TestEntity # a class comment", // + "", // + "STATIC_STRING # a field comment", // + " Ljava/lang/String; # an original signature comment", // + " L1java/lang/String; # an annotated signature comment", // + "", // + "name # a field comment", // + " Ljava/lang/String;", // + " L1java/lang/String;", // + "", // + "keepTest1 # a metod comment", // + " ()Ljava/lang/String;", // + " ()Ljava/lang/String; # @Keep to test preventing generator from changing it to L0", // + "keepTest2", // + " ()Ljava/lang/String;", // + " ()Ljava/lang/String; # @Keep to test preventing removal on minimization" // + )); + } + @Test void testPackageMissingOnClasspath() { final var rootPath = Path.of("src/test/resources/invalid"); diff --git a/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzerTest.java b/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzerTest.java new file mode 100644 index 0000000000..56e676eabc --- /dev/null +++ b/eea-generator/src/test/java/com/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzerTest.java @@ -0,0 +1,285 @@ +/* + * SPDX-FileCopyrightText: © Vegard IT GmbH (https://vegardit.com) and contributors. + * SPDX-License-Identifier: EPL-2.0 + */ +package com.vegardit.no_npe.eea_generator.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.lang.reflect.Method; +import java.util.Comparator; +import java.util.function.Supplier; +import java.util.stream.Stream; + +import org.eclipse.jdt.annotation.Nullable; +import org.junit.jupiter.api.Test; + +import com.vegardit.no_npe.eea_generator.internal.BytecodeAnalyzer.Nullability; + +import io.github.classgraph.ClassGraph; +import io.github.classgraph.ScanResult; + +/** + * @author Sebastian Thomschke (https://sebthom.de), Vegard IT GmbH (https://vegardit.com) + */ +class BytecodeAnalyzerTest { + + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.METHOD) + public @interface ReturnValueNullability { + Nullability value(); + } + + static final String STATIC_NONNULL_STRING = "HI"; + + static @Nullable Object staticNullableObject; + + @ReturnValueNullability(Nullability.NEVER_NULL) + static Object neverReturningNull1() { + /*L0 + LDC "Hey" + ARETURN */ + return "Hey"; + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static Object neverReturningNull10() { + /*L0 + ICONST_0 + NEWARRAY T_INT + ARETURN */ + return new int[0]; + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static Object neverReturningNull11() { + /*L0 + ICONST_0 + ANEWARRAY java/lang/Object + ARETURN */ + return new Object[0]; + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static @Nullable String neverReturningNull12() { + final Object str = "Hey"; + return (String) str; + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static Object neverReturningNull2() { + /*L0 + NEW java/lang/Object + DUP + INVOKESPECIAL java/lang/Object.()V + ARETURN */ + return new Object(); + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static Object neverReturningNull3() { + /*L0 + NEW java/lang/String + DUP + LDC "Test" + INVOKESPECIAL java/lang/String.(Ljava/lang/String;)V + ARETURN */ + return new String("Test"); + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static Object neverReturningNull4() { + /*L0 + NEW java/lang/Object + DUP + INVOKESPECIAL java/lang/Object.()V + INVOKESTATIC java/lang/String.valueOf(Ljava/lang/Object;)Ljava/lang/String; + INVOKEDYNAMIC makeConcatWithConstants(Ljava/lang/String;)Ljava/lang/String; [ + // handle kind 0x6 : INVOKESTATIC + java/lang/invoke/StringConcatFactory.makeConcatWithConstants( + Ljava/lang/invoke/MethodHandles$Lookup; + Ljava/lang/String;Ljava/lang/invoke/MethodType; + Ljava/lang/String;[Ljava/lang/Object; + )Ljava/lang/invoke/CallSite; + arguments: + "\u0001 test" + ] + ARETURN */ + return new Object() + " test"; + } + + // test method to ensure that `return null` in lambdas are not mistaken as null returns + @ReturnValueNullability(Nullability.NEVER_NULL) + static Object neverReturningNull6() { + ((Supplier) () -> null).get(); + return "Hey"; + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static void neverReturningNull7() { + // nothing to do + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static boolean neverReturningNull8() { + return true; + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + static @Nullable Object neverReturningNull9() { + return STATIC_NONNULL_STRING; + } + + @ReturnValueNullability(Nullability.UNKNOWN) + static @Nullable Object returningMaybeNull1() { + return new @Nullable Object[] {null}[0]; + } + + @ReturnValueNullability(Nullability.UNKNOWN) + static @Nullable Object returningMaybeNull2() { + final var env = System.getProperty("Abcdefg1234567"); + @SuppressWarnings("unused") + final var unused = new Object(); + return env; + } + + @ReturnValueNullability(Nullability.UNKNOWN) + static @Nullable String returningMaybeNull3() { + final @Nullable Object env = System.getProperty("Abcdefg1234567"); + return (String) env; + } + + static @Nullable Object returningMaybeNull4() { + return staticNullableObject; + } + + @ReturnValueNullability(Nullability.DEFINITLY_NULL) + static @Nullable Object returningNull1() { + return null; + } + + @ReturnValueNullability(Nullability.DEFINITLY_NULL) + static @Nullable Object returningNull2() { + if (System.currentTimeMillis() == 123) + return "Hey"; + return null; + } + + @ReturnValueNullability(Nullability.DEFINITLY_NULL) + static @Nullable Object returningNull3() { + return System.currentTimeMillis() == 123 ? "Hey" : null; + } + + @ReturnValueNullability(Nullability.DEFINITLY_NULL) + static @Nullable Object returningNull4(final boolean condition) { + return condition ? "Hey" : null; + } + + @ReturnValueNullability(Nullability.DEFINITLY_NULL) + static @Nullable Object returningNull5() { + /*L0 + ACONST_NULL + ASTORE 0 + L1 + ALOAD 0 + ARETURN + L2 + LOCALVARIABLE foo Ljava/lang/String; L1 L2 0 */ + final String foo = null; + return foo; + } + + @ReturnValueNullability(Nullability.DEFINITLY_NULL) + static @Nullable String returningNull6() { + /*L0 + ACONST_NULL + ASTORE 0 + L1 + ALOAD 0 + CHECKCAST java/lang/String + ARETURN + L2 + LOCALVARIABLE str Ljava/lang/Object; L1 L2 0 */ + final Object str = null; + return (String) str; + } + + @ReturnValueNullability(Nullability.POLY_NULL) + static @Nullable Object returningNullIfArgIsNull1(final @Nullable String arg1) { + if (arg1 == null) + return null; + return "Hey"; + } + + @ReturnValueNullability(Nullability.POLY_NULL) + static @Nullable Object returningNullIfArgIsNull2(final @Nullable String arg1, final @Nullable String arg2) { + if (arg1 == null || arg2 == null) + return null; + return "Hey"; + } + + @ReturnValueNullability(Nullability.POLY_NULL) + static @Nullable Object returningNullIfArgIsNull3(final @Nullable String arg1, final @Nullable String arg2) { + if (arg1 == null && arg2 == null) + return null; + return "Hey"; + } + + @ReturnValueNullability(Nullability.POLY_NULL) + static @Nullable Object returningNullIfArgIsNull4(final @Nullable String arg1) { + if (arg1 == null) + return arg1; + return "Hey"; + } + + @ReturnValueNullability(Nullability.NEVER_NULL) + public Object neverReturningNull5(final boolean condition) { + /*L0 + ILOAD 1 + IFEQ L1 + L2 + NEW java/lang/Object + DUP + INVOKESPECIAL java/lang/Object.()V + ARETURN + L1 + FRAME SAME + LDC "Constant String" + ARETURN + L3 + LOCALVARIABLE this Lcom/vegardit/no_npe/eea_generator/internal/BytecodeAnalyzerTest; L0 L3 0 + LOCALVARIABLE condition Z L0 L3 1 */ + if (condition) + return new Object(); + return "Constant String"; + } + + @Test + @SuppressWarnings("null") + void testDetermineMethodReturnTypeNullability() { + final var className = BytecodeAnalyzerTest.class.getName(); + try (ScanResult scanResult = new ClassGraph() // + .enableAllInfo() // + .enableSystemJarsAndModules() // + .acceptClasses(className) // + .scan()) { + + final var classInfo = scanResult.getClassInfo(className); + assert classInfo != null; + + final var analyzer = new BytecodeAnalyzer(classInfo); + + Stream.of(getClass().getDeclaredMethods()).sorted(Comparator.comparing(Method::getName)).forEach(m -> { + final var anno = m.getAnnotation(ReturnValueNullability.class); + if (anno != null) { + assertThat(analyzer.determineMethodReturnTypeNullability(classInfo.getMethodInfo(m.getName()).get(0))).describedAs(m + .getName()).isEqualTo(anno.value()); + } + }); + } + } +} diff --git a/eea-generator/src/test/resources/valid/com/vegardit/no_npe/eea_generator/EEAFileTest$TestEntity.eea b/eea-generator/src/test/resources/valid/com/vegardit/no_npe/eea_generator/EEAFileTest$TestEntity.eea index bdc3b1ef90..1200646a69 100644 --- a/eea-generator/src/test/resources/valid/com/vegardit/no_npe/eea_generator/EEAFileTest$TestEntity.eea +++ b/eea-generator/src/test/resources/valid/com/vegardit/no_npe/eea_generator/EEAFileTest$TestEntity.eea @@ -4,10 +4,16 @@ STATIC_STRING # a field comment Ljava/lang/String; # an original signature comment L1java/lang/String; # an annotated signature comment -name # a method comment +name # a field comment Ljava/lang/String; L1java/lang/String; (Ljava/lang/String;)V (Ljava/lang/String;)V +keepTest1 # a metod comment + ()Ljava/lang/String; + ()Ljava/lang/String; # @Keep to test preventing generator from changing it to L0 +keepTest2 + ()Ljava/lang/String; + ()Ljava/lang/String; # @Keep to test preventing removal on minimization