diff --git a/after/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java b/after/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java index 16817f5..60fc6e4 100644 --- a/after/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java +++ b/after/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java @@ -120,16 +120,11 @@ public static Field getField(final Class cls, final String fieldName, final b } } - final List> interfaces = ClassUtils.getAllInterfaces(cls); - if (interfaces == null) { - return null; - } - // check the public interface case. This must be manually searched for // incase there is a public supersuperclass field hidden by a private/package // superclass field. Field match = null; - for (final Class class1 : interfaces) { + for (final Class class1 : ClassUtils.getAllInterfaces(cls)) { try { final Field test = class1.getField(fieldName); Validate.isTrue(match == null, "Reference to field %s is ambiguous relative to %s" diff --git a/after/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java b/after/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java index c289f44..ce62340 100644 --- a/after/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java +++ b/after/src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java @@ -476,7 +476,7 @@ static Object[] getVarArgs(final Object[] args, final Class[] methodParameter final int varArgLength = args.length - methodParameterTypes.length + 1; if (varArgComponentType == null) { - throw new NullPointerException("last method parameter type is not an array"); + throw new IllegalArgumentException("last method parameter type is not an array"); } Object varArgsArray = Array.newInstance(ClassUtils.primitiveToWrapper(varArgComponentType), varArgLength); diff --git a/report.pdf b/report.pdf index 6ba5627..4df4612 100644 Binary files a/report.pdf and b/report.pdf differ diff --git a/report.tex b/report.tex index 3fb30cd..44d0111 100644 --- a/report.tex +++ b/report.tex @@ -56,7 +56,7 @@ \geometry{left=2cm,right=2cm,top=2cm,bottom=3cm} \title{ \vspace{-5ex} -Assignment 1 -- Software Analysis \\\vspace{0.5cm} +Assignment 2 -- Software Analysis \\\vspace{0.5cm} \Large Static Analysis with Infer \vspace{-1ex} } @@ -136,15 +136,12 @@ on \textit{gitlab.com}. The script \textit{docker-infer.sh} can be executed to automatically run the Infer tool using default options through the course tools docker image \textit{bugcounting/satools:y23}. -The script selects Java 17 to compile the project as this is required to not -make the Animal Sniffer Maven build plugin fail with the message ``This feature -requires ASM7''. - The script executes Infer in Maven capture mode executing the \textit{compile} and \textit{test} targets while disabling the Apache RAT software license -checker (which fails for this release). Since unit tests are executed, running -the script before and after the warning guided refactoring ensures the fixes I -introduce do not introduce regressions. +checker (which fails for this release) and the Animal Sniffer Maven plugin do to +the failure message ``This feature requires ASM7'' it produces if ran. Since +unit tests are executed, running the script before and after the warning guided +refactoring ensures the fixes I introduce do not introduce regressions. The analysis outputs are located in \textit{before/infer-out/report.txt}. @@ -161,8 +158,7 @@ result. \toprule \textbf{File} & \textbf{Line} & \textbf{Kind} & \textbf{True Pos.} & \textbf{Reason why flagged expression is a false positive} \\ \midrule -AnnotationUtils.java & 72 & Null & Yes & \multirow{4}{6cm}{--} \\ -reflect/MethodUtils.java & 486 & Null & Yes & \\ +AnnotationUtils.java & 72 & Null & Yes & \\ reflect/FieldUtils.java & 126 & Null & Yes & \\ concurrent/MultiBackgroundInitializer.java & 160 & Thread Safety & Yes & \\ \midrule @@ -170,9 +166,11 @@ builder/ToStringBuilder.java & 223 & Null & No & \multirow{2}{6cm}{Infer flags the value \texttt{null} when used as a nullable method argument} \\ builder/ReflectionToStringBuilder.java & 131 & Null & No & \\ \midrule -time/DurationUtils.java & 142 & Null & No & The method which may return a null -value returns a non-null value if its parameter is non-null, and a -non-null parameter is given \\ +time/DurationUtils.java & 142 & Null & No & \multirow{3}{6.2cm}{The method which + may return a null value returns a non-null value if its parameter is + non-null, and a non-null parameter is given} \\ +reflect/MethodUtils.java & 486 & Null & No & \\ + \\ \hspace{1cm} \\ \midrule CharSetUtils.java & 181 & Null & No & According to \textit{java.lang} documentation, the method always returns a non-null value \\ @@ -197,9 +195,9 @@ documentation for the inner nested method \\ \bottomrule \end{table} In total Infer reports 13 warnings, 12 of which are null dereference warnings -and 1 is a thread safety violation. Of all warnings, 4 are true positives and 9 -are false positives, resulting in a false positive ratio of over 69\%. These -values are summarized in table \ref{tab:num}. +and 1 is a thread safety violation. Of all warnings, 3 are true positives and 10 +are false positives, resulting in a precision of 23\%. +These values are summarized in table \ref{tab:num}. \begin{table}[H] \centering @@ -208,8 +206,8 @@ values are summarized in table \ref{tab:num}. Total number of warnings: & 13 \\ \midrule Null dereference warnings: & 12 \\ Thread safety violations: & 1 \\ \midrule -True positives: & 4 \\ -False positives: & 9 \\ \bottomrule +True positives: & 3 \\ +False positives: & 10 \\ \bottomrule \end{tabular} \caption{Quantitative results of the static analysis performed by Infer.} \label{tab:num} @@ -226,7 +224,8 @@ In other cases, the flagged expression is guarded by an utility method throwing an exception if its value is indeed \texttt{null}. One of such guards is the \mintinline{java}{static Validate.notNull(Object, String, Object...)} method, which checks if its first argument is null and throws a -\texttt{NullPointerException} if so. +\texttt{NullPointerException} with a ``pretty'' message composed from a format +string and variadic arguments if so. Additionally, Infer seems to struggle with boxed primitives and not understanding that their value is always non-null by construction. As an example @@ -280,5 +279,37 @@ causing null dereferences. In this section I now cover the warnings that are true positives and thus are causes for refactoring. +\begin{center} +\color{red} +- field utils violation of contract + +- missing synchronized in multibackground + +- Annotation utils protected method no contract on field fixed by returning +"empty" value + +- no additional warnings appear after fix + +- cool that a contract violation has been found + +- tests pass after fixes so no regression +\end{center} + +\section{Conclusions} + +\begin{center} +\color{red} +- project is mature, few detections but still a couple for null values which + shows how insidious they are. (Null and thread safety are popular and + relevant to the project) + +- infer compared to pmd gives warnings about potential execution paths (data +flow analysis) while pmd is style only and bound to the language + +- sonarqube kind of covers some ground infer does. it combines the powers of +pmd (patterns common to the language to avoid/pursue) with some data flow +analysis (some nullability check). No overlap with Infer results +\end{center} + \end{document}