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

implement @no_type_check for class defs #6584

Closed
wants to merge 1 commit into from

Conversation

lunixbochs
Copy link

@lunixbochs lunixbochs commented Mar 22, 2019

Fixes #607 (which is incorrectly titled).

This adds TypeInfo.is_no_type_check, and enables TypeInfo.fallback_to_any for classes decorated with @no_type_check, which is checked in a couple of places to abort the type checker early and stub method types as Any.

I'm not sure confident about my use of decorator.name in ('no_type_check', 'typing.no_type_check' in semanal.py. I see RefName.fullname = 'typing.no_type_check' was used elsewhere but it wasn't a RefName in the AST during semanal and I didn't look too far into that.

This is my current test:

from typing import no_type_check, Any

@no_type_check
class TestClass:
    bar: {'x': 1} = 1

    def meth1(): pass
    def meth2(a: {'x': 1}) -> {'x': 1}:
        def garbage_inner(a: {'x': 1}):
            pass

    def test(a: int): pass

TestClass().test(1)
TestClass().doesntexist()

class Test(TestClass):
    def test2(self, a: int): pass

t = Test()
t.meth1()
t.doesntexist2()
t.test(1)
t.test2(1)

x: int = TestClass().bar

With @no_type_check, mypy throws a Missing return statement for the nested garbage_inner definition, but suppresses the rest of the type errors for this class and subclass.

Without @no_type_check, we get this nice pile of errors:

repro.py:4: error: Invalid type comment or annotation
repro.py:6: error: Method must have at least one argument
repro.py:7: error: Invalid type comment or annotation
repro.py:8: error: Invalid type comment or annotation
repro.py:11: error: Self argument missing for a non-static method (or an invalid type for self)
repro.py:13: error: Too many arguments for "test" of "TestClass"
repro.py:14: error: "TestClass" has no attribute "doesntexist"
repro.py:21: error: "Test" has no attribute "doesntexist2"
repro.py:22: error: Too many arguments for "test" of "TestClass"

TODO (I'd like advice from core contributors on these):

  • I think I should write in-tree tests for this, but I could use guidance on that.
  • confirm my decorator.name check is correct.
  • is it possible to suppress the no return error?
  • am I missing any additional obvious class type behavior in my test that should be suppressed/stubbed by the decorator?

@lunixbochs
Copy link
Author

Can a maintainer look at this / comment?

@ilevkivskyi
Copy link
Member

Few comments:

  • This is still a draft, please make it non-draft and resolve merge conflicts if you want to get a review
  • Your decorator.name check is wrong, you should check for a RefExpr and its fullname instead.
  • I don't think we need a new flag for this. I think fallback_to_any plus replicating effect of @no_type_check for every method should be enough.
  • Please add tests for this, I think https://github.com/python/mypy/blob/master/test-data/unit/check-classes.test is the best place.

@lunixbochs
Copy link
Author

lunixbochs commented Mar 6, 2020

  • At the point of SemanticAnalyzer.analyze_class, all defn.decorators are still of type NameExpr. How/where can I get RefExpr decorators instead at that point?
  • I used the special flag because I needed to propagate the behavior to semanal3 and analyze_instance_member_access, which fallback_to_any isn't expressive enough to do. I don't see how to propagate the behavior to methods at all at the point where we're just skipping analyze_class.

I'm also now getting fallback can't be filled out until semanal I guess due to skipping analysis with the new semantic analyzer.

@msullivan
Copy link
Collaborator

I'm going to close this because it is against the old semantic analyzer which is totally gone now. Feel free to redo this against the current code base and I'll review it.

@msullivan msullivan closed this Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support @no_type_check for classes
3 participants