Improve mypy typing

Author: xylix

Makefile

@@ -24,7 +24,7 @@ test-integration:
 	python3 ./tests/integration_tests.py --no-modules
 
 typecheck:
-	mypy aw_qt --ignore-missing-imports
+	mypy aw_qt --strict
 
 precommit:
 	make typecheck

aw_qt/main.py

@@ -1,36 +1,41 @@
 import sys
 import logging
 import argparse
+from typing import Set
+from typing_extensions import TypedDict
 
 from aw_core.log import setup_logging
 
 from .manager import Manager
-
 from . import trayicon
 
 logger = logging.getLogger(__name__)
 
 
-def main():
+def main() -> None:
     args = parse_args()
-    setup_logging("aw-qt", testing=args.testing, verbose=args.testing, log_file=True)
+    setup_logging("aw-qt", testing=args['testing'], verbose=args['testing'], log_file=True)
 
-    _manager = Manager(testing=args.testing)
-    _manager.autostart(args.autostart_modules)
+    _manager = Manager(testing=args['testing'])
+    _manager.autostart(args['autostart_modules'])
 
-    error_code = trayicon.run(_manager, testing=args.testing)
+    error_code = trayicon.run(_manager, testing=args['testing'])
     _manager.stop_all()
 
     sys.exit(error_code)
 
 
-def parse_args():
+CommandLineArgs = TypedDict('CommandLineArgs', {'testing': bool, 'autostart_modules': Set[str]}, total=False)
+
+
+def parse_args() -> CommandLineArgs:
     parser = argparse.ArgumentParser(prog="aw-qt", description='A trayicon and service manager for ActivityWatch')
     parser.add_argument('--testing', action='store_true',
                         help='Run the trayicon and services in testing mode')
     parser.add_argument('--autostart-modules', dest='autostart_modules',
                         type=lambda s: [m for m in s.split(',') if m and m.lower() != "none"],
                         default=None,
                         help='A comma-separated list of modules to autostart, or just `none` to not autostart anything')
-
-    return parser.parse_args()
+    parsed_args = parser.parse_args()
+    dict: CommandLineArgs = {'autostart_modules': parsed_args.autostart_modules, 'testing': parsed_args.testing}
+    return dict

aw_qt/manager.py

@@ -30,7 +30,7 @@ def _locate_bundled_executable(name: str) -> Optional[str]:
     return None
 
 
-def _is_system_module(name) -> bool:
+def _is_system_module(name: str) -> bool:
     """Checks if a module with a particular name exists in PATH"""
     return shutil.which(name) is not None
 
@@ -88,20 +88,20 @@ def __init__(self, name: str, testing: bool = False) -> None:
         self.started = False  # Should be True if module is supposed to be running, else False
         self.testing = testing
         self.location = "system" if _is_system_module(name) else "bundled"
-        self._process = None  # type: Optional[subprocess.Popen]
-        self._last_process = None  # type: Optional[subprocess.Popen]
+        self._process: Optional[subprocess.Popen[str]] = None
+        self._last_process: Optional[subprocess.Popen[str]] = None
 
     def start(self) -> None:
         logger.info("Starting module {}".format(self.name))
 
         # Create a process group, become its leader
         # TODO: This shouldn't go here
         if platform.system() != "Windows":
-            os.setpgrp()  # type: ignore
+            os.setpgrp()
 
         exec_path = _locate_executable(self.name)
         if exec_path is None:
-            return
+            logger.error("Tried to start nonexistent module {}".format(self.name))
         else:
             exec_cmd = [exec_path]
             if self.testing:
@@ -112,8 +112,8 @@ def start(self) -> None:
         # See: https://github.com/ActivityWatch/activitywatch/issues/212
         startupinfo = None
         if platform.system() == "Windows":
-            startupinfo = subprocess.STARTUPINFO()  # type: ignore
-            startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW  # type: ignore
+            startupinfo = subprocess.STARTUPINFO() #type: ignore
+            startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW #type: ignore
         elif platform.system() == "Darwin":
             logger.info("Macos: Disable dock icon")
             import AppKit
@@ -122,7 +122,6 @@ def start(self) -> None:
         # There is a very good reason stdout and stderr is not PIPE here
         # See: https://github.com/ActivityWatch/aw-server/issues/27
         self._process = subprocess.Popen(exec_cmd, universal_newlines=True, startupinfo=startupinfo)
