Skip to content

Disable the loop-iterator-mutation rule in ruff#3450

Merged
svartkanin merged 1 commit intoarchlinux:masterfrom
correctmost:cm/disable-bugbear-rule
May 8, 2025
Merged

Disable the loop-iterator-mutation rule in ruff#3450
svartkanin merged 1 commit intoarchlinux:masterfrom
correctmost:cm/disable-bugbear-rule

Conversation

@correctmost
Copy link
Copy Markdown
Contributor

PR Description:

This allows ruff check --preview to succeed again.

Here's the warning that is now suppressed:

archinstall/lib/pacman/config.py:46:6: B909 Mutation to loop iterable `content` during iteration
   |
44 |                 # also uncomment the next line (Include statement) if it exists and is commented
45 |                 if row + 1 < len(content) and content[row + 1].lstrip().startswith('#'):
46 |                     content[row + 1] = re.sub(r'^#\s*', '', content[row + 1])
   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B909
47 |
48 |         # Write the modified content back to the file
   |

Found 1 error.

This allows ruff check --preview to succeed again.
@correctmost correctmost requested a review from Torxed as a code owner May 8, 2025 20:05
@codefiles
Copy link
Copy Markdown
Contributor

Why not fix it rather than suppress it? Would this be preferable?

diff --git a/archinstall/lib/pacman/config.py b/archinstall/lib/pacman/config.py
index 6ea75d4e..948db2de 100644
--- a/archinstall/lib/pacman/config.py
+++ b/archinstall/lib/pacman/config.py
@@ -33,7 +33,10 @@ class PacmanConfig:

                content = self._config_path.read_text().splitlines(keepends=True)

-               for row, line in enumerate(content):
+               row = 0
+               while row < len(content):
+                       line = content[row]
+
                        # Check if this is a commented repository section that needs to be enabled
                        match = re.match(r'^#\s*\[(.*)\]', line)

@@ -44,6 +47,9 @@ class PacmanConfig:
                                # also uncomment the next line (Include statement) if it exists and is commented
                                if row + 1 < len(content) and content[row + 1].lstrip().startswith('#'):
                                        content[row + 1] = re.sub(r'^#\s*', '', content[row + 1])
+                                       row += 1
+
+                       row += 1

                # Write the modified content back to the file
                with open(self._config_path, 'w') as f:

This also enables skipping content[row + 1] after substitution has been performed on it.

@svartkanin svartkanin merged commit 5cd5d71 into archlinux:master May 8, 2025
8 checks passed
@correctmost correctmost deleted the cm/disable-bugbear-rule branch May 8, 2025 22:18
@correctmost
Copy link
Copy Markdown
Contributor Author

Why not fix it rather than suppress it? Would this be preferable?

I opted to disable the rule for now because a similar redefined-loop-name rule (PLW2901) is disabled. Some of the loop-iterator-mutation violations strike me as stylistic in nature, so I also didn't want to impose a style on the codebase when it wouldn't have a clear impact on correctness or performance.

In the long run, I would like most of the current exclusions to be re-enabled because I think they can eventually help catch more issues. In the short term, I am mainly trying to keep ruff check --preview passing to avoid build breakage from ruff's frequent releases.

The proposed patch looks good, but some people may find it a bit harder to read because of the double increment. Manual increments can also lead to infinite loops if continue statements get added later on. This is why I think the rule is a bit stylistic and opted to disable it :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants