Commit 21d0d376 authored by Eric Bruneton's avatar Eric Bruneton

Merge branch 'check-code-quality-with-pmd' into 'master'

Check the code quality with PMD

See merge request !222
parents 19735cbf e14cdbdf
Pipeline #2988 passed with stage
in 9 minutes and 8 seconds
......@@ -263,8 +263,7 @@ public class Analyzer<V extends Value> implements Opcodes {
List<TryCatchBlockNode> insnHandlers = handlers[insnIndex];
if (insnHandlers != null) {
for (int i = 0; i < insnHandlers.size(); ++i) {
TryCatchBlockNode tryCatchBlock = insnHandlers.get(i);
for (TryCatchBlockNode tryCatchBlock : insnHandlers) {
Type catchType;
if (tryCatchBlock.type == null) {
catchType = Type.getObjectType("java/lang/Throwable");
......@@ -348,8 +347,7 @@ public class Analyzer<V extends Value> implements Opcodes {
// Push the exception handler successors of currentInsn onto instructionIndicesToProcess.
List<TryCatchBlockNode> insnHandlers = handlers[currentInsnIndex];
if (insnHandlers != null) {
for (int i = 0; i < insnHandlers.size(); ++i) {
TryCatchBlockNode tryCatchBlock = insnHandlers.get(i);
for (TryCatchBlockNode tryCatchBlock : insnHandlers) {
instructionIndicesToProcess.add(insnList.indexOf(tryCatchBlock.handler));
}
}
......@@ -393,12 +391,12 @@ public class Analyzer<V extends Value> implements Opcodes {
currentLocal++;
}
Type[] argumentTypes = Type.getArgumentTypes(method.desc);
for (int i = 0; i < argumentTypes.length; ++i) {
for (Type argumentType : argumentTypes) {
frame.setLocal(
currentLocal,
interpreter.newParameterValue(isInstanceMethod, currentLocal, argumentTypes[i]));
interpreter.newParameterValue(isInstanceMethod, currentLocal, argumentType));
currentLocal++;
if (argumentTypes[i].getSize() == 2) {
if (argumentType.getSize() == 2) {
frame.setLocal(currentLocal, interpreter.newEmptyValue(currentLocal));
currentLocal++;
}
......
......@@ -80,6 +80,7 @@ public class BasicValue implements Value {
return type;
}
@Override
public int getSize() {
return type == Type.LONG_TYPE || type == Type.DOUBLE_TYPE ? 2 : 1;
}
......
......@@ -375,9 +375,9 @@ public class BasicVerifier extends BasicInterpreter {
throws AnalyzerException {
int opcode = insn.getOpcode();
if (opcode == MULTIANEWARRAY) {
for (int i = 0; i < values.size(); ++i) {
if (!BasicValue.INT_VALUE.equals(values.get(i))) {
throw new AnalyzerException(insn, null, BasicValue.INT_VALUE, values.get(i));
for (BasicValue value : values) {
if (!BasicValue.INT_VALUE.equals(value)) {
throw new AnalyzerException(insn, null, BasicValue.INT_VALUE, value);
}
}
} else {
......
......@@ -79,13 +79,13 @@ public class Frame<V extends Value> {
}
/**
* Constructs a copy of the given.
* Constructs a copy of the given Frame.
*
* @param frame a frame.
*/
public Frame(final Frame<? extends V> frame) {
this(frame.numLocals, frame.values.length - frame.numLocals);
init(frame);
init(frame); // NOPMD(ConstructorCallsOverridableMethod): can't fix for backward compatibility.
}
/**
......
......@@ -291,7 +291,7 @@ public class SimpleVerifier extends BasicVerifier {
* @return whether 'type' corresponds to an interface.
*/
protected boolean isInterface(final Type type) {
if (currentClass != null && type.equals(currentClass)) {
if (currentClass != null && currentClass.equals(type)) {
return isInterface;
}
return getClass(type).isInterface();
......@@ -306,7 +306,7 @@ public class SimpleVerifier extends BasicVerifier {
* @return the type corresponding to the super class of 'type'.
*/
protected Type getSuperClass(final Type type) {
if (currentClass != null && type.equals(currentClass)) {
if (currentClass != null && currentClass.equals(type)) {
return currentSuperClass;
}
Class<?> superClass = getClass(type).getSuperclass();
......@@ -329,7 +329,7 @@ public class SimpleVerifier extends BasicVerifier {
if (type1.equals(type2)) {
return true;
}
if (currentClass != null && type1.equals(currentClass)) {
if (currentClass != null && currentClass.equals(type1)) {
if (getSuperClass(type2) == null) {
return false;
} else {
......@@ -339,13 +339,12 @@ public class SimpleVerifier extends BasicVerifier {
return isAssignableFrom(type1, getSuperClass(type2));
}
}
if (currentClass != null && type2.equals(currentClass)) {
if (currentClass != null && currentClass.equals(type2)) {
if (isAssignableFrom(type1, currentSuperClass)) {
return true;
}
if (currentClassInterfaces != null) {
for (int i = 0; i < currentClassInterfaces.size(); ++i) {
Type currentClassInterface = currentClassInterfaces.get(i);
for (Type currentClassInterface : currentClassInterfaces) {
if (isAssignableFrom(type1, currentClassInterface)) {
return true;
}
......
......@@ -168,10 +168,12 @@ final class SmallSet<T> extends AbstractSet<T> {
this.secondElement = secondElement;
}
@Override
public boolean hasNext() {
return firstElement != null;
}
@Override
public T next() {
if (firstElement == null) {
throw new NoSuchElementException();
......
......@@ -95,6 +95,7 @@ public class SourceInterpreter extends Interpreter<SourceValue> implements Opcod
break;
default:
size = 1;
break;
}
return new SourceValue(size, insn);
}
......@@ -123,6 +124,7 @@ public class SourceInterpreter extends Interpreter<SourceValue> implements Opcod
break;
default:
size = 1;
break;
}
return new SourceValue(size, insn);
}
......@@ -154,6 +156,7 @@ public class SourceInterpreter extends Interpreter<SourceValue> implements Opcod
break;
default:
size = 1;
break;
}
return new SourceValue(size, insn);
}
......
......@@ -98,6 +98,7 @@ public class SourceValue implements Value {
* @return the size of this value, in 32 bits words. This size is 1 for byte, boolean, char,
* short, int, float, object and array types, and 2 for long and double.
*/
@Override
public int getSize() {
return size;
}
......
......@@ -846,10 +846,10 @@ public class AnalyzerTest {
Frame<BasicValue>[] frames = analyzer.analyze("C", this);
int actualMaxStack = 0;
int actualMaxLocals = 0;
for (int i = 0; i < frames.length; ++i) {
if (frames[i] != null) {
actualMaxStack = Math.max(actualMaxStack, frames[i].getStackSize());
actualMaxLocals = Math.max(actualMaxLocals, frames[i].getLocals());
for (Frame<BasicValue> frame : frames) {
if (frame != null) {
actualMaxStack = Math.max(actualMaxStack, frame.getStackSize());
actualMaxLocals = Math.max(actualMaxLocals, frame.getLocals());
}
}
assertEquals(maxStack, actualMaxStack, "maxStack");
......
......@@ -33,8 +33,9 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
......@@ -67,7 +68,7 @@ public class BasicInterpreterTest extends AsmTest {
public void testMergeWithJsrReachableFromTwoDifferentPaths()
throws IOException, AnalyzerException {
ClassReader classReader =
new ClassReader(new FileInputStream("src/test/resources/Issue316204.class"));
new ClassReader(Files.newInputStream(Paths.get("src/test/resources/Issue316204.class")));
ClassNode classNode = new ClassNode();
classReader.accept(classNode, 0);
Analyzer<BasicValue> analyzer = new Analyzer<>(new BasicInterpreter());
......
......@@ -71,9 +71,9 @@ public class SimpleVerifierTest extends AsmTest implements Opcodes {
methodNode.visitMaxs(10, 10);
anaylzer.analyze("C", methodNode);
Frame<?>[] frames = anaylzer.getFrames();
for (int i = 0; i < frames.length; ++i) {
if (frames[i] != null) {
frames[i].toString();
for (Frame<?> frame : frames) {
if (frame != null) {
frame.toString();
}
}
anaylzer.getHandlers(0);
......
......@@ -46,10 +46,10 @@ public class ValueTest {
@Test
public void testBasicValue() {
assertTrue(BasicValue.UNINITIALIZED_VALUE.equals(new BasicValue(null)));
assertTrue(BasicValue.INT_VALUE.equals(new BasicValue(Type.INT_TYPE)));
assertTrue(BasicValue.INT_VALUE.equals(BasicValue.INT_VALUE));
assertFalse(BasicValue.INT_VALUE.equals(new Object()));
assertEquals(new BasicValue(null), BasicValue.UNINITIALIZED_VALUE);
assertEquals(new BasicValue(Type.INT_TYPE), BasicValue.INT_VALUE);
assertEquals(BasicValue.INT_VALUE, BasicValue.INT_VALUE);
assertNotEquals(new Object(), BasicValue.INT_VALUE);
assertTrue(BasicValue.REFERENCE_VALUE.isReference());
assertTrue(new BasicValue(Type.getObjectType("[I")).isReference());
......@@ -69,10 +69,10 @@ public class ValueTest {
public void testSourceValue() {
assertEquals(2, new SourceValue(2).getSize());
assertTrue(new SourceValue(1).equals(new SourceValue(1)));
assertFalse(new SourceValue(1).equals(new SourceValue(1, new InsnNode(Opcodes.NOP))));
assertFalse(new SourceValue(1).equals(new SourceValue(2)));
assertFalse(new SourceValue(1).equals(null));
assertEquals(new SourceValue(1), new SourceValue(1));
assertNotEquals(new SourceValue(1), new SourceValue(1, new InsnNode(Opcodes.NOP)));
assertNotEquals(new SourceValue(1), new SourceValue(2));
assertNotEquals(new SourceValue(1), null);
assertEquals(0, new SourceValue(1).hashCode());
assertNotEquals(0, new SourceValue(1, new InsnNode(Opcodes.NOP)).hashCode());
......
......@@ -554,8 +554,8 @@ public class AnalyzerAdapter extends MethodVisitor {
if (firstDescriptorChar == '(') {
int numSlots = 0;
Type[] types = Type.getArgumentTypes(descriptor);
for (int i = 0; i < types.length; ++i) {
numSlots += types[i].getSize();
for (Type type : types) {
numSlots += type.getSize();
}
pop(numSlots);
} else if (firstDescriptorChar == 'J' || firstDescriptorChar == 'D') {
......
......@@ -425,6 +425,7 @@ public class GeneratorAdapter extends LocalVariablesSorter {
break;
default:
mv.visitLdcInsn(value);
break;
}
}
}
......@@ -915,6 +916,7 @@ public class GeneratorAdapter extends LocalVariablesSorter {
break;
default:
unboxMethod = null;
break;
}
if (unboxMethod == null) {
checkCast(type);
......
......@@ -162,8 +162,7 @@ public class Method {
}
stringBuilder.append(argumentDescriptor);
} while (currentArgumentEndIndex != -1);
stringBuilder.append(')');
stringBuilder.append(getDescriptorInternal(returnType, defaultPackage));
stringBuilder.append(')').append(getDescriptorInternal(returnType, defaultPackage));
return new Method(methodName, stringBuilder.toString());
}
......
......@@ -451,8 +451,7 @@ public class SerialVersionUIDAdder extends ClassVisitor {
final DataOutput dataOutputStream,
final boolean dotted)
throws IOException {
int size = itemCollection.size();
Item[] items = itemCollection.toArray(new Item[size]);
Item[] items = itemCollection.toArray(new Item[0]);
Arrays.sort(
items,
new Comparator<Item>() {
......
......@@ -95,6 +95,7 @@ public class TryCatchBlockSorter extends MethodNode {
tryCatchBlocks,
new Comparator<TryCatchBlockNode>() {
@Override
public int compare(
final TryCatchBlockNode tryCatchBlockNode1,
final TryCatchBlockNode tryCatchBlockNode2) {
......
......@@ -417,6 +417,16 @@ public class AdviceAdapterTest extends AsmTest {
classWriter.visitField(Opcodes.ACC_STATIC, "f", "[[I", null, null);
MethodVisitor defaultConstructorVisitor =
classWriter.visitMethod(Opcodes.ACC_PUBLIC, "<init>", "()V", null, null);
defaultConstructorVisitor.visitCode();
defaultConstructorVisitor.visitVarInsn(Opcodes.ALOAD, 0);
defaultConstructorVisitor.visitMethodInsn(
Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
defaultConstructorVisitor.visitInsn(Opcodes.RETURN);
defaultConstructorVisitor.visitMaxs(0, 0);
defaultConstructorVisitor.visitEnd();
String descriptor = "(I)V";
MethodVisitor methodVisitor =
classWriter.visitMethod(Opcodes.ACC_PUBLIC, "<init>", descriptor, null, null);
......
......@@ -32,6 +32,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.objectweb.asm.test.Assertions.assertThat;
import java.util.Arrays;
import java.util.Locale;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
......@@ -211,6 +212,8 @@ public class ClassRemapperTest extends AsmTest {
static class UpperCaseRemapper extends Remapper {
private static final Locale LOCALE = Locale.ENGLISH;
private final String internalClassName;
private final String remappedInternalClassName;
......@@ -219,7 +222,7 @@ public class ClassRemapperTest extends AsmTest {
this.remappedInternalClassName =
internalClassName.equals("module-info")
? internalClassName
: internalClassName.toUpperCase();
: internalClassName.toUpperCase(LOCALE);
}
String getRemappedClassName() {
......@@ -245,17 +248,17 @@ public class ClassRemapperTest extends AsmTest {
if (name.equals("<init>") || name.equals("<clinit>")) {
return name;
}
return owner.equals(internalClassName) ? name.toUpperCase() : name;
return owner.equals(internalClassName) ? name.toUpperCase(LOCALE) : name;
}
@Override
public String mapInvokeDynamicMethodName(final String name, final String descriptor) {
return name.toUpperCase();
return name.toUpperCase(LOCALE);
}
@Override
public String mapFieldName(final String owner, final String name, final String descriptor) {
return owner.equals(internalClassName) ? name.toUpperCase() : name;
return owner.equals(internalClassName) ? name.toUpperCase(LOCALE) : name;
}
@Override
......
......@@ -31,9 +31,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.objectweb.asm.test.Assertions.assertThat;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
......@@ -137,7 +138,7 @@ public class LocalVariablesSorterTest extends AsmTest {
@Test
public void testSortLocalVariablesAndInstantiate() throws FileNotFoundException, IOException {
ClassReader classReader =
new ClassReader(new FileInputStream("src/test/resources/Issue317586.class"));
new ClassReader(Files.newInputStream(Paths.get("src/test/resources/Issue317586.class")));
ClassWriter classWriter = new ClassWriter(0);
ClassVisitor classVisitor =
new ClassVisitor(Opcodes.ASM7, classWriter) {
......
......@@ -29,9 +29,8 @@ package org.objectweb.asm.commons;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.Test;
import org.objectweb.asm.Type;
......@@ -102,14 +101,14 @@ public class MethodTest {
@Test
public void testEquals() {
assertFalse(new Method("name", "()V").equals(null));
assertFalse(new Method("name", "()V").equals(new Method("other", "()V")));
assertFalse(new Method("name", "()V").equals(new Method("name", "(I)J")));
assertTrue(new Method("name", "()V").equals(Method.getMethod("void name()")));
assertNotEquals(new Method("name", "()V"), null);
assertNotEquals(new Method("name", "()V"), new Method("other", "()V"));
assertNotEquals(new Method("name", "()V"), new Method("name", "(I)J"));
assertEquals(new Method("name", "()V"), Method.getMethod("void name()"));
}
@Test
public void testHashCode() {
assertTrue(new Method("name", "()V").hashCode() != 0);
assertNotEquals(0, new Method("name", "()V").hashCode());
}
}
......@@ -34,7 +34,9 @@ import java.io.Serializable;
*
* @author Eric Bruneton
*/
class SerialVersionAnonymousInnerClass implements Serializable {
class SerialVersionAnonymousInnerClass implements Serializable { // NOPMD(MissingSerialVersionUID)
// No serial version UID on purpose, to test SerialVersionUIDAdder.
public static final SerialVersionAnonymousInnerClass anonymousInnerClass =
new SerialVersionAnonymousInnerClass() {};
......
......@@ -34,11 +34,14 @@ import java.io.Serializable;
*
* @author Eric Bruneton
*/
class SerialVersionClass implements Serializable {
class SerialVersionClass implements Serializable { // NOPMD(MissingSerialVersionUID)
// No serial version UID on purpose, to test SerialVersionUIDAdder.
protected static final int someField = 32;
static {
assert someField > 0;
}
SerialVersionClass() {}
......
......@@ -374,7 +374,7 @@ public abstract class AsmTest {
arguments.add(Array.get(Array.newInstance(parameterType, 1), 0));
}
constructor.setAccessible(true);
constructor.newInstance(arguments.toArray(new Object[arguments.size()]));
constructor.newInstance(arguments.toArray(new Object[0]));
}
} catch (ClassNotFoundException e) {
// Should never happen given the ByteClassLoader implementation.
......@@ -386,7 +386,8 @@ public abstract class AsmTest {
fail("Can't instantiate class " + className, e);
} catch (InvocationTargetException e) {
// If an exception occurs in the invoked constructor, it means the class was successfully
// verified first.
// verified first. Still, we fail the test to be on the safe side.
fail("An exception occurred in the constructor of " + className, e);
}
return byteClassLoader.classLoaded();
}
......
......@@ -300,10 +300,6 @@ class ClassDump {
dumpConstantValueAttribute(parser, builder);
} else if (attributeName.equals("Code")) {
dumpCodeAttribute(parser, builder);
} else if (attributeName.equals("CodeComment")) {
// empty non-standard attribute used for tests.
} else if (attributeName.equals("Comment")) {
// empty non-standard attribute used for tests.
} else if (attributeName.equals("StackMapTable")) {
dumpStackMapTableAttribute(parser, builder);
} else if (attributeName.equals("Exceptions")) {
......@@ -358,7 +354,8 @@ class ClassDump {
dumpNestMembersAttribute(parser, builder);
} else if (attributeName.equals("StackMap")) {
dumpStackMapAttribute(parser, builder);
} else {
} else if (!attributeName.equals("CodeComment") && !attributeName.equals("Comment")) {
// Not a standard attribute nor one the of empty non-standard attributes used for tests.
throw new IOException("Unknown attribute " + attributeName);
}
}
......@@ -1039,8 +1036,8 @@ class ClassDump {
final int attributeLength, final Parser parser, final Builder builder) throws IOException {
byte[] attributeData = parser.bytes(attributeLength);
StringBuilder stringBuilder = new StringBuilder();
for (int i = 0; i < attributeData.length; ++i) {
stringBuilder.append(attributeData[i]).append(',');
for (byte data : attributeData) {
stringBuilder.append(data).append(',');
}
builder.add("debug_extension: ", stringBuilder.toString());
}
......@@ -2385,6 +2382,7 @@ class ClassDump {
name = stringBuilder.toString();
}
@Override
public int compareTo(final Builder builder) {
return name.compareTo(builder.name);
}
......
......@@ -65,6 +65,7 @@ public class AsmTestTest extends AsmTest {
break;
default:
fail("Unknown API value");
break;
}
}
......@@ -133,6 +134,7 @@ public class AsmTestTest extends AsmTest {
break;
default:
fail("Unknown invalid class");
break;
}
}
}
......@@ -496,10 +496,12 @@ public class InsnList {
}
}
@Override
public boolean hasNext() {
return nextInsn != null;
}
@Override
public Object next() {
if (nextInsn == null) {
throw new NoSuchElementException();
......@@ -511,6 +513,7 @@ public class InsnList {
return result;
}
@Override
public void remove() {
if (remove != null) {
if (remove == nextInsn) {
......@@ -525,10 +528,12 @@ public class InsnList {
}
}
@Override
public boolean hasPrevious() {
return previousInsn != null;
}
@Override
public Object previous() {
AbstractInsnNode result = previousInsn;
nextInsn = result;
......@@ -537,6 +542,7 @@ public class InsnList {
return result;
}
@Override
public int nextIndex() {
if (nextInsn == null) {
return size();
......@@ -547,6 +553,7 @@ public class InsnList {
return nextInsn.index;
}
@Override
public int previousIndex() {
if (previousInsn == null) {
return -1;
......@@ -557,6 +564,7 @@ public class InsnList {
return previousInsn.index;
}
@Override
public void add(final Object o) {
if (nextInsn != null) {
InsnList.this.insertBefore(nextInsn, (AbstractInsnNode) o);
......@@ -569,6 +577,7 @@ public class InsnList {
remove = null;
}
@Override
public void set(final Object o) {
if (remove != null) {
InsnList.this.set(remove, (AbstractInsnNode) o);
......
......@@ -74,6 +74,6 @@ public class ModuleExportNode {
*/
public void accept(final ModuleVisitor moduleVisitor) {
moduleVisitor.visitExport(
packaze, access, modules == null ? null : modules.toArray(new String[modules.size()]));
packaze, access, modules == null ? null : modules.toArray(new String[0]));
}
}
......@@ -74,6 +74,6 @@ public class ModuleOpenNode {
*/
public void accept(final ModuleVisitor moduleVisitor) {
moduleVisitor.visitOpen(
packaze, access, modules == null ? null : modules.toArray(new String[modules.size()]));
packaze, access, modules == null ? null : modules.toArray(new String[0]));
}
}
......@@ -61,6 +61,6 @@ public class ModuleProvideNode {
* @param moduleVisitor a module visitor.
*/
public void accept(final ModuleVisitor moduleVisitor) {
moduleVisitor.visitProvide(service, providers.toArray(new String[providers.size()]));
moduleVisitor.visitProvide(service, providers.toArray(new String[0]));
}
}
......@@ -150,7 +150,7 @@ final class Util {
static <T> List<T> asArrayList(final int length, final T[] array) {
List<T> list = new ArrayList<T>(length);
for (int i = 0; i < length; ++i) {
list.add(array[i]);
list.add(array[i]); // NOPMD(UseArraysAsList): we convert a part of the array.
}
return list;
}
......
......@@ -28,6 +28,7 @@
package org.objectweb.asm.tree;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
......@@ -240,12 +241,12 @@ public class ClassNodeTest extends AsmTest implements Opcodes {
MethodInsnNode methodInsnNode = new MethodInsnNode(INVOKESTATIC, "owner", "name", "()I");
assertEquals(AbstractInsnNode.METHOD_INSN, methodInsnNode.getType());
assertEquals(INVOKESTATIC, methodInsnNode.getOpcode());
assertEquals(false, methodInsnNode.itf);
assertFalse(methodInsnNode.itf);
methodInsnNode = new MethodInsnNode(INVOKEINTERFACE, "owner", "name", "()I");