Turn on strict optional in CI #3646
Conversation
|
Travis failure is just #3543 |
mypy/typeanal.py
Outdated
| @@ -522,8 +523,8 @@ def bind_function_type_variables(self, | |||
| self.fail("Type variable '{}' is bound by an outer class".format(name), defn) | |||
| self.tvar_scope.bind(name, tvar) | |||
| binding = self.tvar_scope.get_binding(tvar.fullname()) | |||
| assert binding is not None | |||
| defs.append(binding) | |||
| if binding is not None: | |||
There was a problem hiding this comment.
This changes semantics. Are you sure that the change is safe?
There was a problem hiding this comment.
Oops, sorry, this is a leftover from debugging the fullname problem, this change is not needed. Fixed.
| @@ -252,7 +252,7 @@ MypyFile:1( | |||
| IntExpr(1)) | |||
| AssignmentStmt:3( | |||
| NameExpr(y* [m]) | |||
| NameExpr(x [m])))) | |||
| NameExpr(x [__main__.A.x])))) | |||
There was a problem hiding this comment.
Why has the test output changed?
There was a problem hiding this comment.
This is because of the mentioned change in semanal.py. Previously fullname was None in these examples, so that they were formatted in simplified manner, now they show their fullnames.
| @@ -68,6 +68,7 @@ def bind(self, name: str, tvar_expr: TypeVarExpr) -> TypeVarDef: | |||
|
|
|||
| def get_binding(self, item: Union[str, SymbolTableNode]) -> Optional[TypeVarDef]: | |||
| fullname = item.fullname if isinstance(item, SymbolTableNode) else item | |||
| assert fullname is not None | |||
There was a problem hiding this comment.
Previously this might have been successful even if fullname or item is None, but apparently the original intention was that fullname should be defined here.
There was a problem hiding this comment.
Yes, I have added a line in semanal.analyze_lvalue, so that class members (i.e. MDEF nodes) always get their fullname generated (I don't know why it was not like this before, maybe just an omission). Now this should never be None.
|
@JukkaL Thanks for review! I fixed the accidental change in the new commit. |
Now
--strict-optionalcan be turned on/off per file.This PR turns on strict optional self-test for most files (there are still 16 files that are skipped, I will clean-up them in subsequent PRs).
There is an observation:
is allowed even with
--strict-optional. I understand this is because of limitations of the type comment syntax, but this leads to some hard to find bugs. It would be great to be able to catch this somehow.