[Truffle] Fix a few obvious FindBugs bugs.

jruby · Jan 5, 2015 · cf6c3c5 · cf6c3c5
1 parent 7179a58
commit cf6c3c5
Show file tree

Hide file tree

Showing 16 changed files with 47 additions and 72 deletions.
diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/BindingNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/BindingNodes.java
@@ -43,7 +43,7 @@ public Object localVariableGet(RubyBinding binding, RubySymbol symbol) {
             Object value = frame.getValue(frame.getFrameDescriptor().findFrameSlot(symbol.toString()));
 
             // TODO(CS): temporary hack for $_
-            if (symbol.equals("$_")) {
+            if (symbol.toString().equals("$_")) {
                 value = GetFromThreadLocalNode.get(getContext(), value);
             }
 

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/DirNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/DirNodes.java
@@ -105,7 +105,7 @@ private static RubyArray glob(final RubyContext context, String glob) {
             String absoluteGlob;
 
             if (!glob.startsWith("/")) {
-                absoluteGlob = new File(".", glob).getAbsolutePath().toString();
+                absoluteGlob = new File(".", glob).getAbsolutePath();
             } else {
                 absoluteGlob = glob;
             }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java
@@ -1115,13 +1115,7 @@ public Object integer(RubyString value) {
             }
 
             try {
-                final int value1 = Integer.parseInt(value.toString());
-
-                if (value1 >= Long.MIN_VALUE && value1 <= Long.MAX_VALUE) {
-                    return value1;
-                } else {
-                    return bignum(value1);
-                }
+                return Integer.parseInt(value.toString());
             } catch (NumberFormatException e) {
                 return bignum(new BigInteger(value.toString()));
             }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/core/StringNodes.java b/core/src/main/java/org/jruby/truffle/nodes/core/StringNodes.java
@@ -1357,13 +1357,7 @@ public Object toI(RubyString string) {
             }
 
             try {
-                final int value = Integer.parseInt(string.toString());
-
-                if (value >= Long.MIN_VALUE && value <= Long.MAX_VALUE) {
-                    return value;
-                } else {
-                    return bignum(value);
-                }
+                return Integer.parseInt(string.toString());
             } catch (NumberFormatException e) {
                 return bignum(new BigInteger(string.toString()));
             }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/dispatch/DispatchHeadNode.java b/core/src/main/java/org/jruby/truffle/nodes/dispatch/DispatchHeadNode.java
@@ -118,7 +118,7 @@ public long callLongFixnum(
         }
 
         if (value instanceof Long) {
-            return (int) value;
+            return (long) value;
         }
 
         CompilerDirectives.transferToInterpreter();

diff --git a/.../src/main/java/org/jruby/truffle/nodes/methods/arguments/ReadKeywordRestArgumentNode.java b/.../src/main/java/org/jruby/truffle/nodes/methods/arguments/ReadKeywordRestArgumentNode.java
@@ -46,7 +46,7 @@ public Object execute(VirtualFrame frame) {
 
         outer: for (KeyValue keyValue : HashOperations.verySlowToKeyValues(hash)) {
             for (String excludedKeyword : excludedKeywords) {
-                if (excludedKeyword.toString().equals(keyValue.getKey().toString())) {
+                if (excludedKeyword.equals(keyValue.getKey().toString())) {
                     continue outer;
                 }
             }

diff --git a/core/src/main/java/org/jruby/truffle/nodes/objects/ReadInstanceVariableNode.java b/core/src/main/java/org/jruby/truffle/nodes/objects/ReadInstanceVariableNode.java
@@ -121,25 +121,21 @@ public Object isDefined(VirtualFrame frame) {
 
         final RubyContext context = getContext();
 
-        try {
-            final Object receiverObject = receiver.execute(frame);
+        final Object receiverObject = receiver.execute(frame);
 
-            if (receiverObject instanceof RubyBasicObject) {
-                final RubyBasicObject receiverRubyObject = (RubyBasicObject) receiverObject;
+        if (receiverObject instanceof RubyBasicObject) {
+            final RubyBasicObject receiverRubyObject = (RubyBasicObject) receiverObject;
 
-                final Shape layout = receiverRubyObject.getDynamicObject().getShape();
-                final Property storageLocation = layout.getProperty(readNode.getName());
+            final Shape layout = receiverRubyObject.getDynamicObject().getShape();
+            final Property storageLocation = layout.getProperty(readNode.getName());
 
-                if (storageLocation != null) {
-                    return context.makeString("instance-variable");
-                } else {
-                    return getContext().getCoreLibrary().getNilObject();
-                }
+            if (storageLocation != null) {
+                return context.makeString("instance-variable");
             } else {
-                return false;
+                return getContext().getCoreLibrary().getNilObject();
             }
-        } catch (Exception e) {
-            return getContext().getCoreLibrary().getNilObject();
+        } else {
+            return false;
         }
     }
 

diff --git a/core/src/main/java/org/jruby/truffle/runtime/ObjectIDOperations.java b/core/src/main/java/org/jruby/truffle/runtime/ObjectIDOperations.java
@@ -36,16 +36,16 @@
  */
 public abstract class ObjectIDOperations {
 
-    public static int FALSE = 0;
-    public static int TRUE = 2;
-    public static int NIL = 4;
-    public static int FIRST_OBJECT_ID = 6;
+    public static final int FALSE = 0;
+    public static final int TRUE = 2;
+    public static final int NIL = 4;
+    public static final int FIRST_OBJECT_ID = 6;
 
-    private static BigInteger LARGE_FIXNUM_FLAG = BigInteger.ONE.shiftLeft(64);
-    private static BigInteger FLOAT_FLAG = BigInteger.ONE.shiftLeft(65);
+    private static final BigInteger LARGE_FIXNUM_FLAG = BigInteger.ONE.shiftLeft(64);
+    private static final BigInteger FLOAT_FLAG = BigInteger.ONE.shiftLeft(65);
 
-    private static long SMALL_FIXNUM_MIN = -(1L << 62);
-    private static long SMALL_FIXNUM_MAX = (1L << 62) - 1;
+    private static final long SMALL_FIXNUM_MIN = -(1L << 62);
+    private static final long SMALL_FIXNUM_MAX = (1L << 62) - 1;
 
     // primitive => ID
 

diff --git a/core/src/main/java/org/jruby/truffle/runtime/RubyOperations.java b/core/src/main/java/org/jruby/truffle/runtime/RubyOperations.java
@@ -72,7 +72,8 @@ public Map<Object, Object> getInstanceVariables(RubyBasicObject receiver) {
 
     @CompilerDirectives.TruffleBoundary
     public Object[] getFieldNames(RubyBasicObject receiver) {
-        return receiver.getDynamicObject().getShape().getKeyList().toArray(new Object[0]);
+        List<Object> keys = receiver.getDynamicObject().getShape().getKeyList();
+        return keys.toArray(new Object[keys.size()]);
     }
 
     @CompilerDirectives.TruffleBoundary

diff --git a/core/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java b/core/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java
@@ -124,7 +124,7 @@ public static double toDouble(Object value) {
 
         assert value != null;
 
-        if (value instanceof RubyNilClass || value instanceof RubyNilClass) {
+        if (value instanceof RubyNilClass) {
             return 0;
         }
 

diff --git a/core/src/main/java/org/jruby/truffle/runtime/core/RubyBasicObject.java b/core/src/main/java/org/jruby/truffle/runtime/core/RubyBasicObject.java
@@ -36,7 +36,7 @@ public class RubyBasicObject {
     public static final InternalName OBJECT_ID_IDENTIFIER = new InternalName("object_id");
     public static final InternalName TAINTED_IDENTIFIER = new InternalName("tainted?");
 
-    public static Layout LAYOUT = Layout.createLayout(Layout.INT_TO_LONG);
+    public static final Layout LAYOUT = Layout.createLayout(Layout.INT_TO_LONG);
 
     private final DynamicObject dynamicObject;
 

diff --git a/core/src/main/java/org/jruby/truffle/runtime/core/RubyFiber.java b/core/src/main/java/org/jruby/truffle/runtime/core/RubyFiber.java
@@ -32,7 +32,7 @@ public class RubyFiber extends RubyBasicObject {
     private interface FiberMessage {
     }
 
-    private class FiberResumeMessage implements FiberMessage {
+    private static class FiberResumeMessage implements FiberMessage {
 
         private final RubyThread thread;
         private final RubyFiber sendingFiber;
@@ -58,10 +58,10 @@ public Object getArg() {
 
     }
 
-    private class FiberExitMessage implements FiberMessage {
+    private static class FiberExitMessage implements FiberMessage {
     }
 
-    public class FiberExitException extends ControlFlowException {
+    public static class FiberExitException extends ControlFlowException {
 
         private static final long serialVersionUID = 1522270454305076317L;
 

diff --git a/core/src/main/java/org/jruby/truffle/runtime/core/RubyModule.java b/core/src/main/java/org/jruby/truffle/runtime/core/RubyModule.java
@@ -18,6 +18,7 @@
 import com.oracle.truffle.api.frame.FrameSlot;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.utilities.CyclicAssumption;
+
 import org.jruby.runtime.Visibility;
 import org.jruby.truffle.nodes.RubyNode;
 import org.jruby.truffle.nodes.objects.Allocator;
@@ -456,7 +457,10 @@ public boolean hasNext() {
         }
 
         @Override
-        public RubyModule next() {
+        public RubyModule next() throws NoSuchElementException {
+            if (!hasNext()) {
+                throw new NoSuchElementException();
+            }
             ModuleChain mod = module;
             module = module.getParentModule();
             return mod.getActualModule();

diff --git a/core/src/main/java/org/jruby/truffle/runtime/core/RubyRegexp.java b/core/src/main/java/org/jruby/truffle/runtime/core/RubyRegexp.java
@@ -163,29 +163,15 @@ public Object matchCommon(ByteList bytes, boolean operator, boolean setNamedCapt
 
                 // Copied from jruby/RubyRegexp - see copyright notice there
 
-                if (region == null) {
-                    if (nth >= 1 || (nth < 0 && ++nth <= 0)) {
-                        value = getContext().getCoreLibrary().getNilObject();
-                    } else {
-                        final int start = matcher.getBegin();
-                        final int end = matcher.getEnd();
-                        if (start != -1) {
-                            value = new RubyString(context.getCoreLibrary().getStringClass(), bytes.makeShared(start, end - start).dup());
-                        } else {
-                            value = getContext().getCoreLibrary().getNilObject();
-                        }
-                    }
+                if (nth >= region.numRegs || (nth < 0 && (nth+=region.numRegs) <= 0)) {
+                    value = getContext().getCoreLibrary().getNilObject();
                 } else {
-                    if (nth >= region.numRegs || (nth < 0 && (nth+=region.numRegs) <= 0)) {
-                        value = getContext().getCoreLibrary().getNilObject();
+                    final int start = region.beg[nth];
+                    final int end = region.end[nth];
+                    if (start != -1) {
+                        value = new RubyString(context.getCoreLibrary().getStringClass(), bytes.makeShared(start, end - start).dup());
                     } else {
-                        final int start = region.beg[nth];
-                        final int end = region.end[nth];
-                        if (start != -1) {
-                            value = new RubyString(context.getCoreLibrary().getStringClass(), bytes.makeShared(start, end - start).dup());
-                        } else {
-                            value = getContext().getCoreLibrary().getNilObject();
-                        }
+                        value = getContext().getCoreLibrary().getNilObject();
                     }
                 }
 

diff --git a/core/src/main/java/org/jruby/truffle/runtime/subsystems/ObjectSpaceManager.java b/core/src/main/java/org/jruby/truffle/runtime/subsystems/ObjectSpaceManager.java
@@ -34,7 +34,7 @@
  */
 public class ObjectSpaceManager {
 
-    private class FinalizerReference extends WeakReference<RubyBasicObject> {
+    private static class FinalizerReference extends WeakReference<RubyBasicObject> {
 
         public List<RubyProc> finalizers = new LinkedList<>();
 

diff --git a/core/src/main/java/org/jruby/truffle/translator/BodyTranslator.java b/core/src/main/java/org/jruby/truffle/translator/BodyTranslator.java
@@ -515,7 +515,7 @@ public RubyNode visitCallNodeExtraArgument(CallNode node, RubyNode extraArgument
         return translated;
     }
 
-    protected class ArgumentsAndBlockTranslation {
+    protected static class ArgumentsAndBlockTranslation {
 
         private final RubyNode block;
         private final RubyNode[] arguments;
@@ -2011,7 +2011,7 @@ public RubyNode visitOpAsgnAndNode(org.jruby.ast.OpAsgnAndNode node) {
 
     @Override
     public RubyNode visitOpAsgnNode(org.jruby.ast.OpAsgnNode node) {
-        if (node.getOperatorName() == "||") {
+        if (node.getOperatorName().equals("||")) {
             // Why does this ||= come through as a visitOpAsgnNode and not a visitOpAsgnOrNode?
 
             final String temp = environment.allocateLocalTemp("opassign");
@@ -2222,7 +2222,7 @@ public RubyNode visitRegexpNode(org.jruby.ast.RegexpNode node) {
 
     public static boolean all7Bit(byte[] bytes) {
         for (int n = 0; n < bytes.length; n++) {
-            if (bytes[n] < 0 || bytes[n] > Byte.MAX_VALUE) {
+            if (bytes[n] < 0) {
                 return false;
             }