Relax ignore-missing-imports for modules that have stubs now and update mypy (#11006)

Updating mypy past version 0.9 means that third-party stubs are no-longer distributed with typeshed. See http://mypy-lang.blogspot.com/2021/06/mypy-0900-released.html for details.
We therefore pull in stub packages in setup.py

Additionally, some modules that we were previously ignoring import failures for now have stubs. So let's use them.

The rest of this change consists of fixups to make the newer mypy + stubs pass CI.

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>

Author: David Robertson

changelog.d/11006.misc

@@ -0,0 +1 @@
+Bump mypy version for CI to 0.910, and pull in new type stubs for dependencies.

mypy.ini

@@ -198,98 +198,97 @@ disallow_untyped_defs = True
 [mypy-tests.storage.test_user_directory]
 disallow_untyped_defs = True
 
-[mypy-pymacaroons.*]
-ignore_missing_imports = True
+;; Dependencies without annotations
+;; Before ignoring a module, check to see if type stubs are available.
+;; The `typeshed` project maintains stubs here:
+;;     https://github.com/python/typeshed/tree/master/stubs
+;; and for each package `foo` there's a corresponding `types-foo` package on PyPI,
+;; which we can pull in as a dev dependency by adding to `setup.py`'s
+;; `CONDITIONAL_REQUIREMENTS["mypy"]` list.
 
-[mypy-zope]
+[mypy-authlib.*]
 ignore_missing_imports = True
 
 [mypy-bcrypt]
 ignore_missing_imports = True
 
-[mypy-constantly]
-ignore_missing_imports = True
-
-[mypy-twisted.*]
+[mypy-canonicaljson]
 ignore_missing_imports = True
 
-[mypy-treq.*]
+[mypy-constantly]
 ignore_missing_imports = True
 
-[mypy-hyperlink]
+[mypy-daemonize]
 ignore_missing_imports = True
 
 [mypy-h11]
 ignore_missing_imports = True
 
-[mypy-msgpack]
-ignore_missing_imports = True
-
-[mypy-opentracing]
+[mypy-hiredis]
 ignore_missing_imports = True
 
-[mypy-OpenSSL.*]
+[mypy-hyperlink]
 ignore_missing_imports = True
 
-[mypy-netaddr]
+[mypy-ijson.*]
 ignore_missing_imports = True
 
-[mypy-saml2.*]
+[mypy-jaeger_client.*]
 ignore_missing_imports = True
 
-[mypy-canonicaljson]
+[mypy-josepy.*]
 ignore_missing_imports = True
 
-[mypy-jaeger_client.*]
+[mypy-jwt.*]
 ignore_missing_imports = True
 
-[mypy-jsonschema]
+[mypy-lxml]
 ignore_missing_imports = True
 
-[mypy-signedjson.*]
+[mypy-msgpack]
 ignore_missing_imports = True
 
-[mypy-prometheus_client.*]
+[mypy-nacl.*]
 ignore_missing_imports = True
 
-[mypy-service_identity.*]
+[mypy-netaddr]
 ignore_missing_imports = True
 
-[mypy-daemonize]
+[mypy-opentracing]
 ignore_missing_imports = True
 
-[mypy-sentry_sdk]
+[mypy-phonenumbers.*]
 ignore_missing_imports = True
 
-[mypy-PIL.*]
+[mypy-prometheus_client.*]
 ignore_missing_imports = True
 
-[mypy-lxml]
+[mypy-pymacaroons.*]
 ignore_missing_imports = True
 
-[mypy-jwt.*]
+[mypy-pympler.*]
 ignore_missing_imports = True
 
-[mypy-authlib.*]
+[mypy-rust_python_jaeger_reporter.*]
 ignore_missing_imports = True
 
-[mypy-rust_python_jaeger_reporter.*]
+[mypy-saml2.*]
 ignore_missing_imports = True
 
-[mypy-nacl.*]
+[mypy-sentry_sdk]
 ignore_missing_imports = True
 
-[mypy-hiredis]
+[mypy-service_identity.*]
 ignore_missing_imports = True
 
-[mypy-josepy.*]
+[mypy-signedjson.*]
 ignore_missing_imports = True
 
-[mypy-pympler.*]
+[mypy-treq.*]
 ignore_missing_imports = True
 
-[mypy-phonenumbers.*]
+[mypy-twisted.*]
 ignore_missing_imports = True
 
-[mypy-ijson.*]
+[mypy-zope]
 ignore_missing_imports = True

setup.py

@@ -112,7 +112,16 @@ def exec_file(path_segments):
     "pygithub==1.55",
 ]
 
