Skip to content

Commit 75f2bb1

Browse files
mebigfatguymebigfatguy
committed
Merge branch 'findbugs' into spotbugs
 Conflicts: src/main/java/com/mebigfatguy/fbcontrib/detect/UseVarArgs.java
2 parents a804ca3 + aeb64ad commit 75f2bb1

File tree

2 files changed

+185
-174
lines changed

2 files changed

+185
-174
lines changed

‎etc/messages.xml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,9 +1173,13 @@ a[0] = new A(); // results in ArrayStoreException (Runtime)
11731173
<Detector class="com.mebigfatguy.fbcontrib.detect.UseVarArgs">
11741174
<Details>
11751175
<![CDATA[
1176-
<p>This detector looks for definitions of methods that have an array as the last parameter.
1176+
<p>This detector looks for problems and possibilities to use var args better. Specially looks for
1177+
<ul>
1178+
<li>definitions of methods that have an array as the last parameter.
11771179
Since this class is compiled with Java 1.5 or better, it would be more flexible for clients of this
1178-
method to define this parameter as a vararg parameter.</p>
1180+
method to define this parameter as a vararg parameter</li>
1181+
<li>classes that have samed named vararg and non vararg methods not clearly differentiated.</li>
1182+
<li>Explicitly passing null to a var arg parameter</li></ul>.</p>
11791183
<p>It is a fast detector.</p>
11801184
]]>
11811185
</Details>

‎src/main/java/com/mebigfatguy/fbcontrib/detect/UseVarArgs.java

