Simplify Tags (#12490)

- Use a set to store the list of tags
- Cache evaluated expressions
- Extract locally-defined function into a private method

doc/usage/configuration.rst

@@ -51,9 +51,9 @@ Important points to note:
 
 * There is a special object named ``tags`` available in the config file.
   It can be used to query and change the tags (see :ref:`tags`).  Use
-  ``tags.has('tag')`` to query, ``tags.add('tag')`` and ``tags.remove('tag')``
+  ``'tag' in tags`` to query, ``tags.add('tag')`` and ``tags.remove('tag')``
   to change. Only tags set via the ``-t`` command-line option or via
-  ``tags.add('tag')`` can be queried using ``tags.has('tag')``.
+  ``tags.add('tag')`` can be queried using ``'tag' in tags``.
   Note that the current builder tag is not available in ``conf.py``, as it is
   created *after* the builder is initialized.
 

sphinx/application.py

@@ -140,7 +140,7 @@ def __init__(self, srcdir: str | os.PathLike[str], confdir: str | os.PathLike[st
                  buildername: str, confoverrides: dict | None = None,
                  status: IO | None = sys.stdout, warning: IO | None = sys.stderr,
                  freshenv: bool = False, warningiserror: bool = False,
-                 tags: list[str] | None = None,
+                 tags: Sequence[str] = (),
                  verbosity: int = 0, parallel: int = 0, keep_going: bool = False,
                  pdb: bool = False) -> None:
         """Initialize the Sphinx application.

sphinx/builders/__init__.py

@@ -92,8 +92,8 @@ def __init__(self, app: Sphinx, env: BuildEnvironment) -> None:
         self.tags: Tags = app.tags
         self.tags.add(self.format)
         self.tags.add(self.name)
-        self.tags.add("format_%s" % self.format)
-        self.tags.add("builder_%s" % self.name)
+        self.tags.add(f'format_{self.format}')
+        self.tags.add(f'builder_{self.name}')
 
         # images that need to be copied over (source -> dest)
         self.images: dict[str, str] = {}

sphinx/builders/gettext.py

@@ -119,8 +119,8 @@ def _relpath(s: str) -> str:
 class I18nTags(Tags):
     """Dummy tags module for I18nBuilder.
 
-    To translate all text inside of only nodes, this class
-    always returns True value even if no tags are defined.
+    To ensure that all text inside ``only`` nodes is translated,
+    this class always returns ``True`` regardless the defined tags.
     """
 
     def eval_condition(self, condition: Any) -> bool:

sphinx/testing/util.py

@@ -21,7 +21,7 @@
 from sphinx.util.docutils import additional_nodes
 
 if TYPE_CHECKING:
-    from collections.abc import Mapping
+    from collections.abc import Mapping, Sequence
     from pathlib import Path
     from typing import Any
     from xml.etree.ElementTree import ElementTree
@@ -112,7 +112,7 @@ def __init__(
         confoverrides: dict[str, Any] | None = None,
         status: StringIO | None = None,
         warning: StringIO | None = None,
-        tags: list[str] | None = None,
+        tags: Sequence[str] = (),
         docutils_conf: str | None = None,  # extra constructor argument
         parallel: int = 0,
         # additional arguments at the end to keep the signature

sphinx/util/tags.py

@@ -1,36 +1,36 @@
 from __future__ import annotations
 
+import warnings
 from typing import TYPE_CHECKING
 
-from jinja2 import nodes
-from jinja2.environment import Environment
-from jinja2.parser import Parser
+import jinja2.environment
+import jinja2.nodes
+import jinja2.parser
 
-if TYPE_CHECKING:
-    from collections.abc import Iterator
-
-    from jinja2.nodes import Node
+from sphinx.deprecation import RemovedInSphinx90Warning
 
+if TYPE_CHECKING:
+    from collections.abc import Iterator, Sequence
+    from typing import Literal
 
-env = Environment()
+_ENV = jinja2.environment.Environment()
 
 
-class BooleanParser(Parser):
-    """
-    Only allow condition exprs and/or/not operations.
-    """
+class BooleanParser(jinja2.parser.Parser):
+    """Only allow conditional expressions and binary operators."""
 
-    def parse_compare(self) -> nodes.Expr:
-        node: nodes.Expr
+    def parse_compare(self) -> jinja2.nodes.Expr:
+        node: jinja2.nodes.Expr
         token = self.stream.current
         if token.type == 'name':
-            if token.value in ('true', 'false', 'True', 'False'):
-                node = nodes.Const(token.value in ('true', 'True'),
-                                   lineno=token.lineno)
-            elif token.value in ('none', 'None'):
-                node = nodes.Const(None, lineno=token.lineno)
+            if token.value in {'true', 'True'}:
+                node = jinja2.nodes.Const(True, lineno=token.lineno)
+            elif token.value in {'false', 'False'}:
+                node = jinja2.nodes.Const(False, lineno=token.lineno)
+            elif token.value in {'none', 'None'}:
+                node = jinja2.nodes.Const(None, lineno=token.lineno)
             else:
-                node = nodes.Name(token.value, 'load', lineno=token.lineno)
+                node = jinja2.nodes.Name(token.value, 'load', lineno=token.lineno)
             next(self.stream)
         elif token.type == 'lparen':
             next(self.stream)
@@ -42,47 +42,71 @@ def parse_compare(self) -> nodes.Expr:
 
 
 class Tags:
-    def __init__(self, tags: list[str] | None = None) -> None:
-        self.tags = dict.fromkeys(tags or [], True)
+    def __init__(self, tags: Sequence[str] = ()) -> None:
+        self._tags = set(tags or ())
+        self._condition_cache: dict[str, bool] = {}
 
-    def has(self, tag: str) -> bool:
-        return tag in self.tags
+    def __str__(self) -> str:
+        return f'{self.__class__.__name__}({", ".join(sorted(self._tags))})'
 
-    __contains__ = has
+    def __repr__(self) -> str:
+        return f'{self.__class__.__name__}({tuple(sorted(self._tags))})'
 
     def __iter__(self) -> Iterator[str]:
-        return iter(self.tags)
+        return iter(self._tags)
+
+    def __contains__(self, tag: str) -> bool:
+        return tag in self._tags
+
+    def has(self, tag: str) -> bool:
+        return tag in self._tags
 
     def add(self, tag: str) -> None:
-        self.tags[tag] = True
+        self._tags.add(tag)
 
     def remove(self, tag: str) -> None:
-        self.tags.pop(tag, None)
+        self._tags.discard(tag)
+
+    @property
+    def tags(self) -> dict[str, Literal[True]]:
+        warnings.warn('Tags.tags is deprecated, use methods on Tags.',
+                      RemovedInSphinx90Warning, stacklevel=2)
+        return dict.fromkeys(self._tags, True)
 
     def eval_condition(self, condition: str) -> bool:
+        """Evaluate a boolean condition.
+
+        Only conditional expressions and binary operators (and, or, not)
+        are permitted, and operate on tag names, where truthy values mean
+        the tag is present and vice versa.
+        """
+        if condition in self._condition_cache:
+            return self._condition_cache[condition]
+
         # exceptions are handled by the caller
-        parser = BooleanParser(env, condition, state='variable')
+        parser = BooleanParser(_ENV, condition, state='variable')
         expr = parser.parse_expression()
         if not parser.stream.eos:
             msg = 'chunk after expression'
             raise ValueError(msg)
 
-        def eval_node(node: Node | None) -> bool:
-            if isinstance(node, nodes.CondExpr):
-                if eval_node(node.test):
-                    return eval_node(node.expr1)
-                else:
-                    return eval_node(node.expr2)
-            elif isinstance(node, nodes.And):
-                return eval_node(node.left) and eval_node(node.right)
-            elif isinstance(node, nodes.Or):
-                return eval_node(node.left) or eval_node(node.right)
-            elif isinstance(node, nodes.Not):
-                return not eval_node(node.node)
-            elif isinstance(node, nodes.Name):
-                return self.tags.get(node.name, False)
-            else:
-                msg = 'invalid node, check parsing'
-                raise ValueError(msg)
+        evaluated = self._condition_cache[condition] = self._eval_node(expr)
+        return evaluated
 
-        return eval_node(expr)
+    def _eval_node(self, node: jinja2.nodes.Node | None) -> bool:
+        if isinstance(node, jinja2.nodes.CondExpr):
+            if self._eval_node(node.test):
+                return self._eval_node(node.expr1)
+            else:
+                return self._eval_node(node.expr2)
+        elif isinstance(node, jinja2.nodes.And):
+            return self._eval_node(node.left) and self._eval_node(node.right)
+        elif isinstance(node, jinja2.nodes.Or):
+            return self._eval_node(node.left) or self._eval_node(node.right)
+        elif isinstance(node, jinja2.nodes.Not):
+            return not self._eval_node(node.node)
+        elif isinstance(node, jinja2.nodes.Name):
+            return node.name in self._tags
+        else:
+            msg = 'invalid node, check parsing'
+            raise ValueError(msg)