Skip to content
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.

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

Don't check plugin-generated functions #16524

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
7 changes: 4 additions & 3 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,9 @@ def check_func_item(

If type_override is provided, use it as the function type.
"""
if defn.is_plugin_generated():
return

self.dynamic_funcs.append(defn.is_dynamic() and not type_override)

with self.enter_partial_types(is_function=True):
Expand Down Expand Up @@ -1324,9 +1327,7 @@ def check_func_def(
)

# Ignore plugin generated methods, these usually don't need any bodies.
if defn.info is not FUNC_NO_INFO and (
defn.name not in defn.info.names or defn.info.names[defn.name].plugin_generated
):
if defn.is_plugin_generated():
show_error = False

# Ignore also definitions that appear in `if TYPE_CHECKING: ...` blocks.
Expand Down
6 changes: 6 additions & 0 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,12 @@ def __init__(
if self.arguments[i] is None and i < self.max_fixed_argc():
self.min_args = i + 1

def is_plugin_generated(self) -> bool:
if self.info is FUNC_NO_INFO:
return False
sym = self.info.names.get(self.name)
return not sym or sym.plugin_generated

def max_fixed_argc(self) -> int:
return self.max_pos

Expand Down
4 changes: 4 additions & 0 deletions mypy/plugins/attrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,10 @@ def _attribute_from_attrib_maker(
converter = convert
converter_info = _parse_converter(ctx, converter)

if init_type is None and ctx.api.options.disallow_untyped_defs:
assert lhs.node is not None
ctx.api.msg.need_annotation_for_var(lhs.node, stmt)

name = unmangle(lhs.name)
return Attribute(
name, ctx.cls.info, attr_has_default, init, kw_only, converter_info, stmt, init_type
Expand Down
18 changes: 9 additions & 9 deletions test-data/unit/check-flags.test
Original file line number Diff line number Diff line change
Expand Up @@ -1303,18 +1303,18 @@ def f(i: int, s):
main:3: error: Function is missing a return type annotation
main:3: error: Function is missing a type annotation for one or more arguments

[case testDisallowIncompleteDefsAttrsNoAnnotations]
# flags: --disallow-incomplete-defs
[case testDisallowUntypedDefsAttrsNoAnnotations]
# flags: --disallow-untyped-defs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to also test with --disallow-any-explicit --disallow-any-decorated, which were used in #16454?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a test for the attrs "regression".

I didn't actually add a regression test for #16454, perhaps I should.

I remember I tried at first, and it was made tricky due --disallow-any-explicit --disallow-any-decorated flagging errors in the typeshed fixtures. We normally exclude errors in the typeshed from mypy's output, and this should apparently be fixed for test fixtures.

Copy link
Member

Choose a reason for hiding this comment

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

The easiest workaround might be to just # type: ignore the fixtures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about 33d06cc?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm I'm a little worried about it (see my comment). If there is no other way, it's fine to omit the test cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I follow.

testDisallowUntypedDefsAttrsNoAnnotations is a "casualty" of this change. I can definitely add --disallow-any-explicit --disallow-any-decorated to the flags in this test and it still passes. I'm just concerned that it'll muddy the test, by confusing the reader about what I'm testing, i.e. it'll be akin to adding --strict-equality to the flags. Yes, it still passes, but it's not what's under test.

import attrs

@attrs.define
class Unannotated:
foo = attrs.field()
foo = attrs.field() # E: Need type annotation for "foo"

[builtins fixtures/plugin_attrs.pyi]

[case testDisallowIncompleteDefsAttrsWithAnnotations]
# flags: --disallow-incomplete-defs
[case testDisallowUntypedDefsAttrsWithAnnotations]
# flags: --disallow-untyped-defs
import attrs

@attrs.define
Expand All @@ -1323,14 +1323,14 @@ class Annotated:

[builtins fixtures/plugin_attrs.pyi]

[case testDisallowIncompleteDefsAttrsPartialAnnotations]
# flags: --disallow-incomplete-defs
[case testDisallowUntypedDefsAttrsPartialAnnotations]
# flags: --disallow-untyped-defs
import attrs

@attrs.define
class PartiallyAnnotated: # E: Function is missing a type annotation for one or more arguments
class PartiallyAnnotated:
bar: int = attrs.field()
baz = attrs.field()
baz = attrs.field() # E: Need type annotation for "baz"

[builtins fixtures/plugin_attrs.pyi]

Expand Down