[ty] Fix bad diagnostic range for incorrect implicit __init_subclass__ calls#24541
[ty] Fix bad diagnostic range for incorrect implicit __init_subclass__ calls#24541AlexWaygood merged 1 commit intomainfrom
__init_subclass__ calls#24541Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 87.72%. The percentage of expected errors that received a diagnostic held steady at 82.85%. The number of fully passing files held steady at 74/132. |
Memory usage reportMemory usage unchanged ✅ |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
40 | 0 | 0 |
invalid-return-type |
1 | 0 | 0 |
| Total | 41 | 0 | 0 |
Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.
| // or `__new__`/`__prepare__` on the metaclass -- but positional arguments are not, and neither | ||
| // is the special keyword argument `metaclass`. These need to be excluded from the | ||
| // argument index when looking up the relevant keyword-argument node. | ||
| (ast::AnyNodeRef::StmtClassDef(class_def), Some(argument_index)) => { |
There was a problem hiding this comment.
Feels more intuitive if the index is into args.iter_source_order, so we don't have to re-encode this logic here? But not blocking if that's the way it works today.
There was a problem hiding this comment.
hmm, not totally sure I understand what you're suggesting -- are you saying you think iter_source_order() should just skip past metaclass keywords on an ast::Arguments? That feels a bit too opinionated for a method in the ruff_python_ast crate
There was a problem hiding this comment.
I think I'm asking why the argument_index passed into this function hasn't already taken the metaclass agument into account. Like why do we have to translate it to an index that omits metaclass here?
There was a problem hiding this comment.
that would be pretty hard to fix unless we avoided using our generalised call-binding machinery for validating __init_subclass__ calls. And there are loads of benefits to using our generalised call-binding machinery for doing that validation -- we get overload evaluation for free, generics solving for free, etc.
|
eff2067 to
dd11fc4
Compare
dd11fc4 to
9451849
Compare
| let range = all_arguments_range(node); | ||
| if let Some(builder) = context.report_lint(&CALL_NON_CALLABLE, range) { |
There was a problem hiding this comment.
I don't love the way we now have to remember to call let range = all_arguments_range(node); every time we call context.report_lint() in Bindings::reoprt_diagnostic(). But this is sort-of a pre-existing problem in this file in other places: we already have to remember to call Self::get_node() every time we call context.report_lint() in BindingError::report_diagnostic().
* main: [ty] Fix bad diagnostic range for incorrect implicit `__init_subclass__` calls (#24541) [ty] Add a `SupportedPythonVersion` enum (#24412) [ty] Ignore unsupported editor-selected Python versions (#24498) [ty] Add snapshots for `__init_subclass__` diagnostics (#24539) [ty] Minor fix in tests (#24538) [ty] Allow `Final` variable assignments in `__post_init__` (#24529) [ty] Expand test suite for assignment errors (#24537) [ty] Use `map`, not `__map`, as the name of the mapping parameter in `TypedDict` `__init__` methods (#24535) [ty] Rework logic for synthesizing `TypedDict` methods (#24534) [flake8-bandit] Fix S103 false positives and negatives in mask analysis (#24424) [ty] mdtest.py: update dependencies (#24533) Rename patterns and arguments source order iterator method (#24532) [ty] Omit invalid keyword arguments from `TypedDict` signature (#24522) [ty] support super() in metaclass methods (#24483) [ty] Synthesize `__init__` for `TypedDict` (#24476)
* main: Bump typing conformance suite commit to latest upstream (#24553) [ty] Reject deleting`Final` attributes (#24508) [ty] Respect property deleters in attribute deletion checks (#24500) [ty] stop unioning Unknown into types of un-annotated attributes (#24531) [ty] Fix bad diagnostic range for incorrect implicit `__init_subclass__` calls (#24541) [ty] Add a `SupportedPythonVersion` enum (#24412) [ty] Ignore unsupported editor-selected Python versions (#24498) [ty] Add snapshots for `__init_subclass__` diagnostics (#24539) [ty] Minor fix in tests (#24538) [ty] Allow `Final` variable assignments in `__post_init__` (#24529) [ty] Expand test suite for assignment errors (#24537) [ty] Use `map`, not `__map`, as the name of the mapping parameter in `TypedDict` `__init__` methods (#24535) [ty] Rework logic for synthesizing `TypedDict` methods (#24534) [flake8-bandit] Fix S103 false positives and negatives in mask analysis (#24424) [ty] mdtest.py: update dependencies (#24533) Rename patterns and arguments source order iterator method (#24532) [ty] Omit invalid keyword arguments from `TypedDict` signature (#24522) [ty] support super() in metaclass methods (#24483) [ty] Synthesize `__init__` for `TypedDict` (#24476)
Summary
Incorrect keyword arguments passed to a class currently cause us to emit a diagnostic with a range spanning the whole class node. That can be a huge range if the class contains multiple statements, as demonstrated by the snapshots added in #24539. This PR fixes that.
Test Plan
Snapshots updated