From 379d2b1078e1f72b0336097184b6e7cfbfa40ae6 Mon Sep 17 00:00:00 2001 From: jaspals Date: Mon, 30 Mar 2026 16:29:51 +0000 Subject: [PATCH 1/5] Add non-blocking plugin convention checks pre-commit hook - scripts/plugin_convention_warnings.py: AST checks for CMD_/Field conventions - .pre-commit-config.yaml: local hook (always_run, exits 0) Made-with: Cursor --- .pre-commit-config.yaml | 9 + scripts/plugin_convention_warnings.py | 253 ++++++++++++++++++++++++++ 2 files changed, 262 insertions(+) create mode 100644 scripts/plugin_convention_warnings.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 03f93420..3283d9ff 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,4 +1,13 @@ repos: + - repo: local + hooks: + - id: plugin-convention-warnings + name: plugin convention warnings (non-blocking) + entry: python3 scripts/plugin_convention_warnings.py + language: system + pass_filenames: false + always_run: true + verbose: true - repo: https://github.com/pre-commit/pre-commit-hooks rev: v5.0.0 hooks: diff --git a/scripts/plugin_convention_warnings.py b/scripts/plugin_convention_warnings.py new file mode 100644 index 00000000..33f7d400 --- /dev/null +++ b/scripts/plugin_convention_warnings.py @@ -0,0 +1,253 @@ +#!/usr/bin/env python3 +"""Static checks for nodescraper plugin conventions. + +Emits warnings to stderr for: + - *collector* / *analyzer* modules: string command class attributes must be named + CMD or with the CMD_ prefix (framework attrs like DATA_MODEL are skipped). + - *collector_args.py* / *analyzer_args.py*: each field in *Args classes must use + pydantic Field(...) with a non-empty description= for CLI help and docs. + +Always exits 0 so pre-commit never blocks commits; violations are advisory only. +""" + +from __future__ import annotations + +import ast +import re +import sys +from pathlib import Path + +PLUGIN_ROOT = Path(__file__).resolve().parent.parent / "nodescraper" / "plugins" + +# Class-level names in collectors/analyzers that are not shell-command strings. +_CMD_CHECK_SKIP_NAMES = frozenset( + { + "AMD_SMI_EXE", + "DATA_MODEL", + "SUPPORTED_OS_FAMILY", + "COLLECTOR", + "ANALYZER", + "COLLECTOR_ARGS", + "ANALYZER_ARGS", + "TYPE_CHECKING", + } +) + + +def _is_stringish(expr: ast.expr) -> bool: + if isinstance(expr, ast.Constant) and isinstance(expr.value, str): + return True + if isinstance(expr, ast.JoinedStr): + return True + return False + + +def _stringish_preview(expr: ast.expr) -> str | None: + """Best-effort static string for command-like heuristics (f-strings may be partial).""" + if isinstance(expr, ast.Constant) and isinstance(expr.value, str): + return expr.value + if isinstance(expr, ast.JoinedStr): + parts: list[str] = [] + for elt in expr.values: + if isinstance(elt, ast.Constant) and isinstance(elt.value, str): + parts.append(elt.value) + else: + parts.append("\x00") # dynamic segment + return "".join(parts) if parts else "" + return None + + +def _looks_like_shell_command_literal(s: str) -> bool: + """True if this class-level string is plausibly a shell/CLI invocation (not IDs, tokens, paths).""" + s = s.strip() + if not s: + return False + if re.fullmatch(r"0x[0-9a-fA-F]+", s): + return False + # OS / config tokens such as PRETTY_NAME, VERSION_ID + if re.fullmatch(r"[A-Z][A-Z0-9_]+", s): + return False + # Filenames / simple paths (no shell metacharacters) + if "." in s and not re.search(r"[\s|;&$`]", s): + return False + if re.search(r"[\s|;&$`<>]", s): + return True + # Typical one-word inband commands: uptime, sysctl, dmesg, amd-smi, etc. + if re.fullmatch(r"[a-z][a-z0-9_.-]*", s, flags=re.IGNORECASE): + return True + return False + + +def _base_name(node: ast.expr) -> str | None: + if isinstance(node, ast.Name): + return node.id + if isinstance(node, ast.Subscript): + return _base_name(node.value) + if isinstance(node, ast.Attribute): + return node.attr + return None + + +def _is_collector_or_analyzer_class(cls: ast.ClassDef) -> bool: + return cls.name.endswith("Collector") or cls.name.endswith("Analyzer") + + +def _field_call_name(func: ast.expr) -> bool: + if isinstance(func, ast.Name) and func.id == "Field": + return True + if isinstance(func, ast.Attribute) and func.attr == "Field": + return True + return False + + +def _field_has_nonempty_description(call: ast.Call) -> bool: + for kw in call.keywords: + if kw.arg != "description" or kw.value is None: + continue + v = kw.value + if isinstance(v, ast.Constant) and isinstance(v.value, str) and v.value.strip(): + return True + return False + + +def _check_cmd_prefixes(path: Path, tree: ast.Module) -> list[str]: + msgs: list[str] = [] + for node in tree.body: + # Keeps only classes whose names end with Collector or Analyzer (e.g. ProcessCollector, PcieAnalyzer). + if not isinstance(node, ast.ClassDef) or not _is_collector_or_analyzer_class(node): + continue + for stmt in node.body: + if not isinstance(stmt, ast.Assign) or len(stmt.targets) != 1: + continue + t = stmt.targets[0] + if not isinstance(t, ast.Name): + continue + name = t.id + if name.startswith("_") or name in _CMD_CHECK_SKIP_NAMES: + continue + if not _is_stringish(stmt.value): + continue + preview = _stringish_preview(stmt.value) + if preview is None or not _looks_like_shell_command_literal(preview): + continue + if name == "CMD" or name.startswith("CMD_"): + continue + msgs.append( + f"{path}:{stmt.lineno}: [{node.name}] command-like class attribute {name!r} " + "should be renamed to CMD or to start with CMD_." + ) + return msgs + + +def _is_args_class(cls: ast.ClassDef) -> bool: + if not cls.name.endswith("Args"): + return False + if not cls.bases: + return False + for b in cls.bases: + bn = _base_name(b) + if bn in ("BaseModel", "CollectorArgs", "AnalyzerArgs"): + return True + if bn and bn.endswith("Args"): + return True + return False + + +def _annotation_mentions_classvar(ann: ast.expr | None) -> bool: + if ann is None: + return False + if isinstance(ann, ast.Name) and ann.id == "ClassVar": + return True + if isinstance(ann, ast.Subscript): + return _annotation_mentions_classvar(ann.value) + if isinstance(ann, ast.Attribute) and ann.attr == "ClassVar": + return True + if isinstance(ann, ast.BinOp) and isinstance(ann.op, ast.BitOr): + return _annotation_mentions_classvar(ann.left) or _annotation_mentions_classvar(ann.right) + return False + + +def _check_args_fields(path: Path, tree: ast.Module) -> list[str]: + msgs: list[str] = [] + for node in tree.body: + if not isinstance(node, ast.ClassDef) or not _is_args_class(node): + continue + for stmt in node.body: + if isinstance(stmt, ast.AnnAssign): + if _annotation_mentions_classvar(stmt.annotation): + continue + if not isinstance(stmt.target, ast.Name): + continue + field_name = stmt.target.id + if field_name.startswith("_") or field_name in ("model_config",): + continue + if stmt.value is None: + msgs.append( + f"{path}:{stmt.lineno}: [{node.name}] {field_name}: " + "use Field(..., description='...') for every Args field." + ) + continue + if isinstance(stmt.value, ast.Call) and _field_call_name(stmt.value.func): + if not _field_has_nonempty_description(stmt.value): + msgs.append( + f"{path}:{stmt.lineno}: [{node.name}] {field_name}: " + "Field(...) must include a non-empty description= for help text." + ) + else: + msgs.append( + f"{path}:{stmt.lineno}: [{node.name}] {field_name}: " + "must assign pydantic Field(...) with description=." + ) + elif isinstance(stmt, ast.Assign) and len(stmt.targets) == 1: + t = stmt.targets[0] + if not isinstance(t, ast.Name): + continue + field_name = t.id + if field_name.startswith("_") or field_name in ("model_config",): + continue + val = stmt.value + if isinstance(val, ast.Call) and _field_call_name(val.func): + if not _field_has_nonempty_description(val): + msgs.append( + f"{path}:{stmt.lineno}: [{node.name}] {field_name}: " + "Field(...) must include a non-empty description= for help text." + ) + return msgs + + +def main() -> None: + if not PLUGIN_ROOT.is_dir(): + sys.stderr.write(f"warning: plugins directory not found: {PLUGIN_ROOT}\n") + return + + all_msgs: list[str] = [] + for path in sorted(PLUGIN_ROOT.rglob("*.py")): + rel = path.relative_to(PLUGIN_ROOT.parent.parent) + name = path.name + try: + src = path.read_text(encoding="utf-8") + tree = ast.parse(src, filename=str(path)) + except (OSError, SyntaxError) as e: + all_msgs.append(f"{rel}: could not parse: {e}") + continue + + if "collector" in name and name.endswith(".py"): + all_msgs.extend(_check_cmd_prefixes(rel, tree)) + if "analyzer" in name and name.endswith(".py"): + all_msgs.extend(_check_cmd_prefixes(rel, tree)) + + if name == "collector_args.py" or name == "analyzer_args.py": + all_msgs.extend(_check_args_fields(rel, tree)) + + if all_msgs: + sys.stderr.write("plugin convention warnings (commit not blocked):\n") + for m in all_msgs: + sys.stderr.write(f" WARNING: {m}\n") + else: + # Match the style of hooks like mypy ("Success: no issues found") for clean runs. + sys.stdout.write("Success: no plugin convention warnings (commit not blocked).\n") + sys.exit(0) + + +if __name__ == "__main__": + main() From ef5a6afbe2a60da79de98c01e8b2a76d206a2c11 Mon Sep 17 00:00:00 2001 From: jaspals Date: Mon, 6 Apr 2026 16:31:52 +0000 Subject: [PATCH 2/5] moved plugin convention script --- .../scripts}/plugin_convention_warnings.py | 16 +++------------- .pre-commit-config.yaml | 2 +- 2 files changed, 4 insertions(+), 14 deletions(-) rename {scripts => .github/scripts}/plugin_convention_warnings.py (93%) mode change 100644 => 100755 diff --git a/scripts/plugin_convention_warnings.py b/.github/scripts/plugin_convention_warnings.py old mode 100644 new mode 100755 similarity index 93% rename from scripts/plugin_convention_warnings.py rename to .github/scripts/plugin_convention_warnings.py index 33f7d400..f2d83793 --- a/scripts/plugin_convention_warnings.py +++ b/.github/scripts/plugin_convention_warnings.py @@ -1,15 +1,4 @@ #!/usr/bin/env python3 -"""Static checks for nodescraper plugin conventions. - -Emits warnings to stderr for: - - *collector* / *analyzer* modules: string command class attributes must be named - CMD or with the CMD_ prefix (framework attrs like DATA_MODEL are skipped). - - *collector_args.py* / *analyzer_args.py*: each field in *Args classes must use - pydantic Field(...) with a non-empty description= for CLI help and docs. - -Always exits 0 so pre-commit never blocks commits; violations are advisory only. -""" - from __future__ import annotations import ast @@ -17,7 +6,8 @@ import sys from pathlib import Path -PLUGIN_ROOT = Path(__file__).resolve().parent.parent / "nodescraper" / "plugins" +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +PLUGIN_ROOT = _REPO_ROOT / "nodescraper" / "plugins" # Class-level names in collectors/analyzers that are not shell-command strings. _CMD_CHECK_SKIP_NAMES = frozenset( @@ -222,7 +212,7 @@ def main() -> None: all_msgs: list[str] = [] for path in sorted(PLUGIN_ROOT.rglob("*.py")): - rel = path.relative_to(PLUGIN_ROOT.parent.parent) + rel = path.relative_to(_REPO_ROOT) name = path.name try: src = path.read_text(encoding="utf-8") diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3283d9ff..85a64e4f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,7 +3,7 @@ repos: hooks: - id: plugin-convention-warnings name: plugin convention warnings (non-blocking) - entry: python3 scripts/plugin_convention_warnings.py + entry: python3 .github/scripts/plugin_convention_warnings.py language: system pass_filenames: false always_run: true From f74981b8f9feddcf5368c5271a9ee821fc68c70e Mon Sep 17 00:00:00 2001 From: jaspals Date: Mon, 6 Apr 2026 16:32:20 +0000 Subject: [PATCH 3/5] moved plugin convention script --- .github/scripts/plugin_convention_warnings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/plugin_convention_warnings.py b/.github/scripts/plugin_convention_warnings.py index f2d83793..1d94d828 100755 --- a/.github/scripts/plugin_convention_warnings.py +++ b/.github/scripts/plugin_convention_warnings.py @@ -235,7 +235,7 @@ def main() -> None: sys.stderr.write(f" WARNING: {m}\n") else: # Match the style of hooks like mypy ("Success: no issues found") for clean runs. - sys.stdout.write("Success: no plugin convention warnings (commit not blocked).\n") + sys.stdout.write("Success: no plugin convention warnings.\n") sys.exit(0) From b0cf67bba7501c15675f57f3376751fb5bf4fb61 Mon Sep 17 00:00:00 2001 From: jaspals Date: Mon, 6 Apr 2026 16:37:46 +0000 Subject: [PATCH 4/5] removed comment --- .github/scripts/plugin_convention_warnings.py | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/scripts/plugin_convention_warnings.py b/.github/scripts/plugin_convention_warnings.py index 1d94d828..4b5d3a5a 100755 --- a/.github/scripts/plugin_convention_warnings.py +++ b/.github/scripts/plugin_convention_warnings.py @@ -234,7 +234,6 @@ def main() -> None: for m in all_msgs: sys.stderr.write(f" WARNING: {m}\n") else: - # Match the style of hooks like mypy ("Success: no issues found") for clean runs. sys.stdout.write("Success: no plugin convention warnings.\n") sys.exit(0) From 8b04125bc17de6613e50739359ecf93d8c7c3182 Mon Sep 17 00:00:00 2001 From: jaspals Date: Tue, 7 Apr 2026 21:32:33 +0000 Subject: [PATCH 5/5] added doc string --- .github/scripts/plugin_convention_warnings.py | 19 +++++++++++++++++++ CONTRIBUTING.md | 11 +++++++++++ 2 files changed, 30 insertions(+) diff --git a/.github/scripts/plugin_convention_warnings.py b/.github/scripts/plugin_convention_warnings.py index 4b5d3a5a..caaf03f6 100755 --- a/.github/scripts/plugin_convention_warnings.py +++ b/.github/scripts/plugin_convention_warnings.py @@ -1,4 +1,21 @@ #!/usr/bin/env python3 +"""Checks conventions under ``nodescraper/plugins`` (stderr warnings only; non-blocking). + +1. **Command strings in collectors/analyzers** , for ``Collector`` + or ``Analyzer`` classes: a *class-level* assignment to a string (or f-string) that + looks like a shell/CLI invocation must use the name ``CMD`` or + ``CMD_`` (e.g. ``CMD_LIST``). Names starting with ``_`` and names + listed in ``_CMD_CHECK_SKIP_NAMES`` are ignored; see + ``_looks_like_shell_command_literal`` for what counts as command-like. + +2. **Args models** — In ``collector_args.py`` and ``analyzer_args.py``, + for classes named ``*Args`` that subclass ``BaseModel``, ``CollectorArgs``, + ``AnalyzerArgs``, or another ``*Args``: each public field should assign + ``pydantic.Field(...)`` with a non-empty ``description=`` (for help/CLI + text). ``ClassVar`` fields, ``_``-prefixed names, and ``model_config`` are + skipped. +""" + from __future__ import annotations import ast @@ -101,6 +118,7 @@ def _field_has_nonempty_description(call: ast.Call) -> bool: def _check_cmd_prefixes(path: Path, tree: ast.Module) -> list[str]: + """Rule #1: warn when a command-like class attr is not ``CMD`` / ``CMD_*``.""" msgs: list[str] = [] for node in tree.body: # Keeps only classes whose names end with Collector or Analyzer (e.g. ProcessCollector, PcieAnalyzer). @@ -158,6 +176,7 @@ def _annotation_mentions_classvar(ann: ast.expr | None) -> bool: def _check_args_fields(path: Path, tree: ast.Module) -> list[str]: + """Rule #2: warn when Args fields lack ``Field`` with a non-empty ``description``.""" msgs: list[str] = [] for node in tree.body: if not isinstance(node, ast.ClassDef) or not _is_args_class(node): diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index de74bbae..da2ac19d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -90,3 +90,14 @@ You can also run hooks manually: # Run all hooks on all files pre-commit run --all-files ``` + +### Plugin conventions + +We follow a few plugin design conventions so that +generation and downstream doc tooling run cleanly—for example, naming +command strings on `*Collector` / `*Analyzer` classes as `CMD` or `CMD_*`, and +using `pydantic.Field(..., description=...)` on args models. The +`plugin-convention-warnings` hook in pre-commit runs +[`.github/scripts/plugin_convention_warnings.py`](.github/scripts/plugin_convention_warnings.py) +to flag violations (warnings only); read the script’s module docstring for the +exact rules.