Merge pull request #3319 from jruby/test-findbugs-2

find-bugs 2
jruby · Sep 11, 2015 · 976c968 · 976c968
2 parents 8b68a14 + e82eaeb
commit 976c968
Showing 44 changed files with 171 additions and 194 deletions.
diff --git a/core/src/main/java/org/jruby/BasicObjectStub.java b/core/src/main/java/org/jruby/BasicObjectStub.java
@@ -190,7 +190,7 @@ public static RubyString convertToString(IRubyObject self) {
     public static IRubyObject anyToString(IRubyObject self) {
         String cname = getMetaClass(self).getRealClass().getName();
         /* 6:tags 16:addr 1:eos */
-        RubyString str = getRuntime(self).newString("#<" + cname + ":0x" + Integer.toHexString(System.identityHashCode(self)) + ">");
+        RubyString str = getRuntime(self).newString("#<" + cname + ":0x" + Integer.toHexString(System.identityHashCode(self)) + '>');
         str.setTaint(isTaint(self));
         return str;
     }

diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -2864,7 +2864,7 @@ public RubyModule getClassFromPath(String path) {
 
             IRubyObject cc = c.getConstant(str);
             if(!(cc instanceof RubyModule)) {
-                throw newTypeError("" + path + " does not refer to class/module");
+                throw newTypeError(path + " does not refer to class/module");
             }
             c = (RubyModule)cc;
         }

diff --git a/core/src/main/java/org/jruby/RubyDir.java b/core/src/main/java/org/jruby/RubyDir.java
@@ -701,8 +701,8 @@ protected static JRubyFile getDirForRmdir(final Ruby runtime, final String path)
         // dir can't be read, so we check permissions first
 
         // no permission
-        if (directory.getParentFile().exists() &&
-                !directory.getParentFile().canWrite()) {
+        File parentFile = directory.getParentFile();
+        if (parentFile.exists() && ! parentFile.canWrite()) {
             throw runtime.newErrnoEACCESError(path);
         }
 

diff --git a/core/src/main/java/org/jruby/RubyFile.java b/core/src/main/java/org/jruby/RubyFile.java
@@ -353,11 +353,12 @@ public IRubyObject chmod(ThreadContext context, IRubyObject arg) {
         checkClosed(context);
         int mode = (int) arg.convertToInteger().getLongValue();
 
-        if (!new File(getPath()).exists()) {
-            throw context.runtime.newErrnoENOENTError(getPath());
+        final String path = getPath();
+        if ( ! new File(path).exists() ) {
+            throw context.runtime.newErrnoENOENTError(path);
         }
 
-        return context.runtime.newFixnum(context.runtime.getPosix().chmod(getPath(), mode));
+        return context.runtime.newFixnum(context.runtime.getPosix().chmod(path, mode));
     }
 
     @JRubyMethod(required = 2)
@@ -373,11 +374,12 @@ public IRubyObject chown(ThreadContext context, IRubyObject arg1, IRubyObject ar
             group = RubyNumeric.num2int(arg2);
         }
 
-        if (!new File(getPath()).exists()) {
-            throw context.runtime.newErrnoENOENTError(getPath());
+        final String path = getPath();
+        if ( ! new File(path).exists() ) {
+            throw context.runtime.newErrnoENOENTError(path);
         }
 
-        return context.runtime.newFixnum(context.runtime.getPosix().chown(getPath(), owner, group));
+        return context.runtime.newFixnum(context.runtime.getPosix().chown(path, owner, group));
     }
 
     @JRubyMethod
@@ -436,8 +438,9 @@ public static IRubyObject path(ThreadContext context, IRubyObject self, IRubyObj
     @JRubyMethod(name = {"path", "to_path"})
     public IRubyObject path(ThreadContext context) {
         IRubyObject newPath = context.runtime.getNil();
-        if (getPath() != null) {
-            newPath = context.runtime.newString(getPath());
+        final String path = getPath();
+        if (path != null) {
+            newPath = context.runtime.newString(path);
             newPath.setTaint(true);
         }
         return newPath;
@@ -1733,7 +1736,7 @@ public static String expandUserPath(ThreadContext context, String path, boolean
                     throw context.runtime.newArgumentError("user " + user + " does not exist");
                 }
 
-                path = "" + dir + (pathLength == userEnd ? "" : path.substring(userEnd));
+                path = dir + (pathLength == userEnd ? "" : path.substring(userEnd));
 
                 // getpwd (or /etc/passwd fallback) returns a home which is not absolute!!! [mecha-unlikely]
                 if (raiseOnRelativePath && !isAbsolutePath(path)) throw context.runtime.newArgumentError("non-absolute home of " + user);

diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java
@@ -2041,7 +2041,7 @@ public void clear() {
     public boolean equals(Object other) {
         if (!(other instanceof RubyHash)) return false;
         if (this == other) return true;
-        return op_equal(getRuntime().getCurrentContext(), (RubyHash)other).isTrue() ? true : false;
+        return op_equal(getRuntime().getCurrentContext(), (RubyHash)other).isTrue();
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java
@@ -4319,44 +4319,42 @@ public ByteList allocate(int size) {
     protected IOOptions updateIOOptionsFromOptions(ThreadContext context, RubyHash options, IOOptions ioOptions) {
         if (options == null || options.isNil()) return ioOptions;
 
-        Ruby runtime = context.runtime;
+        final Ruby runtime = context.runtime;
 
-        if (options.containsKey(runtime.newSymbol("mode"))) {
-            ioOptions = parseIOOptions(options.fastARef(runtime.newSymbol("mode")));
+        final RubySymbol mode = runtime.newSymbol("mode");
+        if (options.containsKey(mode)) {
+            ioOptions = parseIOOptions(options.fastARef(mode));
         }
 
         // This duplicates the non-error behavior of MRI 1.9: the
         // :binmode option is ORed in with other options. It does
         // not obliterate what came before.
 
-        if (options.containsKey(runtime.newSymbol("binmode")) &&
-                options.fastARef(runtime.newSymbol("binmode")).isTrue()) {
-
+        final RubySymbol binmode = runtime.newSymbol("binmode");
+        if (options.containsKey(binmode) && options.fastARef(binmode).isTrue()) {
             ioOptions = newIOOptions(runtime, ioOptions, ModeFlags.BINARY);
         }
 
         // This duplicates the non-error behavior of MRI 1.9: the
         // :binmode option is ORed in with other options. It does
         // not obliterate what came before.
 
-        if (options.containsKey(runtime.newSymbol("binmode")) &&
-                options.fastARef(runtime.newSymbol("binmode")).isTrue()) {
-
+        if (options.containsKey(binmode) && options.fastARef(binmode).isTrue()) {
             ioOptions = newIOOptions(runtime, ioOptions, ModeFlags.BINARY);
         }
 
-        if (options.containsKey(runtime.newSymbol("textmode")) &&
-                options.fastARef(runtime.newSymbol("textmode")).isTrue()) {
-
+        final RubySymbol textmode = runtime.newSymbol("textmode");
+        if (options.containsKey(textmode) && options.fastARef(textmode).isTrue()) {
             ioOptions = newIOOptions(runtime, ioOptions, ModeFlags.TEXT);
         }
 
+        final RubySymbol open_args = runtime.newSymbol("open_args");
         // TODO: Waaaay different than MRI.  They uniformly have all opening logic
         // do a scan of args before anything opens.  We do this logic in a less
         // consistent way.  We should consider re-impling all IO/File construction
         // logic.
-        if (options.containsKey(runtime.newSymbol("open_args"))) {
-            IRubyObject args = options.fastARef(runtime.newSymbol("open_args"));
+        if (options.containsKey(open_args)) {
+            IRubyObject args = options.fastARef(open_args);
 
             RubyArray openArgs = args.convertToArray();
 
@@ -4669,11 +4667,6 @@ public IRubyObject getline(Ruby runtime, ByteList separator, long limit) {
         return getline(runtime.getCurrentContext(), runtime.newString(separator), limit, null);
     }
 
-    @Deprecated
-    private IRubyObject getline(ThreadContext context, IRubyObject separator, ByteListCache cache) {
-        return getline(context, separator, -1, cache);
-    }
-
     @Deprecated
     public IRubyObject getline(ThreadContext context, ByteList separator) {
         return getline(context, RubyString.newString(context.runtime, separator), -1, null);

diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -552,7 +552,8 @@ private String calculateName() {
     private String calculateAnonymousName() {
         if (anonymousName == null) {
             // anonymous classes get the #<Class:0xdeadbeef> format
-            StringBuilder anonBase = new StringBuilder("#<" + metaClass.getRealClass().getName() + ":0x");
+            StringBuilder anonBase = new StringBuilder(24);
+            anonBase.append("#<").append(metaClass.getRealClass().getName()).append(":0x");
             anonBase.append(Integer.toHexString(System.identityHashCode(this))).append('>');
             anonymousName = anonBase.toString();
         }
@@ -1163,8 +1164,9 @@ public void removeMethod(ThreadContext context, String name) {
 
         // We can safely reference methods here instead of doing getMethods() since if we
         // are adding we are not using a IncludedModule.
-        synchronized(methodLocation.getMethodsForWrite()) {
-            DynamicMethod method = (DynamicMethod) methodLocation.getMethodsForWrite().remove(name);
+        Map<String, DynamicMethod> methodsForWrite = methodLocation.getMethodsForWrite();
+        synchronized (methodsForWrite) {
+            DynamicMethod method = (DynamicMethod) methodsForWrite.remove(name);
             if (method == null) {
                 throw runtime.newNameError("method '" + name + "' not defined in " + getName(), name);
             }

diff --git a/core/src/main/java/org/jruby/RubyProc.java b/core/src/main/java/org/jruby/RubyProc.java
@@ -211,13 +211,14 @@ public IRubyObject to_s() {
 
     @JRubyMethod(name = "to_s", alias = "inspect")
     public IRubyObject to_s19() {
-        StringBuilder sb = new StringBuilder("#<Proc:0x" + Integer.toString(block.hashCode(), 16));
-        String file = block.getBody().getFile();
+        StringBuilder sb = new StringBuilder(32);
+        sb.append("#<Proc:0x").append(Integer.toString(block.hashCode(), 16));
 
+        String file = block.getBody().getFile();
         if (file != null) sb.append('@').append(file).append(':').append(block.getBody().getLine() + 1);
 
         if (isLambda()) sb.append(" (lambda)");
-        sb.append(">");
+        sb.append('>');
 
         IRubyObject string = RubyString.newString(getRuntime(), sb.toString());
 

diff --git a/core/src/main/java/org/jruby/RubyString.java b/core/src/main/java/org/jruby/RubyString.java
@@ -2129,7 +2129,7 @@ public RubyString concat19(ThreadContext context, IRubyObject other) {
         if (other instanceof RubyFixnum) {
             long c = RubyNumeric.num2long(other);
             if (c < 0) {
-                throw runtime.newRangeError("" + c + " out of char range");
+                throw runtime.newRangeError(c + " out of char range");
             }
             return concatNumeric(runtime, (int)(c & 0xFFFFFFFF));
         } else if (other instanceof RubyBignum) {
@@ -3715,19 +3715,21 @@ private static IRubyObject scanOnce(ThreadContext context, RubyString str, IRuby
 
         if (patternSearch(context, pat, str, startp[0], true) >= 0) {
             match = (RubyMatchData)context.getBackRef();
-            if (match.begin(0) == match.end(0)) {
+            final int matchEnd = match.end(0);
+            if (match.begin(0) == matchEnd) {
                 Encoding enc = str.getEncoding();
                 /*
                  * Always consume at least one character of the input string
                  */
-                if (str.size() > match.end(0)) {
-                    startp[0] = match.end(0) + encFastMBCLen(str.value.unsafeBytes(), str.value.begin() + match.end(0),
-                            str.value.begin() + str.value.realSize(), enc);
+                if (str.size() > matchEnd) {
+                    final ByteList strValue = str.value;
+                    startp[0] = matchEnd + encFastMBCLen(strValue.unsafeBytes(), strValue.begin() + matchEnd,
+                            strValue.begin() + strValue.realSize(), enc);
                 } else {
-                    startp[0] = match.end(0) + 1;
+                    startp[0] = matchEnd + 1;
                 }
             } else {
-                startp[0] = match.end(0);
+                startp[0] = matchEnd;
             }
             if (match.numRegs() == 1) {
                 return RubyRegexp.nth_match(0, match);
@@ -5185,11 +5187,10 @@ public IRubyObject to_c(ThreadContext context) {
 
         RubyArray a = RubyComplex.str_to_c_internal(context, s);
 
-        if (!a.eltInternal(0).isNil()) {
-            return a.eltInternal(0);
-        } else {
-            return RubyComplex.newComplexCanonicalize(context, RubyFixnum.zero(runtime));
-        }
+        IRubyObject first = a.eltInternal(0);
+        if ( ! first.isNil() ) return first;
+
+        return RubyComplex.newComplexCanonicalize(context, RubyFixnum.zero(runtime));
     }
 
     /** string_to_r
@@ -5205,11 +5206,10 @@ public IRubyObject to_r(ThreadContext context) {
 
         RubyArray a = RubyRational.str_to_r_internal(context, s);
 
-        if (!a.eltInternal(0).isNil()) {
-            return a.eltInternal(0);
-        } else {
-            return RubyRational.newRationalCanonicalize(context, RubyFixnum.zero(runtime));
-        }
+        IRubyObject first = a.eltInternal(0);
+        if ( ! first.isNil() ) return first;
+
+        return RubyRational.newRationalCanonicalize(context, RubyFixnum.zero(runtime));
     }
 
     public static RubyString unmarshalFrom(UnmarshalStream input) throws java.io.IOException {

diff --git a/core/src/main/java/org/jruby/ast/DNode.java b/core/src/main/java/org/jruby/ast/DNode.java
@@ -20,7 +20,7 @@ public DNode(ISourcePosition position) {
     public DNode(ISourcePosition position, Encoding encoding) {
         super(position);
 
-        assert encoding != null: "" + getClass().getName() + " passed in a null encoding";
+        assert encoding != null: getClass().getName() + " passed in a null encoding";
 
         this.encoding = encoding;
     }

diff --git a/core/src/main/java/org/jruby/ast/java_signature/AnnotationParameter.java b/core/src/main/java/org/jruby/ast/java_signature/AnnotationParameter.java
@@ -29,6 +29,6 @@ public AnnotationExpression getExpression() {
 
     @Override
     public String toString() {
-        return "" + name + "=" + expression;
+        return name + '=' + expression;
     }
 }
diff --git a/core/src/main/java/org/jruby/compiler/JITCompiler.java b/core/src/main/java/org/jruby/compiler/JITCompiler.java
@@ -429,7 +429,8 @@ static void log(RubyModule implementationClass, String file, int line, String na
 
         name = isBlock ? "" : "." + name;
 
-        StringBuilder builder = new StringBuilder(message + ":" + className + name + " at " + file + ":" + line);
+        StringBuilder builder = new StringBuilder(32);
+        builder.append(message).append(':').append(className).append(name).append(" at ").append(file).append(':').append(line);
 
         if (reason.length > 0) {
             builder.append(" because of: \"");

diff --git a/core/src/main/java/org/jruby/embed/ScriptingContainer.java b/core/src/main/java/org/jruby/embed/ScriptingContainer.java
@@ -1696,7 +1696,7 @@ public void setReader(Reader reader) {
     public Reader getReader() {
         Map map = getAttributeMap();
         if (map.containsKey(AttributeName.READER)) {
-            return (Reader) getAttributeMap().get(AttributeName.READER);
+            return (Reader) map.get(AttributeName.READER);
         }
         return null;
     }
@@ -1761,7 +1761,7 @@ public void resetWriter() {
     public Writer getWriter() {
         Map map = getAttributeMap();
         if (map.containsKey(AttributeName.WRITER)) {
-            return (Writer) getAttributeMap().get(AttributeName.WRITER);
+            return (Writer) map.get(AttributeName.WRITER);
         }
         return null;
     }
@@ -1825,7 +1825,7 @@ public void resetErrorWriter() {
     public Writer getErrorWriter() {
         Map map = getAttributeMap();
         if (map.containsKey(AttributeName.ERROR_WRITER)) {
-            return (Writer) getAttributeMap().get(AttributeName.ERROR_WRITER);
+            return (Writer) map.get(AttributeName.ERROR_WRITER);
         }
         return null;
     }

diff --git a/core/src/main/java/org/jruby/exceptions/RaiseException.java b/core/src/main/java/org/jruby/exceptions/RaiseException.java
@@ -96,37 +96,39 @@ public RaiseException(Ruby runtime, RubyClass excptnClass, String msg, boolean n
         if (msg == null) {
             msg = "No message available";
         }
-        providedMessage = "(" + excptnClass.getName() + ") " + msg;
+        providedMessage = '(' + excptnClass.getName() + ") " + msg;
         this.nativeException = nativeException;
         if (DEBUG) {
             Thread.dumpStack();
         }
+        final ThreadContext context = runtime.getCurrentContext();
         setException((RubyException) Helpers.invoke(
-                runtime.getCurrentContext(),
+                context,
                 excptnClass,
                 "new",
-                RubyString.newUnicodeString(excptnClass.getRuntime(), msg)),
+                RubyString.newUnicodeString(runtime, msg)),
                 nativeException);
-        preRaise(runtime.getCurrentContext());
+        preRaise(context);
     }
 
     public RaiseException(Ruby runtime, RubyClass excptnClass, String msg, IRubyObject backtrace, boolean nativeException) {
         super(msg);
         if (msg == null) {
             msg = "No message available";
         }
-        providedMessage = "(" + excptnClass.getName() + ") " + msg;
+        providedMessage = '(' + excptnClass.getName() + ") " + msg;
         this.nativeException = nativeException;
         if (DEBUG) {
             Thread.dumpStack();
         }
+        final ThreadContext context = runtime.getCurrentContext();
         setException((RubyException) Helpers.invoke(
-                runtime.getCurrentContext(),
+                context,
                 excptnClass,
                 "new",
-                RubyString.newUnicodeString(excptnClass.getRuntime(), msg)),
+                RubyString.newUnicodeString(runtime, msg)),
                 nativeException);
-        preRaise(runtime.getCurrentContext(), backtrace);
+        preRaise(context, backtrace);
     }
 
     public RaiseException(RubyException exception, boolean isNativeException) {

diff --git a/core/src/main/java/org/jruby/ext/bigdecimal/RubyBigDecimal.java b/core/src/main/java/org/jruby/ext/bigdecimal/RubyBigDecimal.java
@@ -1321,10 +1321,10 @@ public IRubyObject infinite_p(ThreadContext context) {
     public IRubyObject inspect(ThreadContext context) {
         StringBuilder val = new StringBuilder("#<BigDecimal:");
 
-        val.append(Integer.toHexString(System.identityHashCode(this))).append(",");
-        val.append("'").append(callMethod(context, "to_s")).append("'").append(",");
-        val.append(getSignificantDigits().length()).append("(");
-        val.append(((getAllDigits().length() / 4) + 1) * 4).append(")").append(">");
+        val.append( Integer.toHexString(System.identityHashCode(this)) ).append(',');
+        val.append('\'').append( callMethod(context, "to_s") ).append('\'').append(',');
+        val.append( getSignificantDigits().length() ).append('(');
+        val.append( ((getAllDigits().length() / 4) + 1) * 4 ).append(')').append('>');
 
         return getRuntime().newString(val.toString());
     }