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

report warn-redundant-annotation (similar to warn-redundant-cast) #18540

Open
asottile opened this issue Jan 26, 2025 · 3 comments
Open

report warn-redundant-annotation (similar to warn-redundant-cast) #18540

asottile opened this issue Jan 26, 2025 · 3 comments
Labels

Comments

@asottile
Copy link
Contributor

Feature

I frequently see people new to python typing over typing everything -- think x: int = function_that_obviously_returns_int(). I would like this to be an optional error

Pitch

in most cases it's somewhat harmless -- it adds a little bit of extra reading and occasionally a slight amount of runtime overhead. but in some cases it makes refactoring and typing a burdensome task (especially if the manual annotation was wrong or subtly wrong, or slightly incorrect, or correct at the time but inconsistent after a refactor).

I would like to suggest a new feature which allows mypy to report this as an optional diagnostic when someone adds an inline annotation which is unnecessary (when mypy already knows the rvalue's type given its expression).


I initially attempted to write something like this as a mypy plugin but I don't think the plugin interface actually gives enough interface points to make this diagnostic possible so I instead looked at how much work it would be to implement this in mypy directly. at a first pass it seems to be not that difficult!

diff --git a/mypy/checker.py b/mypy/checker.py
index 3d0f40283..9229c197c 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -2980,6 +2980,9 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
         Handle all kinds of assignment statements (simple, indexed, multiple).
         """
 
+        if self.expr_checker.accept(s.rvalue) == s.type:
+            self.msg.redundant_annotation(s.type, s.rvalue)
+
         # Avoid type checking type aliases in stubs to avoid false
         # positives about modern type syntax available in stubs such
         # as X | Y.
diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py
index 8f650aa30..5eeb7139c 100644
--- a/mypy/errorcodes.py
+++ b/mypy/errorcodes.py
@@ -164,6 +164,9 @@ NO_UNTYPED_CALL: Final = ErrorCode(
     "Disallow calling functions without type annotations from annotated functions",
     "General",
 )
+REDUNDANT_ANNOTATION: Final = ErrorCode(
+    "redundant-annotation", "Check that the annotation is necessary or can be omitted", "General"
+)
 REDUNDANT_CAST: Final = ErrorCode(
     "redundant-cast", "Check that cast changes type of expression", "General"
 )
diff --git a/mypy/messages.py b/mypy/messages.py
index b63310825..8e9648ed0 100644
--- a/mypy/messages.py
+++ b/mypy/messages.py
@@ -1768,6 +1768,13 @@ class MessageBuilder:
             f'Cannot instantiate type "Type[{format_type_bare(item, self.options)}]"', context
         )
 
+    def redundant_annotation(self, typ: Type, context: Context) -> None:
+        self.fail(
+            f"Annotation {format_type(typ, self.options)} is redundant (inferred type is the same)",
+            context,
+            code=codes.REDUNDANT_ANNOTATION,
+        )
+
     def redundant_cast(self, typ: Type, context: Context) -> None:
         self.fail(
             f"Redundant cast to {format_type(typ, self.options)}",

with an example file like this:

def f() -> int: ...

def g():
   x: int = f()  # stop doing this please!
   ...

my hacked version of mypy produces the desired diagnostic (though I didn't do the part where it's disabled by default and opted into but I just wanted to show a proof of concept!):

$ rm -rf .mypy_cache/ && mypy t.py --show-traceback
t.py:13: error: Annotation "int" is redundant (inferred type is the same)  [redundant-annotation]
Found 1 error in 1 file (checked 1 source file)
@sobolevn
Copy link
Member

sobolevn commented Jan 27, 2025

Great feature! I always wanted something like this, PR is very welcome. Feel free to ping me on it.

I would suggest using is_same_type instead of == though.

@asottile
Copy link
Contributor Author

asottile commented Feb 2, 2025

updated my patch a little bit after playing around with warn-redundant-cast a bit:

diff --git a/mypy/checker.py b/mypy/checker.py
index c69b80a55..9cd8bc60a 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -3077,6 +3077,16 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
         Handle all kinds of assignment statements (simple, indexed, multiple).
         """
 
+        if (
+            not s.is_final_def
+            and not s.is_alias_def
+            and s.unanalyzed_type is not None
+            and s.type is not None
+            and not is_same_type(s.type, AnyType(TypeOfAny.special_form))
+            and is_same_type(s.type, self.expr_checker.accept(s.rvalue))
+        ):
+            self.msg.redundant_annotation(s.type, s.rvalue)
+
         # Avoid type checking type aliases in stubs to avoid false
         # positives about modern type syntax available in stubs such
         # as X | Y.

and I'm slightly worried that this idea is dead-in-the-water.

on the one hand, it points out some pretty nice fixes such as:

edges: dict[str, list[str]] = {"A": ["B", "C"], "B": ["C"], "C": ["B", "D"], "D": []}

on the other hand it also points out some stuff that might be there intentionally to prevent a typo such as:

legacy_bundled_packages: dict[str, str] = {

(I also would need to do some special casing for NamedTuple / dataclass / etc. since it currently flags defaults there) -- oh and ClassVar too

thoughts?

@sobolevn
Copy link
Member

sobolevn commented Feb 2, 2025

thoughts?

Maybe we can start small maybe, for non-generic types only? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants