You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/03/19 13:20:52 UTC
[groovy] 01/04: reduce SpotBugs warnings
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 8913c291bc2737894c24590b4adbdb4239cef5d2
Author: Paul King <pa...@asert.com.au>
AuthorDate: Wed Mar 18 18:21:34 2020 +1000
reduce SpotBugs warnings
---
build.gradle | 5 +-
.../groovy/swing/binding/JListProperties.groovy | 18 +++++--
.../groovy/groovy/swing/factory/BindFactory.groovy | 4 +-
.../main/java/groovy/swing/table/TableSorter.java | 2 +-
.../org/apache/groovy/swing/binding/BindPath.java | 12 +++--
.../apache/groovy/swing/binding/BindingProxy.java | 3 +-
.../groovy/groovy/text/SimpleTemplateEngine.java | 5 +-
.../groovy/text/StreamingTemplateEngine.java | 9 ++--
.../main/groovy/groovy/text/TemplateEngine.java | 57 +++++++++++++++++-----
.../main/groovy/groovy/text/XmlTemplateEngine.java | 10 ++--
10 files changed, 89 insertions(+), 36 deletions(-)
diff --git a/build.gradle b/build.gradle
index 3f579b1..c7decd5 100644
--- a/build.gradle
+++ b/build.gradle
@@ -177,8 +177,6 @@ dependencies {
transitive = false
}
- compileOnly "com.github.spotbugs:spotbugs-annotations:$spotbugsAnnotationsVersion"
-
implementation("org.codehaus.gpars:gpars:$gparsVersion") {
exclude(group: 'org.codehaus.groovy', module: 'groovy-all')
}
@@ -369,6 +367,9 @@ task bootstrapJar(type:Jar ) {
}
allprojects {
+ dependencies {
+ compileOnly "com.github.spotbugs:spotbugs-annotations:$spotbugsAnnotationsVersion"
+ }
tasks.withType(JavaCompile) {
options.encoding = 'UTF-8'
diff --git a/subprojects/groovy-swing/src/main/groovy/groovy/swing/binding/JListProperties.groovy b/subprojects/groovy-swing/src/main/groovy/groovy/swing/binding/JListProperties.groovy
index bd2beea..fec5147 100644
--- a/subprojects/groovy-swing/src/main/groovy/groovy/swing/binding/JListProperties.groovy
+++ b/subprojects/groovy-swing/src/main/groovy/groovy/swing/binding/JListProperties.groovy
@@ -18,13 +18,15 @@
*/
package groovy.swing.binding
+import groovy.transform.Synchronized
import org.apache.groovy.swing.binding.FullBinding
import org.apache.groovy.swing.binding.PropertyBinding
import org.apache.groovy.swing.binding.SourceBinding
import org.apache.groovy.swing.binding.TargetBinding
import org.apache.groovy.swing.binding.TriggerBinding
-import javax.swing.*
+import javax.swing.JList
+import javax.swing.ListModel
import javax.swing.event.ListDataEvent
import javax.swing.event.ListDataListener
import javax.swing.event.ListSelectionEvent
@@ -128,19 +130,27 @@ class JListElementsBinding extends AbstractSyntheticBinding implements ListDataL
}
class JListSelectedElementBinding extends AbstractSyntheticBinding implements PropertyChangeListener, ListSelectionListener {
- JList boundList
+ private JList boundList
+
+ @Synchronized JList getBoundList() {
+ return boundList
+ }
+
+ @Synchronized void setBoundList(JList boundList) {
+ this.boundList = boundList
+ }
protected JListSelectedElementBinding(PropertyBinding source, TargetBinding target, String propertyName) {
super(source, target, JList.class, propertyName)
}
- synchronized void syntheticBind() {
+ @Synchronized void syntheticBind() {
boundList = (JList) ((PropertyBinding) sourceBinding).getBean()
boundList.addPropertyChangeListener("selectionModel", this)
boundList.addListSelectionListener(this)
}
- synchronized void syntheticUnbind() {
+ @Synchronized void syntheticUnbind() {
boundList.removePropertyChangeListener("selectionModel", this)
boundList.removeListSelectionListener(this)
boundList = null
diff --git a/subprojects/groovy-swing/src/main/groovy/groovy/swing/factory/BindFactory.groovy b/subprojects/groovy-swing/src/main/groovy/groovy/swing/factory/BindFactory.groovy
index 05d61cf..48d9512 100644
--- a/subprojects/groovy-swing/src/main/groovy/groovy/swing/factory/BindFactory.groovy
+++ b/subprojects/groovy-swing/src/main/groovy/groovy/swing/factory/BindFactory.groovy
@@ -231,7 +231,7 @@ class BindFactory extends AbstractFactory {
fb.bind()
}
- if ((attributes.group instanceof AggregateBinding) && (fb instanceof BindingUpdatable)) {
+ if ((attributes.group instanceof AggregateBinding) && fb != null) {
attributes.remove('group').addBinding(fb)
}
@@ -364,7 +364,7 @@ class BindFactory extends AbstractFactory {
List propertiesToBeSkipped = ['group']
bindAttrs.each { k, v -> if (!(k in propertiesToBeSkipped)) fb."$k" = v }
- if ((bindAttrs.group instanceof AggregateBinding) && (fb instanceof BindingUpdatable)) {
+ if ((bindAttrs.group instanceof AggregateBinding) && fb != null) {
bindAttrs.group.addBinding(fb)
}
diff --git a/subprojects/groovy-swing/src/main/java/groovy/swing/table/TableSorter.java b/subprojects/groovy-swing/src/main/java/groovy/swing/table/TableSorter.java
index 018db20..d46ca9d 100644
--- a/subprojects/groovy-swing/src/main/java/groovy/swing/table/TableSorter.java
+++ b/subprojects/groovy-swing/src/main/java/groovy/swing/table/TableSorter.java
@@ -229,7 +229,7 @@ space and avoid unnecessary heap allocation.
if (high - low < 2) {
return;
}
- int middle = (low + high) / 2;
+ int middle = (low + high) >>> 1;
shuttlesort(to, from, low, middle);
shuttlesort(to, from, middle, high);
diff --git a/subprojects/groovy-swing/src/main/java/org/apache/groovy/swing/binding/BindPath.java b/subprojects/groovy-swing/src/main/java/org/apache/groovy/swing/binding/BindPath.java
index 74450a5..d86a926 100644
--- a/subprojects/groovy-swing/src/main/java/org/apache/groovy/swing/binding/BindPath.java
+++ b/subprojects/groovy-swing/src/main/java/org/apache/groovy/swing/binding/BindPath.java
@@ -210,22 +210,26 @@ public class BindPath {
for (Map.Entry<String, TriggerBinding> syntheticEntry : synthetics.entrySet()) {
if (syntheticEntry.getKey().endsWith(endName)) {
if (localSynthetics == null) {
- localSynthetics = new TreeMap();
+ localSynthetics = new TreeMap<>();
}
localSynthetics.put(syntheticEntry.getKey(), syntheticEntry.getValue());
}
}
}
+ synchronized Map<String, TriggerBinding> getLocalSynthetics() {
+ return localSynthetics;
+ }
+
public TriggerBinding getSyntheticTriggerBinding(Object newObject) {
+ final Map<String, TriggerBinding> localSynthetics = getLocalSynthetics();
if (localSynthetics == null) {
return null;
}
- Class currentClass = newObject.getClass();
+ Class<?> currentClass = newObject.getClass();
while (currentClass != null) {
// should we check interfaces as well? if so at what level?
- TriggerBinding trigger =
- localSynthetics.get(currentClass.getName() + "#" + propertyName);
+ TriggerBinding trigger = localSynthetics.get(currentClass.getName() + "#" + propertyName);
if (trigger != null) {
return trigger;
}
diff --git a/subprojects/groovy-swing/src/main/java/org/apache/groovy/swing/binding/BindingProxy.java b/subprojects/groovy-swing/src/main/java/org/apache/groovy/swing/binding/BindingProxy.java
index 25abed2..dc4df33 100644
--- a/subprojects/groovy-swing/src/main/java/org/apache/groovy/swing/binding/BindingProxy.java
+++ b/subprojects/groovy-swing/src/main/java/org/apache/groovy/swing/binding/BindingProxy.java
@@ -72,6 +72,7 @@ public class BindingProxy extends GroovyObjectSupport implements BindingUpdatabl
public Object getProperty(String property) {
PropertyBinding pb;
+ final Object model = getModel();
synchronized (propertyBindings) {
// should we verify the property is valid?
pb = propertyBindings.get(property);
@@ -88,7 +89,7 @@ public class BindingProxy extends GroovyObjectSupport implements BindingUpdatabl
}
public void setProperty(String property, Object value) {
- throw new ReadOnlyPropertyException(property, model.getClass());
+ throw new ReadOnlyPropertyException(property, getModel().getClass());
}
public void bind() {
diff --git a/subprojects/groovy-templates/src/main/groovy/groovy/text/SimpleTemplateEngine.java b/subprojects/groovy-templates/src/main/groovy/groovy/text/SimpleTemplateEngine.java
index 801874c..ba38410 100644
--- a/subprojects/groovy-templates/src/main/groovy/groovy/text/SimpleTemplateEngine.java
+++ b/subprojects/groovy-templates/src/main/groovy/groovy/text/SimpleTemplateEngine.java
@@ -33,6 +33,7 @@ import java.io.PrintWriter;
import java.io.Reader;
import java.io.Writer;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Processes template source files substituting variables and expressions into
@@ -91,7 +92,7 @@ import java.util.Map;
*/
public class SimpleTemplateEngine extends TemplateEngine {
private boolean verbose;
- private static int counter = 1;
+ private static AtomicInteger counter = new AtomicInteger(0);
private GroovyShell groovyShell;
private boolean escapeBackslash;
@@ -121,7 +122,7 @@ public class SimpleTemplateEngine extends TemplateEngine {
System.out.println("\n-- script end --\n");
}
try {
- template.script = groovyShell.parse(script, "SimpleTemplateScript" + counter++ + ".groovy");
+ template.script = groovyShell.parse(script, "SimpleTemplateScript" + counter.incrementAndGet() + ".groovy");
} catch (Exception e) {
throw new GroovyRuntimeException("Failed to parse template script (your template may contain an error or be trying to use expressions not currently supported): " + e.getMessage());
}
diff --git a/subprojects/groovy-templates/src/main/groovy/groovy/text/StreamingTemplateEngine.java b/subprojects/groovy-templates/src/main/groovy/groovy/text/StreamingTemplateEngine.java
index 09b8b0c..1afcd1b 100644
--- a/subprojects/groovy-templates/src/main/groovy/groovy/text/StreamingTemplateEngine.java
+++ b/subprojects/groovy-templates/src/main/groovy/groovy/text/StreamingTemplateEngine.java
@@ -18,6 +18,7 @@
*/
package groovy.text;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import groovy.lang.Closure;
import groovy.lang.GroovyClassLoader;
import groovy.lang.GroovyCodeSource;
@@ -42,6 +43,7 @@ import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Processes template source files substituting variables and expressions into
@@ -148,7 +150,7 @@ public class StreamingTemplateEngine extends TemplateEngine {
private static final String TEMPLATE_SCRIPT_PREFIX = "StreamingTemplateScript";
private final ClassLoader parentLoader;
- private static int counter = 1;
+ private static AtomicInteger counter = new AtomicInteger(0);
/**
* Create a streaming template engine instance using the default class loader
@@ -372,11 +374,12 @@ public class StreamingTemplateEngine extends TemplateEngine {
}
}
+ @SuppressFBWarnings(value = "SR_NOT_CHECKED", justification = "safe to ignore return value of skip from reader backed by StringReader")
private int getLinesInSource() throws IOException {
int result = 0;
try (LineNumberReader reader = new LineNumberReader(new StringReader(templateSource.toString()))) {
- reader.skip(Long.MAX_VALUE);
+ reader.skip(Long.MAX_VALUE); // SR_NOT_CHECKED
result = reader.getLineNumber();
}
@@ -600,7 +603,7 @@ public class StreamingTemplateEngine extends TemplateEngine {
});
final Class groovyClass;
try {
- groovyClass = loader.parseClass(new GroovyCodeSource(target.toString(), TEMPLATE_SCRIPT_PREFIX + counter++ + ".groovy", "x"));
+ groovyClass = loader.parseClass(new GroovyCodeSource(target.toString(), TEMPLATE_SCRIPT_PREFIX + counter.incrementAndGet() + ".groovy", "x"));
} catch (MultipleCompilationErrorsException e) {
throw mangleMultipleCompilationErrorsException(e, sections);
diff --git a/subprojects/groovy-templates/src/main/groovy/groovy/text/TemplateEngine.java b/subprojects/groovy-templates/src/main/groovy/groovy/text/TemplateEngine.java
index f2282c5..cd43526 100644
--- a/subprojects/groovy-templates/src/main/groovy/groovy/text/TemplateEngine.java
+++ b/subprojects/groovy-templates/src/main/groovy/groovy/text/TemplateEngine.java
@@ -18,42 +18,73 @@
*/
package groovy.text;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import groovy.util.CharsetToolkit;
import org.codehaus.groovy.control.CompilationFailedException;
-import org.codehaus.groovy.runtime.DefaultGroovyMethodsSupport;
import java.io.File;
-import java.io.FileReader;
+import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.StringReader;
import java.net.URL;
+import java.nio.charset.Charset;
/**
- * Represents an API to any template engine which is basically a factory of Template instances from a given text input.
+ * A template engine is a factory for creating a Template instance for a given text input.
*/
public abstract class TemplateEngine {
+ /**
+ * Creates a template by reading content from the Reader.
+ */
public abstract Template createTemplate(Reader reader) throws CompilationFailedException, ClassNotFoundException, IOException;
-
+
+ /**
+ * Creates a template from the String contents.
+ */
public Template createTemplate(String templateText) throws CompilationFailedException, ClassNotFoundException, IOException {
return createTemplate(new StringReader(templateText));
}
-
+
+ /**
+ * Creates a template from the File contents.
+ * If the encoding for the file can be determined, that encoding will be used, otherwise the default encoding will be used.
+ * Consider using {@link #createTemplate(File, Charset)} if you need to explicitly set the encoding.
+ */
public Template createTemplate(File file) throws CompilationFailedException, ClassNotFoundException, IOException {
- Reader reader = new FileReader(file);
- try {
+ CharsetToolkit toolkit = new CharsetToolkit(file);
+ try (Reader reader = toolkit.getReader()) {
+ return createTemplate(reader);
+ }
+ }
+
+ /**
+ * Creates a template from the File contents using the given charset encoding.
+ */
+ public Template createTemplate(File file, Charset cs) throws CompilationFailedException, ClassNotFoundException, IOException {
+ try (Reader reader = new InputStreamReader(new FileInputStream(file), cs)) {
return createTemplate(reader);
- } finally {
- DefaultGroovyMethodsSupport.closeWithWarning(reader);
}
}
+ /**
+ * Creates a template from the content found at the URL using the default encoding.
+ * Please consider using {@link #createTemplate(URL, Charset)}.
+ */
+ @SuppressFBWarnings(value = "DM_DEFAULT_ENCODING", justification = "left for legacy reasons but users expected to heed warning")
public Template createTemplate(URL url) throws CompilationFailedException, ClassNotFoundException, IOException {
- Reader reader = new InputStreamReader(url.openStream());
- try {
+ try (Reader reader = new InputStreamReader(url.openStream())) {
+ return createTemplate(reader);
+ }
+ }
+
+ /**
+ * Creates a template from the content found at the URL using the given charset encoding.
+ */
+ public Template createTemplate(URL url, Charset cs) throws CompilationFailedException, ClassNotFoundException, IOException {
+ try (Reader reader = new InputStreamReader(url.openStream(), cs)) {
return createTemplate(reader);
- } finally {
- DefaultGroovyMethodsSupport.closeWithWarning(reader);
}
}
}
diff --git a/subprojects/groovy-templates/src/main/groovy/groovy/text/XmlTemplateEngine.java b/subprojects/groovy-templates/src/main/groovy/groovy/text/XmlTemplateEngine.java
index 147697d..f4a0075 100644
--- a/subprojects/groovy-templates/src/main/groovy/groovy/text/XmlTemplateEngine.java
+++ b/subprojects/groovy-templates/src/main/groovy/groovy/text/XmlTemplateEngine.java
@@ -41,6 +41,7 @@ import java.io.Writer;
import java.lang.ref.WeakReference;
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Template engine for use in templating scenarios where both the template
@@ -104,7 +105,7 @@ import java.util.Map;
*/
public class XmlTemplateEngine extends TemplateEngine {
- private static int counter = 1;
+ private static AtomicInteger counter = new AtomicInteger(0);
private static class GspPrinter extends XmlNodePrinter {
@@ -255,8 +256,9 @@ public class XmlTemplateEngine extends TemplateEngine {
}
public String toString() {
- if (result.get() != null) {
- return result.get().toString();
+ Object o = result.get();
+ if (o != null) {
+ return o.toString();
}
String string = writeTo(new StringBuilderWriter(1024)).toString();
result = new WeakReference<>(string);
@@ -308,7 +310,7 @@ public class XmlTemplateEngine extends TemplateEngine {
Script script;
try {
- script = groovyShell.parse(writer.toString(), "XmlTemplateScript" + counter++ + ".groovy");
+ script = groovyShell.parse(writer.toString(), "XmlTemplateScript" + counter.incrementAndGet() + ".groovy");
} catch (Exception e) {
throw new GroovyRuntimeException("Failed to parse template script (your template may contain an error or be trying to use expressions not currently supported): " + e.getMessage());
}