Skip to content

Conversation

@kedder
Copy link

@kedder kedder commented Feb 3, 2019

Only show error when not-instantiatable class is instantiated. This will allow plugins to allow instantiation of certain classes, even if they are abstract or specify a protocol. Or, conversely, disallow instantiation even if class is not abstract or protocol.

The particular problem that called for this solution: mypy-zope plugin tries to implement zope interfaces as "protocols". Zope has the adaptation pattern that looks like instantiation of an interface:

adapter = IMyInterface(context)

since IMyInterface is marked as "protocol", mypy unconditionally rejects such syntax with the message

error: Cannot instantiate protocol class "IMyInterface"

The proposed change would allow zope plugin to mark IMyInterface as both "protocol" and instantiatable" at the same time, effectively disabling the error message.

Only show error when not-instantiatable class is instantiated. This will
allow plugins to allow instantiation of certain classes, even if they
are abstract or specify a protocol.

The particular problem that called for this solution:
[mypy-zope](https://github.com/Shoobx/mypy-zope) plugin
tries to implement zope interfaces as "protocols". Zope has the adaptation
pattern that looks like instantiation of an interface:

```python
adapter = IMyInterface(context)
```

since `IMyInterface` is marked as "protocol", mypy unconditionally rejects
such syntax with the message

```
error: Cannot instantiate protocol class "IMyInterface"
```

 The proposed change would allow zope plugin to mark
`IMyInterface` as both "protocol" and "instantiatable", effectively
disabling the error message.
@kedder
Copy link
Author

kedder commented Feb 3, 2019

Now with a test that demonstrates how plugins can mark classes to be not-instantiatable without them being abstract or protocol.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

I don't think the need of one plugin justifies addition of a new TypeInfo attribute with a narrow scope. Also this approach doesn't scale, we can't add a new attribute for every plugin use case. Potentially, we could add a plugin hook to filter out errors messages for given error codes in given context (the error codes haven't been added yet, but there is ongoing work, see #6152)

One interesting idea in this PR is to add a method (or property) can_be_instantiated() and use it instead of the current checks.

@kedder
Copy link
Author

kedder commented Feb 3, 2019

I don't think the need of one plugin justifies addition of a new TypeInfo attribute with a narrow scope.

The point is that current rules of what mypy considers to be instantiatable are pretty limited and hardcoded (abstracts and protocols are not, everything else is) and cannot be affected by plugins. There are lot of situations and frameworks where these rules are not sufficient and plugins could be used to support such frameworks, if mypy would be more flexible in this regard.

The plugin example in the description is just to give a broader context. Another useful example is in the test case, just to show that the PR is not specific to a single plugin. But if this is not convincing enough, I won't argue.

@ilevkivskyi
Copy link
Member

But if this is not convincing enough, I won't argue.

TBH, I am still not convinced (mostly because I don't like ad hoc solutions). You might however ask other mypy devs.

@ilevkivskyi
Copy link
Member

As we discussed off-line, a better solution would be to use another hook that filters errors using error codes, after the latter are implemented, see #6472. Closing this one therefore.

@ilevkivskyi ilevkivskyi closed this May 8, 2019
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.

2 participants