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 2022/07/22 06:53:34 UTC
[groovy] branch GROOVY_4_0_X updated (a671a4cea0 -> 1602f63d11)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a change to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
from a671a4cea0 GROOVY-10689: phase can now be changed but keep at original in first instance in GROOVY_4_0_X while we gather feedback
new dd98013978 GROOVY-10696: The DGM removeAll variants which take a closure can be refactored to use removeIf for better efficiency
new 1602f63d11 GROOVY-10697: @ToString throws an NPE for POJO with null field
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/groovy/ast/tools/MethodCallUtils.java | 7 +++++
.../groovy/runtime/DefaultGroovyMethods.java | 26 +++---------------
...per.java => BooleanClosureForMapPredicate.java} | 32 ++++++++--------------
.../runtime/callsite/BooleanClosurePredicate.java} | 27 +++++++++---------
.../transform/ToStringASTTransformation.java | 7 +++--
.../groovy/transform/ToStringTransformTest.groovy | 15 ++++++++++
.../console/ui/AstNodeToScriptAdapterTest.groovy | 2 +-
7 files changed, 56 insertions(+), 60 deletions(-)
copy src/main/java/org/codehaus/groovy/runtime/callsite/{BooleanClosureWrapper.java => BooleanClosureForMapPredicate.java} (56%)
copy src/main/java/{groovy/util/ClosureComparator.java => org/codehaus/groovy/runtime/callsite/BooleanClosurePredicate.java} (57%)
[groovy] 01/02: GROOVY-10696: The DGM removeAll variants which take a closure can be refactored to use removeIf for better efficiency
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit dd98013978df4ce84873ebf733083bd141ed44e4
Author: Paul King <pa...@asert.com.au>
AuthorDate: Thu Jul 21 11:12:38 2022 +1000
GROOVY-10696: The DGM removeAll variants which take a closure can be refactored to use removeIf for better efficiency
---
.../groovy/runtime/DefaultGroovyMethods.java | 26 ++--------
.../callsite/BooleanClosureForMapPredicate.java | 59 ++++++++++++++++++++++
.../runtime/callsite/BooleanClosurePredicate.java | 43 ++++++++++++++++
3 files changed, 106 insertions(+), 22 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
index b9e026c1a1..f9260d6b66 100644
--- a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
+++ b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
@@ -64,6 +64,8 @@ import org.codehaus.groovy.reflection.MixinInMetaClass;
import org.codehaus.groovy.reflection.ReflectionCache;
import org.codehaus.groovy.reflection.ReflectionUtils;
import org.codehaus.groovy.reflection.stdclasses.CachedSAMClass;
+import org.codehaus.groovy.runtime.callsite.BooleanClosureForMapPredicate;
+import org.codehaus.groovy.runtime.callsite.BooleanClosurePredicate;
import org.codehaus.groovy.runtime.callsite.BooleanClosureWrapper;
import org.codehaus.groovy.runtime.callsite.BooleanReturningMethodInvoker;
import org.codehaus.groovy.runtime.dgmimpl.NumberNumberDiv;
@@ -4995,17 +4997,7 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport {
* @since 1.7.2
*/
public static <T> boolean removeAll(Collection<T> self, @ClosureParams(FirstParam.FirstGenericType.class) Closure condition) {
- Iterator iter = InvokerHelper.asIterator(self);
- BooleanClosureWrapper bcw = new BooleanClosureWrapper(condition);
- boolean result = false;
- while (iter.hasNext()) {
- Object value = iter.next();
- if (bcw.call(value)) {
- iter.remove();
- result = true;
- }
- }
- return result;
+ return self.removeIf(new BooleanClosurePredicate<>(condition));
}
/**
@@ -5027,17 +5019,7 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport {
* @since 2.5.0
*/
public static <K, V> boolean removeAll(Map<K, V> self, @ClosureParams(MapEntryOrKeyValue.class) Closure condition) {
- Iterator<Map.Entry<K, V>> iter = self.entrySet().iterator();
- BooleanClosureWrapper bcw = new BooleanClosureWrapper(condition);
- boolean result = false;
- while (iter.hasNext()) {
- Map.Entry<K, V> entry = iter.next();
- if (bcw.callForMap(entry)) {
- iter.remove();
- result = true;
- }
- }
- return result;
+ return self.entrySet().removeIf(new BooleanClosureForMapPredicate<>(condition));
}
/**
diff --git a/src/main/java/org/codehaus/groovy/runtime/callsite/BooleanClosureForMapPredicate.java b/src/main/java/org/codehaus/groovy/runtime/callsite/BooleanClosureForMapPredicate.java
new file mode 100644
index 0000000000..5d97a06b04
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/runtime/callsite/BooleanClosureForMapPredicate.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.runtime.callsite;
+
+import groovy.lang.Closure;
+
+import java.util.Map;
+import java.util.function.Predicate;
+
+/**
+ * Helper class for internal use only.
+ * This creates a Predicate by calling a {@link Closure} and converting the result to a boolean.
+ * {@link BooleanReturningMethodInvoker} is used for caching.
+ */
+public class BooleanClosureForMapPredicate<K, V> implements Predicate<Map.Entry<K, V>> {
+ private final BooleanReturningMethodInvoker bmi;
+ private final Closure wrapped;
+ private final int numberOfArguments;
+
+ public BooleanClosureForMapPredicate(Closure wrapped) {
+ this.wrapped = wrapped;
+ bmi = new BooleanReturningMethodInvoker("call");
+ numberOfArguments = wrapped.getMaximumNumberOfParameters();
+ }
+
+ private boolean call(Object... args) {
+ return bmi.invoke(wrapped, args);
+ }
+
+ /**
+ * If the call to the backing {@link Closure} is done on a {@link Closure}
+ * taking one argument, then we give in the {@link Map.Entry}, otherwise we will
+ * give in the key and value.
+ */
+ @Override
+ public boolean test(Map.Entry<K, V> entry) {
+ if (numberOfArguments == 2) {
+ return call(entry.getKey(), entry.getValue());
+ } else {
+ return call(entry);
+ }
+ }
+}
diff --git a/src/main/java/org/codehaus/groovy/runtime/callsite/BooleanClosurePredicate.java b/src/main/java/org/codehaus/groovy/runtime/callsite/BooleanClosurePredicate.java
new file mode 100644
index 0000000000..2ad3b3e9db
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/runtime/callsite/BooleanClosurePredicate.java
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.runtime.callsite;
+
+import groovy.lang.Closure;
+
+import java.util.function.Predicate;
+
+/**
+ * Helper class for internal use only.
+ * This creates a Predicate by calling a {@link Closure} and converting the result to a boolean.
+ * {@link BooleanReturningMethodInvoker} is used for caching.
+ */
+public class BooleanClosurePredicate<T> implements Predicate<T> {
+ private final BooleanReturningMethodInvoker bmi;
+ private final Closure wrapped;
+
+ public BooleanClosurePredicate(Closure wrapped) {
+ this.wrapped = wrapped;
+ this.bmi = new BooleanReturningMethodInvoker("call");
+ }
+
+ @Override
+ public boolean test(T arg) {
+ return bmi.invoke(wrapped, arg);
+ }
+}
[groovy] 02/02: GROOVY-10697: @ToString throws an NPE for POJO with null field
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 1602f63d11db8edb1f2848295fa9d9a305699567
Author: Paul King <pa...@asert.com.au>
AuthorDate: Thu Jul 21 16:42:59 2022 +1000
GROOVY-10697: @ToString throws an NPE for POJO with null field
---
.../java/org/apache/groovy/ast/tools/MethodCallUtils.java | 7 +++++++
.../groovy/transform/ToStringASTTransformation.java | 7 ++++---
.../groovy/transform/ToStringTransformTest.groovy | 15 +++++++++++++++
.../groovy/console/ui/AstNodeToScriptAdapterTest.groovy | 2 +-
4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/src/main/java/org/apache/groovy/ast/tools/MethodCallUtils.java b/src/main/java/org/apache/groovy/ast/tools/MethodCallUtils.java
index 6b06eb1136..820cbc6a4e 100644
--- a/src/main/java/org/apache/groovy/ast/tools/MethodCallUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/MethodCallUtils.java
@@ -23,7 +23,10 @@ import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.stmt.Statement;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.isNullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ternaryX;
/**
* Utility class for commonly called methods
@@ -43,4 +46,8 @@ public class MethodCallUtils {
toString.setImplicitThis(false);
return toString;
}
+
+ public static Expression maybeNullToStringX(final Expression object) {
+ return ternaryX(isNullX(object), constX("null"), toStringX(object));
+ }
}
diff --git a/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
index 1924ea6864..759201e10d 100644
--- a/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
@@ -38,7 +38,7 @@ import org.codehaus.groovy.ast.stmt.Statement;
import org.codehaus.groovy.control.CompilePhase;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.runtime.DefaultGroovyMethods;
-import org.codehaus.groovy.runtime.InvokerHelper;
+import org.codehaus.groovy.runtime.FormatHelper;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import java.util.ArrayList;
@@ -49,6 +49,7 @@ import java.util.Set;
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.apache.groovy.ast.tools.MethodCallUtils.appendS;
+import static org.apache.groovy.ast.tools.MethodCallUtils.maybeNullToStringX;
import static org.apache.groovy.ast.tools.MethodCallUtils.toStringX;
import static org.codehaus.groovy.ast.ClassHelper.make;
import static org.codehaus.groovy.ast.tools.GeneralUtils.assignS;
@@ -82,7 +83,7 @@ public class ToStringASTTransformation extends AbstractASTTransformation {
static final ClassNode MY_TYPE = make(MY_CLASS);
static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
private static final ClassNode STRINGBUILDER_TYPE = make(StringBuilder.class);
- private static final ClassNode INVOKER_TYPE = make(InvokerHelper.class);
+ private static final ClassNode FORMAT_TYPE = make(FormatHelper.class);
private static final ClassNode POJO_TYPE = make(POJO.class);
private static final String TO_STRING = "toString";
private static final String UNDER_TO_STRING = "_toString";
@@ -272,7 +273,7 @@ public class ToStringASTTransformation extends AbstractASTTransformation {
final Statement appendValue = ignoreNulls ? ifS(notNullX(value), thenBlock) : thenBlock;
appendCommaIfNotFirst(thenBlock, result, first, delims);
appendPrefix(thenBlock, result, name, includeNames, delims);
- Expression toString = pojo ? toStringX(value) : callX(INVOKER_TYPE, TO_STRING, value);
+ Expression toString = pojo ? maybeNullToStringX(value) : callX(FORMAT_TYPE, TO_STRING, value);
if (canBeSelf) {
thenBlock.addStatement(ifElseS(
sameX(value, varX("this")),
diff --git a/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy b/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy
index 12eb461640..9625ab5cdf 100644
--- a/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/ToStringTransformTest.groovy
@@ -514,4 +514,19 @@ class ToStringTransformTest extends GroovyShellTestCase {
"""
}
+ // GROOVY-10697
+ void testToStringOnPojoWithNullField() {
+ assertScript '''
+ import groovy.transform.*
+
+ @ToString(pojo=true)
+ @CompileStatic
+ class Clazz {
+ String field = null
+ }
+
+ assert new Clazz().toString() == 'Clazz(null)'
+ '''
+ }
+
}
diff --git a/subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy b/subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy
index b8a9cec72b..136862307a 100644
--- a/subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy
+++ b/subprojects/groovy-console/src/test/groovy/groovy/console/ui/AstNodeToScriptAdapterTest.groovy
@@ -579,7 +579,7 @@ final class AstNodeToScriptAdapterTest extends GroovyTestCase {
String result = compileToScript(script, CompilePhase.CANONICALIZATION)
// we had problems with the ast transform passing a VariableExpression as StaticMethodCallExpression arguments
- assert result.contains("_result.append(org.codehaus.groovy.runtime.InvokerHelper.toString(this.getWhen())")
+ assert result.contains("_result.append(org.codehaus.groovy.runtime.FormatHelper.toString(this.getWhen())")
}
void testAtImmutableClassWithProperties() {