Skip to content

Don't check plugin-generated functions #16524

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,8 @@ def parse_file(
self.errors.ignored_files.add(path)
tree = parse(source, path, id, self.errors, options=options)
tree._fullname = id
if options.use_builtins_fixtures and id in CORE_BUILTIN_MODULES:
tree._is_typeshed_file = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd rather not do this if this can silence errors if test fixtures have issues. It would be better to either add a minimal custom fixture that is only used in the relevant test cases or to omit the test cases that need this (I trust that you've tested it already).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_is_typeshed_file does not silence all errors, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The premise of errors in fixtures becoming obscured concerned me too, but having the test environment closer to actual environment seemed more prudent.

If you feel particularly iffy about it, sure, I can add # type:ignore[misc] to a handful of places in the fixtures.

self.add_stats(
files_parsed=1,
modules_parsed=int(not tree.is_stub),
Expand Down
9 changes: 6 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_synthetic():
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_synthetic():
show_error = False

# Ignore also definitions that appear in `if TYPE_CHECKING: ...` blocks.
Expand Down Expand Up @@ -4875,6 +4876,8 @@ def visit_del_stmt(self, s: DelStmt) -> None:
)

def visit_decorator(self, e: Decorator) -> None:
if e.func.is_synthetic():
return
for d in e.decorators:
if isinstance(d, RefExpr):
if d.fullname == "typing.no_type_check":
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_synthetic(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
5 changes: 5 additions & 0 deletions mypy/plugins/attrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,11 @@ def _attribute_from_attrib_maker(
rvalue,
code=LITERAL_REQ,
)

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, alias, ctx.cls.info, attr_has_default, init, kw_only, converter_info, stmt, init_type
Expand Down
15 changes: 15 additions & 0 deletions test-data/unit/check-dataclasses.test
Original file line number Diff line number Diff line change
Expand Up @@ -2544,3 +2544,18 @@ class Base:
class Child(Base):
y: int
[builtins fixtures/dataclasses.pyi]

[case testDataclassDisallowAny]
# flags: --disallow-any-explicit --disallow-any-decorated

from dataclasses import dataclass
from typing import Any

# Ensures that when Any-typed fields end up in synthetic functions,
# those functions are not flagged since there's nothing the user can do about that
@dataclass
class C:
a: Any # E: Explicit "Any" is not allowed

[typing fixtures/typing-full.pyi]
[builtins fixtures/dataclasses.pyi]
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