Lines changed: 179 additions & 172 deletions
Original file line numberDiff line numberDiff line change
@@ -43,176 +43,183 @@
4343
*/
4444
public class UseVarArgs extends PreorderVisitor implements Detector {
4545

46-
public static final String SIG_STRING_ARRAY_TO_VOID = new SignatureBuilder()
47-
.withParamTypes(SignatureBuilder.SIG_STRING_ARRAY).toString();
48-
49-
private final BugReporter bugReporter;
50-
private JavaClass javaClass;
51-
52-
public UseVarArgs(BugReporter bugReporter) {
53-
this.bugReporter = bugReporter;
54-
}
55-
56-
/**
57-
* overrides the visitor to make sure that the class was compiled by java 1.5 or
58-
* later.
59-
*
60-
* @param classContext the context object of the currently parsed class
61-
*/
62-
@Override
63-
public void visitClassContext(ClassContext classContext) {
64-
try {
65-
javaClass = classContext.getJavaClass();
66-
if (javaClass.getMajor() >= Const.MAJOR_1_5) {
67-
javaClass.accept(this);
68-
}
69-
} finally {
70-
javaClass = null;
71-
}
72-
}
73-
74-
/**
75-
* overrides the visitor to look for methods that has an array as a last
76-
* parameter of an array type, where the base type is not like the previous
77-
* parameter nor something like a char or byte array.
78-
*
79-
* @param obj the currently parse method
80-
*/
81-
@Override
82-
public void visitMethod(Method obj) {
83-
try {
84-
if (obj.isSynthetic()) {
85-
return;
86-
}
87-
88-
if (Values.CONSTRUCTOR.equals(getMethodName()) && javaClass.getClassName().contains("$")) {
89-
return;
90-
}
91-
92-
List<String> types = SignatureUtils.getParameterSignatures(obj.getSignature());
93-
if ((types.isEmpty()) || (types.size() > 2)) {
94-
return;
95-
}
96-
97-
if ((obj.getAccessFlags() & Const.ACC_VARARGS) != 0) {
98-
return;
99-
}
100-
101-
String lastParmSig = types.get(types.size() - 1);
102-
if (!lastParmSig.startsWith(Values.SIG_ARRAY_PREFIX)
103-
|| lastParmSig.startsWith(Values.SIG_ARRAY_OF_ARRAYS_PREFIX)) {
104-
return;
105-
}
106-
107-
if (SignatureBuilder.SIG_BYTE_ARRAY.equals(lastParmSig)
108-
|| SignatureBuilder.SIG_CHAR_ARRAY.equals(lastParmSig)) {
109-
return;
110-
}
111-
112-
if (hasSimilarParms(types)) {
113-
return;
114-
}
115-
116-
if (obj.isStatic() && "main".equals(obj.getName()) && SIG_STRING_ARRAY_TO_VOID.equals(obj.getSignature())) {
117-
return;
118-
}
119-
120-
if (!obj.isPrivate() && !obj.isStatic() && isInherited(obj)) {
121-
return;
122-
}
123-
124-
super.visitMethod(obj);
125-
bugReporter.reportBug(new BugInstance(this, BugType.UVA_USE_VAR_ARGS.name(), LOW_PRIORITY).addClass(this)
126-
.addMethod(this));
127-
128-
} catch (ClassNotFoundException cnfe) {
129-
bugReporter.reportMissingClass(cnfe);
130-
}
131-
}
132-
133-
/**
134-
* overrides the visitor, but not used
135-
*/
136-
@Override
137-
public void report() {
138-
// needed by Detector interface but not used
139-
}
140-
141-
/**
142-
* determines whether a bunch of types are similar and thus would be confusing
143-
* to have one be a varargs.
144-
*
145-
* @param argTypes the parameter signatures to check
146-
* @return whether the parameter are similar
147-
*/
148-
@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")
149-
private static boolean hasSimilarParms(List<String> argTypes) {
150-
151-
for (int i = 0; i < (argTypes.size() - 1); i++) {
152-
if (argTypes.get(i).startsWith(Values.SIG_ARRAY_PREFIX)) {
153-
return true;
154-
}
155-
}
156-
157-
String baseType = argTypes.get(argTypes.size() - 1);
158-
while (baseType.startsWith(Values.SIG_ARRAY_PREFIX)) {
159-
baseType = baseType.substring(1);
160-
}
161-
162-
for (int i = 0; i < (argTypes.size() - 1); i++) {
163-
if (argTypes.get(i).equals(baseType)) {
164-
return true;
165-
}
166-
}
167-
168-
return false;
169-
}
170-
171-
/**
172-
* looks to see if this method is derived from a super class. If it is we don't
173-
* want to report on it, as that would entail changing a whole hierarchy
174-
*
175-
* @param m the current method
176-
* @return if the method is inherited
177-
*
178-
* @throws ClassNotFoundException if the super class(s) aren't found
179-
*/
180-
private boolean isInherited(Method m) throws ClassNotFoundException {
181-
JavaClass[] infs = javaClass.getAllInterfaces();
182-
for (JavaClass inf : infs) {
183-
if (hasMethod(inf, m)) {
184-
return true;
185-
}
186-
}
187-
188-
JavaClass[] sups = javaClass.getSuperClasses();
189-
for (JavaClass sup : sups) {
190-
if (hasMethod(sup, m)) {
191-
return true;
192-
}
193-
}
194-
195-
return false;
196-
}
197-
198-
/**
199-
* looks to see if a class has a method with a specific name and signature
200-
*
201-
* @param c the class to check
202-
* @param candidateMethod the method to look for
203-
*
204-
* @return whether this class has the exact method
205-
*/
206-
private static boolean hasMethod(JavaClass c, Method candidateMethod) {
207-
String name = candidateMethod.getName();
208-
String sig = candidateMethod.getSignature();
209-
210-
for (Method method : c.getMethods()) {
211-
if (!method.isStatic() && method.getName().equals(name) && method.getSignature().equals(sig)) {
212-
return true;
213-
}
214-
}
215-
216-
return false;
217-
}
46+
public static final String SIG_STRING_ARRAY_TO_VOID = new SignatureBuilder()
47+
.withParamTypes(SignatureBuilder.SIG_STRING_ARRAY).toString();
48+
49+
private final BugReporter bugReporter;
50+
private JavaClass javaClass;
51+
52+
public UseVarArgs(BugReporter bugReporter) {
53+
this.bugReporter = bugReporter;
54+
}
55+
56+
/**
57+
* overrides the visitor to make sure that the class was compiled by java 1.5 or
58+
* later.
59+
*
60+
* @param classContext the context object of the currently parsed class
61+
*/
62+
@Override
63+
public void visitClassContext(ClassContext classContext) {
64+
try {
65+
javaClass = classContext.getJavaClass();
66+
if (javaClass.getMajor() >= Const.MAJOR_1_5) {
67+
javaClass.accept(this);
68+
}
69+
} finally {
70+
javaClass = null;
71+
}
72+
}
73+
74+
/**
75+
* overrides the visitor to look for methods that has an array as a last
76+
* parameter of an array type, where the base type is not like the previous
77+
* parameter nor something like a char or byte array.
78+
*
79+
* @param obj the currently parse method
80+
*/
81+
@Override
82+
public void visitMethod(Method obj) {
83+
try {
84+
if (obj.isSynthetic()) {
85+
return;
86+
}
87+
88+
boolean isVarMethod = (obj.getAccessFlags() & Const.ACC_VARARGS) != 0;
89+
90+
boolean isConvertable = !isVarMethod && methodHasConvertableLastParam(obj);
91+
92+
super.visitMethod(obj);
93+
94+
if (isConvertable) {
95+
bugReporter.reportBug(new BugInstance(this, BugType.UVA_USE_VAR_ARGS.name(), LOW_PRIORITY)
96+
.addClass(this).addMethod(this));
97+
}
98+
99+
} catch (ClassNotFoundException cnfe) {
100+
bugReporter.reportMissingClass(cnfe);
101+
}
102+
}
103+
104+
public boolean methodHasConvertableLastParam(Method method) throws ClassNotFoundException {
105+
if (Values.CONSTRUCTOR.equals(getMethodName()) && javaClass.getClassName().contains("$")) {
106+
return false;
107+
}
108+
List<String> types = SignatureUtils.getParameterSignatures(method.getSignature());
109+
if ((types.isEmpty()) || (types.size() > 2)) {
110+
return false;
111+
}
112+
113+
String lastParmSig = types.get(types.size() - 1);
114+
if (!lastParmSig.startsWith(Values.SIG_ARRAY_PREFIX)
115+
|| lastParmSig.startsWith(Values.SIG_ARRAY_OF_ARRAYS_PREFIX)) {
116+
return false;
117+
}
118+
119+
if (SignatureBuilder.SIG_BYTE_ARRAY.equals(lastParmSig)
120+
|| SignatureBuilder.SIG_CHAR_ARRAY.equals(lastParmSig)) {
121+
return false;
122+
}
123+
124+
if (hasSimilarParms(types)) {
125+
return false;
126+
}
127+
128+
if (method.isStatic() && "main".equals(method.getName())
129+
&& SIG_STRING_ARRAY_TO_VOID.equals(method.getSignature())) {
130+
return false;
131+
}
132+
133+
if (!method.isPrivate() && !method.isStatic() && isInherited(method)) {
134+
return false;
135+
}
136+
137+
return true;
138+
}
139+
140+
/**
141+
* overrides the visitor, but not used
142+
*/
143+
@Override
144+
public void report() {
145+
// needed by Detector interface but not used
146+
}
147+
148+
/**
149+
* determines whether a bunch of types are similar and thus would be confusing
150+
* to have one be a varargs.
151+
*
152+
* @param argTypes the parameter signatures to check
153+
* @return whether the parameter are similar
154+
*/
155+
@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")
156+
private static boolean hasSimilarParms(List<String> argTypes) {
157+
158+
for (int i = 0; i < (argTypes.size() - 1); i++) {
159+
if (argTypes.get(i).startsWith(Values.SIG_ARRAY_PREFIX)) {
160+
return true;
161+
}
162+
}
163+
164+
String baseType = argTypes.get(argTypes.size() - 1);
165+
while (baseType.startsWith(Values.SIG_ARRAY_PREFIX)) {
166+
baseType = baseType.substring(1);
167+
}
168+
169+
for (int i = 0; i < (argTypes.size() - 1); i++) {
170+
if (argTypes.get(i).equals(baseType)) {
171+
return true;
172+
}
173+
}
174+
175+
return false;
176+
}
177+
178+
/**
179+
* looks to see if this method is derived from a super class. If it is we don't
180+
* want to report on it, as that would entail changing a whole hierarchy
181+
*
182+
* @param m the current method
183+
* @return if the method is inherited
184+
*
185+
* @throws ClassNotFoundException if the super class(s) aren't found
186+
*/
187+
private boolean isInherited(Method m) throws ClassNotFoundException {
188+
JavaClass[] infs = javaClass.getAllInterfaces();
189+
for (JavaClass inf : infs) {
190+
if (hasMethod(inf, m)) {
191+
return true;
192+
}
193+
}
194+
195+
JavaClass[] sups = javaClass.getSuperClasses();
196+
for (JavaClass sup : sups) {
197+
if (hasMethod(sup, m)) {
198+
return true;
199+
}
200+
}
201+
202+
return false;
203+
}
204+
205+
/**
206+
* looks to see if a class has a method with a specific name and signature
207+
*
208+
* @param c the class to check
209+
* @param candidateMethod the method to look for
210+
*
211+
* @return whether this class has the exact method
212+
*/
213+
private static boolean hasMethod(JavaClass c, Method candidateMethod) {
214+
String name = candidateMethod.getName();
215+
String sig = candidateMethod.getSignature();
216+
217+
for (Method method : c.getMethods()) {
218+
if (!method.isStatic() && method.getName().equals(name) && method.getSignature().equals(sig)) {
219+
return true;
220+
}
221+
}
222+
223+
return false;
224+
}
218225
}

0 commit comments

Comments
 (0)