B018 false positive on attribute access with side effect

[jaraco]

Using ruff 0.3.4 and attempting to adopt bugbear (B) checks in the keyring project, I'm encountering what appear to be false positives:

 keyring main @ ruff check --select B018 --fix --unsafe-fixes
keyring/backend.py:71:13: B018 Found useless expression. Either assign it to a variable or remove it.
keyring/backends/SecretService.py:35:13: B018 Found useless expression. Either assign it to a variable or remove it.
keyring/backends/Windows.py:14:9: B018 Found useless expression. Either assign it to a variable or remove it.
keyring/backends/Windows.py:21:9: B018 Found useless expression. Either assign it to a variable or remove it.
keyring/core.py:137:5: B018 Found useless expression. Either assign it to a variable or remove it.
keyring/testing/backend.py:177:13: B018 Found useless expression. Either assign it to a variable or remove it.

(there was one other prior to jaraco/keyring@7881db6)

Looking at keyring.backend:71, it's apparent that the "useless expression" is cls.priority. The protocol for keyring, however, is for a backend to raise an exception in the .priority access if the backend is not viable at all.

Looking at keyring.backends.Windows:14, the purpose of the attribute access is explicitly documented (force demand import to import the module).

The advice is wrong. In particular, the expression isn't useless (the application would fail without the side effect of accessing the property), and the guidance is misleading (assigning to a variable would introduce more lint and removing it would break the the behavior).

I can imagine one possible workaround is to replace direct attribute access with a getattr call:

diff --git a/keyring/backend.py b/keyring/backend.py
index a21418f..e082fbe 100644
--- a/keyring/backend.py
+++ b/keyring/backend.py
@@ -68,7 +68,7 @@ class KeyringBackend(metaclass=KeyringBackendMeta):
     @properties.classproperty
     def viable(cls):
         with errors.ExceptionRaisedContext() as exc:
-            cls.priority
+            getattr(cls, 'priority')
         return not exc
 
     @classmethod
diff --git a/keyring/backends/Windows.py b/keyring/backends/Windows.py
index 7208ed1..5d057ec 100644
--- a/keyring/backends/Windows.py
+++ b/keyring/backends/Windows.py
@@ -11,7 +11,7 @@ with ExceptionRaisedContext() as missing_deps:
         from win32ctypes.pywin32 import pywintypes, win32cred
 
         # force demand import to raise ImportError
-        win32cred.__name__
+        getattr(win32cred, '__name__')
     except ImportError:
         # fallback to pywin32
         import pywintypes

However, this approach makes the code less readable and idiosyncratic (the canonical way to access an attribute is with dot notation), and making the change introduces B009 errors:

 keyring main @ ruff check --select B009
keyring/backend.py:71:13: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
keyring/backends/Windows.py:14:9: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.

Another workaround could be to disable B018 checks for all projects (or possibly just this project; I'm not yet sure how much this issue is isolated to keyring). That would be somewhat undesirable. It would be nice to get an error for failing to call a method (e.g. self._init).

The workaround I'm leaning toward is to put noqa: B018 on each of these expressions, but that feels messy.

Since I'm not confident that ruff could readily distinguish between a property access that has a side-effect and a useless property access, I wonder if the documentation should at least capture this scenario and make a recommendation on what to do in that case.

[jaraco]

I did search the flake8-bugbear issue tracker (and this one) to see if this issue had been discussed previously, but came up empty.

[autinerd]

I am reviewing the ruff rules in Home Assistant and my solution is to use _ = <attribute> to make it clear that it is deliberately accessing the attribute without using the value (see home-assistant/core#113582)

Of course normally reading attributes or properties should not have side-effects, but there are occasions where it is done. The underscore as throw-away assignment is also quite common in the programming environment (e.g. in Rust)

[charliermarsh]

Ah yeah, I think suggesting _ = ... is a pretty reasonable workaround (or adding a # noqa: B018). I just can't think of a way for us to know that an attribute access is intentionally included to trigger a side effect. We can add this to the docs.