Skip to content

Disallow Any on public interface types #6815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Disallow Any on public interface types #6815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ Nicholas Murphy
Niclas Olofsson
Nicolas Delaby
Nikolay Kondratyev
Nipunn Koorapati
Oleg Pidsadnyi
Oleg Sushchenko
Oliver Bestwalter
Expand Down
15 changes: 15 additions & 0 deletions setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,18 @@ strict_equality = True
warn_redundant_casts = True
warn_return_any = True
warn_unused_configs = True

# Hold the public interface to a higher standard
[mypy-pytest]
# Disallow dynamic typing
disallow_any_decorated = True
disallow_any_expr = True
disallow_any_generics = True
disallow_any_unimported = True
disallow_subclassing_any = True

# Disallow untyped definitions/calls
disallow_incomplete_defs = True
disallow_untyped_calls = True
disallow_untyped_decorators = True
disallow_untyped_defs = True
4 changes: 3 additions & 1 deletion src/_pytest/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
__all__ = ["__version__"]

try:
from ._version import version as __version__
from ._version import version

__version__ = version # type: str
except ImportError:
# broken installation, we don't even try
# unknown only works because we do poor mans version compare
Expand Down
51 changes: 50 additions & 1 deletion src/pytest/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,57 @@
from _pytest.warning_types import PytestUnknownMarkWarning
from _pytest.warning_types import PytestWarning

# For mypy Any type checking purposes.
# This file sets disallow_any_expr to ensure that the public API
# does not have dynamic typing via Any. Manually using each public API
# type as an expression to enforce this.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be moved to __init__.pyi?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can explicitly put types into an init.pyi, but that seems less desirable than having the code annotated inline. With the current setup, the code is annotated inline, and the
disallow_any_expr ensures that Any is not used as an annotation in this file.

disallow_any_expr was implemented into mypy here python/mypy#3519
Notably, it requires that each item exists in the file as an expression. The import itself does not count as an expr here - so we need to have a series of a = a style expression assigments

Copy link
Member

@Zac-HD Zac-HD Feb 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case I'd prefer to simply not use disallow_any_expr for now; while I dislike this part of the patch the rest is fantastic and I'd like to merge it soon. We can revisit the more controversial part later 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nipunn1313 I've meant to move the "Manually using each public API type as an expression to enforce this." part of it. It is only boilerplate, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi - sorry for the delay - I was on vacation.

Unfortunately - I do not believe we will get any protection unless we use the variables as expressions within the file. The imports themselves are not type checked or protected - only when used as expressions.

If we use a .pyi file - then the annotations in the .pyi file override the annotations in the actual .py file - and we will lose annotation coverage from the actual code - which is the purpose of #3342 py.typed strategy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disallow_any_expr is actually the most important piece - as it's the one that is guaranteeing that the public interface is typed.

The public interface is only the __init__.py file - so without this boilerplate - we're not actually adding much value.

__version__ = __version__
register_assert_rewrite = register_assert_rewrite
_setup_collect_fakemodule = _setup_collect_fakemodule
cmdline = cmdline
ExitCode = ExitCode
# hookimpl = hookimpl
# hookspec = hookspec
main = main
UsageError = UsageError
__pytestPDB = __pytestPDB
_fillfuncargs = _fillfuncargs
fixture = fixture
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixture and yield_fixture are currently untyped. Shouldn't it fail for them (for example)?

yield_fixture = yield_fixture
freeze_includes = freeze_includes
Session = Session
mark = mark
param = param
Collector = Collector
File = File
Item = Item
exit = exit
fail = fail
importorskip = importorskip
skip = skip
xfail = xfail
Class = Class
Function = Function
Instance = Instance
Module = Module
Package = Package
approx = approx
raises = raises
deprecated_call = deprecated_call
warns = warns
PytestAssertRewriteWarning = PytestAssertRewriteWarning
PytestCacheWarning = PytestCacheWarning
PytestCollectionWarning = PytestCollectionWarning
PytestConfigWarning = PytestConfigWarning
PytestDeprecationWarning = PytestDeprecationWarning
PytestExperimentalApiWarning = PytestExperimentalApiWarning
PytestUnhandledCoroutineWarning = PytestUnhandledCoroutineWarning
PytestUnknownMarkWarning = PytestUnknownMarkWarning
PytestWarning = PytestWarning

set_trace = __pytestPDB.set_trace

# Allow set_trace() to be typed with None
set_trace = __pytestPDB.set_trace # type: ignore

__all__ = [
"__version__",
Expand Down