diff options
author | Stephen Rosen <sirosen@globus.org> | 2022-05-29 12:28:43 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-05-29 11:28:43 -0500 |
commit | 9e161031bbde781f8dae2387946bf55724095cfe (patch) | |
tree | ff37e0f82645e0ed2d1b128b83afec56ffe155d7 | |
parent | ff2b1624c08ba80b6a5132601fddc4835467c9a0 (diff) | |
download | pyparsing-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.py | 6 | ||||
-rw-r--r-- | tests/README.md | 9 | ||||
-rw-r--r-- | tests/mypy-ignore-cases/forward_methods.py | 14 | ||||
-rw-r--r-- | tox.ini | 11 |
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] @@ -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 |