-
         self.started = True
 
     def stop(self) -> None:
@@ -137,7 +136,7 @@ def stop(self) -> None:
             logger.warning("Tried to stop module {}, but it wasn't running".format(self.name))
         else:
             if not self._process:
-                logger.error("No reference to process object")
+                logger.error("")
             logger.debug("Stopping module {}".format(self.name))
             if self._process:
                 self._process.terminate()
@@ -180,6 +179,7 @@ def __init__(self, testing: bool = False) -> None:
         self.settings: AwQtSettings = AwQtSettings(testing)
         self.modules: Dict[str, Module] = {}
         self.autostart_modules: Set[str] = set(self.settings.autostart_modules)
+        self.testing = testing
 
         for name in self.settings.possible_modules:
             if _locate_executable(name):
@@ -189,7 +189,7 @@ def __init__(self, testing: bool = False) -> None:
         # Is this actually a good way to do this? merged from dev/autodetect-modules
         self.discover_modules()
 
-    def discover_modules(self):
+    def discover_modules(self) -> None:
         # These should always be bundled with aw-qt
         found_modules = set(_discover_modules_bundled())
         found_modules |= set(_discover_modules_system())
@@ -199,17 +199,19 @@ def discover_modules(self):
             if m_name not in self.modules:
                 self.modules[m_name] = Module(m_name, testing=self.testing)
 
-    def get_unexpected_stops(self):
+    def get_unexpected_stops(self) -> List[Module]:
         return list(filter(lambda x: x.started and not x.is_alive(), self.modules.values()))
 
-    def start(self, module_name):
+    def start(self, module_name: str) -> None:
         if module_name in self.modules.keys():
             self.modules[module_name].start()
+        else:
+            logger.debug("Manager tried to start nonexistent module {}".format(module_name))
 
-    def autostart(self, autostart_modules):
-        if autostart_modules is None:
+    def autostart(self, autostart_modules: Set[str] = set()) -> None:
+        if autostart_modules and len(autostart_modules) > 0:
             logger.info("Modules to start weren't specified in CLI arguments. Falling back to configuration.")
-            autostart_modules = self.settings.autostart_modules
+            autostart_modules = set(self.settings.autostart_modules)
 
         # We only want to autostart modules that are both in found modules and are asked to autostart.
         autostart_modules = autostart_modules.intersection(set(self.modules.keys()))
@@ -223,7 +225,7 @@ def autostart(self, autostart_modules):
         for module_name in autostart_modules:
             self.start(module_name)
 
-    def stop_all(self):
+    def stop_all(self) -> None:
         for module in filter(lambda m: m.is_alive(), self.modules.values()):
             module.stop()
 

aw_qt/trayicon.py

@@ -5,29 +5,30 @@
 import os
 import subprocess
 from collections import defaultdict
+from typing import Any, DefaultDict, List, Optional
 
 from PyQt5 import QtCore
 from PyQt5.QtWidgets import QApplication, QSystemTrayIcon, QMessageBox, QMenu, QWidget, QPushButton
 from PyQt5.QtGui import QIcon
 
 import aw_core
 
-from .manager import Manager
+from .manager import Manager, Module
 
 logger = logging.getLogger(__name__)
 
 
-def open_webui(root_url):
+def open_webui(root_url: str) -> None:
     print("Opening dashboard")
     webbrowser.open(root_url)
 
 
-def open_apibrowser(root_url):
+def open_apibrowser(root_url: str) -> None:
     print("Opening api browser")
     webbrowser.open(root_url + "/api")
 
 
-def open_dir(d):
+def open_dir(d: str)-> None:
     """From: http://stackoverflow.com/a/1795849/965332"""
     if sys.platform == 'win32':
         os.startfile(d)
@@ -38,17 +39,18 @@ def open_dir(d):
 
 
 class TrayIcon(QSystemTrayIcon):
-    def __init__(self, manager: Manager, icon, parent=None, testing=False) -> None:
+    def __init__(self, manager: Manager, icon: QIcon, parent: Optional[QWidget]=None, testing: bool=False) -> None:
         QSystemTrayIcon.__init__(self, icon, parent)
+        self._parent = parent # QSystemTrayIcon also tries to save parent info but it screws up the type info
         self.setToolTip("ActivityWatch" + (" (testing)" if testing else ""))
 
         self.manager = manager
         self.testing = testing
 
         self._build_rootmenu()
 
