tooling: Fixes/cleanups/tests for rst_check.py

docs/root/version_history/current.rst

@@ -39,7 +39,7 @@ Bug Fixes
 *Changes expected to improve the state of the world and are unlikely to have negative effects*
 
 * access log: fix ``%UPSTREAM_CLUSTER%`` when used in http upstream access logs. Previously, it was always logging as an unset value.
-* access log: fix `%UPSTREAM_CLUSTER%` when used in http upstream access logs. Previously, it was always logging as an unset value.
+* access log: fix ``%UPSTREAM_CLUSTER%`` when used in http upstream access logs. Previously, it was always logging as an unset value.
 * aws request signer: fix the AWS Request Signer extension to correctly normalize the path and query string to be signed according to AWS' guidelines, so that the hash on the server side matches. See `AWS SigV4 documentaion <https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html>`_.
 * cluster: delete pools when they're idle to fix unbounded memory use when using PROXY protocol upstream with tcp_proxy. This behavior can be temporarily reverted by setting the ``envoy.reloadable_features.conn_pool_delete_when_idle`` runtime guard to false.
 * xray: fix the AWS X-Ray tracer bug where span's error, fault and throttle information was not reported properly as per the `AWS X-Ray documentation <https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html>`_. Before this fix, server error was reported under 'annotations' section of the segment data.
@@ -59,7 +59,7 @@ New Features
 * http: added :ref:`string_match <envoy_v3_api_field_config.route.v3.HeaderMatcher.string_match>` in the header matcher.
 * http: added support for :ref:`max_requests_per_connection <envoy_v3_api_field_config.core.v3.HttpProtocolOptions.max_requests_per_connection>` for both upstream and downstream connections.
 * jwt_authn: added support for :ref:`Jwt Cache <envoy_v3_api_field_extensions.filters.http.jwt_authn.v3.JwtProvider.jwt_cache_config>` and its size can be specified by :ref:`jwt_cache_size <envoy_v3_api_field_extensions.filters.http.jwt_authn.v3.JwtCacheConfig.jwt_cache_size>`.
-* listener: new listener metric `downstream_cx_transport_socket_connect_timeout` to track transport socket timeouts.
+* listener: new listener metric ``downstream_cx_transport_socket_connect_timeout`` to track transport socket timeouts.
 * rbac: added :ref:`destination_port_range <envoy_v3_api_field_config.rbac.v3.Permission.destination_port_range>` for matching range of destination ports.
 
 Deprecated
@@ -71,4 +71,3 @@ Deprecated
 * listener: :ref:`reuse_port <envoy_v3_api_field_config.listener.v3.Listener.reuse_port>` has been
   deprecated in favor of :ref:`enable_reuse_port <envoy_v3_api_field_config.listener.v3.Listener.enable_reuse_port>`.
   At the same time, the default has been changed from false to true. See above for more information.
-

tools/docs/rst_check.py

@@ -1,23 +1,30 @@
+import pathlib
 import re
 import sys
-from typing import Iterator
+from functools import cached_property
+from typing import Iterator, List, Pattern
 
 from tools.base import checker
 
-INVALID_REFLINK = re.compile(r".* ref:.*")
-REF_WITH_PUNCTUATION_REGEX = re.compile(r".*\. <[^<]*>`\s*")
-RELOADABLE_FLAG_REGEX = re.compile(r".*(...)(envoy.reloadable_features.[^ ]*)\s.*")
-VERSION_HISTORY_NEW_LINE_REGEX = re.compile(r"\* ([a-z \-_]+): ([a-z:`]+)")
-VERSION_HISTORY_SECTION_NAME = re.compile(r"^[A-Z][A-Za-z ]*$")
+INVALID_REFLINK = r".* ref:.*"
+REF_WITH_PUNCTUATION_REGEX = r".*\. <[^<]*>`\s*"
+VERSION_HISTORY_NEW_LINE_REGEX = r"\* ([a-z \-_]+): ([a-z:`]+)"
+VERSION_HISTORY_SECTION_NAME = r"^[A-Z][A-Za-z ]*$"
 # Make sure backticks come in pairs.
 # Exceptions: reflinks (ref:`` where the backtick won't be preceded by a space
 #             links `title <link>`_ where the _ is checked for in the regex.
 BAD_TICKS_REGEX = re.compile(r".* `[^`].*`[^_]")
 
+# TODO(phlax):
+#   - generalize these checks to all rst files
+#   - improve checks/handling of "default role"/inline literals
+#       (perhaps using a sphinx plugin)
+#   - add rstcheck and/or rstlint
 
-class CurrentVersionFile(object):
 
-    def __init__(self, path):
+class CurrentVersionFile:
+
+    def __init__(self, path: pathlib.Path):
         self._path = path
 
     @property
@@ -26,8 +33,20 @@ def lines(self) -> Iterator[str]:
             for line in f.readlines():
                 yield line.strip()
 
