Merge pull request #1068 from pelson/fix/safer_pysafe

Type hint the pysafe function, ensure it isn't leaked in public API, and enhance the testing

jpype/_pykeywords.py

@@ -15,21 +15,66 @@
 #   See NOTICE file for details.
 #
 # *****************************************************************************
+from __future__ import annotations
+
+import typing
 
 # This is a superset of the keywords in Python.
 # We use this so that jpype is a bit more version independent.
-# Removing keywords from this list impacts the exposed interfaces, and therefore is a breaking change. 
-_KEYWORDS = set((
-    'False', 'None', 'True', 'and', 'as', 'assert', 'async',
-    'await', 'break', 'class', 'continue', 'def', 'del', 'elif', 'else',
-    'except', 'exec', 'finally', 'for', 'from', 'global', 'if', 'import',
-    'in', 'is', 'lambda', 'nonlocal', 'not', 'or', 'pass', 'print',
-    'raise', 'return', 'try', 'while', 'with', 'yield'
-))
+# Adding and removing keywords from this list impacts the exposed interfaces,
+# and therefore is technically a breaking change.
+_KEYWORDS = {
+    'False',
+    'None',
+    'True',
+    'and',
+    'as',
+    'assert',
+    'async',
+    'await',
+    'break',
+    'class',
+    'continue',
+    'def',
+    'del',
+    'elif',
+    'else',
+    'except',
+    'exec',
+    'finally',
+    'for',
+    'from',
+    'global',
+    'if',
+    'import',
+    'in',
+    'is',
+    'lambda',
+    'nonlocal',
+    'not',
+    'or',
+    'pass',
+    'print',  # No longer a Python 3 keyword. Kept for backwards compatibility.
+    'raise',
+    'return',
+    'try',
+    'while',
+    'with',
+    'yield',
+}
+
 
+def pysafe(s: str) -> typing.Optional[str]:
+    """
+    Given an identifier name in Java, return an equivalent identifier name in
+    Python that is guaranteed to not collide with the Python grammar.
 
-def pysafe(s):
-    if s.startswith("__"):
+    """
+    if s.startswith("__") and s.endswith("__") and len(s) >= 4:
+        # Dunder methods should not be considered safe.
+        # (see system defined names in the Python documentation
+        # https://docs.python.org/3/reference/lexical_analysis.html#reserved-classes-of-identifiers
+        # )
         return None
     if s in _KEYWORDS:
         return s + "_"

jpype/beans.py

@@ -42,7 +42,7 @@
 """
 import _jpype
 from . import _jcustomizer
-from ._pykeywords import pysafe
+from ._pykeywords import pysafe as _pysafe
 
 
 def _extract_accessor_pairs(members):
@@ -89,10 +89,10 @@ class _BeansCustomizer(object):
     def __jclass_init__(self):
         accessor_pairs = _extract_accessor_pairs(self.__dict__)
         for attr_name, (getter, setter) in accessor_pairs.items():
-            attr_name = pysafe(attr_name)
+            attr_name = _pysafe(attr_name)
 
             # Don't mess with an existing member
-            if attr_name in self.__dict__:
+            if attr_name is None or attr_name in self.__dict__:
                 continue
 
             # Add the property

test/harness/jpype/attr/TestKeywords.java

@@ -0,0 +1,30 @@
+/* ****************************************************************************
+  Licensed 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.
+
+  See NOTICE file for details.
+**************************************************************************** */
+package jpype.attr;
+
+public class TestKeywords
+{
+
+  public String __leading_double_underscore()
+  {
+    return "foo";
+  }
+
+  public String __dunder_name__()
+  {
+    return "foo";
+  }
+}

test/jpypetest/common.py

@@ -127,8 +127,6 @@ def setUp(self):
             jpype.startJVM(jvm_path, *args,
                            convertStrings=self._convertStrings)
         self.jpype = jpype.JPackage('jpype')
-        if sys.version < '3':
-            self.assertCountEqual = self.assertItemsEqual
 
     def tearDown(self):
         pass

test/jpypetest/test_keywords.py

@@ -15,20 +15,50 @@
 #   See NOTICE file for details.
 #
 # *****************************************************************************
+import keyword  # From the standard library.
+
+import pytest
+
 import jpype
 import common
-import keyword
 
 
-class KeywordsTestCase(common.JPypeTestCase):
-    def setUp(self):
-        common.JPypeTestCase.setUp(self)
+@pytest.mark.parametrize(
+    "identifier",
+    list(keyword.kwlist) + [
+        '__del__',
+        # Print is no longer a keyword in Python 3, but still protected to
+        # avoid API breaking in JPype v1.
+        'print',
+        '____',  # Not allowed.
+    ]
+)
+def testPySafe__Keywords(identifier):
+    safe = jpype._pykeywords.pysafe(identifier)
+    if identifier.startswith("__"):
+        assert safe is None
+    else:
+        assert isinstance(safe, str), f"Fail on keyword {identifier}"
+        assert safe.endswith("_")
+
+
+@pytest.mark.parametrize(
+    "identifier",
+    [
+        'notSpecial',
+        '__notSpecial',
+        'notSpecial__',
+        '_notSpecial_',
+        '_not__special_',
+        '__', '___',  # Technically these are fine.
+    ])
+def testPySafe__NotKeywords(identifier):
+    safe = jpype._pykeywords.pysafe(identifier)
+    assert safe == identifier
+
 
-    def testKeywords(self):
-        for kw in keyword.kwlist:
-            safe = jpype._pykeywords.pysafe(kw)
-            if kw.startswith("_"):
-                continue
-            self.assertEqual(type(safe), str, "Fail on keyword %s" % kw)
-            self.assertTrue(safe.endswith("_"))
-        self.assertEqual(jpype._pykeywords.pysafe("__del__"), None)
+class AttributeTestCase(common.JPypeTestCase):
+    def testPySafe(self):
+        cls = jpype.JPackage("jpype").attr.TestKeywords
+        self.assertTrue(hasattr(cls, "__leading_double_underscore"))
+        self.assertFalse(hasattr(cls, "__dunder_name__"))