-    def _build_rootmenu(self):
-        menu = QMenu(self.parent())
+    def _build_rootmenu(self) -> None:
+        menu = QMenu(self._parent)
 
         root_url = "http://localhost:{port}".format(port=5666 if self.testing else 5600)
 
@@ -80,8 +82,8 @@ def _build_rootmenu(self):
 
         self.setContextMenu(menu)
 
-        def show_module_failed_dialog(module):
-            box = QMessageBox(self.parent())
+        def show_module_failed_dialog(module: Module) -> None:
+            box = QMessageBox(self._parent)
             box.setIcon(QMessageBox.Warning)
             box.setText("Module {} quit unexpectedly".format(module.name))
             box.setDetailedText(module.read_log())
@@ -93,10 +95,10 @@ def show_module_failed_dialog(module):
 
             box.show()
 
-        def rebuild_modules_menu():
+        def rebuild_modules_menu() -> None:
             for action in modulesMenu.actions():
                 if action.isEnabled():
-                    name = action.module.name
+                    name = action.data().name
                     alive = self.manager.modules[name].is_alive()
                     action.setChecked(alive)
                     # print(module.text(), alive)
@@ -105,7 +107,7 @@ def rebuild_modules_menu():
             QtCore.QTimer.singleShot(2000, rebuild_modules_menu)
         QtCore.QTimer.singleShot(2000, rebuild_modules_menu)
 
-        def check_module_status():
+        def check_module_status() -> None:
             unexpected_exits = self.manager.get_unexpected_stops()
             if unexpected_exits:
                 for module in unexpected_exits:
@@ -116,19 +118,19 @@ def check_module_status():
             QtCore.QTimer.singleShot(2000, rebuild_modules_menu)
         QtCore.QTimer.singleShot(2000, check_module_status)
 
-    def _build_modulemenu(self, moduleMenu):
+    def _build_modulemenu(self, moduleMenu: QMenu) -> None:
         moduleMenu.clear()
 
-        def add_module_menuitem(module):
+        def add_module_menuitem(module: Module) -> None:
             title = module.name
             ac = moduleMenu.addAction(title, lambda: module.toggle())
-            # Kind of nasty, but a quick way to affiliate the module to the menu action for when it needs updating
-            ac.module = module
+
+            ac.setData(module)
             ac.setCheckable(True)
             ac.setChecked(module.is_alive())
 
         # Merged from branch dev/autodetect-modules, still kind of in progress with making this actually work
-        modules_by_location = defaultdict(lambda: list())
+        modules_by_location: DefaultDict[str, List[Module]] = defaultdict(lambda: list())
         for module in sorted(self.manager.modules.values(), key=lambda m: m.name):
             modules_by_location[module.location].append(module)
 
@@ -140,7 +142,7 @@ def add_module_menuitem(module):
             add_module_menuitem(self.manager.modules[module.name])
 
 
-def exit(manager: Manager):
+def exit(manager: Manager) -> None:
     # TODO: Do cleanup actions
     # TODO: Save state for resume
     print("Shutdown initiated, stopping all services...")
@@ -151,16 +153,17 @@ def exit(manager: Manager):
     QApplication.quit()
 
 
-def run(manager, testing=False):
+def run(manager: Manager, testing: bool = False) -> Any:
     logger.info("Creating trayicon...")
     # print(QIcon.themeSearchPaths())
 
     app = QApplication(sys.argv)
 
+    # FIXME: remove ignores after https://github.com/python/mypy/issues/2955 has been fixed
     # Without this, Ctrl+C will have no effect
-    signal.signal(signal.SIGINT, lambda: exit(manager))
+    signal.signal(signal.SIGINT, lambda: exit(manager)) #type: ignore
     # Ensure cleanup happens on SIGTERM
-    signal.signal(signal.SIGTERM, lambda: exit(manager))
+    signal.signal(signal.SIGTERM, lambda: exit(manager)) #type: ignore
 
     timer = QtCore.QTimer()
     timer.start(100)  # You may change this if you wish.

mypy.ini

@@ -0,0 +1,6 @@
+[mypy]
+python_version = 3.6
+ignore_missing_imports = True
+
+[mypy-*.resources]
+ignore_errors = True