Repository: canonical/operator
Pull request #1388 (2024-09-24 04:14:39)

ci: spell-check the code as part of linting
Add a `codespell` run as part of the `tox -e lint` environment, to pick up any spelling errors that might be missed in review. It's also added to the pre-commit checks, for anyone using those.

The PR also makes 4 corrections, which codespell found. Two are debatable (whether or not to hyphenate) but two are definite errors.

This adds a new (lint-time) dependency of codespell - it's also used in the default charmcraft profile, I've used it before, and it has a [good Snyk score](https://snyk.io/advisor/python/codespell) so it seems ok to me. I have lightly skimmed the code, but not read it in depth.

tonyandrewmeyer: > Out of interest, what does this check? Just code comments and strings?

I think it's doing a regular expression match for `[\w\-'’]+` by default (you can customise this). The doc says:

>> Fix common misspellings in text files. It's designed primarily for checking misspelled words in source code (backslash escapes are skipped), but it can be used with other files as well. It does not check for word membership in a complete dictionary, but instead looks for a set of common misspellings. Therefore it should catch errors like "adn", but it will not catch "adnasdfasdf". This also means it shouldn't generate false-positives when you use a niche term it doesn't know about.

I think this 'look for misspellings' is how it manages to avoid having to look specifically for words in comments, strings, and so on. If I add a line `adn = 42` then it'll flag that as a potential misspelling. There are ways to disable the check for specific lines, but if we start having to do that all over the place then I'd rather get rid of it or use a different tool. I think that it found 2 (to 4) legitimate mistakes and had no false positives is a positive sign, though.

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index d4ed43f..1649f28 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v4.5.0
     hooks:
       - id: check-added-large-files
       - id: check-ast
@@ -24,3 +24,10 @@ repos:
         args: [ --preview ]
       - id: ruff-format
         args: [ --preview ]
+  # Spellcheck the code.
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.2.4
+    hooks:
+    - id: codespell
+      additional_dependencies:
+        - tomli
diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md
index b5bc1b1..0732215 100644
--- a/CODE_OF_CONDUCT.md
+++ b/CODE_OF_CONDUCT.md
@@ -6,7 +6,7 @@ In the interest of fostering an open and welcoming environment, we as
 contributors and maintainers pledge to making participation in our project and
 our community a harassment-free experience for everyone, regardless of age, body
 size, disability, ethnicity, sex characteristics, gender identity and expression,
-level of experience, education, socio-economic status, nationality, personal
+level of experience, education, socioeconomic status, nationality, personal
 appearance, race, religion, or sexual identity and orientation.
 
 ## Our Standards
diff --git a/ops/_private/harness.py b/ops/_private/harness.py
index 0933440..c762b45 100644
--- a/ops/_private/harness.py
+++ b/ops/_private/harness.py
@@ -380,7 +380,7 @@ class Harness(Generic[CharmType]):
         if self._charm is not None:
             raise RuntimeError('cannot call the begin method on the harness more than once')
 
-        # The Framework adds attributes to class objects for events, etc. As such, we can't re-use
+        # The Framework adds attributes to class objects for events, etc. As such, we can't reuse
         # the original class against multiple Frameworks. So create a locally defined class
         # and register it.
         # TODO: jam 2020-03-16 We are looking to changes this to Instance attributes instead of
diff --git a/ops/charm.py b/ops/charm.py
index edef8db..0e68618 100644
--- a/ops/charm.py
+++ b/ops/charm.py
@@ -1105,12 +1105,13 @@ class CollectStatusEvent(LifecycleEvent):
 
     The order of priorities is as follows, from highest to lowest:
 
-    * error
     * blocked
     * maintenance
     * waiting
     * active
-    * unknown
+
+    It is an error to call :meth:`add_status` with an instance of
+    :class:`ErrorStatus` or :class:`UnknownStatus`.
 
     If there are multiple statuses with the same priority, the first one added
     wins (and if an event is observed multiple times, the handlers are called
@@ -1151,6 +1152,10 @@ class CollectStatusEvent(LifecycleEvent):
         """
         if not isinstance(status, model.StatusBase):
             raise TypeError(f'status should be a StatusBase, not {type(status).__name__}')
+        if status.name not in model._SETTABLE_STATUS_NAMES:
+            raise model.InvalidStatusError(
+                f'status.name must be in {model._SETTABLE_STATUS_NAMES}, not {status.name!r}'
+            )
         model_ = self.framework.model
         if self.handle.kind == 'collect_app_status':
             if not isinstance(status, model.ActiveStatus):
diff --git a/ops/model.py b/ops/model.py
index d03c4b5..3cc268f 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -3133,7 +3133,7 @@ def _format_action_result_dict(
     parent_key: Optional[str] = None,
     output: Optional[Dict[str, str]] = None,
 ) -> Dict[str, str]:
-    """Turn a nested dictionary into a flattened dictionary, using '.' as a key seperator.
+    """Turn a nested dictionary into a flattened dictionary, using '.' as a key separator.
 
     This is used to allow nested dictionaries to be translated into the dotted format required by
     the Juju `action-set` hook tool in order to set nested data on an action.
@@ -3973,7 +3973,7 @@ class CloudSpec:
     """A list of CA certificates."""
 
     skip_tls_verify: bool = False
-    """Whether to skip TLS verfication."""
+    """Whether to skip TLS verification."""
 
     is_controller_cloud: bool = False
     """If this is the cloud used by the controller, defaults to False."""
diff --git a/pyproject.toml b/pyproject.toml
index 4bfa2d1..2379629 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -224,3 +224,7 @@ reportUnnecessaryComparison = false
 reportUnnecessaryTypeIgnoreComment = "error"
 disableBytesTypePromotions = false
 stubPath = ""
+
+[tool.codespell]
+skip = './docs/_build,.venv,venv,build'
+quiet-level = 3
diff --git a/tox.ini b/tox.ini
index cf097a1..505aa79 100644
--- a/tox.ini
+++ b/tox.ini
@@ -68,9 +68,11 @@ commands =
 description = Check code against coding style standards
 deps =
     ruff==0.4.5
+    codespell==2.3.0
 commands =
     ruff check --preview
     ruff format --preview --check
+    codespell
 
 [testenv:static]
 description = Run static type checker


Test patch:
diff --git a/test/test_charm.py b/test/test_charm.py
index 0939fbb..5872a4e 100644
--- a/test/test_charm.py
+++ b/test/test_charm.py
@@ -959,7 +959,6 @@ def test_add_status_type_error(request: pytest.FixtureRequest, fake_script: Fake
         (['waiting', 'blocked'], 'blocked'),
         (['waiting', 'maintenance'], 'maintenance'),
         (['active', 'waiting'], 'waiting'),
-        (['active', 'unknown'], 'active'),
     ],
 )
 def test_collect_status_priority_valid(
@@ -994,6 +993,7 @@ def test_collect_status_priority_valid(
     [
         ['blocked', 'error'],
         ['unknown'],
+        ['active', 'unknown'],
     ],
 )
 def test_collect_status_priority_invalid(
@@ -1015,7 +1015,7 @@ def test_collect_status_priority_invalid(
 
     framework = create_framework(request)
     charm = MyCharm(framework, statuses=statuses)
-    with pytest.raises(ModelError):
+    with pytest.raises(ops.InvalidStatusError):
         ops.charm._evaluate_status(charm)