mypy icon indicating copy to clipboard operation
mypy copied to clipboard

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

Open asottile opened this issue 11 months ago • 4 comments

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)

asottile avatar Jan 26 '25 21:01 asottile

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.

sobolevn avatar Jan 27 '25 12:01 sobolevn

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: https://github.com/python/mypy/blob/f44a60dd9d02ce496561c08ded134d5e2e3bc8ca/mypy/test/testgraph.py#L32

on the other hand it also points out some stuff that might be there intentionally to prevent a typo such as: https://github.com/python/mypy/blob/f44a60dd9d02ce496561c08ded134d5e2e3bc8ca/mypy/stubinfo.py#L34

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

thoughts?

asottile avatar Feb 02 '25 21:02 asottile

thoughts?

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

sobolevn avatar Feb 02 '25 22:02 sobolevn

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

I'm assuming this new check will be opt-in, and it could always be disabled for specific code via # type: ignore

Why not make a PR, even if it's Draft?

belm0 avatar Jun 11 '25 15:06 belm0