The goal of this PR is to allow typeshed to not ignore NQA102. It does so by making the typeshed_primer job non-blocking (meaning it won't fail even if Flake8 errors are raised), and instead add a comment to the PR to show if the PR would produce any Flake8 error in typeshed. This way, typeshed doesn't need to pre-emptively ignore new errors for flake8-pyi CI to pass.

This is heavily based on https://github.com/python/typeshed/blob/main/.github/workflows/mypy_primer_comment.yml , but simplified a bit and I extracted the JS scripts for better readability. (CC: @AlexWaygood )

Note: .github/workflows/check.yml contains extra auto-formatting changes. I don't think they make the PR harder to review (especially if you hide whitespace changes), but I can always revert and/or split it off.

An attempt at implementing #168 (closes #168).

Marking this as a draft for now, as I don't think the error message is very good, and I'm not sure how to improve it. I would also be curious to hear what others think of the overall idea before I put any more work in, as well.

This package can be very helpful for keeping stub files healthy, but development has kind of stalled. For example, typeshed now has some lint-type checks in a custom file (check_new_syntax.py). I'd like to keep such checks in this package in the future, so they can be reused by other repos than typeshed and so we can take advantage of the flake8 ecosystem (e.g., standardized noqa comments).

Here's a few process improvements I'm thinking of:

• [x] More maintainers. I think @srittau is already a maintainer, but @Akuli @hauntsaninja @AlexWaygood would you be interested in joining as maintainers?
• [ ] Making releases easier. Perhaps we should set up a CI pipeline that automatically releases every merged PR to PyPI, so we can quickly upgrade over at typeshed.
• [x] Moving this repo over to a more general org. @ambv, maybe it can go into github.com/psf?

I'd like a check similar to Y041, but for unions with object. Especially for object | None, coming from a Typescript background, it is not obvious to me that this type union is redundant, and I often make that mistake (or forget about it and leave it in a stub).

Closes #45.

@AlexWaygood this is roughly what I had in mind when suggesting a new code, just a simple Use PEP 604 union types instead of {Optional,Union} message. @srittau unfortunately, the check for just importing Optional/Union seems to be not enough, since then any typing.Optional/typing.Union usages are skipped. But luckily the _check_import_or_attribute method already covers that :crossed_fingers:

Helps work towards #80 and #32. I've tried to keep these error codes completely cross-compatible between Python 2 and Python 3 -- I think it would be best to leave Python-3-only changes (e.g. disallowing typing.ContextManager and typing_extensions.OrderedDict) can be left for another error code.

I'm trying to package your module as an rpm package. So I'm using the typical PEP517 based build, install and test cycle used on building packages from non-root account.

• python3 -sBm build -w
• install .whl file in </install/prefix>
• run pytest with PYTHONPATH pointing to sitearch and sitelib inside </install/prefix>

Here is pytest output:

+ PYTHONPATH=/home/tkloczko/rpmbuild/BUILDROOT/python-flake8-pyi-20.10.0-2.fc35.x86_64/usr/lib64/python3.8/site-packages:/home/tkloczko/rpmbuild/BUILDROOT/python-flake8-pyi-20.10.0-2.fc35.x86_64/usr/lib/python3.8/site-packages
+ /usr/bin/pytest -ra
=========================================================================== test session starts ============================================================================
platform linux -- Python 3.8.12, pytest-6.2.5, py-1.11.0, pluggy-0.13.1
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
Using --randomly-seed=3873507879
rootdir: /home/tkloczko/rpmbuild/BUILD/flake8-pyi-20.10.0
plugins: mock-3.6.1, cov-2.12.1, anyio-3.3.4, flaky-3.7.0, console-scripts-1.2.0, asyncio-0.16.0, freezegun-0.4.2, flake8-1.0.7, rerunfailures-9.1.1, yagot-0.5.0, forked-1.4.0, ordering-0.6, xdist-2.5.0, Faker-10.0.0, benchmark-3.4.1, pyfakefs-4.5.3, datadir-1.3.1, regressions-2.2.0, timeout-2.0.2, randomly-3.10.3, perf-0.10.1, trio-0.7.0, requests-mock-1.9.3, hypothesis-6.31.5, easy-server-0.8.0
collected 15 items

tests/test_pyi.py .....F.......F.                                                                                                                                    [100%]

================================================================================= FAILURES =================================================================================
______________________________________________________________________ PyiTestCase.test_function_def _______________________________________________________________________

self = <tests.test_pyi.PyiTestCase testMethod=test_function_def>

    def test_function_def(self) -> None:
        stdout_lines = (
            '5:5: Y009 Empty body should contain "...", not "pass"',
            '19:5: Y010 Function body must contain only "..."',
            '23:5: Y010 Function body must contain only "..."',
        )
>       self.checkFileOutput("emptyfunctions.pyi", stdout_lines=stdout_lines)

tests/test_pyi.py:182:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_pyi.py:83: in checkFileOutput
    check_output(
tests/test_pyi.py:78: in check_output
    self.assertMultiLineEqual(expected, actual)
E   AssertionError: 'empt[85 chars]pyi:19:5: Y010 Function body must contain only[71 chars]..."' != 'empt[85 chars]pyi:10:1: B903 Data class should either be imm[585 chars]..."'
E     emptyfunctions.pyi:5:5: Y009 Empty body should contain "...", not "pass"
E   + emptyfunctions.pyi:10:1: B903 Data class should either be immutable or use __slots__ to save memory. Use collections.namedtuple to generate an immutable class, or enumerate the attributes in a __slot__ declaration in the class to leave attributes mutable.
E   + emptyfunctions.pyi:14:1: B903 Data class should either be immutable or use __slots__ to save memory. Use collections.namedtuple to generate an immutable class, or enumerate the attributes in a __slot__ declaration in the class to leave attributes mutable.
E     emptyfunctions.pyi:19:5: Y010 Function body must contain only "..."
E     emptyfunctions.pyi:23:5: Y010 Function body must contain only "..."
_______________________________________________________________________ PyiTestCase.test_empty_init ________________________________________________________________________

self = <tests.test_pyi.PyiTestCase testMethod=test_empty_init>

    def test_empty_init(self) -> None:
        # should have no output if it's not explicitly selected
>       self.checkFileOutput("emptyinit.pyi", stdout_lines=())

tests/test_pyi.py:186:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_pyi.py:83: in checkFileOutput
    check_output(
tests/test_pyi.py:78: in check_output
    self.assertMultiLineEqual(expected, actual)
E   AssertionError: '' != 'emptyinit.pyi:1:1: B903 Data class should[204 chars]ble.'
E   + emptyinit.pyi:1:1: B903 Data class should either be immutable or use __slots__ to save memory. Use collections.namedtuple to generate an immutable class, or enumerate the attributes in a __slot__ declaration in the class to leave attributes mutable.
========================================================================= short test summary info ==========================================================================
FAILED tests/test_pyi.py::PyiTestCase::test_function_def - AssertionError: 'empt[85 chars]pyi:19:5: Y010 Function body must contain only[71 chars]..."' != 'empt[85 chars...
FAILED tests/test_pyi.py::PyiTestCase::test_empty_init - AssertionError: '' != 'emptyinit.pyi:1:1: B903 Data class should[204 chars]ble.'
====================================================================== 2 failed, 13 passed in 14.35s =======================================================================
error: Bad exit status from /var/tmp/rpm-tmp.mkEK9d (%check)

Typeshed currently has a script that checks a bunch of things that belong here IMO: https://github.com/python/typeshed/blob/af8e37d2736d3a6fbed8f5dfad71580dd417c612/tests/check_new_syntax.py

• [x] Collections classes in typing: typing.Counter --> collections.Counter etc (related: #46)
• [x] Built-in generics: List --> list etc
• [x] typing.(Async)ContextManager --> contextlib.Abstract(Async)ContextManager
• [x] Preferring typing over typing_extensions when available: typing_extensions.ClassVar --> typing.ClassVar etc
• [x] Union[str, int] --> str | int (#45)
• [x] Banning from collections.abc import Set, because it's confusingly different from the built-in set
• [ ] Ordering sys.version_info checks so that the most recent version is first
• [x] Ban explicitly inheriting from object in Python 3-only stubs.
• [x] Disallow using typing.Text in Python 3-only stubs.

~~We should probably create one error code per Python version, so that it's easy to e.g. enable errors for not using features new in Python 3.8+. Another option would be to make the errors depend on sys.version_info (maybe too clever, would need good documentation).~~

flake8 5.0.0 was just released, and flake8-pyi crashes if run with the new version of flake8.

In the short term, we should maybe pin the required version of flake8 in our setup.py to <5.0.0 and do a hotfix release.

In order to become compatible with flake8 5.0.0, we'll probably need to either address #183 or just change the monkey-patching of flake8 that we do.

Thoughts?

And delete the machinery for disabling errors.

This made this check run on all tests, which helped me find a bug (it triggered on all). I can split the test changes into a separate PR if preferred.

I'll submit a separate PR to typeshed to disable this check for now.

Fixes #75. Fixes #86.

Disabled-by-default error codes are problematic; see #75.

Y092 finds a fairly harmless error and has significant false positives (python/typeshed#6930). I'm removing it so we can get rid of disabled-by-default errors and simplify our setup.

flake8-pyi currently emits a false-positive error for this code, which should be fine:

from typing_extensions import Final as _Final

CONSTANT: _Final = 5

This might be tricky to fix, since we don't currently track whether imports are aliased at all. Came up in https://github.com/python/typeshed/pull/9470

Since we use CalVer, we have one "major version bump" a year: the first release after the new year. It might be better to plan breaking/backwards-incompatible changes for the major version bump. Does anybody have anything they'd like to be included in the next release?

Things I'd like to do:

• #269.
• I'd also add typing.Match and typing.Pattern to the imports forbidden by Y022 (Python 3.6 is now long EOL, and unsupported by at least typeshed and mypy).

If a function has many keyword arguments that do not change across all overloads (for example pandas.read_csv). It is more readable and shorter to move all the overload-invariant keywords into a TypedDict and suggest using the Unpack feature which is supported by at least pyright and mypy.

This would probably need some heuristic of when is it worth recommending Unpack: Using a TypedDict for less than insert magic number keywords is probably not helpful.

Is there any option to say that we need compatibility with Python 3.7? Or are we expected to add any incompatible error codes to the ignore list?

e.g. Y022 is not possible to fix until Python 3.9 is the minimum supported version (in our library).

Repro:

from typing import List
class Outer:
    class Inner1: ...
    class Inner2(List[Outer.Inner1]): ...

Both mypy and pyright are happy with this flake8 complains that it can't identify the name Outer

Within a .py file, this construct would not be ok, but it appears to be ok within a .pyi file.

This may be because mypy/pyright do a 2-pass check. I am not sure how flake8/flake8-pyi works here.