Give Pyright what it wants (alias attributes everywhere) (#3114)

* Pyright wants aliases everywhere

* Changelog

* Make backslash replacement more correct

* Debug which class doesn't exist

* Debug a bit better

* Fix weirdness around PosixPath needing to be resolved

* Appease type checker

* Don't even consider tests in the alias test

* Turn paths into strings before indexing

* Explain the test better

* Fixes for pre-commit

* Reformat according to black

* One last pre-commit fix

* Simplify alias test by monkeypatching attrs.

* Skip pyright init attributes test if plugin has not run

* Catch any other renaming too

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Handle autoattribs

* Disambiguate

* Synchronise name

* Update import mode because it worked locally

* Check suspicions

* Double check

* Don't rely on installing `_trio_check_attrs_aliases.py` as a module

* importlib import mode means we can use conftest again!

* Just do something hacky instead

* Debug and remove `-Wall` for later PR

* Hopefully fix some things

* Give into perl

* Add perl to alpine (should work now)

---------

Co-authored-by: Spencer Brown <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 3 people

.github/workflows/ci.yml

@@ -189,7 +189,8 @@ jobs:
         # can't use setup-python because that python doesn't seem to work;
         # `python3-dev` (rather than `python:alpine`) for some ctypes reason,
         # `nodejs` for pyright (`node-env` pulls in nodejs but that takes a while and can time out the test).
-        run: apk update && apk add python3-dev bash nodejs
+        # `perl` for a platform independent `sed -i` alternative
+        run: apk update && apk add python3-dev bash nodejs perl
       - name: Enter virtual environment
         run: python -m venv .venv
       - name: Run tests

ci.sh

@@ -116,23 +116,29 @@ else
     echo "::group::Setup for tests"
 
     # We run the tests from inside an empty directory, to make sure Python
-    # doesn't pick up any .py files from our working dir. Might have been
-    # pre-created by some of the code above.
+    # doesn't pick up any .py files from our working dir. Might have already
+    # been created by a previous run.
     mkdir empty || true
     cd empty
 
     INSTALLDIR=$(python -c "import os, trio; print(os.path.dirname(trio.__file__))")
-    cp ../pyproject.toml "$INSTALLDIR"
+    cp ../pyproject.toml "$INSTALLDIR"  # TODO: remove this
 
     # get mypy tests a nice cache
     MYPYPATH=".." mypy --config-file= --cache-dir=./.mypy_cache -c "import trio" >/dev/null 2>/dev/null || true
 
     # support subprocess spawning with coverage.py
     echo "import coverage; coverage.process_startup()" | tee -a "$INSTALLDIR/../sitecustomize.py"
 
+    perl -i -pe 's/-p trio\._tests\.pytest_plugin//' "$INSTALLDIR/pyproject.toml"
+
     echo "::endgroup::"
     echo "::group:: Run Tests"
-    if COVERAGE_PROCESS_START=$(pwd)/../pyproject.toml coverage run --rcfile=../pyproject.toml -m pytest -ra --junitxml=../test-results.xml --run-slow "${INSTALLDIR}" --verbose --durations=10 $flags; then
+    if PYTHONPATH=../tests COVERAGE_PROCESS_START=$(pwd)/../pyproject.toml \
+            coverage run --rcfile=../pyproject.toml -m \
+            pytest -ra --junitxml=../test-results.xml \
+            -p _trio_check_attrs_aliases --verbose --durations=10 \
+            -p trio._tests.pytest_plugin --run-slow $flags "${INSTALLDIR}"; then
         PASSED=true
     else
         PASSED=false

newsfragments/3114.bugfix.rst

@@ -0,0 +1 @@
+Ensure that Pyright recognizes our underscore prefixed attributes for attrs classes.

pyproject.toml

@@ -203,7 +203,7 @@ reportUnnecessaryTypeIgnoreComment = true
 typeCheckingMode = "strict"
 
 [tool.pytest.ini_options]
-addopts = ["--strict-markers", "--strict-config", "-p trio._tests.pytest_plugin"]
+addopts = ["--strict-markers", "--strict-config", "-p trio._tests.pytest_plugin", "--import-mode=importlib"]
 faulthandler_timeout = 60
 filterwarnings = [
   "error",

src/trio/_core/_local.py

@@ -38,8 +38,8 @@ class RunVar(Generic[T]):
 
     """
 
-    _name: str
-    _default: T | type[_NoValue] = _NoValue
+    _name: str = attrs.field(alias="name")
+    _default: T | type[_NoValue] = attrs.field(default=_NoValue, alias="default")
 
     def get(self, default: T | type[_NoValue] = _NoValue) -> T:
         """Gets the value of this :class:`RunVar` for the current run call."""

src/trio/_core/_run.py

@@ -543,9 +543,13 @@ class CancelScope:
     cancelled_caught: bool = attrs.field(default=False, init=False)
 
     # Constructor arguments:
-    _relative_deadline: float = attrs.field(default=inf, kw_only=True)
-    _deadline: float = attrs.field(default=inf, kw_only=True)
-    _shield: bool = attrs.field(default=False, kw_only=True)
+    _relative_deadline: float = attrs.field(
+        default=inf,
+        kw_only=True,
+        alias="relative_deadline",
+    )
+    _deadline: float = attrs.field(default=inf, kw_only=True, alias="deadline")
+    _shield: bool = attrs.field(default=False, kw_only=True, alias="shield")
 
     def __attrs_post_init__(self) -> None:
         if isnan(self._deadline):

src/trio/_core/_tests/tutil.py

@@ -12,7 +12,7 @@
 
 import pytest
 
-# See trio/_tests/conftest.py for the other half of this
+# See trio/_tests/pytest_plugin.py for the other half of this
 from trio._tests.pytest_plugin import RUN_SLOW
 
 if TYPE_CHECKING:

src/trio/_tests/test_exports.py

@@ -19,11 +19,10 @@
 
 import trio
 import trio.testing
-from trio._tests.pytest_plugin import skip_if_optional_else_raise
+from trio._tests.pytest_plugin import RUN_SLOW, skip_if_optional_else_raise
 
 from .. import _core, _util
 from .._core._tests.tutil import slow
-from .pytest_plugin import RUN_SLOW
 
 if TYPE_CHECKING:
     from collections.abc import Iterable, Iterator
@@ -574,3 +573,37 @@ def test_classes_are_final() -> None:
                 continue
 
             assert class_is_final(class_)
+
+
+# Plugin might not be running, especially if running from an installed version.
+@pytest.mark.skipif(
+    not hasattr(attrs.field, "trio_modded"),
+    reason="Pytest plugin not installed.",
+)
+def test_pyright_recognizes_init_attributes() -> None:
+    """Check whether we provide `alias` for all underscore prefixed attributes.
+
+    Attrs always sets the `alias` attribute on fields, so a pytest plugin is used
+    to monkeypatch `field()` to record whether an alias was defined in the metadata.
+    See `_trio_check_attrs_aliases`.
+    """
+    for module in PUBLIC_MODULES:
+        for class_ in module.__dict__.values():
+            if not attrs.has(class_):
+                continue
+            if isinstance(class_, _util.NoPublicConstructor):
+                continue
+
+            attributes = [
+                attr
+                for attr in attrs.fields(class_)
+                if attr.init
+                if attr.alias
+                not in (
+                    attr.name,
+                    # trio_original_args may not be present in autoattribs
+                    attr.metadata.get("trio_original_args", {}).get("alias"),
+                )
+            ]
+
+            assert attributes == [], class_

tests/_trio_check_attrs_aliases.py

@@ -0,0 +1,22 @@
+"""Plugins are executed by Pytest before test modules.
+
+We use this to monkeypatch attrs.field(), so that we can detect if aliases are used for test_exports.
+"""
+
+from typing import Any
+
+import attrs
+
+orig_field = attrs.field
+
+
+def field(**kwargs: Any) -> Any:
+    original_args = kwargs.copy()
+    metadata = kwargs.setdefault("metadata", {})
+    metadata["trio_original_args"] = original_args
+    return orig_field(**kwargs)
+
+
+# Mark it as being ours, so the test knows it can actually run.
+field.trio_modded = True  # type: ignore
+attrs.field = field