From 1859d0f44ad900506f020d8f768f90a65488c890 Mon Sep 17 00:00:00 2001
From: iphydf <iphydf@users.noreply.github.com>
Date: Fri, 4 Feb 2022 15:05:19 +0000
Subject: [PATCH] cleanup: Ensure we limit the system headers included in .h
 files.

Most system headers contain functions (e.g. `memcpy` in `string.h`)
which aren't needed in our own header files. For the most part, our own
headers should only include types needed to declare our own types and
functions. We now enforce this so we think twice about which headers we
really need in the .h files.
---
 .circleci/config.yml           |  1 +
 .github/workflows/ci.yml       |  6 ++--
 .gitignore                     |  1 +
 other/analysis/check_includes  | 57 ++++++++++++++++++++++++++++++++++
 other/analysis/check_recursion |  2 +-
 toxav/audio.c                  |  1 +
 toxav/msi.h                    |  2 +-
 toxcore/ccompat.h              |  1 -
 toxcore/onion_client.c         |  1 +
 9 files changed, 65 insertions(+), 7 deletions(-)
 create mode 100755 other/analysis/check_includes

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 6bb53ccedd..3beb3dc076 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -89,6 +89,7 @@ jobs:
       - run: *apt_install
       - run: apt-get install -y --no-install-recommends cppcheck g++ llvm-dev
       - checkout
+      - run: other/analysis/check_includes
       - run: other/analysis/check_logger_levels
       - run: other/analysis/run-check-recursion
       - run: other/analysis/run-clang
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 5baed17599..8fd89868a5 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -17,10 +17,8 @@ jobs:
         run: pip install mypy
       - name: Run mypy
         run: |
-          mypy --strict \
-            $(echo $(find . -name "*.py" -and -not -name "conanfile.py") \
-                   $(grep -lR '^#!.*python3' .) \
-              | tr ' ' '\n' | sort -u | tr '\n' ' ')
+          (find . -name "*.py" -and -not -name "conanfile.py"; grep -lR '^#!.*python') \
+              | xargs -n1 -P8 mypy --strict
 
   build-msan:
     runs-on: ubuntu-latest
diff --git a/.gitignore b/.gitignore
index 2586b9a0ae..9dcf09b692 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,7 @@
 .DS_Store
 .DS_Store?
 ._*
+.mypy_cache
 .Spotlight-V100
 .Trash*
 Icon?
diff --git a/other/analysis/check_includes b/other/analysis/check_includes
new file mode 100755
index 0000000000..38d67feab2
--- /dev/null
+++ b/other/analysis/check_includes
@@ -0,0 +1,57 @@
+#!/usr/bin/env python3
+import subprocess
+import sys
+from typing import Tuple
+
+ALLOWLIST: Tuple[str, ...] = (
+    # system headers
+    "pthread.h",
+    "stdarg.h",
+    "stdbool.h",
+    "stddef.h",
+    "stdint.h",
+    # toxav stuff, maybe not worth abstracting away
+    "opus.h",
+    "vpx/vp8cx.h",
+    "vpx/vp8dx.h",
+    "vpx/vpx_decoder.h",
+    "vpx/vpx_encoder.h",
+    "vpx/vpx_image.h",
+)
+
+out = (subprocess.run(
+    [
+        "grep", "-R", "^#include <.*>", "other", "toxav", "toxcore",
+        "toxencryptsave"
+    ],
+    check=True,
+    capture_output=True,
+).stdout.decode("utf-8").rstrip())
+
+errors = 0
+for line in out.split("\n"):
+    # other/fun can do what it wants.
+    if "/fun/" in line:
+        continue
+    filename, include = line.split(":", 1)
+    # We only check headers.
+    if not filename.endswith(".h"):
+        continue
+    # ccompat.h can include some things we don't really want elsewhere.
+    allowlist = ALLOWLIST
+    if filename == "toxcore/ccompat.h":
+        allowlist += ("alloca.h", "malloc.h", "stdlib.h")
+    header = include[include.index("<") + 1:include.index(">")]
+    if header not in allowlist:
+        source = filename[:-2] + ".c"
+        print(
+            f"{filename}: includes system header <{header}>, which is not allowed in .h files"
+        )
+        print(
+            " " * len(filename) +
+            f"  consider including it in {source} and exporting an abstraction, instead"
+        )
+        errors += 1
+
+if errors:
+    sys.exit(1)
diff --git a/other/analysis/check_recursion b/other/analysis/check_recursion
index 28dd5746bd..1ae093dff9 100755
--- a/other/analysis/check_recursion
+++ b/other/analysis/check_recursion
@@ -27,7 +27,7 @@ def load_callgraph() -> Dict[str, List[str]]:
     Returns graph as dict[str, list[str]] containing nodes with their outgoing
     edges.
     """
-    graph = collections.defaultdict(set)
+    graph: Dict[str, Set[str]] = collections.defaultdict(set)
     cur = None
     for line in fileinput.input():
         found = re.search("Call graph node for function: '(.*)'", line)
diff --git a/toxav/audio.c b/toxav/audio.c
index 39882778b8..5369a1e15b 100644
--- a/toxav/audio.c
+++ b/toxav/audio.c
@@ -4,6 +4,7 @@
  */
 #include "audio.h"
 
+#include <assert.h>
 #include <stdlib.h>
 #include <string.h>
 
diff --git a/toxav/msi.h b/toxav/msi.h
index a8ca3ad3a5..83d2a968c0 100644
--- a/toxav/msi.h
+++ b/toxav/msi.h
@@ -5,8 +5,8 @@
 #ifndef C_TOXCORE_TOXAV_MSI_H
 #define C_TOXCORE_TOXAV_MSI_H
 
-#include <inttypes.h>
 #include <pthread.h>
+#include <stdint.h>
 
 #include "audio.h"
 #include "video.h"
diff --git a/toxcore/ccompat.h b/toxcore/ccompat.h
index f03dbad11f..d5fd632028 100644
--- a/toxcore/ccompat.h
+++ b/toxcore/ccompat.h
@@ -8,7 +8,6 @@
 #ifndef C_TOXCORE_TOXCORE_CCOMPAT_H
 #define C_TOXCORE_TOXCORE_CCOMPAT_H
 
-#include <assert.h>
 #include <stdbool.h>
 
 bool unused_for_tokstyle(void);
diff --git a/toxcore/onion_client.c b/toxcore/onion_client.c
index 8004e0b7c9..cf5fe647f7 100644
--- a/toxcore/onion_client.c
+++ b/toxcore/onion_client.c
@@ -9,6 +9,7 @@
  */
 #include "onion_client.h"
 
+#include <assert.h>
 #include <stdlib.h>
 #include <string.h>