ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Thanks for looking into this!

Has this change been discussed anywhere on an issue? I'm not sure we want to add more hard-coded exceptions to this rule, especially when you can suppress this diagnostic by configuring the extend-allowed-calls setting.

It has not been discussed anywhere.
I just thought multiprocessing.Value might qualify if sqlalchemy did.
Will look into the CI tonight, if this is not rejected before :)

It looks like the sqlalchemy special case was added a while before the config option. Can you check the ecosystem results too? It looks like this change is affecting Value types beyond multiprocessing.

Certainly 🙂

Indeed, I'm currently also hitting django.db.models.Value, which was not my intention.

So actually django.db.models.Value underwent the same discussion in #10356.
At that point the configuration option @ntBre talked about did not exist, yet.
The discussion went with "default ignore", allow-list later.

edited the italic
The signature of django.db.models.Value does allow providing the wrapped value as keyword parameter though, which is not the case for multiprocessing.Value(*args).

Adding it per default allowed is therefore (in my opinion) still a good choice.

It would require a new helper in rust though, which does something similar to is_user_allowed_func_call and allows access to checker.semantic().

@ntBre could you approve the workflows once again?
I just hit cargo fmt and clippy.

I think someone did approve the workflows, and the remaining test failures are real.

I'm sort of confused by this comment:

The signature of django.db.models.Value does allow keyword parameters though, which is not the case for multiprocessing.Value(*args).

I'm looking at the help output for multiprocessing.Value in the REPL:

Help on method Value in module multiprocessing.context:

Value(typecode_or_type, *args, lock=True) method of multiprocessing.context.DefaultContext instance
    Returns a synchronized shared object

which indicates that lock can be provided as a keyword. That also works in the REPL directly:

>>> import multiprocessing
>>> from ctypes import c_bool
>>> multiprocessing.Value(c_bool, lock=False)
c_bool(False)

I'm not very familiar with the usage of multiprocessing.Value, but that seems like another way to suppress the diagnostic instead of using extend-allowed-calls or adding this as a hard-coded exception.

Ah yes, a nasty pitfall.
I will answer properly tomorrow and address the remaining test issues, but your assumption is not correct: lock is not the wrapped value.

>>> import multiprocessing
>>> from ctypes import c_bool
>>> multiprocessing.Value(c_bool, lock=True)
<Synchronized wrapper for c_bool(False)

*args is really the only way to feed values into multiprocessing.Value.

Looking at it I do not think I broke anything but the snaps, which are IIRC (because of my additional three lines in the FBT.py) off by three.
I'll fix those tomorrow morning, then everything should be neat.

Thanks for your patience :)

Ah of course, lock is keyword-only because of *args. Thanks for clarifying :)

I think I've come around on this. It makes sense to me to include an exception for the standard library, especially if we're checking the QualifiedName. Let me know when you're ready for another CI run and a review, or if you end up needing help with the snapshots.

Took me a second to realize cargo-insta exists and updating the snap file is not a manual editing process.

After reviewing the three snap files, I can confirm, the only thing I did was changes of line numbers in them due to my addition in FBT.py.

Another option would be to add my new FBT lines at the very bottom,
but grouping it with other FBT003 rules justifies the modification of the snapshots, imo.

If somebody could give CI another kick, I'd appreciate it.

Nice, thank you!