Merge pull request #2023 from vyos/yuriy/doc-linter-cleanup-followups

ci(doc-linter): fix 12 accumulated bugs flagged across PR #2014/#2019/#2020 reviews

1 files changed, 147 insertions, 74 deletions

diff --git a/scripts/doc-linter.py b/scripts/doc-linter.py
index 8e74600f..b95ea505 100644
--- a/scripts/doc-linter.py
+++ b/scripts/doc-linter.py
@@ -2,14 +2,14 @@ import os
 import re
 import ipaddress
 import sys
-import ast
+import json
 
 IPV4SEG  = r'(?:25[0-5]|(?:2[0-4]|1{0,1}[0-9]){0,1}[0-9])'
 IPV4ADDR = r'\b(?:(?:' + IPV4SEG + r'\.){3,3}' + IPV4SEG + r')\b'
 IPV6SEG  = r'(?:(?:[0-9a-fA-F]){1,4})'
 IPV6GROUPS = (
     r'(?:' + IPV6SEG + r':){7,7}' + IPV6SEG,                  # 1:2:3:4:5:6:7:8
-    r'(?:\s' + IPV6SEG + r':){1,7}:',                           # 1::                                 1:2:3:4:5:6:7::
+    r'\b(?:' + IPV6SEG + r':){1,7}:',                           # 1::                                 1:2:3:4:5:6:7::
     r'(?:' + IPV6SEG + r':){1,6}:' + IPV6SEG,                 # 1::8               1:2:3:4:5:6::8   1:2:3:4:5:6::8
     r'(?:' + IPV6SEG + r':){1,5}(?::' + IPV6SEG + r'){1,2}',  # 1::7:8             1:2:3:4:5::7:8   1:2:3:4:5::8
     r'(?:' + IPV6SEG + r':){1,4}(?::' + IPV6SEG + r'){1,3}',  # 1::6:7:8           1:2:3:4::6:7:8   1:2:3:4::8
@@ -23,10 +23,6 @@ IPV6GROUPS = (
 )
 IPV6ADDR = '|'.join(['(?:{})'.format(g) for g in IPV6GROUPS[::-1]])  # Reverse rows for greedy match
 
-MAC = r'([0-9A-F]{2}[:-]){5}([0-9A-F]{2})'
-
-NUMBER = r"([\s']\d+[\s'])"
-
 SUPPORTED_EXTS = ('.md', '.rst', '.txt')
 
 # Linter only applies to published documentation sources under docs/. Repo-root
@@ -34,9 +30,15 @@ SUPPORTED_EXTS = ('.md', '.rst', '.txt')
 # meta, not docs content, and are out of scope.
 DOCS_ROOT = 'docs'
 
+# Subtrees under docs/ that are excluded from the build (per docs/conf.py)
+# and therefore also excluded from lint.
+DOCS_EXCLUDED_SUBDIRS = ('_build', '_rst_legacy')
+
 
 def is_docs_path(path):
-    """Return True iff `path` resolves under the repo's `docs/` tree.
+    """Return True iff `path` resolves under the repo's `docs/` tree AND
+    is not inside one of the build-excluded subtrees (``_build``,
+    ``_rst_legacy``).
 
     Accepts both repo-relative and absolute paths. Both `path` and
     `DOCS_ROOT` are resolved with `os.path.realpath`, so symlinks are
@@ -50,15 +52,63 @@ def is_docs_path(path):
     abs_path = os.path.realpath(path)
     abs_docs = os.path.realpath(DOCS_ROOT)
     try:
-        return os.path.commonpath([abs_path, abs_docs]) == abs_docs
+        if os.path.commonpath([abs_path, abs_docs]) != abs_docs:
+            return False
     except ValueError:
         # commonpath raises on mixed drives (Windows) or empty input.
         return False
+    for excluded in DOCS_EXCLUDED_SUBDIRS:
+        abs_excluded = os.path.realpath(os.path.join(DOCS_ROOT, excluded))
+        try:
+            if os.path.commonpath([abs_path, abs_excluded]) == abs_excluded:
+                return False
+        except ValueError:
+            continue
+    return True
 
 # MyST / Markdown fenced code block: leading whitespace + 3+ backticks or 3+ colons.
 # Same character and length-or-greater closes.
 MD_FENCE_RE = re.compile(r'^(\s*)(`{3,}|:{3,})(.*)$')
 
+# RST `.. code-block::` directive: leading whitespace + literal `.. code-block::`.
+# Anchored to start-of-line so prose mentions like `\`\`.. code-block::\`\`` inside
+# a Markdown paragraph (see docs/documentation.md) don't false-trigger the
+# code-block tracker.
+RST_CODEBLOCK_RE = re.compile(r'^(\s*)\.\.\s+code-block::')
+
+# MyST directive names whose body is code-like — line-length exception applies.
+# Anything else with a `{directive}` info string is treated as prose-bearing
+# (`{note}`, `{warning}`, `{tip}`, `{deprecated}`, …); their content is normal
+# Markdown and gets line-length checked. Plain fences with no info string OR a
+# bare language tag (`python`, `bash`, `yaml`, …) are code blocks per
+# CommonMark and always skip line-length.
+CODE_BEARING_DIRECTIVES = frozenset({
+    'code-block', 'code', 'sourcecode',
+    'cfgcmd', 'opcmd', 'cmdinclude', 'cmdincludemd',
+    'literalinclude', 'parsed-literal', 'raw',
+    'command-output',
+    'eval-rst',  # body is RST; its own line-length is handled via the
+                 # `.. code-block::` tracker for nested code blocks.
+})
+
+
+def _fence_is_code(info):
+    """Return True iff the fence opener `info` string designates a code-like block.
+
+    Used to gate the line-length-check skip — only code-like fences should
+    suppress line-length linting; prose-bearing directive fences like
+    `{note}` / `{warning}` must still have their content checked.
+    """
+    info = info.strip()
+    if not info:
+        return True  # plain ``` is code per CommonMark
+    if not info.startswith('{'):
+        return True  # python, bash, yaml, text, etc.
+    # `{code-block} python` -> `code-block`; `{note}` -> `note`.
+    name = info[1:].split('}', 1)[0].strip()
+    return name in CODE_BEARING_DIRECTIVES
+
+
 SUPPRESSION_MARKER_RE = {
     'rst': {
         'stop': re.compile(r'^\s*\.\.\s+stop_vyoslinter\s*$'),
@@ -92,52 +142,42 @@ def is_suppression_marker(line, kind, in_md_fence, in_rst_codeblock,
     return False
 
 
-def lint_mac(cnt, line):
-    """Flag MAC addresses outside the RFC 7042 documentation range."""
-    mac = re.search(MAC, line, re.I)
-    if mac is not None:
-        mac = mac.group()
-        u_mac = re.search(r'((00)[:-](53)([:-][0-9A-F]{2}){4})', mac, re.I)
-        m_mac = re.search(r'((90)[:-](10)([:-][0-9A-F]{2}){4})', mac, re.I)
-        if u_mac is None and m_mac is None:
-            return (f"Use MAC reserved for Documentation (RFC7042): {mac}", cnt, 'error')
-
-
 def lint_ipv4(cnt, line):
-    """Flag IPv4 addresses outside RFC 5737 / private / multicast ranges."""
-    ip = re.search(IPV4ADDR, line, re.I)
-    if ip is not None:
-        ip = ipaddress.ip_address(ip.group().strip(' '))
+    """Flag IPv4 addresses outside RFC 5737 / private / multicast ranges.
+
+    Iterates over every IPv4 match on the line — a line with an
+    allowed address followed by a real public address must not
+    pass just because the first match is allowed.
+    """
+    for match in re.finditer(IPV4ADDR, line, re.I):
+        ip = ipaddress.ip_address(match.group().strip(' '))
         # https://docs.python.org/3/library/ipaddress.html#ipaddress.IPv4Address.is_private
         if ip.is_private:
-            return None
+            continue
         if ip.is_multicast:
-            return None
+            continue
         if ip.is_global is False:
-            return None
-        return (f"Use IPv4 reserved for Documentation (RFC 5737) or private Space: {ip}", cnt, 'error')
+            continue
+        return (f"Use IPv4 reserved for Documentation (RFC 5737) or private space: {ip}", cnt, 'error')
+    return None
 
 
 def lint_ipv6(cnt, line):
-    """Flag IPv6 addresses outside RFC 3849 / private / multicast ranges."""
-    ip = re.search(IPV6ADDR, line, re.I)
-    if ip is not None:
-        ip = ipaddress.ip_address(ip.group().strip(' '))
+    """Flag IPv6 addresses outside RFC 3849 / private / multicast ranges.
+
+    Iterates over every IPv6 match on the line — same all-matches
+    discipline as `lint_ipv4`.
+    """
+    for match in re.finditer(IPV6ADDR, line, re.I):
+        ip = ipaddress.ip_address(match.group().strip(' '))
         if ip.is_private:
-            return None
+            continue
         if ip.is_multicast:
-            return None
+            continue
         if ip.is_global is False:
-            return None
-        return (f"Use IPv6 reserved for Documentation (RFC 3849) or private Space: {ip}", cnt, 'error')
-
-
-def lint_AS(cnt, line):
-    """Placeholder for future AS-number documentation-range checks (RFC 5398)."""
-    number = re.search(NUMBER, line, re.I)
-    if number:
-        pass
-        # find a way to detect AS numbers
+            continue
+        return (f"Use IPv6 reserved for Documentation (RFC 3849) or private space: {ip}", cnt, 'error')
+    return None
 
 
 def lint_linelen(cnt, line):
@@ -150,10 +190,12 @@ def handle_file_action(filepath):
     """Run all lint checks on one file, respecting fence/code-block and suppression context."""
     errors = []
     file_ext = os.path.splitext(filepath)[1].lower()
-    # Stack of open MD/MyST fences: (char, min_len). Supports nesting like
-    # `:::{note}` containing `::::{code-block}`, where the inner opener does
-    # not close the outer note (CommonMark closing rule: matching closer must
-    # have no info string after the fence chars).
+    # Stack of open MD/MyST fences: (char, min_len, is_code). Supports
+    # nesting like `:::{note}` containing `::::{code-block}`, where the
+    # inner opener does not close the outer note (CommonMark closing rule:
+    # matching closer must have no info string after the fence chars).
+    # `is_code` records whether THAT fence is a code-bearing one — gates the
+    # line-length skip per the innermost (top-of-stack) entry.
     md_fence_stack = []
     md_fence_is_eval_rst = False
     in_rst_codeblock = False
@@ -186,25 +228,40 @@ def handle_file_action(filepath):
                 else:
                     if not md_fence_stack:
                         md_fence_is_eval_rst = info.startswith('{eval-rst}')
-                    md_fence_stack.append((fence_char, fence_len))
+                    md_fence_stack.append((fence_char, fence_len, _fence_is_code(info)))
 
             in_md_fence = bool(md_fence_stack)
+            in_md_code_fence = bool(md_fence_stack) and md_fence_stack[-1][2]
 
             # RST `.. code-block::` tracking (existing semantics for .rst/.txt).
             # Each `.. code-block::` directive resets the tracked indent so a
             # later dedent past that column exits the block — even when the
             # directive itself appears inside an already-open outer block.
-            if in_rst_codeblock:
-                if len(line) > rst_codeblock_indent and not line[rst_codeblock_indent].isspace():
+            #
+            # Exit when the next non-blank line's leading-whitespace count is
+            # <= the directive's column. The body of an RST code-block must
+            # be indented MORE than the directive itself, so leading-ws at or
+            # below `rst_codeblock_indent` signals the block has ended.
+            #
+            # Previously this used `len(line) > rst_codeblock_indent and not
+            # line[rst_codeblock_indent].isspace()`, which silently skipped
+            # the exit check on lines shorter than `rst_codeblock_indent + 1`
+            # chars — e.g., a 3-char dedented line under a directive at
+            # column 4 left the block open, suppressing line-length checks
+            # downstream.
+            if in_rst_codeblock and line.strip():
+                leading = len(line) - len(line.lstrip())
+                if leading <= rst_codeblock_indent:
                     in_rst_codeblock = False
-            if ".. code-block::" in line:
-                in_rst_codeblock = True
-                rst_codeblock_indent = 0
-                for ch in line:
-                    if ch.isspace():
-                        rst_codeblock_indent += 1
-                    else:
-                        break
+            # Only treat `.. code-block::` as a directive opener in RST/TXT
+            # files or inside an `{eval-rst}` MyST fence. In plain Markdown
+            # the same characters can appear as prose (e.g., backtick-quoted
+            # mentions of the directive name) and must not open a block.
+            if file_ext in ('.rst', '.txt') or md_fence_is_eval_rst:
+                rst_open = RST_CODEBLOCK_RE.match(line)
+                if rst_open:
+                    in_rst_codeblock = True
+                    rst_codeblock_indent = len(rst_open.group(1))
 
             if is_suppression_marker(
                 line, 'stop', in_md_fence, in_rst_codeblock,
@@ -220,15 +277,15 @@ def handle_file_action(filepath):
             if not start_vyoslinter:
                 continue
 
-            test_line_length = not (in_md_fence or in_rst_codeblock)
+            # Only code-bearing fences (plain ```python```, `{code-block}`,
+            # `{cfgcmd}`, etc.) skip line-length. Prose-bearing directives
+            # like `{note}` and `{warning}` have their content checked.
+            test_line_length = not (in_md_code_fence or in_rst_codeblock)
 
-            err_mac = lint_mac(cnt, line.strip())
-            # disable mac detection for the moment, too many false positives
-            err_mac = None
             err_ip4 = lint_ipv4(cnt, line.strip())
             err_ip6 = lint_ipv6(cnt, line.strip())
             err_len = lint_linelen(cnt, line) if test_line_length else None
-            for e in (err_mac, err_ip4, err_ip6, err_len):
+            for e in (err_ip4, err_ip6, err_len):
                 if e:
                     errors.append(e)
 
@@ -249,32 +306,48 @@ def handle_file_action(filepath):
 def main():
     """Entry point: lint the changed-file list from argv, or fall back to walking `docs/`."""
     bool_error = True
-    print('start')
     # Only the argv-parsing step is wrapped in try/except. Errors raised by
     # handle_file_action() must propagate so CI failures stay visible instead
     # of silently triggering a full docs/ walk.
-    try:
-        files = ast.literal_eval(sys.argv[1])
-    except (IndexError, SyntaxError, ValueError):
-        # No argv or malformed list -> fall back to walking DOCS_ROOT.
+    #
+    # Accepts one or more positional argv entries, each a JSON array of
+    # paths (e.g., `'["foo.md", "bar.rst"]'`). Arrays are merged and
+    # deduplicated before linting. CI passes `files_modified`,
+    # `files_added`, etc. as separate args — see lint-doc.yml.
+    if len(sys.argv) <= 1:
         files = None
+    else:
+        try:
+            files = []
+            for arg in sys.argv[1:]:
+                if arg.strip():
+                    files.extend(json.loads(arg))
+            files = sorted(set(files))
+        except (json.JSONDecodeError, TypeError):
+            # Malformed input -> fall back to walking DOCS_ROOT.
+            files = None
 
     if files is not None:
         for file in files:
             if (
                 file.endswith(SUPPORTED_EXTS)
-                and "_build" not in file
                 and is_docs_path(file)
             ):
                 if handle_file_action(file) is False:
                     bool_error = False
     else:
-        for root, _dirs, files in os.walk(DOCS_ROOT):
-            path = root.split(os.sep)
+        # In-place dirs prune: skip descending into build-excluded subtrees.
+        # is_docs_path() also rejects paths under those subtrees, so the
+        # prune is an optimization (avoids walking thousands of archived
+        # legacy RST files) and the filter is correctness.
+        for root, dirs, files in os.walk(DOCS_ROOT):
+            dirs[:] = [d for d in dirs if d not in DOCS_EXCLUDED_SUBDIRS]
             for file in files:
-                if file.endswith(SUPPORTED_EXTS) and "_build" not in path:
-                    fpath = '/'.join(path)
-                    filepath = f"{fpath}/{file}"
+                filepath = os.path.join(root, file)
+                if (
+                    file.endswith(SUPPORTED_EXTS)
+                    and is_docs_path(filepath)
+                ):
                     if handle_file_action(filepath) is False:
                         bool_error = False