From b597daf4ac4bf6536e04bdf901ecaf11566504a1 Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Tue, 11 Aug 2026 20:26:43 +0200 Subject: [PATCH] Remove the log-origin column Remove the column that named the Update-time module each log line came from. Closes #230. --- .claude/CLAUDE.md | 4 +- CHANGELOG.md | 4 + src/update_time/io/log.py | 92 ++++-------------- src/update_time/references/__init__.py | 5 - src/update_time/references/github.py | 6 +- tests/update_time/helpers.py | 2 +- tests/update_time/io/test_log.py | 76 +-------------- tools/fixit_rules.py | 123 +++++++++++++++++++++++++ tools/generate_log_svg.py | 11 +-- tools/vulture-whitelist.py | 31 ++++--- 10 files changed, 175 insertions(+), 179 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 525bc72..e5addfb 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -20,7 +20,7 @@ Rename a Python name with `just rename` rather than by substitution: it resolves When reviewing code, whether standalone or as part of the TDD refactor step, pay attention to the following: -- **Comments**: a comment or docstring says what *this* code does as it stands, never where the increment is heading, and only what the signature and the code don't already say. Delete a sentence that narrates or justifies an edit, that says what the code is not or is unlike, or that restates a parameter, return type, or default. Delete one that describes what a caller or a collaborator does, or that names a concept from a layer further out than the module it sits in. A test's docstring answers to the same rules: it says which case the test pins, while the reason the behaviour exists belongs in the code under test and in the README. Keep a contrast only when the reader has to act on the difference. When a signature changes, the docstring usually needs a word in its summary, not a new sentence. A docstring that matches the one beside it has been copied rather than checked: read both against these rules, since a defect copied reads as a convention. +- **Comments**: a comment or docstring says what *this* code does as it stands, never where the increment is heading, and only what the signature and the code don't already say. Delete a sentence that narrates or justifies an edit, that says what the code is not or is unlike, or that restates a parameter, return type, or default. The summary already names what the function returns, so don't add a sentence saying what the return value is for. Delete one that describes what a caller or a collaborator does, or that names a concept from a layer further out than the module it sits in. A test's docstring answers to the same rules: it says which case the test pins, while the reason the behaviour exists belongs in the code under test and in the README. Keep a contrast only when the reader has to act on the difference. When a signature changes, the docstring usually needs a word in its summary, not a new sentence. A docstring that matches the one beside it has been copied rather than checked: read both against these rules, since a defect copied reads as a convention. - **Duplication**: look for the same decision taken in more than one place, rather than for repeated lines. A rule every module has to remember to apply is duplication too: prefer stating it once, somewhere it cannot be forgotten. A helper callers have to remember to call can be forgotten as well, so prefer a check that reads the code itself. In tests, repeated setup or a repeated assertion is worth naming as a helper. - **Reuse**: look for an existing type, test helper, or fixture before writing a new one. The misses to watch for: a fixture's value spelled out as a literal, a mixin's setup redone inline, and the same builder defined in two modules rather than in the shared one. - **Complexity**: a function should hold one decision. Watch for nesting, for flag parameters that make one function do two things, and for long parameter lists. When a docstring needs several sentences to describe the control flow, the code is doing too much, rather than the docstring being too short. @@ -50,7 +50,7 @@ A few rules that keep the cycle honest: 1. New tests follow the conventions of the nearest existing test for the same kind of behaviour, unless that test breaks a rule below. They cover the cases it covers too: a construct of an existing shape has the same edges, so read those off that test before choosing which case to start with. A table of cases loops with `subTest` naming each case, rather than repeating an assertion or a helper call, and reports one failure per failing case, so predict that many failures. 2. Build a behaviour before its off-switch: don't test an opt-out, a flag, or any other suppression until the thing it suppresses exists. 3. Assert what does and doesn't happen, rather than saying it in a docstring: an assertion is checked on every run, a docstring claim is checked by nobody. A test that asserts nothing was found passes just as well when nothing was examined, so assert that something was. A test's name is not evidence of what it guards, and neither is a green run, so settle what a guard catches with `just mutate`, a duplicate you would fold or delete included. Pick the stub from the regression that guard defends against, not from the nearest line to mutate: one that leaves the guard green says nothing about it, and one that fails a dozen others says little more, since it shows the suite reacting rather than that guard. -4. A test that pins down existing behaviour drives no code, so predict it passes and say so; it closes a gap in intent, not in behaviour. +4. A test that pins down existing behaviour drives no code, so predict it passes and say so; it closes a gap in intent, not in behaviour. A step that only removes behaviour drives no test either, so don't offer one asserting the removed thing is gone. Delete the tests that guarded it, and predict what the deletion leaves behind: the suite green, the count down by exactly the tests deleted, and a name those tests were the last outside caller of now private. 5. Coverage must stay at 100%, which `just test` already enforces, so a passing run needs no separate coverage command. A gap after implementing points at a test case worth adding, not at a line worth excluding. A step must not end with the new code uncovered: when the test you chose mocks the collaborator the new code lives on, add a second test in the same step that reaches that code. The one file the gate doesn't reach is `tools/fixit_rules.py`, which coverage omits: pin each of its branches with the VALID and INVALID cases `just fixit` runs. 6. Treat a failing existing test as a signal: work out whether its premise legitimately changed and say why, rather than patching the assertion to match the new output. 7. A green run can prove nothing, so check it ran what you think: read the tail of `just test` and `just check` yourself instead of grepping or counting their output, since a filter that matches nothing exits non-zero and silently skips the rest of an `&&` chain. A filter that prints nothing has told you nothing, so never read that silence as a pass. Compare the test count whenever imports move or test methods are renamed, since a module that fails to import, or a method whose new name collides with an existing one, drops tests without a word. The same goes for the output you quote from: never read a count off something you piped through `head`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f03fcb..003c106 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] +### Changed + +- Remove the column that named the Update-time module each log line came from. Closes [#230](https://github.com/ICTU/update-time/issues/230). + ### Fixed - Read a GitHub repository URL in three more spellings: git's scp-like `git@github.com:owner/repo` form, npm's `github:owner/repo` host shorthand, and npm's bare `owner/repo` shorthand. A pre-commit hook whose repository uses the scp-like form is updated instead of being silently left alone. The changelog of a package whose npm `repository` uses any of the three is found instead of being reported missing. Closes [#224](https://github.com/ICTU/update-time/issues/224). diff --git a/src/update_time/io/log.py b/src/update_time/io/log.py index e7f46b7..8687bee 100644 --- a/src/update_time/io/log.py +++ b/src/update_time/io/log.py @@ -2,10 +2,8 @@ import logging import re -import sys from dataclasses import dataclass from logging import DEBUG, ERROR, INFO, WARNING -from pathlib import Path from typing import TYPE_CHECKING from rich.console import Console @@ -21,7 +19,7 @@ from update_time.primitives.timestamp import days_since if TYPE_CHECKING: - from types import FrameType + from pathlib import Path from requests import Response from rich.text import Text @@ -117,65 +115,23 @@ def _restyle_delimited(text: Text, pattern: re.Pattern[str], style: str, *, keep # The theme adds the styles `LogHighlighter` applies: `repr.digest` for a whole `sha256:` digest and `repr.dependency` # (bold white) for a dependency name; a file location reuses Rich's built-in `repr.filename`, so it needs no entry. -# When colour is off, all render as plain text. The theme and formats are shared with `docs/generate_log_svg.py`, -# which logs its sample through `Logger` itself, so the README screenshot renders exactly like the real output. +# When colour is off, all render as plain text. The theme is shared with `tools/generate_log_svg.py`, which logs its +# sample through `Logger` and `configure_logging`, so the README screenshot renders exactly like the real output. LOG_THEME = Theme({"repr.digest": "dim", "repr.dependency": "bold white"}) -LOG_TIME_FORMAT = "[%X]" -LOG_MESSAGE_FORMAT = "%(message)s" +_LOG_TIME_FORMAT = "[%X]" +_LOG_MESSAGE_FORMAT = "%(message)s" -# This wrapper, and the packages that log on behalf of the updaters (see `attribute_logs_to_caller`). Frames in -# these are skipped when determining a log record's origin, so the reported origin is the updater that triggered the -# log rather than this wrapper or the shared machinery in between. -_wrapper_file = Path(__file__).resolve() -_helper_packages: set[Path] = set() - - -def attribute_logs_to_caller(package_file: str) -> None: - """Register a package whose frames should be skipped when determining a log record's origin. - - A package whose modules log on behalf of the updaters registers itself, passing its `__init__.py`'s `__file__`, - so the frames of every module in it are walked past and a log record is attributed to the updater that triggered - it rather than to the shared machinery in between. Registration covers the whole package, so a module added to it - needs none of its own. - """ - _helper_packages.add(Path(package_file).resolve().parent) - - -def _is_helper_frame(filename: str) -> bool: - """Return whether the frame's file is this wrapper or a module in a registered package.""" - path = Path(filename).resolve() - return path == _wrapper_file or any(path.is_relative_to(package) for package in _helper_packages) - - -def _caller_stacklevel() -> int: - """Return the stacklevel of the first frame outside this wrapper and the registered packages. - - A fixed stacklevel can't work because some log methods are called directly by an updater while others - are dispatched through a registered package (with extra comprehension frames in between), so walk - the stack to find the originating updater frame instead. - """ - level = 1 # Start at the frame that emits the record (Logger._log) and skip helper frames from there. - try: - frame: FrameType | None = sys._getframe(level) # noqa: SLF001 - except ValueError: # pragma: no cover - return level - while frame is not None and _is_helper_frame(frame.f_code.co_filename): - level += 1 - frame = frame.f_back - return level +def configure_logging(console: Console, level: str) -> RichHandler: + """Send every record at the level or above to the console, and return the handler that renders it there.""" + handler = RichHandler(console=console, highlighter=LogHighlighter(), show_path=False) + logging.basicConfig(level=level, datefmt=_LOG_TIME_FORMAT, format=_LOG_MESSAGE_FORMAT, handlers=[handler]) + return handler @dataclass(frozen=True) class LogMessage: - """A log message: the level it is logged at and the format string the arguments are interpolated into. - - A message's level is a property of the message, not of the call that emits it, so the two are declared together - and `Logger._log` emits at the level the message names. The message object itself is logged, which is what the - standard library expects of a message that is not a string: it calls `str()` on it when a handler formats the - record. Passing the object rather than the format string keeps the level available to whoever holds the message, - such as the log tests and the screenshot generator. - """ + """A log message: the level it is logged at and the format string the arguments are interpolated into.""" level: int format: str @@ -190,11 +146,7 @@ def __repr__(self) -> str: def _redundant_marker(reason: str) -> str: - """Return the warning a marker that holds nothing back is reported as, for the reason the caller gives. - - Every scope reports its own reason, and the sentence they share is spelled here, so the redundant markers read - alike whichever check found one inert. - """ + """Return the warning a marker that holds nothing back is reported as, for the reason the caller gives.""" return ( f"Redundant update-time marker %(directive)s for %(dependency)s in %(location)s: {reason}, " "so the marker holds nothing back" @@ -214,20 +166,12 @@ def forget_shown_changelogs(self) -> None: self._logged_changes.clear() def _log(self, message: LogMessage, **fields: object) -> None: - """Emit a log record at the message's own level, attributing it to the updater that triggered it. - - Every message interpolates its arguments by name (`%(location)s`), so the fields are passed as the single - mapping the standard library's `%`-formatting fills them from. - """ - self.log.log(message.level, message, self._rendered(fields), stacklevel=_caller_stacklevel()) + """Emit a log record at the message's own level.""" + self.log.log(message.level, message, self._rendered(fields)) @classmethod def _rendered(cls, fields: dict[str, object]) -> dict[str, object]: - """Return the fields with the ones the highlighter styles wrapped in their delimiter, and the rest as they are. - - Rendering them here rather than in each log method is what keeps a message's arguments plain domain values at - the call site, and leaves one place that decides what the highlighter has to style. - """ + """Return the fields with the ones the highlighter styles wrapped in their delimiter.""" return {name: cls._render_field(name, value) for name, value in fields.items()} @classmethod @@ -887,11 +831,7 @@ def get_logger(name: str) -> Logger: the root logger only the first time — when it has no handlers yet — instead of building a handler on every call. """ if not logging.getLogger().handlers: - console = Console(stderr=True, theme=LOG_THEME) - handler = RichHandler(console=console, highlighter=LogHighlighter()) - logging.basicConfig( - level=LOG_LEVEL.get(), datefmt=LOG_TIME_FORMAT, format=LOG_MESSAGE_FORMAT, handlers=[handler] - ) + configure_logging(Console(stderr=True, theme=LOG_THEME), LOG_LEVEL.get()) logger = Logger(name) _LOGGERS.append(logger) return logger diff --git a/src/update_time/references/__init__.py b/src/update_time/references/__init__.py index c5b1bc0..32e9de3 100644 --- a/src/update_time/references/__init__.py +++ b/src/update_time/references/__init__.py @@ -1,6 +1 @@ """Decide which version each pinned reference should update to, honouring its marker, and rewrite it in place.""" - -from update_time.io.log import attribute_logs_to_caller - -# Every module in this package logs on behalf of the updaters, so records point at the updater. -attribute_logs_to_caller(__file__) diff --git a/src/update_time/references/github.py b/src/update_time/references/github.py index 96a4f8a..d8d203c 100644 --- a/src/update_time/references/github.py +++ b/src/update_time/references/github.py @@ -41,7 +41,7 @@ def _sha_pinned_reference(match: re.Match[str], dependency: str) -> Reference: return Reference(dependency, match.group("version") if current_sha else match.group("tag"), current_sha) -def latest_pin(reference: Reference, marker: Marker, location: Location, log: Logger) -> DependencyVersion | None: +def _latest_pin(reference: Reference, marker: Marker, location: Location, log: Logger) -> DependencyVersion | None: """Return the latest version to (re)pin the GitHub reference to, or None to leave it unchanged. Which version to update to is `latest_version`'s decision, resolving through `sources.github`; layered on top @@ -99,12 +99,12 @@ class PinUpdater: def update_line(self, match: re.Match[str], location: Location, marker: Marker, dependency: str = "") -> str: """Return the line with the reference (re)pinned to the latest version, or unchanged when it stays put. - Unchanged covers each case `latest_pin` declines: an invalid current version, a marker holding the update + Unchanged covers each case `_latest_pin` declines: an invalid current version, a marker holding the update back, no commit SHA to pin to, and a reference already pinned and up to date. The dependency comes from the regexp's `dependency` group; a `rev:` takes it from the `repo:` above, so it names it in `dependency` instead. """ reference = _sha_pinned_reference(match, matched_dependency(match, dependency)) - latest = latest_pin(reference, marker, location, self.logger) + latest = _latest_pin(reference, marker, location, self.logger) if latest is None: return match.string return replace_match(match, self.spell(reference, latest)) diff --git a/tests/update_time/helpers.py b/tests/update_time/helpers.py index 81bdbe2..98dd112 100644 --- a/tests/update_time/helpers.py +++ b/tests/update_time/helpers.py @@ -165,7 +165,7 @@ def _expected_call(message: LogMessage, fields: dict[str, object]) -> _Call: — a `Location` rather than its delimiter-wrapped text — and which fields carry a delimiter is asserted where that rendering itself is tested, in the logger's unit tests. """ - return call(message, Logger._rendered(fields), stacklevel=ANY) + return call(message, Logger._rendered(fields)) def assert_logged(self, message: LogMessage, **fields: object) -> None: """Assert the message was the only record logged at its level, with the given fields.""" diff --git a/tests/update_time/io/test_log.py b/tests/update_time/io/test_log.py index c6fb9bc..7635c5b 100644 --- a/tests/update_time/io/test_log.py +++ b/tests/update_time/io/test_log.py @@ -5,7 +5,6 @@ import re from datetime import UTC, datetime, timedelta, timezone from pathlib import Path -from tempfile import TemporaryDirectory from unittest import TestCase from unittest.mock import ANY, Mock, patch @@ -27,12 +26,9 @@ reset_changelog_suppression, ) from update_time.primitives.location import Location -from update_time.references import file -from update_time.references.github import latest_pin -from update_time.references.resolve import latest_version -from tests.update_time.fixtures import COMMIT_SHA, DIGEST, DIGEST1, DIGEST2 -from tests.update_time.helpers import bound, new_version_getter +from tests.update_time.fixtures import DIGEST, DIGEST1, DIGEST2 +from tests.update_time.helpers import bound class GetLoggerTests(TestCase): @@ -122,10 +118,7 @@ def test_a_location_field_is_wrapped_and_a_plain_field_is_not(self, mock_log: Mo message = LogMessage(logging.INFO, "Skipping %(location)s: %(reason)s") Logger("fields")._log(message, location=_create_location("Dockerfile", 1), reason="it is compiled") mock_log.assert_called_once_with( - message.level, - message, - {"location": _at("Dockerfile:1"), "reason": "it is compiled"}, - stacklevel=ANY, + message.level, message, {"location": _at("Dockerfile:1"), "reason": "it is compiled"} ) @patch("logging.Logger.log") @@ -137,12 +130,7 @@ def test_the_dependency_field_is_wrapped_in_its_delimiter(self, mock_log: Mock): """ message = LogMessage(logging.ERROR, "No valid version found for %(dependency)s") Logger("fields")._log(message, dependency="actions/checkout") - mock_log.assert_called_once_with( - message.level, - message, - {"dependency": dependency("actions/checkout")}, - stacklevel=ANY, - ) + mock_log.assert_called_once_with(message.level, message, {"dependency": dependency("actions/checkout")}) @patch("logging.Logger.log") @@ -159,7 +147,7 @@ def assert_last_message(self, mock_log: Mock, message: LogMessage, rendered: str Records emitted before the most recent one are ignored. """ - mock_log.assert_called_with(message.level, message, ANY, stacklevel=ANY) + mock_log.assert_called_with(message.level, message, ANY) _level, template, fields = mock_log.call_args.args self.assertEqual(sorted(fields), sorted(re.findall(r"%\((\w+)\)", str(template)))) self.assertEqual(str(template) % fields, rendered) @@ -665,60 +653,6 @@ def test_dependency_style_is_bold_white(self): self.assertEqual(str(handler.console.get_style("repr.dependency")), "bold white") -class LogOriginTests(TestCase): - """Tests that log records are attributed to the originating updater, not to the shared machinery in between.""" - - def assert_origin_is_this_test(self, records: list[logging.LogRecord]) -> None: - """Assert that every record names this test file, the caller of the shared machinery, as its origin.""" - self.assertEqual({Path(record.pathname).name for record in records}, {"test_log.py"}) - - def test_direct_call_is_attributed_to_the_caller(self): - """Test that a log method called directly reports the calling line as its origin.""" - logger = Logger("origin direct") - with self.assertLogs(logger.log, level="DEBUG") as captured: - logger.path(Path.cwd()) - self.assert_origin_is_this_test(captured.records) - - def test_file_rewrite_is_attributed_to_the_caller(self): - """Test that logs emitted while rewriting a file report the rewriting's caller as their origin.""" - logger = Logger("origin rewrite") - with TemporaryDirectory() as directory: - (Path(directory) / "config.yml").write_text("dependency: 1.0\n") - with ( - patch("pathlib.Path.cwd", Mock(return_value=Path(directory))), - self.assertLogs(logger.log, level="DEBUG") as captured, - ): - file.update_files( - "*.yml", - regexp=r"(?Pdependency): (?P[\d.]+)", - get_new_version=new_version_getter("2.0"), - logger=logger, - start=Path(directory), - ) - self.assert_origin_is_this_test(captured.records) - - def test_version_decision_is_attributed_to_the_caller(self): - """Test that a warning from the shared version decision reports its caller, not the decision, as origin.""" - logger = Logger("origin decision") - stale = DependencyVersion("2.0", newest_published=datetime.now(UTC) - timedelta(days=1000)) - reference = Reference("dependency", "1.0") - with self.assertLogs(logger.log, level="DEBUG") as captured: - latest_version(reference, lambda *_args: stale, Marker(), Location(Path.cwd()), logger) - self.assert_origin_is_this_test(captured.records) - - def test_github_pin_decision_is_attributed_to_the_caller(self): - """Test that a pin from the shared GitHub decision reports its caller, not the decision, as origin.""" - logger = Logger("origin github") - latest = DependencyVersion("2.0", sha=COMMIT_SHA) - reference = Reference("owner/action", "1.0") - with ( - patch("update_time.references.github.get_latest_version", Mock(return_value=latest)), - self.assertLogs(logger.log, level="DEBUG") as captured, - ): - latest_pin(reference, Marker(), Location(Path.cwd()), logger) - self.assert_origin_is_this_test(captured.records) - - class LoggerMessageTest(TestCase): """Test that Logger's message templates and its log methods pair one-to-one. diff --git a/tools/fixit_rules.py b/tools/fixit_rules.py index 9c5ce14..d209d26 100644 --- a/tools/fixit_rules.py +++ b/tools/fixit_rules.py @@ -1,6 +1,7 @@ """Local fixit rules, enabled via `[tool.fixit]` in pyproject.toml.""" import inspect +import re import unittest import libcst as cst @@ -209,6 +210,128 @@ def visit_For(self, node: cst.For) -> None: self.report(node) +# An identifier, as the code spells a name, and an identifier a docstring quotes between backticks. A backticked +# run holding anything else — a `path:line`, an `# update-time: ignore` marker — is skipped here, and the twin +# check would skip it in any case, since no name the code binds has that shape. +_IDENTIFIER = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") +_BACKTICKED_NAME = re.compile(f"`({_IDENTIFIER.pattern})`") + + +def _twin(name: str) -> str: + """Return the name under the opposite visibility: the private spelling of a public name, and the other way on.""" + return name.removeprefix("_") if name.startswith("_") else f"_{name}" + + +def _quoted_words(module: cst.Module) -> set[str]: + """Return the identifier-shaped words the module's strings spell out, the backticked runs in them left out. + + A word spelled out in a string is data the module works with — a path in a URL, a key, a word of a message — + so a docstring quoting that word quotes the data rather than a name the code binds. + """ + strings = m.findall(module, m.SimpleString() | m.FormattedString()) + sources = (_BACKTICKED_NAME.sub(" ", module.code_for_node(string)) for string in strings) + return {word for source in sources for word in _IDENTIFIER.findall(source)} + + +_QUERY_ENDPOINT = '''\ +def _query(reference): + return {"version": reference} + + +def reported_vulnerabilities(reference): + """Return what the `query` endpoint reports for the reference.""" + return fetch("https://api.osv.dev/v1/query", json=_query(reference)) +''' + +_USED_NAME = '''\ +def _pin(reference): + return reference + + +def pin(references): + """Return each reference pinned by `_pin`.""" + return [_pin(reference) for reference in references] +''' + +_NAME_FROM_ANOTHER_MODULE = '''\ +def changes(dependency): + """Return what `get_changes` reports for the dependency.""" + return dependency +''' + +_RENAMED_PIN = '''\ +def _latest_pin(reference): + return reference + + +class PinUpdater: + def update_line(self, match): + """Return the line, unchanged in each case `latest_pin` declines.""" + return _latest_pin(match) +''' + +_MADE_PUBLIC = '''\ +def helper(value): + return value + + +def caller(value): + """Return the value `_helper` produced.""" + return helper(value) +''' + + +class RenamedNameInDocstring(LintRule): + """Require a name a docstring quotes between backticks to be spelled the way the code spells it. + + Renaming a name leaves the docstrings quoting it untouched, `just rename` by design, so the old spelling + survives there and nothing else reports it. A quoted name the code has only under the opposite visibility is + one of those leftovers. + """ + + VALID = [ + Valid(_QUERY_ENDPOINT), # A word a string spells out is data, whatever the code binds beside it. + Valid(_USED_NAME), # The code mentions the quoted name itself, so no rename moved it. + Valid(_NAME_FROM_ANOTHER_MODULE), # Neither spelling is here, so the name belongs to another module. + ] + INVALID = [ + Invalid(_RENAMED_PIN), # The name was made private, and the docstring kept the public spelling. + Invalid(_MADE_PUBLIC), # The same the other way on: made public, and the docstring kept the private one. + ] + + def __init__(self) -> None: + """Start with no names, until the module they are read from is visited.""" + super().__init__() + self._code_names: set[str] = set() + self._quoted_words: set[str] = set() + + def visit_Module(self, node: cst.Module) -> None: + """Collect what the module names and what it spells out, then check its own docstring against them.""" + self._code_names = {cst.ensure_type(name, cst.Name).value for name in m.findall(node, m.Name())} + self._quoted_words = _quoted_words(node) + self._report_renamed(node) + + def visit_FunctionDef(self, node: cst.FunctionDef) -> None: + """Check the function's docstring here, so a report lands on the function rather than on the module.""" + self._report_renamed(node) + + def visit_ClassDef(self, node: cst.ClassDef) -> None: + """Check the class's docstring here, so a report lands on the class rather than on the module.""" + self._report_renamed(node) + + def _report_renamed(self, node: cst.Module | cst.FunctionDef | cst.ClassDef) -> None: + """Report each name the docstring quotes that the code mentions only under the opposite visibility. + + A word the module's strings spell out is data it works with, whatever the code binds beside it, so a + docstring quoting that word is left alone. + """ + for name in _BACKTICKED_NAME.findall(node.get_docstring(clean=False) or ""): + if name in self._quoted_words: + continue + if name not in self._code_names and _twin(name) in self._code_names: + self.report(node, message=f"Rewrite `{name}` as `{_twin(name)}`: a rename left the docstring behind") + + class SubTestOutsideLoop(LintRule): """Require a `subTest` in an `assert*` helper to sit inside a loop over cases the helper itself owns. diff --git a/tools/generate_log_svg.py b/tools/generate_log_svg.py index e91d0ae..18658d0 100644 --- a/tools/generate_log_svg.py +++ b/tools/generate_log_svg.py @@ -1,7 +1,7 @@ """Render the log-output screenshot (docs/log-output.svg) shown in the README. -A handful of representative log lines are emitted through Update-time's own `Logger` and rendered with its -highlighter and theme, then exported as an SVG, so the README shows exactly how the coloured output looks. +A handful of representative log lines are emitted through Update-time's own `Logger` and rendered by the handler +and theme a run uses, then exported as an SVG, so the README shows exactly how the coloured output looks. """ import io @@ -12,12 +12,11 @@ from pathlib import Path from rich.console import Console -from rich.logging import RichHandler from update_time.domain.staleness import STALE_AFTER from update_time.domain.version import SHA256_HEX_CHARS, DependencyVersion from update_time.domain.vulnerability import Vulnerability -from update_time.io.log import LOG_MESSAGE_FORMAT, LOG_THEME, LOG_TIME_FORMAT, Logger, LogHighlighter +from update_time.io.log import LOG_THEME, Logger, configure_logging from update_time.primitives.location import Location @@ -65,9 +64,7 @@ def generate() -> LogOutput: file=io.StringIO(), # capture the live render; we only want the exported SVG and text, not stdout noise theme=LOG_THEME, ) - handler = RichHandler(console=console, highlighter=LogHighlighter(), show_path=False) - handler.addFilter(_FixedTime()) - logging.basicConfig(level="INFO", datefmt=LOG_TIME_FORMAT, format=LOG_MESSAGE_FORMAT, handlers=[handler]) + configure_logging(console, "INFO").addFilter(_FixedTime()) log = Logger("update-time") # A representative digest, padded to the exact length of a real one so `LogHighlighter` recognises and dims it. diff --git a/tools/vulture-whitelist.py b/tools/vulture-whitelist.py index d4977d4..2609c7b 100644 --- a/tools/vulture-whitelist.py +++ b/tools/vulture-whitelist.py @@ -1,14 +1,17 @@ -database_specific # unused variable (src/update_time/sources/osv.py:34) -upload_time_iso_8601 # unused variable (src/update_time/sources/pypi.py:44) -AssertEqualActualFirst # unused class (tools/fixit_rules.py:65) -VALID # unused variable (tools/fixit_rules.py:77) -INVALID # unused variable (tools/fixit_rules.py:86) -_.leave_FunctionDef # unused method (tools/fixit_rules.py:132) -original_node # unused variable (tools/fixit_rules.py:132) -_.visit_AnnAssign # unused method (tools/fixit_rules.py:141) -SubTestPerCase # unused class (tools/fixit_rules.py:174) -VALID # unused variable (tools/fixit_rules.py:184) -INVALID # unused variable (tools/fixit_rules.py:197) -SubTestOutsideLoop # unused class (tools/fixit_rules.py:212) -VALID # unused variable (tools/fixit_rules.py:222) -INVALID # unused variable (tools/fixit_rules.py:231) +database_specific # unused variable (src/update_time/sources/osv.py:47) +upload_time_iso_8601 # unused variable (src/update_time/sources/pypi.py:84) +AssertEqualActualFirst # unused class (tools/fixit_rules.py:66) +VALID # unused variable (tools/fixit_rules.py:78) +INVALID # unused variable (tools/fixit_rules.py:87) +_.leave_FunctionDef # unused method (tools/fixit_rules.py:133) +original_node # unused variable (tools/fixit_rules.py:133) +_.visit_AnnAssign # unused method (tools/fixit_rules.py:142) +SubTestPerCase # unused class (tools/fixit_rules.py:175) +VALID # unused variable (tools/fixit_rules.py:185) +INVALID # unused variable (tools/fixit_rules.py:198) +RenamedNameInDocstring # unused class (tools/fixit_rules.py:259) +VALID # unused variable (tools/fixit_rules.py:264) +INVALID # unused variable (tools/fixit_rules.py:265) +SubTestOutsideLoop # unused class (tools/fixit_rules.py:301) +VALID # unused variable (tools/fixit_rules.py:311) +INVALID # unused variable (tools/fixit_rules.py:320)