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 2018/07/25 02:48:59 UTC

groovy git commit: GROOVY-8639: @Sortable annotation is not able to use accessible parent properties (closes #775)

Repository: groovy
Updated Branches:
  refs/heads/master 7d0eaedca -> 426a54166


GROOVY-8639: @Sortable annotation is not able to use accessible parent properties (closes #775)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/426a5416
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/426a5416
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/426a5416

Branch: refs/heads/master
Commit: 426a54166b8ae53942e818e60892cacb7dfd9a12
Parents: 7d0eaed
Author: Paul King <pa...@asert.com.au>
Authored: Fri Jul 13 19:24:51 2018 +1000
Committer: Paul King <pa...@asert.com.au>
Committed: Wed Jul 25 12:47:47 2018 +1000

----------------------------------------------------------------------
 src/main/groovy/groovy/transform/Sortable.java  | 32 ++++++++-
 .../transform/SortableASTTransformation.java    | 36 ++++++----
 src/spec/doc/core-metaprogramming.adoc          | 27 ++++++-
 .../test/CodeGenerationASTTransformsTest.groovy | 75 ++++++++++++++++++++
 4 files changed, 154 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/426a5416/src/main/groovy/groovy/transform/Sortable.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/transform/Sortable.java b/src/main/groovy/groovy/transform/Sortable.java
index 060aeb0..9a28f9b 100644
--- a/src/main/groovy/groovy/transform/Sortable.java
+++ b/src/main/groovy/groovy/transform/Sortable.java
@@ -26,7 +26,7 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * A class annotation used to make a class Comparable by multiple Comparators.
+ * A class annotation used to make a class Comparable by (potentially) multiple Comparators.
  *
  * As an example, given this class:
  * <pre>
@@ -174,4 +174,34 @@ public @interface Sortable {
      * @since 2.5.0
      */
     boolean reversed() default false;
+
+    /**
+     * Whether to include super properties in the comparison algorithm.
+     * Groovy properties and potentially JavaBean properties (in that order) from superclasses come before
+     * the members from a subclass (unless 'includes' is used to determine the order).
+     *
+     * @since 2.5.2
+     */
+    boolean includeSuperProperties() default false;
+
+    /**
+     * Whether to include all properties (as per the JavaBean spec) in the comparison algorithm.
+     * Groovy recognizes any field-like definitions with no explicit visibility as property definitions
+     * and always includes them in the comparison algorithm. Groovy also treats any explicitly created getXxx() or isYyy()
+     * methods as property getters as per the JavaBean specification.
+     * Set this flag to false explicitly exclude such properties.
+     * JavaBean properties come after any Groovy properties for a given class
+     * (unless 'includes' is used to determine the order).
+     *
+     * @since 2.5.2
+     */
+    boolean allProperties() default true;
+
+    /**
+     * Whether to include all fields and/or properties in the comparison algorithm, including those with names that
+     * are considered internal.
+     *
+     * @since 2.5.2
+     */
+    boolean allNames() default false;
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/426a5416/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java
index 0485e6d..68c0c72 100644
--- a/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/SortableASTTransformation.java
@@ -42,7 +42,9 @@ import org.codehaus.groovy.runtime.StringGroovyMethods;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
 import static org.codehaus.groovy.ast.ClassHelper.make;
@@ -59,6 +61,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.declS;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.eqX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.equalsNullX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.fieldX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.ifS;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.neX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.notNullX;
@@ -98,17 +101,20 @@ public class SortableASTTransformation extends AbstractASTTransformation {
         }
     }
 
-    private void createSortable(AnnotationNode annotation, ClassNode classNode) {
-        List<String> includes = getMemberStringList(annotation, "includes");
-        List<String> excludes = getMemberStringList(annotation, "excludes");
-        boolean reversed = memberHasValue(annotation, "reversed", true);
-        if (!checkIncludeExcludeUndefinedAware(annotation, excludes, includes, MY_TYPE_NAME)) return;
-        if (!checkPropertyList(classNode, includes, "includes", annotation, MY_TYPE_NAME, false)) return;
-        if (!checkPropertyList(classNode, excludes, "excludes", annotation, MY_TYPE_NAME, false)) return;
+    private void createSortable(AnnotationNode anno, ClassNode classNode) {
+        List<String> includes = getMemberStringList(anno, "includes");
+        List<String> excludes = getMemberStringList(anno, "excludes");
+        boolean reversed = memberHasValue(anno, "reversed", true);
+        boolean includeSuperProperties = memberHasValue(anno, "includeSuperProperties", true);
+        boolean allNames = memberHasValue(anno, "allNames", true);
+        boolean allProperties = !memberHasValue(anno, "allProperties", false);
+        if (!checkIncludeExcludeUndefinedAware(anno, excludes, includes, MY_TYPE_NAME)) return;
+        if (!checkPropertyList(classNode, includes, "includes", anno, MY_TYPE_NAME, false, includeSuperProperties, allProperties)) return;
+        if (!checkPropertyList(classNode, excludes, "excludes", anno, MY_TYPE_NAME, false, includeSuperProperties, allProperties)) return;
         if (classNode.isInterface()) {
-            addError(MY_TYPE_NAME + " cannot be applied to interface " + classNode.getName(), annotation);
+            addError(MY_TYPE_NAME + " cannot be applied to interface " + classNode.getName(), anno);
         }
-        List<PropertyNode> properties = findProperties(annotation, classNode, includes, excludes);
+        List<PropertyNode> properties = findProperties(anno, classNode, includes, excludes, allProperties, includeSuperProperties, allNames);
         implementComparable(classNode);
 
         classNode.addMethod(new MethodNode(
@@ -211,12 +217,16 @@ public class SortableASTTransformation extends AbstractASTTransformation {
         ));
     }
 
-    private List<PropertyNode> findProperties(AnnotationNode annotation, ClassNode classNode, final List<String> includes, final List<String> excludes) {
+    private List<PropertyNode> findProperties(AnnotationNode annotation, final ClassNode classNode, final List<String> includes,
+                                              final List<String> excludes, final boolean allProperties,
+                                              final boolean includeSuperProperties, final boolean allNames) {
+        Set<String> names = new HashSet<String>();
+        List<PropertyNode> props = getAllProperties(names, classNode, classNode, true, false, allProperties,
+                false, includeSuperProperties, false, false, allNames, false);
         List<PropertyNode> properties = new ArrayList<PropertyNode>();
-        for (PropertyNode property : classNode.getProperties()) {
+        for (PropertyNode property : props) {
             String propertyName = property.getName();
-            if (property.isStatic() ||
-                    (excludes != null && excludes.contains(propertyName)) ||
+            if ((excludes != null && excludes.contains(propertyName)) ||
                     includes != null && !includes.contains(propertyName)) continue;
             properties.add(property);
         }

http://git-wip-us.apache.org/repos/asf/groovy/blob/426a5416/src/spec/doc/core-metaprogramming.adoc
----------------------------------------------------------------------
diff --git a/src/spec/doc/core-metaprogramming.adoc b/src/spec/doc/core-metaprogramming.adoc
index 32ac759..da25e57 100644
--- a/src/spec/doc/core-metaprogramming.adoc
+++ b/src/spec/doc/core-metaprogramming.adoc
@@ -1321,8 +1321,9 @@ The `Ruby` version can be disabled by setting the `auto` flag to `false`.
 [[xform-Sortable]]
 ===== `@groovy.transform.Sortable`
 
-The `@Sortable` AST transformation is used to help write classes that are `Comparable` and easily sorted by
-numerous properties. It is easy to use as shown in the following example where we annotate the `Person` class:
+The `@Sortable` AST transformation is used to help write classes that are `Comparable` and easily sorted
+typically by numerous properties. It is easy to use as shown in the following example where we annotate
+the `Person` class:
 
 [source,groovy]
 ----
@@ -1380,6 +1381,28 @@ This `Person` class can be used as follows:
 include::{projectdir}/src/spec/test/CodeGenerationASTTransformsTest.groovy[tags=sortable_custom_usage,indent=0]
 ----
 
+The behavior of the `@Sortable` AST transformation can be further changed using the following additional parameters:
+
+[cols="1,1,2,3a",options="header"]
+|=======================================================================
+|Attribute|Default value|Description|Example
+|allProperties|True|Should JavaBean properties (ordered after native properties) be used|
+[source,groovy]
+----
+include::{projectdir}/src/spec/test/CodeGenerationASTTransformsTest.groovy[tags=sortable_example_allProperties,indent=0]
+----
+|allNames|False|Should properties with "internal" names be used|
+[source,groovy]
+----
+include::{projectdir}/src/spec/test/CodeGenerationASTTransformsTest.groovy[tags=sortable_example_allNames,indent=0]
+----
+|includeSuperProperties|False|Should super properties also be used (ordered first)|
+[source,groovy]
+----
+include::{projectdir}/src/spec/test/CodeGenerationASTTransformsTest.groovy[tags=sortable_example_superProperties,indent=0]
+----
+|=======================================================================
+
 [[xform-Builder]]
 ===== `@groovy.transform.builder.Builder`
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/426a5416/src/spec/test/CodeGenerationASTTransformsTest.groovy
----------------------------------------------------------------------
diff --git a/src/spec/test/CodeGenerationASTTransformsTest.groovy b/src/spec/test/CodeGenerationASTTransformsTest.groovy
index efc73ae..769dcf5 100644
--- a/src/spec/test/CodeGenerationASTTransformsTest.groovy
+++ b/src/spec/test/CodeGenerationASTTransformsTest.groovy
@@ -1212,6 +1212,81 @@ public int compareTo(java.lang.Object obj) {
 // end::sortable_custom_generated_compareTo[]
 */
         '''
+
+        assertScript '''
+import groovy.transform.*
+
+// tag::sortable_example_superProperties[]
+class Person {
+  String name
+}
+
+@Canonical(includeSuperProperties = true)
+@Sortable(includeSuperProperties = true)
+class Citizen extends Person {
+  String country
+}
+
+def people = [
+  new Citizen('Bob', 'Italy'),
+  new Citizen('Cathy', 'Hungary'),
+  new Citizen('Cathy', 'Egypt'),
+  new Citizen('Bob', 'Germany'),
+  new Citizen('Alan', 'France')
+]
+
+assert people.sort()*.name == ['Alan', 'Bob', 'Bob', 'Cathy', 'Cathy']
+assert people.sort()*.country == ['France', 'Germany', 'Italy', 'Egypt', 'Hungary']
+// end::sortable_example_superProperties[]
+        '''
+
+        assertScript '''
+import groovy.transform.*
+
+// tag::sortable_example_allNames[]
+import groovy.transform.*
+
+@Canonical(allNames = true)
+@Sortable(allNames = false)
+class Player {
+  String $country
+  String name
+}
+
+def finalists = [
+  new Player('USA', 'Serena'),
+  new Player('USA', 'Venus'),
+  new Player('USA', 'CoCo'),
+  new Player('Croatian', 'Mirjana')
+]
+
+assert finalists.sort()*.name == ['Mirjana', 'CoCo', 'Serena', 'Venus']
+// end::sortable_example_allNames[]
+        '''
+
+        assertScript '''
+import groovy.transform.*
+
+// tag::sortable_example_allProperties[]
+import groovy.transform.*
+
+@Canonical(includeFields = true)
+@Sortable(allProperties = true, includes = 'nameSize')
+class Player {
+  String name
+  int getNameSize() { name.size() }
+}
+
+def finalists = [
+  new Player('Serena'),
+  new Player('Venus'),
+  new Player('CoCo'),
+  new Player('Mirjana')
+]
+
+assert finalists.sort()*.name == ['CoCo', 'Venus', 'Serena', 'Mirjana']
+// end::sortable_example_allProperties[]
+        '''
     }
 
     void testBuilderSimple() {