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

Check that collection generics are parametrised #118

Closed
kkom opened this issue Jun 26, 2021 · 4 comments
Closed

Check that collection generics are parametrised #118

kkom opened this issue Jun 26, 2021 · 4 comments
Labels
wontfix This will not be worked on

Comments

@kkom
Copy link

kkom commented Jun 26, 2021

Description

Introduce warnings checking that collection generics are parametrised (even just by typing.Any).

I'm happy to work on it, but I would like to first hear feedback if this check makes sense, if yes what generics should be covered, should it be disabled by default for backwards compatibility and lastly – what are some good commits that I can follow to see how to implement it.

Rationale/Use Case

I often see people annotate their code with really unhelpful types such as list, typing.List, dict or typing.Dict purely to satisfy flake8-annotations warnings.

The idea is to nudge the developer into considering how their generic will be parametrised.

I.e. this would fail:

def my_fun(data: dict) -> None:
  return

But that would be fine:

from typing import Any

def my_fun(data: dict[Any, Any]) -> None:
  return

Of course, the idea is that this would make the developer consider declaring the type parameters even more precisely, e.g.:

from typing import Any

def my_fun(data: dict[str, Any]) -> None:
  return

Or even:

def my_fun(data: dict[str, int]) -> None:
  return

CC: @flaviojdz – we spoke about it a few months ago!

@sco1
Copy link
Owner

sco1 commented Jun 26, 2021

I don't think this can be sufficiently well implemented within Flake8's framework without magic, since we have no ability to trace imports between files. Even if imports were just ignored, this plugin currently has no mechanism for tracking local state, and likely isn't worth the complexity, which is why #98 and #99 use config magic to accomplish their respective tasks.

I can sympathize with the motivation, but I think if you're reaching for something like this you're better off with mypy, which has a disallow_any_generics flag for exactly this problem.

@kkom
Copy link
Author

kkom commented Jun 26, 2021

Thanks for the quick response @sco1!

I wasn't aware of disallow_any_generics - this looks like a perfect solution, especially as we already use mypy in the project.

This is good to close then, thanks for the tip!

@kkom kkom closed this as completed Jun 26, 2021
@kkom
Copy link
Author

kkom commented Jun 26, 2021

Just out of curiosity – I also saw https://github.com/best-doctor/flake8-annotations-complexity which intuitively should have access to the kind of information we would need here. I suppose that the AST helpers from that project count as the magic you were referring to?

Disclaimer: I haven't read carefully the code of neither this nor that plugin.

@sco1
Copy link
Owner

sco1 commented Jun 27, 2021

Consider the following:

import typing as t

SOME_TYPE = t.List[int]


def foo() -> t.List[int]:
    ...

def bar() -> SOME_TYPE:
    ...

Our returns end up looking something like:

# foo
Subscript(value=Attribute(value=Name(id='t', ctx=Load()), attr='List', ctx=Load()), slice=Name(id='int', ctx=Load()), ctx=Load())

# bar
Name(id='SOME_TYPE', ctx=Load())

For bar, you cannot determine from its FunctionDef node alone whether or not the generic explicitly specifies type parameters. You need to be keeping track of module & context state (e.g. tracing imports, conditionals, redefinitions, etc.) in order to have a chance at doing this. It's doable, but it's complicated.

flake8-annotations-complexity doesn't do this either, it's only calculating complexity of the annotations defined in the FunctionDef node, or variable annotation.

@sco1 sco1 added the wontfix This will not be worked on label Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants