Skip to content
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

QtCore.QCoreApplication.instance() may return None #126

Merged
merged 2 commits into from Dec 31, 2020
Merged

Conversation

altendky
Copy link
Collaborator

No description provided.

@altendky altendky merged commit 6630551 into master Dec 31, 2020
@altendky altendky deleted the altendky-patch-3 branch December 31, 2020 17:02
@The-Compiler
Copy link
Collaborator

While this is true in theory, I wonder if it's worth the pain it causes in practice? In qutebrowser, with this change I get:

Error: qutebrowser/utils/qtutils.py:129: error: Item "None" of "Optional[QCoreApplication]" has no attribute "arguments"  [union-attr]
        args = QApplication.instance().arguments()
               ^
Error: qutebrowser/utils/debug.py:317: error: Item "None" of "Optional[QCoreApplication]" has no attribute "allWidgets"  [union-attr]
        widgets = QApplication.instance().allWidgets()
                  ^
Error: qutebrowser/utils/debug.py:344: error: Argument 2 to "_get_pyqt_objects" has incompatible type "Optional[QObject]"; expected "QObject"  [arg-type]
        _get_pyqt_objects(pyqt_lines, start_obj)
                                      ^
Error: qutebrowser/utils/version.py:532: error: Item "None" of "Optional[QCoreApplication]" has no attribute "launch_time"  [union-attr]
        launch_time = QApplication.instance().launch_time
                      ^
Error: qutebrowser/keyinput/modeman.py:310: error: Item "None" of "Optional[QCoreApplication]" has no attribute "focusWidget"  [union-attr]
                focus_widget = QApplication.instance().focusWidget()
                               ^
Error: qutebrowser/misc/crashdialog.py:245: error: Item "None" of "Optional[QCoreApplication]" has no attribute "launch_time"  [union-attr]
                launch_time = application.launch_time.ctime()
                              ^
Error: qutebrowser/misc/sessions.py:286: error: Item "None" of "Optional[QCoreApplication]" has no attribute "activeWindow"  [union-attr]
                active_window = QApplication.instance().activeWindow()
                                ^
Error: qutebrowser/mainwindow/windowundo.py:52: error: Item "None" of "Optional[QCoreApplication]" has no attribute "window_closing"  [union-attr]
            QApplication.instance().window_closing.connect(self._on_window_closing)
            ^
Error: qutebrowser/mainwindow/mainwindow.py:103: error: Item "None" of "Optional[QCoreApplication]" has no attribute "alert"  [union-attr]
            QApplication.instance().alert(window)
            ^
Error: qutebrowser/mainwindow/mainwindow.py:278: error: Item "None" of "Optional[QCoreApplication]" has no attribute "new_window"  [union-attr]
            QApplication.instance().new_window.emit(self)
            ^
Error: qutebrowser/browser/commands.py:68: error: Item "None" of "Optional[QCoreApplication]" has no attribute "arguments"  [union-attr]
            args = QApplication.instance().arguments()
                   ^
Error: qutebrowser/misc/quitter.py:270: error: Item "None" of "Optional[QCoreApplication]" has no attribute "exit"  [union-attr]
            QApplication.instance().exit(status)
            ^
Error: qutebrowser/misc/quitter.py:317: error: Item "None" of "Optional[QCoreApplication]" has no attribute "lastWindowClosed"  [union-attr]
        qapp.lastWindowClosed.connect(instance.on_last_window_closed)
        ^
qutebrowser/browser/webkit/network/networkmanager.py:192: error: Argument 1 to "setParent" of "QObject" has incompatible type "Optional[QCoreApplication]"; expected "QObject" 
[arg-type]
            cookie_jar.setParent(app)
                                 ^
qutebrowser/browser/webkit/network/networkmanager.py:202: error: Argument 1 to "setParent" of "QObject" has incompatible type "Optional[QCoreApplication]"; expected "QObject" 
[arg-type]
            cache.diskcache.setParent(app)
                                      ^
Error: qutebrowser/misc/backendproblem.py:239: error: Item "None" of "Optional[QCoreApplication]" has no attribute "platformName"  [union-attr]
            platform = QApplication.instance().platformName()
                       ^
