-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[refurb
] Implement unnecessary-enumerate
(FURB148
)
#7454
Conversation
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
Isn't this explicitly what itertools.islice is designed for? Specifically, |
Yes, and it's what I would personally do. But, looking online, there are alternative methods without a consensus over which is best. My concern is that a non-trivial number of people would consider an Very happy to adapt the merge request to add an EDIT: The |
The first upvoted answer wouldn't work on generators. The 2nd upvoted
answer is verbose and just suggests calling next on the iterator, leaving
islice as the most upvoted answer in that StackOverflow post.
…On Sun, Sep 17, 2023 at 3:57 PM Tom Kuson ***@***.***> wrote:
Isn't this explicitly what itertools.islice
<https://docs.python.org/3/library/itertools.html#itertools.islice> is
designed for? Specifically, itertools.islice(start_index, None).
Yes, and it's what I would personally do. But, looking online
<https://stackoverflow.com/questions/10079216/skip-first-entry-in-for-loop-in-python>,
there are alternative methods without a consensus over which is best. My
concern is that a non-trivial number of people would consider an
itertools.isclice auto-fix (or alternative, for that matter) unexpected
behaviour.
Very happy to adapt the merge request to add an islice auto-fix if the
maintainers support it, however!
—
Reply to this email directly, view it on GitHub
<#7454 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPVMX6MLD56J5SXFBJCRMLX25I35ANCNFSM6AAAAAA43RTTEQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
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.
This is looking good, thanks for doing this!
I think the implementation is similar to https://docs.astral.sh/ruff/rules/incorrect-dict-iterator/, maybe in the future we could combine and parameterize the implementation.
/// Returns `true` if the given expression is either an unused value or a tuple of unused values. | ||
pub fn is_unused(expr: &Expr, semantic: &SemanticModel) -> bool { |
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 wonder if this could be made a method on the SemanticModel
:
impl SemanticModel {
// ...
pub fn is_expr_unused(&self, expr: &Expr) -> bool {
// ...
}
}
checker.semantic().is_expr_unused(expr)
But then it would mean that we would need to visit all relevant expressions in this context which I think is out of scope for this PR.
if let Some(suggestion) = iterable_suggestion | ||
.as_ref() | ||
.and_then(SourceCodeSnippet::full_display) |
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.
You could probably just use map
instead:
if let Some(suggestion) = iterable_suggestion | |
.as_ref() | |
.and_then(SourceCodeSnippet::full_display) | |
if let Some(suggestion) = iterable_suggestion.map(SourceCodeSnippet::full_display) |
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.
The suggestion causes a type mismatch, is there a better way?
if let Some(suggestion) = iterable_suggestion | ||
.as_ref() | ||
.and_then(SourceCodeSnippet::full_display) |
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.
Same suggestion as above.
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
…te.rs" This reverts commit 62c84be.
Thanks! I made some modifications. I think the |
8a311a9
to
faa4576
Compare
Oops, that was silly of me. Thanks! |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://docs.astral.sh/ruff) ([source](https://togithub.com/astral-sh/ruff), [changelog](https://togithub.com/astral-sh/ruff/releases)) | `==0.0.290` -> `==0.0.291` | [![age](https://developer.mend.io/api/mc/badges/age/pypi/ruff/0.0.291?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/ruff/0.0.291?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/ruff/0.0.290/0.0.291?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/ruff/0.0.290/0.0.291?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>astral-sh/ruff (ruff)</summary> ### [`v0.0.291`](https://togithub.com/astral-sh/ruff/releases/tag/v0.0.291) [Compare Source](https://togithub.com/astral-sh/ruff/compare/v0.0.290...v0.0.291) <!-- Release notes generated using configuration in .github/release.yml at v0.0.291 --> #### What's Changed ##### Deprecations **The `format` command-line argument and configuration option has been renamed to `output-format`.** While Ruff will continue to respect `format` when passed as a command-line argument or configuration option, this backwards-compatible support will be dropped in a future release. See: [https://github.com/astral-sh/ruff/pull/7514](https://togithub.com/astral-sh/ruff/pull/7514). ##### Rules - \[`flake8-bandit`] Implement `S201`: `flask-debug-true` by [@​mkniewallner](https://togithub.com/mkniewallner) in [https://github.com/astral-sh/ruff/pull/7503](https://togithub.com/astral-sh/ruff/pull/7503) - \[`flake8-bandit`] Implement `S507`: `ssh_no_host_key_verification` by [@​mkniewallner](https://togithub.com/mkniewallner) in [https://github.com/astral-sh/ruff/pull/7528](https://togithub.com/astral-sh/ruff/pull/7528) - \[`flake8-logging`] Implement `LOG002`: `invalid-get-logger-argument` by [@​dhruvmanila](https://togithub.com/dhruvmanila) in [https://github.com/astral-sh/ruff/pull/7399](https://togithub.com/astral-sh/ruff/pull/7399) - \[`flake8-logging`] Implement `LOG007`: `exception-without-exc-info` by [@​qdegraaf](https://togithub.com/qdegraaf) in [https://github.com/astral-sh/ruff/pull/7410](https://togithub.com/astral-sh/ruff/pull/7410) - \[`refurb`] Implement `FURB140`: `reimplemented-starmap` by [@​SavchenkoValeriy](https://togithub.com/SavchenkoValeriy) in [https://github.com/astral-sh/ruff/pull/7253](https://togithub.com/astral-sh/ruff/pull/7253) - \[`refurb`] Implement `FURB148`: `unnecessary-enumerate` by [@​tjkuson](https://togithub.com/tjkuson) in [https://github.com/astral-sh/ruff/pull/7454](https://togithub.com/astral-sh/ruff/pull/7454) - \[`ruff`] Detect `asyncio.get_running_loop` calls in RUF006 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7562](https://togithub.com/astral-sh/ruff/pull/7562) ##### Settings - Show `--no-X` variants in CLI help by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7504](https://togithub.com/astral-sh/ruff/pull/7504) - Rename `format` option to `output-format` by [@​MichaReiser](https://togithub.com/MichaReiser) in [https://github.com/astral-sh/ruff/pull/7514](https://togithub.com/astral-sh/ruff/pull/7514) - Enable tab completion for `ruff rule` by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7560](https://togithub.com/astral-sh/ruff/pull/7560) ##### Bug Fixes - Add padding to prevent some autofix errors by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7461](https://togithub.com/astral-sh/ruff/pull/7461) - Remove parentheses when rewriting assert calls to statements by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7464](https://togithub.com/astral-sh/ruff/pull/7464) - Avoid flagging starred elements in C402 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7466](https://togithub.com/astral-sh/ruff/pull/7466) - Extend `bad-dunder-method-name` to permit `attrs` dunders by [@​tjkuson](https://togithub.com/tjkuson) in [https://github.com/astral-sh/ruff/pull/7472](https://togithub.com/astral-sh/ruff/pull/7472) - Avoid N802 violations for [@​overload](https://togithub.com/overload) methods by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [https://github.com/astral-sh/ruff/pull/7498](https://togithub.com/astral-sh/ruff/pull/7498) - Avoid flagging starred expressions in UP007 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7505](https://togithub.com/astral-sh/ruff/pull/7505) - Ensure that LOG007 only triggers on `.exception()` calls by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7524](https://togithub.com/astral-sh/ruff/pull/7524) - Use strict sorted and union for NoQA mapping insertion by [@​dhruvmanila](https://togithub.com/dhruvmanila) in [https://github.com/astral-sh/ruff/pull/7531](https://togithub.com/astral-sh/ruff/pull/7531) - Avoid inserting imports directly after continuation by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7553](https://togithub.com/astral-sh/ruff/pull/7553) - Add padding in `PERF102` fixes by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7554](https://togithub.com/astral-sh/ruff/pull/7554) - Avoid invalid fix for parenthesized values in F601 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7559](https://togithub.com/astral-sh/ruff/pull/7559) - Treat `os.error` as an `OSError` alias by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7582](https://togithub.com/astral-sh/ruff/pull/7582) - Extend `bad-dunder-method-name` to permit `__html__` by [@​jaap3](https://togithub.com/jaap3) in [https://github.com/astral-sh/ruff/pull/7492](https://togithub.com/astral-sh/ruff/pull/7492) - Fix stylist indentation with a formfeed by [@​konstin](https://togithub.com/konstin) in [https://github.com/astral-sh/ruff/pull/7489](https://togithub.com/astral-sh/ruff/pull/7489) #### New Contributors - [@​MicaelJarniac](https://togithub.com/MicaelJarniac) made their first contribution in [https://github.com/astral-sh/ruff/pull/5498](https://togithub.com/astral-sh/ruff/pull/5498) - [@​maheshsaripalli9](https://togithub.com/maheshsaripalli9) made their first contribution in [https://github.com/astral-sh/ruff/pull/7552](https://togithub.com/astral-sh/ruff/pull/7552) - [@​T-256](https://togithub.com/T-256) made their first contribution in [https://github.com/astral-sh/ruff/pull/7585](https://togithub.com/astral-sh/ruff/pull/7585) **Full Changelog**: astral-sh/ruff@v0.0.290...v0.0.291 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/allenporter/flux-local). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi45Ny4xIiwidXBkYXRlZEluVmVyIjoiMzYuOTcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Summary
Implement
no-ignored-enumerate-items
asunnecessary-enumerate
(FURB148
).The auto-fix considers if a
start
argument is passed to theenumerate()
function. If only the index is used, then the suggested fix is to pass thestart
value to therange()
function. So,becomes
If the index is ignored and only the value is ignored, and if a start value greater than zero is passed to
enumerate()
, the rule doesn't produce a suggestion. I couldn't find a unanimously accepted best way to iterate over a collection whilst skipping the first n elements. The rule still triggers, however.Related to #1348.
Test Plan
cargo test