2072565 - Fix ruff PLW2901 warnings (for-loop variable overwritten) in python/mozbuild

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, task)

Description

[Sylvestre Ledru [:Sylvestre]]

Filing as a good first bug to learn workflows.

ruff reports 3 PLW2901 warnings ("for loop variable overwritten by assignment target") in python/mozbuild. In each case the loop body immediately reassigns the loop variable to a normalized form of itself, which makes the original value unrecoverable and confuses readers about what the variable holds at any point.

The fix is to assign to a new, clearly named local instead of clobbering the loop variable, then use that new name in the rest of the loop body.

The three sites:

1. python/mozbuild/mozbuild/backend/common.py#493 -- for s in source_filenames: then s = mozpath.relpath(s, output_directory)
   https://searchfox.org/firefox-main/rev/211abcf08683d4089ebc30897460213f560ed59f/python/mozbuild/mozbuild/backend/common.py#493

2. python/mozbuild/mozbuild/frontend/emitter.py#2012 -- for mpath in mpmanifest.manifests(): then mpath = mozpath.normpath(mpath)
   https://searchfox.org/firefox-main/rev/211abcf08683d4089ebc30897460213f560ed59f/python/mozbuild/mozbuild/frontend/emitter.py#2012

3. python/mozbuild/mozbuild/vendor/vendor_manifest.py#520 -- for file in self.convert_patterns_to_paths(...): then file = mozpath.normsep(file)
   https://searchfox.org/firefox-main/rev/211abcf08683d4089ebc30897460213f560ed59f/python/mozbuild/mozbuild/vendor/vendor_manifest.py#520

In the third one, the loop variable is also named file, which shadows the file builtin. Renaming it to something like path fixes that at the same time.

Take care in each case to update every later use of the variable inside the loop body, not just the assignment. In the vendor_manifest.py case in particular, the normalized value is what the if ... not in to_keep test and the mozfile.remove call must keep using, so the behaviour stays the same.

To verify the fix:

./mach lint -W -l ruff python/mozbuild/mozbuild/backend/common.py python/mozbuild/mozbuild/frontend/emitter.py python/mozbuild/mozbuild/vendor/vendor_manifest.py
./mach python-test python/mozbuild/mozbuild/test

The three PLW2901 warnings should be gone and the tests should still pass. Note that these files also carry UP031 warnings, which are being handled in separate bugs -- please leave those alone here so the patches do not conflict.

Tutorial to contribute:
https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
https://firefox-source-docs.mozilla.org/contributing/stack_quickref.html

Please don't ask for the bug to be assigned. It will be automatically assigned to the first patch.

Comment 1

[Daniel mungai chege]

Attachment: Bug 2072565 - Avoid overwriting loop variables in mozbuild. r?#firefox-build-system-reviewers

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8efb69491b74
https://hg.mozilla.org/integration/autoland/rev/2612a9b7b007
Avoid overwriting loop variables in mozbuild. r=firefox-build-system-reviewers,ahochheiden

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2612a9b7b007