-CONDITIONAL_REQUIREMENTS["mypy"] = ["mypy==0.812", "mypy-zope==0.2.13"]
+CONDITIONAL_REQUIREMENTS["mypy"] = [
+    "mypy==0.910",
+    "mypy-zope==0.3.2",
+    "types-bleach>=4.1.0",
+    "types-jsonschema>=3.2.0",
+    "types-Pillow>=8.3.4",
+    "types-pyOpenSSL>=20.0.7",
+    "types-PyYAML>=5.4.10",
+    "types-setuptools>=57.4.0",
+]
 
 # Dependencies which are exclusively required by unit test code. This is
 # NOT a list of all modules that are necessary to run the unit tests.

synapse/config/tls.py

@@ -172,9 +172,12 @@ def is_disk_cert_valid(self, allow_self_signed=True):
                 )
 
         # YYYYMMDDhhmmssZ -- in UTC
-        expires_on = datetime.strptime(
-            tls_certificate.get_notAfter().decode("ascii"), "%Y%m%d%H%M%SZ"
-        )
+        expiry_data = tls_certificate.get_notAfter()
+        if expiry_data is None:
+            raise ValueError(
+                "TLS Certificate has no expiry date, and this is not permitted"
+            )
+        expires_on = datetime.strptime(expiry_data.decode("ascii"), "%Y%m%d%H%M%SZ")
         now = datetime.utcnow()
         days_remaining = (expires_on - now).days
         return days_remaining

synapse/http/client.py

@@ -912,7 +912,7 @@ class InsecureInterceptableContextFactory(ssl.ContextFactory):
 
     def __init__(self):
         self._context = SSL.Context(SSL.SSLv23_METHOD)
-        self._context.set_verify(VERIFY_NONE, lambda *_: None)
+        self._context.set_verify(VERIFY_NONE, lambda *_: False)
 
     def getContext(self, hostname=None, port=None):
         return self._context

synapse/logging/context.py

@@ -52,7 +52,7 @@
 
     is_thread_resource_usage_supported = True
 
-    def get_thread_resource_usage() -> "Optional[resource._RUsage]":
+    def get_thread_resource_usage() -> "Optional[resource.struct_rusage]":
         return resource.getrusage(RUSAGE_THREAD)
 
 
@@ -61,7 +61,7 @@ def get_thread_resource_usage() -> "Optional[resource._RUsage]":
     # won't track resource usage.
     is_thread_resource_usage_supported = False
 
-    def get_thread_resource_usage() -> "Optional[resource._RUsage]":
+    def get_thread_resource_usage() -> "Optional[resource.struct_rusage]":
         return None
 
 
@@ -226,10 +226,10 @@ def __str__(self):
     def copy_to(self, record):
         pass
 
-    def start(self, rusage: "Optional[resource._RUsage]"):
+    def start(self, rusage: "Optional[resource.struct_rusage]"):
         pass
 
-    def stop(self, rusage: "Optional[resource._RUsage]"):
+    def stop(self, rusage: "Optional[resource.struct_rusage]"):
         pass
 
     def add_database_transaction(self, duration_sec):
