From e0821642a9348de819665bd53249bdb078ff2226 Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Thu, 19 May 2022 08:34:13 +0200 Subject: [PATCH 1/2] Type hint the pysafe function, ensure it isn't leaked in public API, and enhance the testing --- jpype/_pykeywords.py | 65 ++++++++++++++++++++++++++++----- jpype/beans.py | 6 +-- test/jpypetest/test_keywords.py | 46 ++++++++++++++++------- 3 files changed, 91 insertions(+), 26 deletions(-) diff --git a/jpype/_pykeywords.py b/jpype/_pykeywords.py index 0c22631b1..70e10856d 100644 --- a/jpype/_pykeywords.py +++ b/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('__'): + # 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 + "_" diff --git a/jpype/beans.py b/jpype/beans.py index e2e3f7b39..2e4bc2d2c 100644 --- a/jpype/beans.py +++ b/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 diff --git a/test/jpypetest/test_keywords.py b/test/jpypetest/test_keywords.py index 6c255770d..381b7f101 100644 --- a/test/jpypetest/test_keywords.py +++ b/test/jpypetest/test_keywords.py @@ -15,20 +15,40 @@ # 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', + ] +) +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("_") + - 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) +@pytest.mark.parametrize( + "identifier", + [ + 'notSpecial', + '__notSpecial', + 'notSpecial__', + '_notSpecial_', + '_not__special_', + ]) +def testPySafe__NotKeywords(identifier): + safe = jpype._pykeywords.pysafe(identifier) + assert safe == identifier From b8e09a6c6f3fd277c4b61e8945ab81f529e3c7cc Mon Sep 17 00:00:00 2001 From: Phil Elson Date: Mon, 23 May 2022 08:56:28 +0200 Subject: [PATCH 2/2] Include a test case for methods with double-underscore names --- jpype/_pykeywords.py | 2 +- test/harness/jpype/attr/TestKeywords.java | 30 +++++++++++++++++++++++ test/jpypetest/common.py | 2 -- test/jpypetest/test_keywords.py | 10 ++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 test/harness/jpype/attr/TestKeywords.java diff --git a/jpype/_pykeywords.py b/jpype/_pykeywords.py index 70e10856d..a438b80d4 100644 --- a/jpype/_pykeywords.py +++ b/jpype/_pykeywords.py @@ -70,7 +70,7 @@ def pysafe(s: str) -> typing.Optional[str]: Python that is guaranteed to not collide with the Python grammar. """ - if s.startswith("__") and s.endswith('__'): + 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 diff --git a/test/harness/jpype/attr/TestKeywords.java b/test/harness/jpype/attr/TestKeywords.java new file mode 100644 index 000000000..03dd4e86e --- /dev/null +++ b/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"; + } +} diff --git a/test/jpypetest/common.py b/test/jpypetest/common.py index 1177eaeb2..7dee50d8a 100644 --- a/test/jpypetest/common.py +++ b/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 diff --git a/test/jpypetest/test_keywords.py b/test/jpypetest/test_keywords.py index 381b7f101..debe4f8ac 100644 --- a/test/jpypetest/test_keywords.py +++ b/test/jpypetest/test_keywords.py @@ -20,6 +20,7 @@ import pytest import jpype +import common @pytest.mark.parametrize( @@ -29,6 +30,7 @@ # 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): @@ -48,7 +50,15 @@ def testPySafe__Keywords(identifier): 'notSpecial__', '_notSpecial_', '_not__special_', + '__', '___', # Technically these are fine. ]) def testPySafe__NotKeywords(identifier): safe = jpype._pykeywords.pysafe(identifier) assert safe == identifier + + +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__"))