Error: qutebrowser/keyinput/eventfilter.py:53: error: Item "None" of "Optional[QCoreApplication]" has no attribute "installEventFilter"  [union-attr]
            QApplication.instance().installEventFilter(self)
            ^
Error: qutebrowser/keyinput/eventfilter.py:57: error: Item "None" of "Optional[QCoreApplication]" has no attribute "removeEventFilter"  [union-attr]
            QApplication.instance().removeEventFilter(self)
            ^
Error: qutebrowser/keyinput/eventfilter.py:68: error: Item "None" of "Optional[QCoreApplication]" has no attribute "activeWindow"  [union-attr]
            active_window = QApplication.instance().activeWindow()
                            ^
Error: qutebrowser/components/readlinecommands.py:42: error: Item "None" of "Optional[QCoreApplication]" has no attribute "focusWidget"  [union-attr]
            w = QApplication.instance().focusWidget()
                ^
Found 20 errors in 14 files (checked 184 source files)

For all those, I know a QApplication exists at that point. I'd now have to replace all occurrences of QApplication.instance().something() with:

app = QApplication.instance()
assert app is not None
app.something()

to appease mypy, for very little benefit (if there was no QApplication, my application would crash at runtime either way).

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this pull request Jan 4, 2021
@BryceBeagle
Copy link
Collaborator

Hmm that's a pretty good point and certainly a common use case. Is there any way to tell mypy that something should usually be not None, but it's technically possible for it to be None?

This situation is making me think of GCC's __builtin_expect or glibc/the Linux kernel's likely/unlikely macros. Assume one path, fall back to the other if it fails.

@The-Compiler
Copy link
Collaborator

I'm not aware of one - if we tell mypy that it can be None, it will require handling it properly everywhere. If we don't tell mypy about it, I suppose it'll complain with if QApplication.instance() is None...

@altendky
Copy link
Collaborator Author

altendky commented Jan 4, 2021

Just to have the original situation that made me notice this listed here... It's nothing beautiful, but so it goes with global persistent state.

https://github.com/altendky/qtrio/blob/16eecdbf8ddb21f9c89917aa99dc4012145e726f/qtrio/_core.py#L513-L540

def maybe_build_application() -> QtCore.QCoreApplication:
    """Create a new Qt application object if one does not already exist.
    Returns:
        The Qt application object.
    """
    application: QtCore.QCoreApplication

    # TODO: https://bugreports.qt.io/browse/PYSIDE-1467
    if qtpy.PYQT5:
        maybe_application = QtWidgets.QApplication.instance()
    elif qtpy.PYSIDE2:
        maybe_application = typing.cast(
            typing.Optional[QtCore.QCoreApplication], QtWidgets.QApplication.instance()
        )
    else:  # pragma: no cover
        raise qtrio.InternalError(
            "You should not be here but you are running neither PyQt5 nor PySide2.",
        )

    if maybe_application is None:
        application = QtWidgets.QApplication(sys.argv[1:])
    else:
        application = maybe_application

    application.setQuitOnLastWindowClosed(False)

    return application

If you don't want to check (I understand that) it could be a cast instead of an assert. Not that that changes much. There could be an exception-raising wrapper function that always returns an instance that is called instead. I hesitate to intentionally make hints wrong especially if the cost is touching just twenty bits of code in a fairly simple way with multiple options. If it is relevant, I'd be perfectly happy to do a PR for qutebrowser to respond to this change. I'll look around in case there's some other way to tell mypy to ignore the optionality.

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this pull request Jan 11, 2021
We know that QApplication.instance() will always be non-None for
practical purposes, but the stubs now (correctly) declare it as
Optional.

See python-qt-tools/PyQt5-stubs#126
The-Compiler added a commit to qutebrowser/qutebrowser that referenced this pull request Jan 11, 2021
We know that QApplication.instance() will always be non-None for
practical purposes, but the stubs now (correctly) declare it as
Optional.

See python-qt-tools/PyQt5-stubs#126
bluebird75 pushed a commit to bluebird75/PyQt5-stubs that referenced this pull request Aug 24, 2021
QtCore.QCoreApplication.instance() may return None
bluebird75 added a commit to bluebird75/PyQt5-stubs that referenced this pull request Apr 26, 2022
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.

None yet

3 participants