@@ -289,7 +289,7 @@ def __init__(
 
         # The thread resource usage when the logcontext became active. None
         # if the context is not currently active.
-        self.usage_start: Optional[resource._RUsage] = None
+        self.usage_start: Optional[resource.struct_rusage] = None
 
         self.main_thread = get_thread_id()
         self.request = None
@@ -410,7 +410,7 @@ def copy_to(self, record) -> None:
         # we also track the current scope:
         record.scope = self.scope
 
-    def start(self, rusage: "Optional[resource._RUsage]") -> None:
+    def start(self, rusage: "Optional[resource.struct_rusage]") -> None:
         """
         Record that this logcontext is currently running.
 
@@ -435,7 +435,7 @@ def start(self, rusage: "Optional[resource._RUsage]") -> None:
         else:
             self.usage_start = rusage
 
-    def stop(self, rusage: "Optional[resource._RUsage]") -> None:
+    def stop(self, rusage: "Optional[resource.struct_rusage]") -> None:
         """
         Record that this logcontext is no longer running.
 
@@ -490,7 +490,7 @@ def get_resource_usage(self) -> ContextResourceUsage:
 
         return res
 
-    def _get_cputime(self, current: "resource._RUsage") -> Tuple[float, float]:
+    def _get_cputime(self, current: "resource.struct_rusage") -> Tuple[float, float]:
         """Get the cpu usage time between start() and the given rusage
 
         Args:

synapse/metrics/background_process_metrics.py

@@ -265,7 +265,7 @@ def __init__(self, name: str, instance_id: Optional[Union[int, str]] = None):
         super().__init__("%s-%s" % (name, instance_id))
         self._proc = _BackgroundProcess(name, self)
 
-    def start(self, rusage: "Optional[resource._RUsage]"):
+    def start(self, rusage: "Optional[resource.struct_rusage]"):
         """Log context has started running (again)."""
 
         super().start(rusage)

synapse/push/mailer.py

@@ -892,7 +892,7 @@ def safe_text(raw_text: str) -> jinja2.Markup:
         A Markup object ready to safely use in a Jinja template.
     """
     return jinja2.Markup(
-        bleach.linkify(bleach.clean(raw_text, tags=[], attributes={}, strip=False))
+        bleach.linkify(bleach.clean(raw_text, tags=[], attributes=[], strip=False))
     )
 
 

synapse/rest/media/v1/__init__.py

@@ -12,33 +12,21 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import PIL.Image
+from PIL.features import check_codec
 
 # check for JPEG support.
-try:
-    PIL.Image._getdecoder("rgb", "jpeg", None)
-except OSError as e:
-    if str(e).startswith("decoder jpeg not available"):
-        raise Exception(
-            "FATAL: jpeg codec not supported. Install pillow correctly! "
-            " 'sudo apt-get install libjpeg-dev' then 'pip uninstall pillow &&"
-            " pip install pillow --user'"
-        )
-except Exception:
-    # any other exception is fine
-    pass
+if not check_codec("jpg"):
+    raise Exception(
+        "FATAL: jpeg codec not supported. Install pillow correctly! "
+        " 'sudo apt-get install libjpeg-dev' then 'pip uninstall pillow &&"
+        " pip install pillow --user'"
+    )
 
 
 # check for PNG support.
-try:
-    PIL.Image._getdecoder("rgb", "zip", None)
-except OSError as e:
-    if str(e).startswith("decoder zip not available"):
-        raise Exception(
-            "FATAL: zip codec not supported. Install pillow correctly! "
-            " 'sudo apt-get install libjpeg-dev' then 'pip uninstall pillow &&"
-            " pip install pillow --user'"
-        )
-except Exception:
-    # any other exception is fine
-    pass
+if not check_codec("zlib"):
+    raise Exception(
+        "FATAL: zip codec not supported. Install pillow correctly! "
+        " 'sudo apt-get install libjpeg-dev' then 'pip uninstall pillow &&"
+        " pip install pillow --user'"
+    )

synapse/rest/media/v1/thumbnailer.py

@@ -61,9 +61,19 @@ def __init__(self, input_path: str):
         self.transpose_method = None
         try:
             # We don't use ImageOps.exif_transpose since it crashes with big EXIF
-            image_exif = self.image._getexif()
+            #
+            # Ignore safety: Pillow seems to acknowledge that this method is
+            # "private, experimental, but generally widely used". Pillow 6
+            # includes a public getexif() method (no underscore) that we might
+            # consider using instead when we can bump that dependency.
+            #
+            # At the time of writing, Debian buster (currently oldstable)
+            # provides version 5.4.1. It's expected to EOL in mid-2022, see
+            # https://wiki.debian.org/DebianReleases#Production_Releases
+            image_exif = self.image._getexif()  # type: ignore
             if image_exif is not None:
                 image_orientation = image_exif.get(EXIF_ORIENTATION_TAG)
+                assert isinstance(image_orientation, int)
                 self.transpose_method = EXIF_TRANSPOSE_MAPPINGS.get(image_orientation)
         except Exception as e:
             # A lot of parsing errors can happen when parsing EXIF
@@ -76,7 +86,10 @@ def transpose(self) -> Tuple[int, int]:
             A tuple containing the new image size in pixels as (width, height).
         """
         if self.transpose_method is not None:
-            self.image = self.image.transpose(self.transpose_method)
+            # Safety: `transpose` takes an int rather than e.g. an IntEnum.
+            # self.transpose_method is set above to be a value in
+            # EXIF_TRANSPOSE_MAPPINGS, and that only contains correct values.
+            self.image = self.image.transpose(self.transpose_method)  # type: ignore[arg-type]
             self.width, self.height = self.image.size
             self.transpose_method = None
             # We don't need EXIF any more
@@ -101,7 +114,7 @@ def aspect(self, max_width: int, max_height: int) -> Tuple[int, int]:
         else:
             return (max_height * self.width) // self.height, max_height
 
-    def _resize(self, width: int, height: int) -> Image:
+    def _resize(self, width: int, height: int) -> Image.Image:
         # 1-bit or 8-bit color palette images need converting to RGB
         # otherwise they will be scaled using nearest neighbour which
         # looks awful.
@@ -151,7 +164,7 @@ def crop(self, width: int, height: int, output_type: str) -> BytesIO:
             cropped = scaled_image.crop((crop_left, 0, crop_right, height))
         return self._encode_image(cropped, output_type)
 
-    def _encode_image(self, output_image: Image, output_type: str) -> BytesIO:
+    def _encode_image(self, output_image: Image.Image, output_type: str) -> BytesIO:
         output_bytes_io = BytesIO()
         fmt = self.FORMATS[output_type]
         if fmt == "JPEG":