fix(java): improve property override detection (#692)

The previous code only checked whether a methodd name was a get/setter
by checking if it's name started by `get` or `set` and was at least 4
characters long. This is insufficient, as a method called `settings`
would be misidentified.

Changed to expect getters to have zero parameters, and setters to have
exactly one parameter and to match a corresponding getter.

Author: RomainMuller

.travis.yml

@@ -14,8 +14,9 @@ script:
   - docker build --pull --build-arg BUILD_TIMESTAMP=$(date -u +'%Y-%m-%dT%H:%M:%SZ') --build-arg COMMIT_ID="${TRAVIS_COMMIT}" -t "jsii/superchain:nightly" ./superchain
   # Building jsii itself within the Docker image
   - docker run --rm -it --net=host -v ${PWD}:${PWD} -w ${PWD} jsii/superchain:nightly ./build.sh
-  # Make sure the build did not change the source tree (git diff-index will return non-zero if that's the case)
-  - git diff-index --exit-code --ignore-space-at-eol --stat HEAD
+  # Make sure the build did not change the source tree
+  - git update-index --refresh
+  - git diff-index --exit-code --stat HEAD
   - untracked=$(git ls-files --others --exclude-standard) && echo "${untracked}" && test -z "${untracked}"
   # Publish the image to DockerHub when relevant
   - echo "TRAVIS_PULL_REQUEST = ${TRAVIS_PULL_REQUEST:-}"

packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiEngine.java

@@ -491,8 +491,8 @@ private static Collection<JsiiOverride> discoverOverrides(final Class<?> classTo
                 String methodName = method.getName();
 
                 // check if this is a property ("getXXX" or "setXXX", oh java!)
-                if (isJavaPropertyMethod(methodName)) {
-                    String propertyName = javaPropertyToJSProperty(methodName);
+                if (isJavaPropertyMethod(method)) {
+                    String propertyName = javaPropertyToJSProperty(method);
 
                     // skip if this property is already in the overrides list
                     if (overrides.containsKey(propertyName)) {

packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/Util.java

@@ -2,6 +2,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.reflect.Method;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -42,21 +43,51 @@ static String readString(final InputStream is) {
 
     /**
      * Checks if the method name looks like a java property getter (getXxx).
-     * @param methodName The name of the method.
-     * @return True if the name looks like getXxx.
+     * @param method The reflected method that may be a get/setter
+     * @return true if the method is a get/setter.
      */
-    static boolean isJavaPropertyMethod(final String methodName) {
-        return (methodName.length() > PROPERTY_METHOD_PREFIX_LEN
-                && (methodName.startsWith("get") || methodName.startsWith(("set"))));
+    static boolean isJavaPropertyMethod(final Method method) {
+        final String methodName = method.getName();
+        if (methodName.length() <= PROPERTY_METHOD_PREFIX_LEN) {
+            // Needs to have at least one character after the "get" or "set" prefix
+            return false;
+        }
+        if (methodName.startsWith("get") && method.getParameterCount() == 0) {
+            // Require something else than a lowercase letter after the "get" prefix
+            return !Character.isLowerCase(methodName.charAt(3));
+        }
+        if (methodName.startsWith("set") && method.getParameterCount() == 1) {
+            // Require something else than a lowercase letter after the "get" prefix
+            return !Character.isLowerCase(methodName.charAt(3))
+                // Require a matching getter
+                && isMatchingGetterPresent(method.getName().replaceFirst("set", "get"),
+                    method.getParameterTypes()[0],
+                    method.getDeclaringClass());
+        }
+        return false;
+    }
+
+    private static boolean isMatchingGetterPresent(final String getterName, final Class<?> returnType, final Class<?> declaring) {
+        try {
+            final Method getter = declaring.getDeclaredMethod(getterName);
+            return getter.getReturnType().equals(returnType);
+        } catch (final NoSuchMethodException nsme) {
+            return declaring.equals(Object.class)
+                ? false
+                : isMatchingGetterPresent(getterName,
+                                          returnType,
+                                          declaring.getSuperclass());
+        }
     }
 
     /**
      * Convert a java property method name (getXxx/setXxx) to a javascript property name (xxx).
-     * @param getterSetterMethod The java method name
+     * @param method The reflected method (assumed to be a get/setter according to #isJavaPropertyMethod)
      * @return The javascript property name
      */
-    static String javaPropertyToJSProperty(final String getterSetterMethod) {
-        if (!isJavaPropertyMethod(getterSetterMethod)) {
+    static String javaPropertyToJSProperty(final Method method) {
+        final String getterSetterMethod = method.getName();
+        if (!isJavaPropertyMethod(method)) {
             throw new JsiiException("Invalid getter/setter method. Must start with get/set");
         }
 

packages/jsii-pacmak/package-lock.json

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    jsii_runtime (0.14.2)
+    jsii_runtime (0.14.3)
 
 GEM
   remote: https://rubygems.org/
@@ -34,4 +34,4 @@ DEPENDENCIES
   test-unit (~> 3.3.3)
 
 BUNDLED WITH
-   1.17.2
+   1.17.3