+    @cached_property
+    def backticks_re(self) -> Pattern[str]:
+        return re.compile(BAD_TICKS_REGEX)
+
+    @cached_property
+    def invalid_reflink_re(self) -> Pattern[str]:
+        return re.compile(INVALID_REFLINK)
+
+    @cached_property
+    def new_line_re(self) -> Pattern[str]:
+        return re.compile(VERSION_HISTORY_NEW_LINE_REGEX)
+
     @property
-    def path(self) -> str:
+    def path(self) -> pathlib.Path:
         return self._path
 
     @property
@@ -37,25 +56,18 @@ def prior_endswith_period(self) -> bool:
             # Don't punctuation-check empty lines.
             or not self.prior_line
             # The text in the :ref ends with a .
-            or
-            (self.prior_line.endswith('`') and REF_WITH_PUNCTUATION_REGEX.match(self.prior_line)))
-
-    def check_flags(self, line: str) -> list:
-        # TODO(phlax): improve checking of inline literals
-        # make sure flags are surrounded by ``s (ie "inline literal")
-        flag_match = RELOADABLE_FLAG_REGEX.match(line)
-        return ([f"Flag {flag_match.groups()[1]} should be enclosed in double back ticks"]
-                if flag_match and not flag_match.groups()[0].startswith(' ``') else [])
-
-    def check_ticks(self, line: str) -> list:
-        ticks_match = BAD_TICKS_REGEX.match(line)
-        return ([f"Backticks should come in pairs (except for links and reflinks): {line}"]
-                if ticks_match else [])
-
-    def check_line(self, line: str) -> list:
-        errors = self.check_reflink(line) + self.check_flags(line)
-        if RELOADABLE_FLAG_REGEX.match(line):
-            errors += self.check_ticks(line)
+            or (self.prior_line.endswith('`') and self.punctuation_re.match(self.prior_line)))
+
+    @cached_property
+    def punctuation_re(self) -> Pattern[str]:
+        return re.compile(REF_WITH_PUNCTUATION_REGEX)
+
+    @cached_property
+    def section_name_re(self) -> Pattern[str]:
+        return re.compile(VERSION_HISTORY_SECTION_NAME)
+
+    def check_line(self, line: str) -> List[str]:
+        errors = self.check_reflink(line) + self.check_ticks(line)
         if line.startswith("* "):
             errors += self.check_list_item(line)
         elif not line:
@@ -66,16 +78,16 @@ def check_line(self, line: str) -> list:
             self.prior_line += line
         return errors
 
-    def check_list_item(self, line: str) -> list:
+    def check_list_item(self, line: str) -> List[str]:
         errors = []
         if not self.prior_endswith_period:
             errors.append(f"The following release note does not end with a '.'\n {self.prior_line}")
 
-        match = VERSION_HISTORY_NEW_LINE_REGEX.match(line)
+        match = self.new_line_re.match(line)
         if not match:
             return errors + [
                 "Version history line malformed. "
-                f"Does not match VERSION_HISTORY_NEW_LINE_REGEX in docs_check.py\n {line}\n"
+                f"Does not match VERSION_HISTORY_NEW_LINE_REGEX\n {line}\n"
                 "Please use messages in the form 'category: feature explanation.', "
                 "starting with a lower-cased letter and ending with a period."
             ]
@@ -97,19 +109,22 @@ def check_list_item(self, line: str) -> list:
         self.set_tokens(line, first_word, next_word)
         return errors
 
-    def check_previous_period(self) -> list:
+    def check_previous_period(self) -> List[str]:
         return ([f"The following release note does not end with a '.'\n {self.prior_line}"]
                 if not self.prior_endswith_period else [])
 
-    def check_reflink(self, line: str) -> list:
-        # TODO(phlax): Check reflinks for all rst files
+    def check_reflink(self, line: str) -> List[str]:
         return ([f"Found text \" ref:\". This should probably be \" :ref:\"\n{line}"]
-                if INVALID_REFLINK.match(line) else [])
+                if self.invalid_reflink_re.match(line) else [])
+
+    def check_ticks(self, line: str) -> List[str]:
+        return ([f"Backticks should come in pairs (except for links and refs): {line}"] if
+                (self.backticks_re.match(line)) else [])
 
     def run_checks(self) -> Iterator[str]:
         self.set_tokens()
         for line_number, line in enumerate(self.lines):
-            if VERSION_HISTORY_SECTION_NAME.match(line):
+            if self.section_name_re.match(line):
                 if line == "Deprecated":
                     break
                 self.set_tokens()
@@ -125,13 +140,14 @@ def set_tokens(self, line: str = "", first_word: str = "", next_word: str = "")
 class RSTChecker(checker.Checker):
     checks = ("current_version",)
 
-    def check_current_version(self):
-        errors = list(CurrentVersionFile("docs/root/version_history/current.rst").run_checks())
+    def check_current_version(self) -> None:
+        errors = list(
+            CurrentVersionFile(pathlib.Path("docs/root/version_history/current.rst")).run_checks())
         if errors:
             self.error("current_version", errors)
 
 
-def main(*args) -> int:
+def main(*args: str) -> int:
     return RSTChecker(*args).run()