Skip to content

Commit

Permalink
Merge branch 'findbugs' into spotbugs
Browse files Browse the repository at this point in the history
 Conflicts:
	src/main/java/com/mebigfatguy/fbcontrib/detect/UseVarArgs.java
  • Loading branch information
mebigfatguy committed Sep 28, 2024
2 parents a804ca3 + aeb64ad commit 75f2bb1
Show file tree
Hide file tree
Showing 2 changed files with 185 additions and 174 deletions.
8 changes: 6 additions & 2 deletions etc/messages.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1173,9 +1173,13 @@ a[0] = new A(); // results in ArrayStoreException (Runtime)
<Detector class="com.mebigfatguy.fbcontrib.detect.UseVarArgs">
<Details>
<![CDATA[
<p>This detector looks for definitions of methods that have an array as the last parameter.
<p>This detector looks for problems and possibilities to use var args better. Specially looks for
<ul>
<li>definitions of methods that have an array as the last parameter.
Since this class is compiled with Java 1.5 or better, it would be more flexible for clients of this
method to define this parameter as a vararg parameter.</p>
method to define this parameter as a vararg parameter</li>
<li>classes that have samed named vararg and non vararg methods not clearly differentiated.</li>
<li>Explicitly passing null to a var arg parameter</li></ul>.</p>
<p>It is a fast detector.</p>
]]>
</Details>
Expand Down
351 changes: 179 additions & 172 deletions src/main/java/com/mebigfatguy/fbcontrib/detect/UseVarArgs.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,176 +43,183 @@
*/
public class UseVarArgs extends PreorderVisitor implements Detector {

public static final String SIG_STRING_ARRAY_TO_VOID = new SignatureBuilder()
.withParamTypes(SignatureBuilder.SIG_STRING_ARRAY).toString();

private final BugReporter bugReporter;
private JavaClass javaClass;

public UseVarArgs(BugReporter bugReporter) {
this.bugReporter = bugReporter;
}

/**
* overrides the visitor to make sure that the class was compiled by java 1.5 or
* later.
*
* @param classContext the context object of the currently parsed class
*/
@Override
public void visitClassContext(ClassContext classContext) {
try {
javaClass = classContext.getJavaClass();
if (javaClass.getMajor() >= Const.MAJOR_1_5) {
javaClass.accept(this);
}
} finally {
javaClass = null;
}
}

/**
* overrides the visitor to look for methods that has an array as a last
* parameter of an array type, where the base type is not like the previous
* parameter nor something like a char or byte array.
*
* @param obj the currently parse method
*/
@Override
public void visitMethod(Method obj) {
try {
if (obj.isSynthetic()) {
return;
}

if (Values.CONSTRUCTOR.equals(getMethodName()) && javaClass.getClassName().contains("$")) {
return;
}

List<String> types = SignatureUtils.getParameterSignatures(obj.getSignature());
if ((types.isEmpty()) || (types.size() > 2)) {
return;
}

if ((obj.getAccessFlags() & Const.ACC_VARARGS) != 0) {
return;
}

String lastParmSig = types.get(types.size() - 1);
if (!lastParmSig.startsWith(Values.SIG_ARRAY_PREFIX)
|| lastParmSig.startsWith(Values.SIG_ARRAY_OF_ARRAYS_PREFIX)) {
return;
}

if (SignatureBuilder.SIG_BYTE_ARRAY.equals(lastParmSig)
|| SignatureBuilder.SIG_CHAR_ARRAY.equals(lastParmSig)) {
return;
}

if (hasSimilarParms(types)) {
return;
}

if (obj.isStatic() && "main".equals(obj.getName()) && SIG_STRING_ARRAY_TO_VOID.equals(obj.getSignature())) {
return;
}

if (!obj.isPrivate() && !obj.isStatic() && isInherited(obj)) {
return;
}

super.visitMethod(obj);
bugReporter.reportBug(new BugInstance(this, BugType.UVA_USE_VAR_ARGS.name(), LOW_PRIORITY).addClass(this)
.addMethod(this));

} catch (ClassNotFoundException cnfe) {
bugReporter.reportMissingClass(cnfe);
}
}

/**
* overrides the visitor, but not used
*/
@Override
public void report() {
// needed by Detector interface but not used
}

/**
* determines whether a bunch of types are similar and thus would be confusing
* to have one be a varargs.
*
* @param argTypes the parameter signatures to check
* @return whether the parameter are similar
*/
@edu.umd.cs.findbugs.annotations.SuppressFBWarnings(value = "LII_LIST_INDEXED_ITERATING", justification = "this doesn't iterate over every element, so we can't use a for-each loop")
private static boolean hasSimilarParms(List<String> argTypes) {

for (int i = 0; i < (argTypes.size() - 1); i++) {
if (argTypes.get(i).startsWith(Values.SIG_ARRAY_PREFIX)) {
return true;
}
}

String baseType = argTypes.get(argTypes.size() - 1);
while (baseType.startsWith(Values.SIG_ARRAY_PREFIX)) {
baseType = baseType.substring(1);
}

for (int i = 0; i < (argTypes.size() - 1); i++) {
if (argTypes.get(i).equals(baseType)) {
return true;
}
}

return false;
}

/**
* looks to see if this method is derived from a super class. If it is we don't
* want to report on it, as that would entail changing a whole hierarchy
*
* @param m the current method
* @return if the method is inherited
*
* @throws ClassNotFoundException if the super class(s) aren't found
*/
private boolean isInherited(Method m) throws ClassNotFoundException {
JavaClass[] infs = javaClass.getAllInterfaces();
for (JavaClass inf : infs) {
if (hasMethod(inf, m)) {
return true;
}
}

JavaClass[] sups = javaClass.getSuperClasses();
for (JavaClass sup : sups) {
if (hasMethod(sup, m)) {
return true;
}
}

return false;
}

/**
* looks to see if a class has a method with a specific name and signature
*
* @param c the class to check
* @param candidateMethod the method to look for
*
* @return whether this class has the exact method
*/
private static boolean hasMethod(JavaClass c, Method candidateMethod) {
String name = candidateMethod.getName();
String sig = candidateMethod.getSignature();

for (Method method : c.getMethods()) {
if (!method.isStatic() && method.getName().equals(name) && method.getSignature().equals(sig)) {
return true;
}
}

return false;
}
public static final String SIG_STRING_ARRAY_TO_VOID = new SignatureBuilder()
.withParamTypes(SignatureBuilder.SIG_STRING_ARRAY).toString();

private final BugReporter bugReporter;
private JavaClass javaClass;

public UseVarArgs(BugReporter bugReporter) {
this.bugReporter = bugReporter;
}

/**
* overrides the visitor to make sure that the class was compiled by java 1.5 or
* later.
*
* @param classContext the context object of the currently parsed class
*/
@Override
public void visitClassContext(ClassContext classContext) {
try {
javaClass = classContext.getJavaClass();
if (javaClass.getMajor() >= Const.MAJOR_1_5) {
javaClass.accept(this);
}
} finally {
javaClass = null;
}
}

/**
* overrides the visitor to look for methods that has an array as a last
* parameter of an array type, where the base type is not like the previous
* parameter nor something like a char or byte array.
*
* @param obj the currently parse method
*/
@Override
public void visitMethod(Method obj) {
try {
if (obj.isSynthetic()) {
return;
}

boolean isVarMethod = (obj.getAccessFlags() & Const.ACC_VARARGS) != 0;

boolean isConvertable = !isVarMethod && methodHasConvertableLastParam(obj);

super.visitMethod(obj);

if (isConvertable) {
bugReporter.reportBug(new BugInstance(this, BugType.UVA_USE_VAR_ARGS.name(), LOW_PRIORITY)
.addClass(this).addMethod(this));
}

} catch (ClassNotFoundException cnfe) {
bugReporter.reportMissingClass(cnfe);
}
}

public boolean methodHasConvertableLastParam(Method method) throws ClassNotFoundException {
if (Values.CONSTRUCTOR.equals(getMethodName()) && javaClass.getClassName().contains("$")) {
return false;
}
List<String> types = SignatureUtils.getParameterSignatures(method.getSignature());
if ((types.isEmpty()) || (types.size() > 2)) {
return false;
}

String lastParmSig = types.get(types.size() - 1);
if (!lastParmSig.startsWith(Values.SIG_ARRAY_PREFIX)
|| lastParmSig.startsWith(Values.SIG_ARRAY_OF_ARRAYS_PREFIX)) {
return false;
}

if (SignatureBuilder.SIG_BYTE_ARRAY.equals(lastParmSig)
|| SignatureBuilder.SIG_CHAR_ARRAY.equals(lastParmSig)) {
return false;
}

if (hasSimilarParms(types)) {
return false;
}

if (method.isStatic() && "main".equals(method.getName())
&& SIG_STRING_ARRAY_TO_VOID.equals(method.getSignature())) {
return false;
}

if (!method.isPrivate() && !method.isStatic() && isInherited(method)) {
return false;
}

return true;
}

/**
* overrides the visitor, but not used
*/
@Override
public void report() {
// needed by Detector interface but not used
}

/**
* determines whether a bunch of types are similar and thus would be confusing
* to have one be a varargs.
*
* @param argTypes the parameter signatures to check
* @return whether the parameter are similar
*/
@edu.umd.cs.findbugs.annotations.SuppressFBWarnings(value = "LII_LIST_INDEXED_ITERATING", justification = "this doesn't iterate over every element, so we can't use a for-each loop")
private static boolean hasSimilarParms(List<String> argTypes) {

for (int i = 0; i < (argTypes.size() - 1); i++) {
if (argTypes.get(i).startsWith(Values.SIG_ARRAY_PREFIX)) {
return true;
}
}

String baseType = argTypes.get(argTypes.size() - 1);
while (baseType.startsWith(Values.SIG_ARRAY_PREFIX)) {
baseType = baseType.substring(1);
}

for (int i = 0; i < (argTypes.size() - 1); i++) {
if (argTypes.get(i).equals(baseType)) {
return true;
}
}

return false;
}

/**
* looks to see if this method is derived from a super class. If it is we don't
* want to report on it, as that would entail changing a whole hierarchy
*
* @param m the current method
* @return if the method is inherited
*
* @throws ClassNotFoundException if the super class(s) aren't found
*/
private boolean isInherited(Method m) throws ClassNotFoundException {
JavaClass[] infs = javaClass.getAllInterfaces();
for (JavaClass inf : infs) {
if (hasMethod(inf, m)) {
return true;
}
}

JavaClass[] sups = javaClass.getSuperClasses();
for (JavaClass sup : sups) {
if (hasMethod(sup, m)) {
return true;
}
}

return false;
}

/**
* looks to see if a class has a method with a specific name and signature
*
* @param c the class to check
* @param candidateMethod the method to look for
*
* @return whether this class has the exact method
*/
private static boolean hasMethod(JavaClass c, Method candidateMethod) {
String name = candidateMethod.getName();
String sig = candidateMethod.getSignature();

for (Method method : c.getMethods()) {
if (!method.isStatic() && method.getName().equals(name) && method.getSignature().equals(sig)) {
return true;
}
}

return false;
}
}

0 comments on commit 75f2bb1

Please sign in to comment.