-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add --warn-implicit-any #3141
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
Add --warn-implicit-any #3141
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3251,12 +3251,15 @@ def name_already_defined(self, name: str, ctx: Context) -> None: | |
self.fail("Name '{}' already defined".format(name), ctx) | ||
|
||
def fail(self, msg: str, ctx: Context, serious: bool = False, *, | ||
blocker: bool = False) -> None: | ||
blocker: bool = False, implicit_any: bool = False) -> None: | ||
if (not serious and | ||
not self.options.check_untyped_defs and | ||
self.function_stack and | ||
self.function_stack[-1].is_dynamic()): | ||
return | ||
if implicit_any: | ||
if not self.options.warn_implicit_any or self.cur_mod_node.is_stub: | ||
return | ||
# In case it's a bug and we don't really have context | ||
assert ctx is not None, msg | ||
self.errors.report(ctx.get_line(), ctx.get_column(), msg, blocker=blocker) | ||
|
@@ -3680,7 +3683,14 @@ def analyze(self, type: Type) -> None: | |
analyzer = TypeAnalyserPass3(self.fail) | ||
type.accept(analyzer) | ||
|
||
def fail(self, msg: str, ctx: Context, *, blocker: bool = False) -> None: | ||
def fail(self, msg: str, ctx: Context, *, blocker: bool = False, | ||
implicit_any: bool = False) -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above - not sure if it's good to have an implicit_any parameter. |
||
if implicit_any: | ||
if not self.options.warn_implicit_any or self.errors.file.endswith('.pyi'): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be a better way for checking for stub files but I can't seem to find it. Why are you limiting the flag to non-stub files anyway? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also can't find it. In stub files, a lot of the times the type argument in genertic types is omitted (e.g., |
||
return | ||
# TempNode, so must have already reported in the first pass | ||
if ctx.get_line() == -1: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a hack but I couldn't find a way to do it better. Perhaps, someone with more mypy experience may help with that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you talking about checking for stubs or checking for temporary nodes? I don't know if I would cosnider either of them a hack (my threshold of hackiness in the error-reporting code is much higher than in normal code); but if anyone has a better solution, it would be nice. |
||
return | ||
self.errors.report(ctx.get_line(), ctx.get_column(), msg) | ||
|
||
def fail_blocker(self, msg: str, ctx: Context) -> None: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,6 +152,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type: | |
elif fullname == 'typing.Tuple': | ||
if len(t.args) == 0 and not t.empty_tuple_index: | ||
# Bare 'Tuple' is same as 'tuple' | ||
self.implicit_any('Tuple without type args', t) | ||
return self.builtin_type('builtins.tuple') | ||
if len(t.args) == 2 and isinstance(t.args[1], EllipsisType): | ||
# Tuple[T, ...] (uniform, variable-length tuple) | ||
|
@@ -174,6 +175,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type: | |
return self.analyze_callable_type(t) | ||
elif fullname == 'typing.Type': | ||
if len(t.args) == 0: | ||
self.implicit_any('Type without type args', t) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There doesn's seem to be a test case for this. It's probably worth having one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
return TypeType(AnyType(), line=t.line) | ||
if len(t.args) != 1: | ||
self.fail('Type[...] must have exactly one type argument', t) | ||
|
@@ -183,6 +185,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type: | |
if self.nesting_level > 0: | ||
self.fail('Invalid type: ClassVar nested inside other type', t) | ||
if len(t.args) == 0: | ||
self.implicit_any('ClassVar without type args', t) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There doesn's seem to be a test case for this. It's probably worth having one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
return AnyType(line=t.line) | ||
if len(t.args) != 1: | ||
self.fail('ClassVar[...] must have at most one type argument', t) | ||
|
@@ -202,6 +205,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type: | |
act_len = len(an_args) | ||
if exp_len > 0 and act_len == 0: | ||
# Interpret bare Alias same as normal generic, i.e., Alias[Any, Any, ...] | ||
self.implicit_any('Generic type without type args', t) | ||
return self.replace_alias_tvars(override, all_vars, [AnyType()] * exp_len, | ||
t.line, t.column) | ||
if exp_len == 0 and act_len == 0: | ||
|
@@ -220,6 +224,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type: | |
# context. This is slightly problematic as it allows using the type 'Any' | ||
# as a base class -- however, this will fail soon at runtime so the problem | ||
# is pretty minor. | ||
self.implicit_any('Assigning value of type Any', t) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There doesn's seem to be a test case for this. It's probably worth having one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
return AnyType() | ||
# Allow unbound type variables when defining an alias | ||
if not (self.aliasing and sym.kind == TVAR and | ||
|
@@ -259,6 +264,7 @@ def visit_unbound_type(self, t: UnboundType) -> Type: | |
fallback=instance) | ||
return instance | ||
else: | ||
self.implicit_any('Fallback', t) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be a debug statement. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, how can you tell that this cannot happen in a production run? |
||
return AnyType() | ||
|
||
def get_type_var_names(self, tp: Type) -> List[str]: | ||
|
@@ -374,6 +380,7 @@ def analyze_callable_type(self, t: UnboundType) -> Type: | |
fallback = self.builtin_type('builtins.function') | ||
if len(t.args) == 0: | ||
# Callable (bare). Treat as Callable[..., Any]. | ||
self.implicit_any('Callable without type args', t) | ||
ret = CallableType([AnyType(), AnyType()], | ||
[nodes.ARG_STAR, nodes.ARG_STAR2], | ||
[None, None], | ||
|
@@ -492,6 +499,10 @@ def builtin_type(self, fully_qualified_name: str, args: List[Type] = None) -> In | |
def tuple_type(self, items: List[Type]) -> TupleType: | ||
return TupleType(items, fallback=self.builtin_type('builtins.tuple', [AnyType()])) | ||
|
||
def implicit_any(self, details: str, t: Type) -> None: | ||
msg = 'Type Any created implicitly: ' + details | ||
self.fail(msg, t, implicit_any=True) # type: ignore | ||
|
||
|
||
class TypeAnalyserPass3(TypeVisitor[None]): | ||
"""Analyze type argument counts and values of generic types. | ||
|
@@ -522,6 +533,8 @@ def visit_instance(self, t: Instance) -> None: | |
if len(t.args) != len(info.type_vars): | ||
if len(t.args) == 0: | ||
# Insert implicit 'Any' type arguments. | ||
if t.type.fullname() not in ('typing.Generator'): | ||
self.implicit_any('{} without type args'.format(t), t) | ||
t.args = [AnyType()] * len(info.type_vars) | ||
return | ||
# Invalid number of type parameters. | ||
|
@@ -624,6 +637,10 @@ def visit_partial_type(self, t: PartialType) -> None: | |
def visit_type_type(self, t: TypeType) -> None: | ||
pass | ||
|
||
def implicit_any(self, details: str, t: Type) -> None: | ||
msg = 'Type Any created implicitly: ' + details | ||
self.fail(msg, t, implicit_any=True) # type: ignore | ||
|
||
|
||
TypeVarList = List[Tuple[str, TypeVarExpr]] | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like adding an extra parameter to fail function might not be the best way to do it.
I would try to keep this function generic for any kind of failures.
Perhaps, you can check if the error is in the caller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Unfortunately, I need to suppress the error message based on the information (
options
, and whether the module being checked is a stub) that's unavailable insideTypeAnalyser
methods where the error occurs. I am not sure what the best fix for this would be. If I don't add an extra parameter tofail
, I'll have to pass all this information intoTypeAnalyser
, but that's not good either sinceTypeAnalyser
should not care about those details.