mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 21:35:58 +00:00
[flake8-bugbear
] Implement return-in-generator
(B901
) (#11644)
## Summary This PR implements the rule B901, which is part of the opinionated rules of `flake8-bugbear`. This rule seems to be desired in `ruff` as per https://github.com/astral-sh/ruff/issues/3758 and https://github.com/astral-sh/ruff/issues/2954#issuecomment-1441162976. ## Test Plan As this PR was made closely following the [CONTRIBUTING.md](8a25531a71/CONTRIBUTING.md
), it tests using the snapshot approach, that is described there. ## Sources The implementation is inspired by [the original implementation in the `flake8-bugbear` repository](d1aec4cbef/bugbear.py (L1092)
). The error message and [test file](d1aec4cbef/tests/b901.py
) where also copied from there. The documentation I came up with on my own and needs improvement. Maybe the example given in https://github.com/astral-sh/ruff/issues/2954#issuecomment-1441162976 could be used, but maybe they are too complex, I'm not sure. ## Open Questions - [ ] Documentation. (See above.) - [x] Can I access the parent in a visitor? The [original implementation](d1aec4cbef/bugbear.py (L1100)
) references the `yield` statement's parent to check if it is an expression statement. I didn't find a way to do this in `ruff` and used the `is_expresssion_statement` field on the visitor instead. What are your thoughts on this? Is it possible and / or desired to access the parent node here? - [x] Is `Option::is_some(...)` -> `...unwrap()` the right thing to do? Referring to [this piece of code](9d5a280f71/crates/ruff_linter/src/rules/flake8_bugbear/rules/return_x_in_generator.rs (L91-L96)
). From my understanding, the `.unwrap()` is safe, because it is checked that `return_` is not `None`. However, I feel like I missed a more elegant solution that does both in one. ## Other I don't know a lot about this rule, I just implemented it because I found it in a https://github.com/astral-sh/ruff/labels/good%20first%20issue. I'm new to Rust, so any constructive critisism is appreciated. --------- Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
parent
91a5fdee7a
commit
312f6640b8
8 changed files with 244 additions and 0 deletions
|
@ -383,6 +383,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Flake8Bugbear, "033") => (RuleGroup::Stable, rules::flake8_bugbear::rules::DuplicateValue),
|
||||
(Flake8Bugbear, "034") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ReSubPositionalArgs),
|
||||
(Flake8Bugbear, "035") => (RuleGroup::Stable, rules::flake8_bugbear::rules::StaticKeyDictComprehension),
|
||||
(Flake8Bugbear, "901") => (RuleGroup::Preview, rules::flake8_bugbear::rules::ReturnInGenerator),
|
||||
(Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
|
||||
(Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),
|
||||
(Flake8Bugbear, "909") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation),
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue