Add type annotations and document their use in doc/patterns.md.

Enforce the annotations in `check.sh` and CI using `pyanalyze`.

Signed-off-by: Daira Emma Hopwood <[email protected]>

.flake8

@@ -2,13 +2,14 @@
 exclude = .git, __pycache__
 
 # Justifications for each ignored error or warning:
+# * E252: equals for a default argument should look distinct from assignment.
 # * E302: always requiring two blank lines rather than one between top-level items is too annoying and nitpicky.
 # * E402: we want imports for tests to go between the main code and the tests.
 # * W503, W504: these give false positives for the style of avoiding \ at the end of each line by using parens.
 #
 # References:
 # - https://flake8.pycqa.org/en/latest/user/error-codes.html
 # - https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes
-ignore = E302, E402, W503, W504
+ignore = E252, E302, E402, W503, W504
 
 max-line-length = 120

.github/workflows/flake8.yml → .github/workflows/lints.yml

@@ -1,4 +1,4 @@
-name: flake8
+name: lints
 
 on: pull_request
 
@@ -16,8 +16,14 @@ jobs:
       - name: Install poetry
         run: pip install --user poetry
 
+      - name: Run poetry check
+        run: poetry check
+
       - name: Install dependencies
         run: poetry install --no-root
 
       - name: Run flake8
         run: poetry run flake8
+
+      - name: Run pyanalyze
+        run: poetry run pyanalyze simtfl

README.md

@@ -27,20 +27,20 @@ Note the caveats: *experimental*, *simulator*, *research*, *potential*.
 
 Design documentation is under the `doc/` directory:
 
-* [Programming patterns for use of simpy](doc/patterns.md).
+* [Programming patterns for use of simpy and type annotations](doc/patterns.md).
 
 You can also generate API documentation by running `./gendoc.sh`. This assumes
 that you have run `poetry install` as shown above. The starting point for the
 generated documentation is <apidoc/simtfl.html>.
 
 ## Contributing
 
-Please use `./check.sh` before submitting a PR. This currently runs `flake8`
-and the unit tests locally.
+Please use `./check.sh` before submitting a PR. This currently runs `flake8`,
+`pyanalyze`, and the unit tests locally.
 
-You can use `./check.sh -k <substring>` to run `flake8` and then only tests
-with names matching the given substring. This will not suppress output to
-stdout or stderr (but `./check.sh -bk <substring>` will).
+You can use `./check.sh -k <substring>` to run `flake8`, `pyanalyze`, and then
+only tests with names matching the given substring. This will not suppress
+output to stdout or stderr (but `./check.sh -bk <substring>` will).
 
 To see other options for running unit tests, use `poetry run python -m unittest -h`.
 

check.sh

@@ -3,9 +3,15 @@
 set -eu
 cd -P -- "$(dirname -- "$(command -v -- "$0")")"
 
+echo Running poetry check...
+poetry check
+
 echo Running flake8...
 poetry run flake8
 
+echo Running pyanalyze...
+poetry run pyanalyze simtfl
+
 echo
 echo Running unit tests...
 args="${*:---buffer}"

doc/patterns.md

@@ -6,8 +6,9 @@ processes are implemented as generators, so that the library can simulate
 timeouts and asynchronous communication (typically faster than real time).
 
 We use the convention of putting "(process)" in the doc comment of these
-functions. They either must use the `yield` construct, *or* return the
-result of calling another "(process)" function (not both).
+functions. They are also annotated with a `ProcessEffect` return type.
+They either must use the `yield` construct, *or* return the result of
+calling another "(process)" function (not both).
 
 Objects that implement processes typically hold the `simpy.Environment` in
 an instance variable `self.env`.
@@ -18,3 +19,67 @@ statements, `return f()` can be used as an optimization.)
 
 A "(process)" function that does nothing should `return skip()`, using
 `simtfl.util.skip`.
+
+
+# Type annotations
+
+The codebase is written to use static type annotations and to pass the
+[pyanalyze](https://pyanalyze.readthedocs.io/en/latest/faq.html) static
+analyzer.
+
+The default annotation for argument and return types is `Any`. This works
+well for interoperating with libraries that don't use static typing, but
+please don't rely on it for code in this project. It is better to add
+explicit `Any` annotations in the few cases where that is needed. This
+means that functions and methods that do not return a value should be
+annotated with `-> None`.
+
+`pyanalyze` has some limitations and is not claimed to be fully sound,
+but it does a pretty good job in practice; the result feels pretty much
+like a statically typed variant of Python. Importing the code it checks
+allows it to be more compatible with some Python idioms. The following
+workarounds for its limitations may be needed:
+
+* It is sometimes unable to see that a `None` value cannot occur in a
+  particular context. In that case, adding an assertion that the value
+  `is not None` may help.
+
+* There is no easy way to precisely check uses of `*args` or `**kwargs`.
+
+* If two files have mutually dependent types, they may end up circularly
+  importing each other, which Python does not support. This is more
+  likely for types than for implementation code. There are several
+  possible workarounds:
+
+  * merge the files;
+  * move part of one file that is causing the circularity into the
+    other;
+  * create an abstract base class for the type that is being used
+    circularly (with methods that raise `NotImplementedError`), and
+    use that as the type instead of the concrete subclass.
+
+* Adding type annotations might require exposing internal classes that
+  would otherwise be intentionally hidden. Since this hiding is normally
+  only possible by convention (e.g. using underscore-prefixed names)
+  in any case, that does not really cause any problem. Please just
+  refrain from directly using the constructors of such classes.
+
+As is often the case for static typing in other languages, it typically
+works best to use more general types for arguments and more specific
+types for results.
+
+Each source file should have `from __future__ import annotations` at
+the top. This (usually) allows types defined in the same file to be
+referenced before their definition, without needing the workaround of
+writing such types as string literals. The preferred style is for this
+line to be immediately followed by other imports that are needed for
+type annotations.
+
+
+# Flake8
+
+We also use [flake8](https://flake8.pycqa.org/en/latest/) to encourage
+a consistent coding style. However, if you disagree with a particular
+error or warning produced by `flake8` and have a good justification for
+why it should not be enforced, please just add it to the `ignore` list
+in `.flake8`, and document the justification there.