Skip to content

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Oct 26, 2024

Fix part of #869 - make sure only valid options are set.

The most recent commit also adds static typing for nox.options.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii mentioned this pull request Oct 28, 2024
@henryiii henryiii marked this pull request as draft October 28, 2024 20:49
@henryiii henryiii marked this pull request as ready for review October 28, 2024 21:13
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii
Copy link
Collaborator Author

I was testing out attrs in order to get validation, and found one type that was wrong - verbose was getting set to None. Since it's never checked for None specifically, I fixed the default.

Here's the patch to move to attrs:

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 1f4d45f..2fab09e 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -37,6 +37,7 @@ repos:
         files: ^nox/
         args: []
         additional_dependencies:
+          - attrs
           - jinja2
           - packaging
           - importlib_metadata
diff --git a/nox/_option_set.py b/nox/_option_set.py
index d0a0297..e99413e 100644
--- a/nox/_option_set.py
+++ b/nox/_option_set.py
@@ -20,7 +20,6 @@ from __future__ import annotations

 import argparse
 import collections
-import dataclasses
 import functools
 from argparse import ArgumentError as ArgumentError  # noqa: PLC0414
 from argparse import ArgumentParser, Namespace
@@ -28,41 +27,30 @@ from collections.abc import Callable, Iterable
 from typing import Any, Literal

 import argcomplete
+import attrs
+import attrs.validators as av

+av_opt_str = av.optional(av.instance_of(str))
+av_opt_list_str = av.optional(av.deep_iterable(member_validator=av.instance_of(str), iterable_validator=attrs.validators.instance_of(list)))
+av_bool = av.instance_of(bool)

-# Python 3.10+ has slots=True (or attrs does), also kwonly=True
-@dataclasses.dataclass
+
+@attrs.define(slots=True, kw_only=True)
 class NoxOptions:
-    __slots__ = (
-        "default_venv_backend",
-        "envdir",
-        "error_on_external_run",
-        "error_on_missing_interpreters",
-        "force_venv_backend",
-        "keywords",
-        "pythons",
-        "report",
-        "reuse_existing_virtualenvs",
-        "reuse_venv",
-        "sessions",
-        "stop_on_first_error",
-        "tags",
-        "verbose",
-    )
-    default_venv_backend: None | str
-    envdir: None | str
-    error_on_external_run: bool
-    error_on_missing_interpreters: bool
-    force_venv_backend: None | str
-    keywords: None | list[str]
-    pythons: None | list[str]
-    report: None | str
-    reuse_existing_virtualenvs: bool
-    reuse_venv: None | Literal["no", "yes", "never", "always"]
-    sessions: None | list[str]
-    stop_on_first_error: bool
-    tags: None | list[str]
-    verbose: bool
+    default_venv_backend: None | str = attrs.field(validator=av_opt_str)
+    envdir: None | str = attrs.field(validator=av_opt_str)
+    error_on_external_run: bool = attrs.field(validator=av_bool)
+    error_on_missing_interpreters: bool = attrs.field(validator=av_bool)
+    force_venv_backend: None | str = attrs.field(validator=av_opt_str)
+    keywords: None | list[str] = attrs.field(validator=av_opt_list_str)
+    pythons: None | list[str] = attrs.field(validator=av_opt_list_str)
+    report: None | str = attrs.field(validator=av_opt_str)
+    reuse_existing_virtualenvs: bool = attrs.field(validator=av_bool)
+    reuse_venv: None | Literal["no", "yes", "never", "always"] = attrs.field(validator=av.optional(av.in_(["no", "yes", "never", "always"])))
+    sessions: None | list[str] = attrs.field(validator=av_opt_list_str)
+    stop_on_first_error: bool = attrs.field(validator=av_bool)
+    tags: None | list[str] = attrs.field(validator=av_opt_list_str)
+    verbose: bool = attrs.field(validator=av_bool)


 class OptionGroup:
diff --git a/pyproject.toml b/pyproject.toml
index 3781be7..df38d44 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -41,6 +41,7 @@ classifiers = [
 ]
 dependencies = [
   "argcomplete<4,>=1.9.4",
+  "attrs>=23.1",
   "colorlog<7,>=2.6.1",
   "packaging>=20.9",
   "tomli>=1; python_version<'3.11'",

Might be an interesting followup. Attrs is a highly respected, zero dependency, small library and used by things like pytest.

@henryiii henryiii merged commit 6edc697 into wntrblm:main Oct 29, 2024
24 checks passed
@henryiii henryiii deleted the henryiii/fix/notoption branch October 29, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants