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 <[email protected]> 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()); }
