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

Split out protocol required members from reportGeneralTypeIssues #6072

Closed
Jonathan-Landeed opened this issue Oct 3, 2023 · 5 comments
Closed
Labels
as designed Not a bug, working as intended enhancement request New feature or request

Comments

@Jonathan-Landeed
Copy link

Jonathan-Landeed commented Oct 3, 2023

Abstract properties are very hard to accomplish in python. There are some methods based around abstract classes and descriptor chaining, but descriptor chaining is going away and generally only worked at instantiation/access time (no type errors). I haven't found anything clean that errors at declaration or import time.

Protocols are a decent way of catching unimplemented abstract properties early. python/cpython#89519 (comment)

"Class derives from one or more protocol classes but does not implement all required members" is currently a reportGeneralTypeIssues error, which are a bit of a catchall and very tempting to disable. Unimplemented required protocol members are almost always going to be addressed, though.

If we could split it out to it's own error it would be great for checking implementations and configs and stuff before runtime.

@erictraut
Copy link
Collaborator

Could someone from the pylance team please transfer this to the pyright project since it's a core type checking feature request? Thanks!

@debonte debonte transferred this issue from microsoft/pylance-release Oct 3, 2023
@erictraut
Copy link
Collaborator

I agree that the reportGeneralTypeIssues is a bit of a catchall, but it's also where the most basic of type checking errors (including type consistency violations) are reported, so I strongly recommend against disabling it. If you disable this diagnostic check, you might as well not bother running a type checker.

Protocols are a relatively advanced type checking feature, so I'm surprised that you want to make use of protocols and enforce them but then ignore more fundamental type violations.

@Jonathan-Landeed
Copy link
Author

I mostly write Rust but also have a Django project. Makes it really hard to use reportGeneralTypeIssues and really annoying when something doesn't error until runtime. Protocols seem like the best way to minimize boilerplate for a lot of reusable behavior (just subclass and set some class attributes).

Also looked at this but it's a runtime error: https://stackoverflow.com/a/54814371/5679413

And honestly protocols seem much simpler than whatever this is: https://stackoverflow.com/a/75253599/5679413

@erictraut
Copy link
Collaborator

As I mentioned, if you're going to use a type checker in Python, the most basic of errors are simple type violations that will be reported in the reportGeneralTypeIssues. If you disable this check, you might as well not run a type checker.

I'm going to close this issue because I don't think it makes sense to move protocol required members to a separate diagnostic rule.

@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2023
@erictraut erictraut added as designed Not a bug, working as intended and removed needs decision labels Oct 4, 2023
@Jonathan-Landeed
Copy link
Author

Jonathan-Landeed commented Oct 4, 2023

If you disable this check, you might as well not run a type checker.

I understand it's your decision, but I literally just described my usecase. If I had the option I would absolutely set up a commit hook to check for protocol violations but ignore reportGeneralTypeIssues. And though I could probably write a script to check the exact error message in that setup, it would be very convenient to have a separate error code to enable checks for in my IDE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants