summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephen Rosen <sirosen@globus.org>2022-05-29 12:28:43 -0400
committerGitHub <noreply@github.com>2022-05-29 11:28:43 -0500
commit9e161031bbde781f8dae2387946bf55724095cfe (patch)
treeff37e0f82645e0ed2d1b128b83afec56ffe155d7
parentff2b1624c08ba80b6a5132601fddc4835467c9a0 (diff)
downloadpyparsing-git-9e161031bbde781f8dae2387946bf55724095cfe.tar.gz
Fix type annotations of Forward dunder-methods (#402)
* Fix type annotations of Forward dunder-methods The `__lshift__`, `__ilshift__`, and `__or__` methods each return a ParserElement object, but have no annotated return type. The result is that the following code will not type check: def foo() -> pp.ParserElement: expr = pp.Forward() expr <<= bar() return expr | pp.Literal("baz") whereas the code will type check if the return line is changed to return pp.MatchFirst([expr, pp.Literal("baz")]) This is a bug in the types which can be resolved fairly simply with some return type annotations. Testing is more complicated. Testing annotation accuracy is a relatively novel space with a few options, none of which can be considered standard as of yet. Many solutions require learning a new toolchain only for that purpose. However, one of the lower-impact options is to use `mypy --warn-unused-ignores` to check that annotations satisfy some constraints. This isn't the most precise test possible, but it's simple and uses a widely known and familiar tool for the job. `tox -e mypy-tests` is a new tox env which calls `mypy` in the desired way. We can confirm with a new test case file that `tox -e mypy-tests` fails prior to this change to `pyparsing/` and that it passes with the change made. * Comment out mypy-test tox env for CI Until CI adjustments are made, it's not possible to add mypy-test to the tox config. It will be run under pypy where it does not work until other changes are made.
-rw-r--r--pyparsing/core.py6
-rw-r--r--tests/README.md9
-rw-r--r--tests/mypy-ignore-cases/forward_methods.py14
-rw-r--r--tox.ini11
4 files changed, 36 insertions, 4 deletions
diff --git a/pyparsing/core.py b/pyparsing/core.py
index 2b93b33..e3bee8d 100644
--- a/pyparsing/core.py
+++ b/pyparsing/core.py
@@ -5178,7 +5178,7 @@ class Forward(ParseElementEnhance):
super().__init__(other, savelist=False)
self.lshift_line = None
- def __lshift__(self, other):
+ def __lshift__(self, other) -> "Forward":
if hasattr(self, "caller_frame"):
del self.caller_frame
if isinstance(other, str_type):
@@ -5195,10 +5195,10 @@ class Forward(ParseElementEnhance):
self.lshift_line = traceback.extract_stack(limit=2)[-2]
return self
- def __ilshift__(self, other):
+ def __ilshift__(self, other) -> "Forward":
return self << other
- def __or__(self, other):
+ def __or__(self, other) -> "ParserElement":
caller_line = traceback.extract_stack(limit=2)[-2]
if (
__diag__.warn_on_match_first_with_lshift_operator
diff --git a/tests/README.md b/tests/README.md
index 87b8195..9c17ee7 100644
--- a/tests/README.md
+++ b/tests/README.md
@@ -8,3 +8,12 @@ After forking the pyparsing repo, and cloning your fork locally, install the lib
Run the tests to ensure your environment is setup
python -m unittest discover tests
+
+### mypy ignore tests
+
+`tests/mypy-ignore-cases/` is populated with python files which are meant to be
+checked using `mypy --warn-unused-ignores`.
+
+To check these files, run
+
+ tox -e mypy-tests
diff --git a/tests/mypy-ignore-cases/forward_methods.py b/tests/mypy-ignore-cases/forward_methods.py
new file mode 100644
index 0000000..ff50f5b
--- /dev/null
+++ b/tests/mypy-ignore-cases/forward_methods.py
@@ -0,0 +1,14 @@
+import pyparsing as pp
+
+# first, some basic validation: forward is a ParserElement, so is Literal
+# MatchFirst([Forward(), Literal(...)]) should also be okay
+e: pp.ParserElement = pp.Forward()
+e = pp.Literal()
+e = pp.MatchFirst([pp.Forward(), pp.Literal("hi there")])
+# confirm that it isn't returning Any because it cannot be assigned to a str
+x: str = pp.Forward() | pp.Literal("oops") # type: ignore[assignment]
+
+# confirm that `Forward.__or__` has the right behavior
+e = pp.Forward() | pp.Literal("nice to meet you")
+# and that it isn't returning Any because it cannot be assigned to an int
+y: int = pp.Forward() | pp.Literal("oops") # type: ignore[assignment]
diff --git a/tox.ini b/tox.ini
index d2f30f1..125208c 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,7 +1,7 @@
[tox]
skip_missing_interpreters=true
envlist =
- py{36,37,38,39,310,py3},pyparsing_packaging
+ py{36,37,38,39,310,py3},mypy-test,pyparsing_packaging
isolated_build = True
[testenv]
@@ -10,6 +10,15 @@ extras=diagrams
commands=
coverage run --parallel --branch -m unittest
+# commented out mypy-test until CI can support running it
+# see: https://github.com/pyparsing/pyparsing/pull/402
+#
+# [testenv:mypy-test]
+# deps = mypy==0.960
+# # note: cd to tests/ to avoid mypy trying to check pyparsing (which fails)
+# changedir = tests
+# commands = mypy --show-error-codes --warn-unused-ignores mypy-ignore-cases/
+
[testenv:pyparsing_packaging]
deps=
pretend