flask-cors
flask-cors copied to clipboard
Added type hints to most classes using mokeytype
Resolves #275
Pretty straightforward process, I installed monkeytype and ran monkeytype run -m unittest in order to run all tests and generate a database of call stacks.
With a small bash loop I applied type hints to all files:
for MODULE in $(monkeytype list-modules)
do
monkeytype apply $MODULE
done
just to be sure, I installed mypy and checked both the flask_cors and the tests packages for typing issues (mypy flask_cors tests): out of a total of 35 errors, 12 were regarding the use of unittest.main() in each test file, the others seem to not be of concern and could be fixed by ensuring stricter type hints. Below is a list of all the errors found
flask_cors/core.py:113: error: Item "str" of "Union[Pattern[Any], str]" has no attribute "pattern"
flask_cors/core.py:120: error: Unsupported right operand type for in ("Optional[Any]")
flask_cors/core.py:133: error: Argument 2 to "try_match_any" has incompatible type "Optional[Any]"; expected "List[Union[str, Pattern[Any]]]"
flask_cors/core.py:158: error: Item "None" of "Optional[Any]" has no attribute "__iter__" (not iterable)
flask_cors/core.py:172: error: Argument 2 to "try_match_any" has incompatible type "Union[str, List[str], int, None]"; expected "List[Union[str, Pattern[Any]]]"
flask_cors/core.py:183: error: Need type annotation for "headers"
flask_cors/core.py:203: error: Unsupported right operand type for in ("Optional[Any]")
flask_cors/core.py:221: error: Argument 1 to "len" has incompatible type "Optional[Any]"; expected "Sized"
flask_cors/core.py:223: error: Argument 2 to "map" has incompatible type "Optional[Any]"; expected "Iterable[Any]"
flask_cors/core.py:250: error: Argument 2 to "get_cors_headers" has incompatible type "Headers"; expected "EnvironHeaders"
flask_cors/core.py:266: error: Unsupported right operand type for in ("Union[Pattern[Any], str]")
flask_cors/core.py:283: error: Incompatible return value type (got "Optional[typing.Match[str]]", expected "Union[difflib.Match, bool, None]")
flask_cors/core.py:285: error: Incompatible return value type (got "Union[typing.Match[Any], None, typing.Match[str]]", expected "Union[difflib.Match, bool, None]")
flask_cors/core.py:288: error: Item "Pattern[Any]" of "Union[Pattern[Any], str]" has no attribute "lower"
flask_cors/core.py:341: error: Item "None" of "Optional[str]" has no attribute "upper"
flask_cors/core.py:353: error: Incompatible return value type (got "Union[List[str], str, Set[str]]", expected "Union[List[str], List[Pattern[Any]], Set[str]]")
flask_cors/core.py:370: error: Argument 1 to "sanitize_regex_param" has incompatible type "Optional[Any]"; expected "Union[List[str], Set[str], Pattern[Any], str]"
flask_cors/core.py:371: error: Argument 1 to "sanitize_regex_param" has incompatible type "Optional[Any]"; expected "Union[List[str], Set[str], Pattern[Any], str]"
flask_cors/extension.py:19: error: Module "urllib" has no attribute "unquote_plus"
flask_cors/extension.py:19: error: Name "unquote_plus" already defined (possibly by an import)
flask_cors/extension.py:176: error: Cannot assign to a method
flask_cors/extension.py:178: error: Cannot assign to a method
tests/base_test.py:46: error: "FlaskCorsTestCase" has no attribute "app"
tests/extension/test_app_extension.py:382: error: Name "unittest" is not defined
tests/decorator/test_w3.py:71: error: Name "unittest" is not defined
tests/decorator/test_vary_header.py:87: error: Name "unittest" is not defined
tests/decorator/test_origins.py:204: error: Name "unittest" is not defined
tests/decorator/test_options.py:81: error: Name "unittest" is not defined
tests/decorator/test_methods.py:61: error: Name "unittest" is not defined
tests/decorator/test_max_age.py:60: error: Name "unittest" is not defined
tests/decorator/test_expose_headers.py:44: error: Name "unittest" is not defined
tests/decorator/test_duplicate_headers.py:33: error: Name "unittest" is not defined
tests/decorator/test_credentials.py:64: error: Name "unittest" is not defined
tests/decorator/test_allow_headers.py:107: error: Name "unittest" is not defined
tests/core/test_override_headers.py:33: error: Name "unittest" is not defined
There are a number of vague type hints (Any, Callable) which could be made more specific, but for now even having vague hints is better than nothing
Hey Luca! Thank you very much for the contribution! I am a big fan of MyPy but don't have experience using it in packaged libraries. Are you familiar with the best practices in terms of maintaining Python version compatibility? If so, I'd love some help with that side of things and then we can merge this and release!
The way I do it is to simply add a type checking step in the CI build step, below is an example of what I used to do with travis:
language: python
python:
- "3.something"
install:
- install_your_packages_etc
script:
- your black, pylint, pytest, and other build step tests
- mypy --strict my_project tests # make mypy strictly check type hints
If your CI pipeline runs against multiple python versions and mypy becomes unsupported in the future you just won't pass the build step and can react accordingly.
All the missing steps to make this project fully compliant with mypy:
- add missing type hints, fixing the errors that pop up when running
mypy flask_cors tests. We need to try and be as specific as possible, since addingmy_var: Anydoesn't improve the type hinting except removing a mypy typing error. If some types are hard to define, we can keep them asAnyand refine them later down the road. - when the project is fully typed, add a
py.typedfile in order to be PEP 561 compliant and let mypy know that it can check the project types when it's used as an external package - add type checking in the build step, as seen above. Initially just a
mypy flask_cors testsentry should suffice
If we want to be extra fancy, we can run mypy in strict mode with the --strict flag, both when checking types locally and in the CI pipeline: It will be very picky about everything, but once those errors are gone you are fully done:
Standard mypy likes this, as it's typed
def test(a: str) -> list:
return a.split(",")
Strict mypy needs the type hints to be fully defined, so no list, dict, tuple, etc. Instead you need to specify what types they will accept
from typing import List
def test(a: str) -> List[str]:
return a.split(",")
Let me know if something is unclear! Happy to contribute more, I just can't ensure consistency as I have a ton of other stuff to do