This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-bcel.git
commit e6740b9f1e0be8bdb4335a28696831de96efb8b7
Author: Gary David Gregory (Code signing key) <ggreg...@apache.org>
AuthorDate: Sat Oct 29 09:19:45 2022 -0400
PMD & Checkstyle: Avoid empty statements and unused args
Enable PMD
---
pom.xml | 2 +-
.../org/apache/bcel/classfile/StackMapEntry.java | 48 ++++++++++++----------
.../org/apache/bcel/generic/ConstantPoolGen.java | 44 ++++++++++----------
src/main/java/org/apache/bcel/generic/PUSH.java | 3 ++
.../bcel/verifier/statics/Pass1Verifier.java | 6 +--
.../structurals/InstConstraintVisitor.java | 12 +++---
6 files changed, 63 insertions(+), 52 deletions(-)
diff --git a/pom.xml b/pom.xml
index 043498e6..e7542006 100644
--- a/pom.xml
+++ b/pom.xml
@@ -220,7 +220,7 @@
</scm>
<build>
- <defaultGoal>clean verify apache-rat:check japicmp:cmp checkstyle:check
javadoc:javadoc</defaultGoal>
+ <defaultGoal>clean verify apache-rat:check japicmp:cmp checkstyle:check
pmd:check javadoc:javadoc</defaultGoal>
<plugins>
<plugin>
<groupId>org.apache.felix</groupId>
diff --git a/src/main/java/org/apache/bcel/classfile/StackMapEntry.java
b/src/main/java/org/apache/bcel/classfile/StackMapEntry.java
index 7d2f9ab6..58b93ed3 100644
--- a/src/main/java/org/apache/bcel/classfile/StackMapEntry.java
+++ b/src/main/java/org/apache/bcel/classfile/StackMapEntry.java
@@ -110,6 +110,12 @@ public final class StackMapEntry implements Node,
Cloneable {
this.typesOfLocals = typesOfLocals != null ? typesOfLocals :
EMPTY_STACK_MAP_TYPE_ARRAY;
this.typesOfStackItems = typesOfStackItems != null ? typesOfStackItems
: EMPTY_STACK_MAP_TYPE_ARRAY;
this.constantPool = constantPool;
+ if (numberOfLocals < 0) {
+ throw new IllegalArgumentException("numberOfLocals < 0");
+ }
+ if (numberOfStackItems < 0) {
+ throw new IllegalArgumentException("numberOfStackItems < 0");
+ }
}
/**
@@ -167,9 +173,7 @@ public final class StackMapEntry implements Node, Cloneable
{
*/
public void dump(final DataOutputStream file) throws IOException {
file.write(frameType);
- if (frameType >= Const.SAME_FRAME && frameType <=
Const.SAME_FRAME_MAX) {
- // nothing to be done
- } else if (frameType >= Const.SAME_LOCALS_1_STACK_ITEM_FRAME &&
frameType <= Const.SAME_LOCALS_1_STACK_ITEM_FRAME_MAX) {
+ if (frameType >= Const.SAME_LOCALS_1_STACK_ITEM_FRAME && frameType <=
Const.SAME_LOCALS_1_STACK_ITEM_FRAME_MAX) {
typesOfStackItems[0].dump(file);
} else if (frameType == Const.SAME_LOCALS_1_STACK_ITEM_FRAME_EXTENDED)
{
file.writeShort(byteCodeOffset);
@@ -193,7 +197,7 @@ public final class StackMapEntry implements Node, Cloneable
{
for (final StackMapType type : typesOfStackItems) {
type.dump(file);
}
- } else {
+ } else if (!(frameType >= Const.SAME_FRAME && frameType <=
Const.SAME_FRAME_MAX)) {
/* Can't happen */
throw new ClassFormatException("Invalid Stack map table tag: " +
frameType);
}
@@ -267,6 +271,16 @@ public final class StackMapEntry implements Node,
Cloneable {
return typesOfStackItems;
}
+ private boolean invalidFrameType(final int f) {
+ // @formatter:off
+ return f != Const.SAME_LOCALS_1_STACK_ITEM_FRAME_EXTENDED
+ && !(f >= Const.CHOP_FRAME && f <= Const.CHOP_FRAME_MAX)
+ && f != Const.SAME_FRAME_EXTENDED
+ && !(f >= Const.APPEND_FRAME && f <= Const.APPEND_FRAME_MAX)
+ && f != Const.FULL_FRAME;
+ // @formatter:on
+ }
+
public void setByteCodeOffset(final int newOffset) {
if (newOffset < 0 || newOffset > 32767) {
throw new IllegalArgumentException("Invalid StackMap offset: " +
newOffset);
@@ -284,12 +298,7 @@ public final class StackMapEntry implements Node,
Cloneable {
} else {
frameType = Const.SAME_LOCALS_1_STACK_ITEM_FRAME + newOffset;
}
- } else if (frameType == Const.SAME_LOCALS_1_STACK_ITEM_FRAME_EXTENDED)
{ // CHECKSTYLE IGNORE EmptyBlock
- } else if (frameType >= Const.CHOP_FRAME && frameType <=
Const.CHOP_FRAME_MAX) { // CHECKSTYLE IGNORE EmptyBlock
- } else if (frameType == Const.SAME_FRAME_EXTENDED) { // CHECKSTYLE
IGNORE EmptyBlock
- } else if (frameType >= Const.APPEND_FRAME && frameType <=
Const.APPEND_FRAME_MAX) { // CHECKSTYLE IGNORE EmptyBlock
- } else if (frameType == Const.FULL_FRAME) { // CHECKSTYLE IGNORE
EmptyBlock
- } else {
+ } else if (invalidFrameType(frameType)) {
throw new IllegalStateException("Invalid StackMap frameType: " +
frameType);
}
byteCodeOffset = newOffset;
@@ -302,20 +311,15 @@ public final class StackMapEntry implements Node,
Cloneable {
this.constantPool = constantPool;
}
- public void setFrameType(final int f) {
- if (f >= Const.SAME_FRAME && f <= Const.SAME_FRAME_MAX) {
- byteCodeOffset = f - Const.SAME_FRAME;
- } else if (f >= Const.SAME_LOCALS_1_STACK_ITEM_FRAME && f <=
Const.SAME_LOCALS_1_STACK_ITEM_FRAME_MAX) {
- byteCodeOffset = f - Const.SAME_LOCALS_1_STACK_ITEM_FRAME;
- } else if (f == Const.SAME_LOCALS_1_STACK_ITEM_FRAME_EXTENDED) { //
CHECKSTYLE IGNORE EmptyBlock
- } else if (f >= Const.CHOP_FRAME && f <= Const.CHOP_FRAME_MAX) { //
CHECKSTYLE IGNORE EmptyBlock
- } else if (f == Const.SAME_FRAME_EXTENDED) { // CHECKSTYLE IGNORE
EmptyBlock
- } else if (f >= Const.APPEND_FRAME && f <= Const.APPEND_FRAME_MAX) {
// CHECKSTYLE IGNORE EmptyBlock
- } else if (f == Const.FULL_FRAME) { // CHECKSTYLE IGNORE EmptyBlock
- } else {
+ public void setFrameType(final int ft) {
+ if (ft >= Const.SAME_FRAME && ft <= Const.SAME_FRAME_MAX) {
+ byteCodeOffset = ft - Const.SAME_FRAME;
+ } else if (ft >= Const.SAME_LOCALS_1_STACK_ITEM_FRAME && ft <=
Const.SAME_LOCALS_1_STACK_ITEM_FRAME_MAX) {
+ byteCodeOffset = ft - Const.SAME_LOCALS_1_STACK_ITEM_FRAME;
+ } else if (invalidFrameType(ft)) {
throw new IllegalArgumentException("Invalid StackMap frameType");
}
- frameType = f;
+ frameType = ft;
}
/**
diff --git a/src/main/java/org/apache/bcel/generic/ConstantPoolGen.java
b/src/main/java/org/apache/bcel/generic/ConstantPoolGen.java
index 3c1556e6..0aeaafc2 100644
--- a/src/main/java/org/apache/bcel/generic/ConstantPoolGen.java
+++ b/src/main/java/org/apache/bcel/generic/ConstantPoolGen.java
@@ -187,27 +187,29 @@ public class ConstantPoolGen {
if (!cpTable.containsKey(key)) {
cpTable.put(key, Integer.valueOf(i));
}
- } else if (c == null) { // entries may be null
- // nothing to do
- } else if (c instanceof ConstantInteger) {
- // nothing to do
- } else if (c instanceof ConstantLong) {
- // nothing to do
- } else if (c instanceof ConstantFloat) {
- // nothing to do
- } else if (c instanceof ConstantDouble) {
- // nothing to do
- } else if (c instanceof
org.apache.bcel.classfile.ConstantMethodType) {
- // TODO should this be handled somehow?
- } else if (c instanceof
org.apache.bcel.classfile.ConstantMethodHandle) {
- // TODO should this be handled somehow?
- } else if (c instanceof org.apache.bcel.classfile.ConstantModule) {
- // TODO should this be handled somehow?
- } else if (c instanceof org.apache.bcel.classfile.ConstantPackage)
{
- // TODO should this be handled somehow?
- } else {
- assert false : "Unexpected constant type: " +
c.getClass().getName();
- }
+ }
+// else if (c == null) { // entries may be null
+// // nothing to do
+// } else if (c instanceof ConstantInteger) {
+// // nothing to do
+// } else if (c instanceof ConstantLong) {
+// // nothing to do
+// } else if (c instanceof ConstantFloat) {
+// // nothing to do
+// } else if (c instanceof ConstantDouble) {
+// // nothing to do
+// } else if (c instanceof
org.apache.bcel.classfile.ConstantMethodType) {
+// // TODO should this be handled somehow?
+// } else if (c instanceof
org.apache.bcel.classfile.ConstantMethodHandle) {
+// // TODO should this be handled somehow?
+// } else if (c instanceof
org.apache.bcel.classfile.ConstantModule) {
+// // TODO should this be handled somehow?
+// } else if (c instanceof
org.apache.bcel.classfile.ConstantPackage) {
+// // TODO should this be handled somehow?
+// } else {
+// // Not helpful, should throw an exception.
+// assert false : "Unexpected constant type: " +
c.getClass().getName();
+// }
}
}
diff --git a/src/main/java/org/apache/bcel/generic/PUSH.java
b/src/main/java/org/apache/bcel/generic/PUSH.java
index c56145ea..b29947dd 100644
--- a/src/main/java/org/apache/bcel/generic/PUSH.java
+++ b/src/main/java/org/apache/bcel/generic/PUSH.java
@@ -16,6 +16,8 @@
*/
package org.apache.bcel.generic;
+import java.util.Objects;
+
import org.apache.bcel.Const;
/**
@@ -30,6 +32,7 @@ public final class PUSH implements CompoundInstruction,
VariableLengthInstructio
* @param value to be pushed
*/
public PUSH(final ConstantPoolGen cp, final boolean value) {
+ Objects.requireNonNull(cp, "cp");
instruction = InstructionConst.getInstruction(Const.ICONST_0 + (value
? 1 : 0));
}
diff --git a/src/main/java/org/apache/bcel/verifier/statics/Pass1Verifier.java
b/src/main/java/org/apache/bcel/verifier/statics/Pass1Verifier.java
index d97e9df8..3cfaec3d 100644
--- a/src/main/java/org/apache/bcel/verifier/statics/Pass1Verifier.java
+++ b/src/main/java/org/apache/bcel/verifier/statics/Pass1Verifier.java
@@ -23,6 +23,7 @@ import org.apache.bcel.verifier.PassVerifier;
import org.apache.bcel.verifier.VerificationResult;
import org.apache.bcel.verifier.Verifier;
import org.apache.bcel.verifier.exc.LoadingException;
+import org.apache.commons.lang3.ArrayUtils;
/**
* This PassVerifier verifies a class file according to pass 1 as described in
The Java Virtual Machine Specification,
@@ -174,12 +175,11 @@ public final class Pass1Verifier extends PassVerifier {
* Currently this returns an empty array of String. One could parse the
error messages of BCEL (written to
* java.lang.System.err) when loading a class file such as detecting
unknown attributes or trailing garbage at the end
* of a class file. However, Markus Dahm does not like the idea so this
method is currently useless and therefore marked
- * as <B>TODO</B>.
+ * as <b>TODO</b>.
*/
@Override
public String[] getMessages() {
- // This method is only here to override the javadoc-comment.
- return super.getMessages();
+ return ArrayUtils.EMPTY_STRING_ARRAY;
}
}
diff --git
a/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java
b/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java
index c6d4e7dc..f0ab7685 100644
---
a/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java
+++
b/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java
@@ -248,9 +248,11 @@ public class InstConstraintVisitor extends EmptyVisitor {
indexOfInt(o, index);
if (!(value instanceof ReferenceType)) {
constraintViolated(o, "The 'value' is not of a ReferenceType but
of type " + value + ".");
- } else {
- // referenceTypeIsInitialized(o, (ReferenceType) value);
}
+ // } else {
+ // referenceTypeIsInitialized(o, (ReferenceType) value);
+ // }
+ //
// Don't bother further with "referenceTypeIsInitialized()", there are
no arrays
// of an uninitialized object type.
if (arrayrefOfArrayType(o, arrayref) && !(((ArrayType)
arrayref).getElementType() instanceof ReferenceType)) {
@@ -1085,8 +1087,8 @@ public class InstConstraintVisitor extends EmptyVisitor {
if (!(t instanceof ObjectType)) {
constraintViolated(o, "The 'objectref' must refer to
an object that's not an array. Found instead: '" + t + "'.");
}
- final ObjectType objreftype = (ObjectType) t;
- if (!(objreftype.equals(curr) ||
objreftype.subclassOf(curr))) {
+ // final ObjectType objreftype = (ObjectType) t;
+ // if (!(objreftype.equals(curr) ||
objreftype.subclassOf(curr))) {
// TODO: One day move to Staerk-et-al's "Set of object
types" instead of "wider" object types
// created during the verification.
// "Wider" object types don't allow us to check for
things like that below.
@@ -1094,7 +1096,7 @@ public class InstConstraintVisitor extends EmptyVisitor {
// "and it's a member of the current class or a
superclass of the current class."+
// " However, the referenced object type
'"+stack().peek()+
// "' is not the current class or a subclass of the
current class.");
- }
+ //}
}
}
