qt brain: Make slot argument optional for disconnect()

[The-Compiler]

Steps

• For new features or bug fixes, add a ChangeLog entry describing what your PR does.
• Write a good description on what the PR does.

If this is an acceptable fix, I'll write a changelog - two more questions:

• Is there a good way I can write a test for this?
• Should this go to main or to a maintenance branch

Description

Discussed here:
#1531 (comment)

PyQt supports calling .disconnect() without any arguments in order to
disconnect all slots:
https://www.riverbankcomputing.com/static/Docs/PyQt6/signals_slots.html#disconnect

Strictly speaking, slot=None is a wrong call, as actually passing None
does not work:
python-qt-tools/PyQt5-stubs#103

However, pylint/astroid does not support overloads needed to properly
express this: pylint-dev/pylint#5264

So, while this is "wrong", it's less wrong than before - without this
change, pylint expects a mandatory argument, thus raising a
false-positive here:

from PyQt5.QtCore import QTimer
t = QTimer()
t.timeout.connect(lambda: None)
t.timeout.disconnect()

despite running fine, pylint complains:

test.py:4:0: E1120: No value for argument 'slot' in method call (no-value-for-parameter)

while with this change, things work fine.

Type of Changes

	Type
✓	🐛 Bug fix

[coveralls]

Pull Request Test Coverage Report for Build 2311757594

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 143 unchanged lines in 4 files lost coverage.
• Overall coverage increased (+0.02%) to 91.665%

---

Files with Coverage Reduction	New Missed Lines	%
astroid/brain/brain_typing.py	10	88.03%
astroid/rebuilder.py	17	96.97%
astroid/brain/brain_builtin_inference.py	26	90.99%
astroid/nodes/scoped_nodes/scoped_nodes.py	90	92.48%

Totals
Change from base Build 2299632152:	0.02%
Covered Lines:	9139
Relevant Lines:	9970

---

💛 - Coveralls

[Pierre-Sassoulas]

Is there a good way I can write a test for this?

Check unittest_brain_qt.py there's a pytest mark for this: https://github.com/PyCQA/astroid/blob/main/tests/unittest_brain_qt.py#L17

Should this go to main or to a maintenance branch

Let's put it on the main, then we can decide if this need to be backported to the maintenance branche or not :)

[The-Compiler]

Check unittest_brain_qt.py there's a pytest mark for this: ...

I've seen that file and tried to write a test inspired by test_implicit_parameters, but I'm lost about checking that it's valid to call this function without any parameters (or somehow checking that this FunctionDef has no mandatory args).

As for the fix itself: One thing we could do to avoid implying that None is okay is to use a _UNSET = object() sentinel as default value instead. I just updated the code accordingly.

Let's put it on the main, then we can decide if this need to be backported to the maintenance branche or not :)

How does the changelog entry work? Do I add it to the earlier version here in the main branch?

[Pierre-Sassoulas]

How does the changelog entry work? Do I add it to the earlier version here in the main branch?

Yes in the 2.11.6 section

[jacobtylerwalls]

Thanks for the fix!

I've seen that file and tried to write a test inspired by test_implicit_parameters, but I'm lost about checking that it's valid to call this function without any parameters (or somehow checking that this FunctionDef has no mandatory args).

You should be able to follow that test to extract the node for the FunctionDef representing disconnect(), and then drill into its .args.kwargs and assert something. The #@ symbol in the code block is for jumping straight to the node you want to extract.

[The-Compiler]

Updated with a test and changelog now. Not too happy with the test since it basically 1:1 checks what the implementation is, but I guess there is not much more we can do if the test should live here and not in pylint.

[Pierre-Sassoulas]

Don't mind the failing test, it's due to #1551

[Pierre-Sassoulas]

👍