chore: expand ruff rule sets and make corresponding minor adjustments

docs/conf.py

@@ -33,8 +33,8 @@ def _compute_navigation_tree(context):
 
 # Pull in fix from https://github.com/sphinx-doc/sphinx/pull/11222/files to fix
 # "invalid signature for autoattribute ('ops.pebble::ServiceDict.backoff-delay')"
-import re
-import sphinx.ext.autodoc
+import re  # noqa: E402
+import sphinx.ext.autodoc  # noqa: E402
 sphinx.ext.autodoc.py_ext_sig_re = re.compile(
     r'''^ ([\w.]+::)?            # explicit module name
           ([\w.]+\.)?            # module and/or class name(s)
@@ -150,15 +150,15 @@ def _compute_navigation_tree(context):
 # Find the current builder
 builder = "dirhtml"
 if '-b' in sys.argv:
-    builder = sys.argv[sys.argv.index('-b')+1]
+    builder = sys.argv[sys.argv.index('-b') + 1]
 
 html_theme = 'furo'
 html_last_updated_fmt = ""
 html_permalinks_icon = "¶"
 html_theme_options = {
     "light_css_variables": {
         "color-sidebar-background-border": "none",
-        "font-stack": "Ubuntu, -apple-system, Segoe UI, Roboto, Oxygen, Cantarell, Fira Sans, Droid Sans, Helvetica Neue, sans-serif",
+        "font-stack": "Ubuntu, -apple-system, Segoe UI, Roboto, Oxygen, Cantarell, Fira Sans, Droid Sans, Helvetica Neue, sans-serif",  # noqa: E501
         "font-stack--monospace": "Ubuntu Mono, Consolas, Monaco, Courier, monospace",
         "color-foreground-primary": "#111",
         "color-foreground-secondary": "var(--color-foreground-primary)",
@@ -168,7 +168,7 @@ def _compute_navigation_tree(context):
         "color-brand-primary": "#111",
         "color-brand-content": "#06C",
         # NOTE: this looks horrible -- commented out
-        #"color-api-background": "#cdcdcd",
+        # "color-api-background": "#cdcdcd",
         "color-inline-code-background": "rgba(0,0,0,.03)",
         "color-sidebar-link-text": "#111",
         "color-sidebar-item-background--current": "#ebebeb",

ops/__init__.py

@@ -41,7 +41,7 @@
 
 # The "from .X import Y" imports below don't explicitly tell Pyright (or MyPy)
 # that those symbols are part of the public API, so we have to add __all__.
-__all__ = [
+__all__ = [  # noqa: RUF022 `__all__` is not sorted
     '__version__',
     'main',
     'pebble',
@@ -173,20 +173,20 @@
 # isort:skip_file
 
 # Import pebble explicitly. It's the one module we don't import names from below.
-from . import pebble  # type: ignore # noqa: F401
+from . import pebble  # type: ignore
 
 # Also import charm explicitly. This is not strictly necessary as the
 # "from .charm" import automatically does that, but be explicit since this
 # import was here previously
-from . import charm  # type: ignore # noqa: F401
+from . import charm  # type: ignore # noqa: F401 `.charm` imported but unused
 
 # Import the main module, which we've overriden in main.py to be callable.
 # This allows "import ops; ops.main(Charm)" to work as expected.
-from . import main  # type: ignore # noqa: F401
+from . import main  # type: ignore
 
 # Explicitly import names from submodules so users can just "import ops" and
 # then use them as "ops.X".
-from .charm import (  # noqa: F401
+from .charm import (
     ActionEvent,
     ActionMeta,
     CharmBase,
@@ -237,7 +237,7 @@
     WorkloadEvent,
 )
 
-from .framework import (  # noqa: F401
+from .framework import (
     BoundEvent,
     BoundStoredState,
     CommitEvent,
@@ -261,9 +261,9 @@
     StoredStateData,
 )
 
-from .jujuversion import JujuVersion  # noqa: F401
+from .jujuversion import JujuVersion
 
-from .model import (  # noqa: F401 E402
+from .model import (
     ActiveStatus,
     Application,
     Binding,
@@ -312,4 +312,4 @@
 # NOTE: don't import testing or Harness here, as that's a test-time concern
 # rather than a runtime concern.
 
-from .version import version as __version__  # noqa: F401 E402
+from .version import version as __version__

ops/_private/__init__.py

@@ -0,0 +1,13 @@
+# Copyright 2019-2024 Canonical Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.

ops/_private/yaml.py

@@ -25,7 +25,7 @@
 
 def safe_load(stream: Union[str, TextIO]) -> Any:
     """Same as yaml.safe_load, but use fast C loader if available."""
-    return yaml.load(stream, Loader=_safe_loader)  # type: ignore
+    return yaml.load(stream, Loader=_safe_loader)  # type: ignore  # noqa: S506
 
 
 def safe_dump(data: Any, stream: Optional[TextIO] = None) -> str:

ops/charm.py

@@ -226,9 +226,9 @@ class StartEvent(HookEvent):
 
     This event is triggered immediately after the first
     :class:`ConfigChangedEvent`. Callback methods bound to the event should be
-    used to ensure that the charm’s software is in a running state. Note that
-    the charm’s software should be configured so as to persist in this state
-    through reboots without further intervention on Juju’s part.
+    used to ensure that the charm's software is in a running state. Note that
+    the charm's software should be configured so as to persist in this state
+    through reboots without further intervention on Juju's part.
     """
 
 
@@ -237,8 +237,8 @@ class StopEvent(HookEvent):
 
     This event is triggered when an application's removal is requested
     by the client. The event fires immediately before the end of the
-    unit’s destruction sequence. Callback methods bound to this event
-    should be used to ensure that the charm’s software is not running,
+    unit's destruction sequence. Callback methods bound to this event
+    should be used to ensure that the charm's software is not running,
     and that it will not start again on reboot.
     """
 
@@ -507,7 +507,7 @@ class RelationChangedEvent(RelationEvent):
     are incomplete, since it can be guaranteed that when the remote unit or
     application changes its settings, the event will fire again.
 
-    The settings that may be queried, or set, are determined by the relation’s
+    The settings that may be queried, or set, are determined by the relation's
     interface.
     """
 
@@ -522,8 +522,8 @@ class RelationDepartedEvent(RelationEvent):
     emitted once for each remaining unit.
 
     Callback methods bound to this event may be used to remove all
-    references to the departing remote unit, because there’s no
-    guarantee that it’s still part of the system; it’s perfectly
+    references to the departing remote unit, because there's no
+    guarantee that it's still part of the system; it's perfectly
     probable (although not guaranteed) that the system running that
     unit has already shut down.
 
@@ -581,7 +581,7 @@ class RelationBrokenEvent(RelationEvent):
     fire to signal that the relationship has been fully terminated.
 
     The event indicates that the current relation is no longer valid, and that
-    the charm’s software must be configured as though the relation had never
+    the charm's software must be configured as though the relation had never
     existed. It will only be called after every callback method bound to
     :class:`RelationDepartedEvent` has been run. If a callback method
     bound to this event is being executed, it is guaranteed that no remote units
@@ -630,16 +630,15 @@ def restore(self, snapshot: Dict[str, Any]):
 
         if storage_name and storage_index is not None:
             storages = self.framework.model.storages[storage_name]
-            self.storage = next((s for s in storages if s.index == storage_index), None)  # type: ignore # noqa
+            self.storage = next((s for s in storages if s.index == storage_index), None)  # type: ignore
             if self.storage is None:
                 msg = 'failed loading storage (name={!r}, index={!r}) from snapshot' \
                     .format(storage_name, storage_index)
                 raise RuntimeError(msg)
             if storage_location is None:
                 raise RuntimeError(
                     'failed loading storage location from snapshot.'
-                    '(name={!r}, index={!r}, storage_location=None)'
-                    .format(storage_name, storage_index))
+                    f'(name={storage_name!r}, index={storage_index!r}, storage_location=None)')
 
             self.storage.location = storage_location
 
@@ -969,9 +968,7 @@ def _on_collect_status(self, event: ops.CollectStatusEvent):
                     event.add_status(ops.BlockedStatus('please set "port" config'))
                     return
                 event.add_status(ops.ActiveStatus())
-
-    .. # noqa (pydocstyle barfs on the above for unknown reasons I've spent hours on)
-    """
+    """  # noqa: D405, D214, D411, D416  Final return confuses docstyle
 
     def add_status(self, status: model.StatusBase):
         """Add a status for evaluation.

ops/framework.py

@@ -60,13 +60,13 @@ class Serializable(typing.Protocol):
     @property
     def handle(self) -> 'Handle': ...  # noqa
     @handle.setter
-    def handle(self, val: 'Handle'): ...  # noqa
+    def handle(self, val: 'Handle'): ...
     def snapshot(self) -> Dict[str, Any]: ...  # noqa
     def restore(self, snapshot: Dict[str, Any]) -> None: ...  # noqa
 
 
 class _StoredObject(Protocol):
-    _under: Any = None  # noqa
+    _under: Any = None
 
 
 StoredObject = Union['StoredList', 'StoredSet', 'StoredDict']

ops/lib/__init__.py

@@ -32,7 +32,7 @@
 from types import ModuleType
 from typing import List
 
-__all__ = ('use', 'autoimport')
+__all__ = ('autoimport', 'use')
 
 logger = logging.getLogger(__name__)
 
@@ -182,7 +182,7 @@ def _join_and(keys: List[str]) -> str:
 class _Missing:
     """Helper to get the difference between what was found and what was needed when logging."""
 
-    def __init__(self, found):
+    def __init__(self, found: bool):
         self._found = found
 
     def __str__(self):
@@ -202,7 +202,7 @@ def _parse_lib(spec: ModuleSpec) -> typing.Optional["_Lib"]:
     logger.debug("    Parsing %r", spec.name)
 
     try:
-        with open(spec.origin, 'rt', encoding='utf-8') as f:
+        with open(spec.origin, encoding='utf-8') as f:
             libinfo = {}
             for n, line in enumerate(f):
                 if len(libinfo) == len(_NEEDED_KEYS):
@@ -255,7 +255,7 @@ def __repr__(self):
         return f"<_Lib {self}>"
 
     def __str__(self):
-        return "{0.name} by {0.author}, API {0.api}, patch {0.patch}".format(self)
+        return f"{self.name} by {self.author}, API {self.api}, patch {self.patch}"
 
     def import_module(self) -> ModuleType:
         if self._module is None:
@@ -264,14 +264,14 @@ def import_module(self) -> ModuleType:
             self._module = module
         return self._module
 
-    def __eq__(self, other):
+    def __eq__(self, other):  # noqa: ANN001
         if not isinstance(other, _Lib):
             return NotImplemented
         a = (self.name, self.author, self.api, self.patch)
         b = (other.name, other.author, other.api, other.patch)
         return a == b
 
-    def __lt__(self, other):
+    def __lt__(self, other):  # noqa: ANN001
         if not isinstance(other, _Lib):
             return NotImplemented
         a = (self.name, self.author, self.api, self.patch)

ops/main.py

@@ -395,10 +395,7 @@ def main(charm_class: Type[ops.charm.CharmBase],
 
     metadata = (charm_dir / 'metadata.yaml').read_text()
     actions_meta = charm_dir / 'actions.yaml'
-    if actions_meta.exists():
-        actions_metadata = actions_meta.read_text()
-    else:
-        actions_metadata = None
+    actions_metadata = actions_meta.read_text() if actions_meta.exists() else None
 
     # If we are in a RelationBroken event, we want to know which relation is
     # broken within the model, not only in the event's `.relation` attribute.