[ty] Add more autofixes - #28029
Conversation
AlexWaygood
added
ty
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 97.69%. The percentage of expected errors that received a diagnostic held steady at 93.71%. The number of fully passing files held steady at 110/136. |
Memory usage reportMemory usage unchanged ✅ |
|
| Project | Old Time | New Time | Change |
|---|---|---|---|
dd-trace-py |
3.36s | 0.99s | -71% |
9db74cb to
744a2ac
Compare
| diagnostic.annotate( | ||
| Annotation::secondary(span).message("Did you mean `NotImplementedError`?"), | ||
| ); | ||
| autofix_with_notimplementederror(context, &mut diagnostic, sub_node); |
There was a problem hiding this comment.
This is a super unlikely case, but given:
try:
raise Exception()
except (NotImplemented, NotImplemented):
passBoth elements share one diagnostic, and each helper call replaces its existing fix via set_fix. The resulting quick fix only produces except (NotImplemented, NotImplementedError):, leaving the diagnostic unresolved, and the preview includes the same help text twice.
This is so unlikely to occur that it probably doesn't matter, though it could be a risk if we add more invalid-exception-caught autofixes in the future (e.g. an autofix for except ValueError() to except ValueError).
There was a problem hiding this comment.
This is a super unlikely case
yeah, my codex kept on telling me off about this too, and I kept on telling it I didn't care 😆
| index | ||
| .place_table(scope) | ||
| .symbol_by_name(name) | ||
| .is_some_and(|symbol| symbol.is_bound() || symbol.is_declared()) |
There was a problem hiding this comment.
Here's another case we currently miss:
class C:
global list
list = 42
value: [int]But we also currently error on that pattern with unresolved-global, so I think it's fine.
There was a problem hiding this comment.
if a user does this then they deserve an incorrect autofix 😆
3 participants
Summary
Add a new
SemanticMode::definitely_has_builtin_binding()that allows us to cheaply and easily query whether a builtin name such aslistorinthas been shadowed by a variable in an enclosing scope when viewed from a given node in the AST. Once this method is in place, it becomes much more trivial to add autofixes to various diagnostics, which this PR therefore does.The new method does not attempt to fully model Python's name-lookup semantics. It doesn't need to -- it's okay to be conservative when offering autofixes. The method therefore errs on the side of caution and only returns
trueif it can be certain that the name passed in was definitely not shadowed in an enclosing scope.The new method is similar to the
SemanticModel::has_builtin_binding()method that Ruff has had for a long time.The new
SemanticModelmethod is pulled out of #27634, where I'm using it for several more diagnostic autofixes that make it much easier to understand how ty is trying to tell you to rewrite your code (in my opinion).Test Plan
mdtests and snapshots