tapestry-5 git commit: TAP5-2268: @Property may generate a method that conflicts with an existing one, without error, and with unpredictable results

[jk...@apache.org]

Repository: tapestry-5
Updated Branches:
  refs/heads/master 8380211bc -> 6c7c090ea


TAP5-2268: @Property may generate a method that conflicts with an existing one, without error, and with unpredictable results


Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/6c7c090e
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/6c7c090e
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/6c7c090e

Branch: refs/heads/master
Commit: 6c7c090eafa1c59972f9e28536ba9c2991fd701e
Parents: 8380211
Author: Felix Scheffer <ma...@felix-scheffer.de>
Authored: Mon Mar 23 15:55:53 2015 +0100
Committer: Jochen Kemnade <jo...@eddyson.de>
Committed: Mon Mar 23 16:43:04 2015 +0100

----------------------------------------------------------------------
 .../tapestry5/internal/plastic/InheritanceData.java  | 10 ++++++++--
 .../plastic/FieldPropertyMethodCreation.groovy       | 15 +++++++++++++++
 .../testsubjects/AccessorsAlreadyExistSubject2.java  | 15 +++++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/6c7c090e/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
index 44c1734..a9dc87d 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
@@ -137,9 +137,15 @@ public class InheritanceData
      * Combines a method name and its desc (which describes parameter types and return value) to form
      * a value, which is how methods are tracked.
      */
-    private String toValue(String name, String desc)
+    private static String toValue(String name, String desc)
     {
-        return name + ":" + desc;
+        // TAP5-2268: ignore return-type to avoid methods with the same number (and type) of parameters but different
+        //            return-types which is illegal in Java.
+        // desc is something like "(I)Ljava/lang/String;", which means: takes an int, returns a String. We strip
+        // everything after the parameter list.
+        int endOfParameterSpecIdx = desc.indexOf(')');
+
+        return name + ":" + desc.substring(0, endOfParameterSpecIdx+1);
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/6c7c090e/plastic/src/test/groovy/org/apache/tapestry5/plastic/FieldPropertyMethodCreation.groovy
----------------------------------------------------------------------
diff --git a/plastic/src/test/groovy/org/apache/tapestry5/plastic/FieldPropertyMethodCreation.groovy b/plastic/src/test/groovy/org/apache/tapestry5/plastic/FieldPropertyMethodCreation.groovy
index 98b3121..62627d8 100644
--- a/plastic/src/test/groovy/org/apache/tapestry5/plastic/FieldPropertyMethodCreation.groovy
+++ b/plastic/src/test/groovy/org/apache/tapestry5/plastic/FieldPropertyMethodCreation.groovy
@@ -1,7 +1,9 @@
 package org.apache.tapestry5.plastic
 
+import spock.lang.Issue
 import testannotations.Property
 import testsubjects.AccessorsAlreadyExistSubject
+import testsubjects.AccessorsAlreadyExistSubject2;
 import testsubjects.CreateAccessorsSubject
 import testsubjects.GenericCreateAccessorsSubject
 
@@ -88,6 +90,19 @@ class FieldPropertyMethodCreation extends AbstractPlasticSpecification
         assert e.message == "Unable to create new accessor method public java.lang.String getValue() on class testsubjects.AccessorsAlreadyExistSubject as the method is already implemented."
     }
 
+    @Issue('https://issues.apache.org/jira/browse/TAP5-2268')
+    def "create getter that already exists with different return type"() {
+      when:
+
+      withAccessors(AccessorsAlreadyExistSubject2, PropertyAccessType.READ_ONLY)
+
+      then:
+
+      def e = thrown(IllegalArgumentException)
+
+      assert e.message == "Unable to create new accessor method public int getValue() on class testsubjects.AccessorsAlreadyExistSubject2 as the method is already implemented."
+    }
+
     def "create setter that already exists"() {
         when:
 

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/6c7c090e/plastic/src/test/java/testsubjects/AccessorsAlreadyExistSubject2.java
----------------------------------------------------------------------
diff --git a/plastic/src/test/java/testsubjects/AccessorsAlreadyExistSubject2.java b/plastic/src/test/java/testsubjects/AccessorsAlreadyExistSubject2.java
new file mode 100644
index 0000000..ba25fb4
--- /dev/null
+++ b/plastic/src/test/java/testsubjects/AccessorsAlreadyExistSubject2.java
@@ -0,0 +1,15 @@
+package testsubjects;
+
+import testannotations.Property;
+
+public class AccessorsAlreadyExistSubject2
+{
+    @Property
+    private int value;
+
+    public String getValue()
+    {
+        return String.valueOf(value);
+    }
+
+}