Implement scoping for productionlist

Fixes sphinx-doc/sphinx#3077

13 files changed, 169 insertions, 15 deletions

diff --git a/CHANGES b/CHANGES
index 1f1f5ebe5..faaef13fd 100644
--- a/CHANGES
+++ b/CHANGES
@@ -19,6 +19,9 @@ Incompatible changes
 * #6830: py domain: ``meta`` fields in info-field-list becomes reserved.  They
   are not displayed on output document now
 * The structure of ``sphinx.events.EventManager.listeners`` has changed
+* Due to the scoping changes for :rst:dir:`productionlist` some uses of
+  :rst:role:`token` must be modified to include the scope which was previously
+  ignored.
 
 Deprecated
 ----------
@@ -39,6 +42,8 @@ Features added
 * #6830: py domain: Add new event: :event:`object-description-transform`
 * Support priority of event handlers. For more detail, see
   :py:meth:`.Sphinx.connect()`
+* #3077: Implement the scoping for :rst:dir:`productionlist` as indicated
+  in the documentation.
 
 Bugs fixed
 ----------
diff --git a/doc/usage/restructuredtext/directives.rst b/doc/usage/restructuredtext/directives.rst
index 92ca3eae2..3e0be1904 100644
--- a/doc/usage/restructuredtext/directives.rst
+++ b/doc/usage/restructuredtext/directives.rst
@@ -1139,7 +1139,7 @@ derived forms), but provides enough to allow context-free grammars to be
 displayed in a way that causes uses of a symbol to be rendered as hyperlinks to
 the definition of the symbol.  There is this directive:
 
-.. rst:directive:: .. productionlist:: [name]
+.. rst:directive:: .. productionlist:: [tokenGroup]
 
    This directive is used to enclose a group of productions.  Each production
    is given on a single line and consists of a name, separated by a colon from
@@ -1147,16 +1147,24 @@ the definition of the symbol.  There is this directive:
    continuation line must begin with a colon placed at the same column as in
    the first line.
 
-   The argument to :rst:dir:`productionlist` serves to distinguish different
-   sets of production lists that belong to different grammars.
+   The ``tokenGroup`` argument to :rst:dir:`productionlist` serves to
+   distinguish different sets of production lists that belong to different
+   grammars.  Multiple production lists with the same ``tokenGroup`` thus
+   define rules in the same scope.
 
    Blank lines are not allowed within ``productionlist`` directive arguments.
 
    The definition can contain token names which are marked as interpreted text
-   (e.g. ``sum ::= `integer` "+" `integer```) -- this generates
+   (e.g. "``sum ::= `integer` "+" `integer```") -- this generates
    cross-references to the productions of these tokens.  Outside of the
-   production list, you can reference to token productions using
+   production list, you can reference token productions using
    :rst:role:`token`.
+   However, if you have given a ``tokenGroup`` argument you must prefix the
+   token name in the cross-reference with the group name and a colon,
+   e.g., "``myTokenGroup:sum``" instead of just "``sum``".
+   If the token group should not be shown in the title of the link either an
+   explicit title can be given (e.g., "``myTitle <myTokenGroup:sum>``"),
+   or the target can be prefixed with a tilde (e.g., "``~myTokenGroup:sum``").
 
    Note that no further reST parsing is done in the production, so that you
    don't have to escape ``*`` or ``|`` characters.
diff --git a/sphinx/domains/std.py b/sphinx/domains/std.py
index 65ebf070e..52d44741d 100644
--- a/sphinx/domains/std.py
+++ b/sphinx/domains/std.py
@@ -406,7 +406,9 @@ class Glossary(SphinxDirective):
         return messages + [node]
 
 
-def token_xrefs(text: str) -> List[Node]:
+def token_xrefs(text: str, productionGroup: str) -> List[Node]:
+    if len(productionGroup) != 0:
+        productionGroup += ':'
     retnodes = []  # type: List[Node]
     pos = 0
     for m in token_re.finditer(text):
@@ -414,7 +416,7 @@ def token_xrefs(text: str) -> List[Node]:
             txt = text[pos:m.start()]
             retnodes.append(nodes.Text(txt, txt))
         refnode = pending_xref(m.group(1), reftype='token', refdomain='std',
-                               reftarget=m.group(1))
+                               reftarget=productionGroup + m.group(1))
         refnode += nodes.literal(m.group(1), m.group(1), classes=['xref'])
         retnodes.append(refnode)
         pos = m.end()
@@ -437,11 +439,11 @@ class ProductionList(SphinxDirective):
     def run(self) -> List[Node]:
         domain = cast(StandardDomain, self.env.get_domain('std'))
         node = addnodes.productionlist()  # type: Element
+        productionGroup = ""
         i = 0
-
         for rule in self.arguments[0].split('\n'):
             if i == 0 and ':' not in rule:
-                # production group
+                productionGroup = rule.strip()
                 continue
             i += 1
             try:
@@ -451,17 +453,38 @@ class ProductionList(SphinxDirective):
             subnode = addnodes.production(rule)
             subnode['tokenname'] = name.strip()
             if subnode['tokenname']:
-                idname = nodes.make_id('grammar-token-%s' % subnode['tokenname'])
+                idname = 'grammar-token2-%s_%s' \
+                         % (nodes.make_id(productionGroup), nodes.make_id(name))
                 if idname not in self.state.document.ids:
                     subnode['ids'].append(idname)
+
+                idnameOld = nodes.make_id('grammar-token-' + name)
+                if idnameOld not in self.state.document.ids:
+                    subnode['ids'].append(idnameOld)
                 self.state.document.note_implicit_target(subnode, subnode)
-                domain.note_object('token', subnode['tokenname'], idname,
+                if len(productionGroup) != 0:
+                    objName = "%s:%s" % (productionGroup, name)
+                else:
+                    objName = name
+                domain.note_object(objtype='token', name=objName, labelid=idname,
                                    location=(self.env.docname, self.lineno))
-            subnode.extend(token_xrefs(tokens))
+            subnode.extend(token_xrefs(tokens, productionGroup))
             node.append(subnode)
         return [node]
 
 
+class TokenXRefRole(XRefRole):
+    def process_link(self, env: "BuildEnvironment", refnode: Element, has_explicit_title: bool,
+                     title: str, target: str) -> Tuple[str, str]:
+        target = target.lstrip('~')  # a title-specific thing
+        if not self.has_explicit_title and title[0] == '~':
+            if ':' in title:
+                _, title = title.split(':')
+            else:
+                title = title[1:]
+        return title, target
+
+
 class StandardDomain(Domain):
     """
     Domain for all objects that don't fit into another domain or are added
@@ -493,7 +516,7 @@ class StandardDomain(Domain):
         'option':  OptionXRefRole(warn_dangling=True),
         'envvar':  EnvVarXRefRole(),
         # links to tokens in grammar productions
-        'token':   XRefRole(),
+        'token':   TokenXRefRole(),
         # links to terms in glossary
         'term':    XRefRole(lowercase=True, innernodeclass=nodes.inline,
                             warn_dangling=True),
diff --git a/tests/roots/test-productionlist/Bare.rst b/tests/roots/test-productionlist/Bare.rst
new file mode 100644
index 000000000..8ea9213f1
--- /dev/null
+++ b/tests/roots/test-productionlist/Bare.rst
@@ -0,0 +1,6 @@
+Bare
+====
+
+.. productionlist::
+	A: `A` | somethingA
+	B: `B` | somethingB
diff --git a/tests/roots/test-productionlist/Dup1.rst b/tests/roots/test-productionlist/Dup1.rst
new file mode 100644
index 000000000..5cd09cb54
--- /dev/null
+++ b/tests/roots/test-productionlist/Dup1.rst
@@ -0,0 +1,5 @@
+Dup1
+====
+
+.. productionlist::
+	Dup: `Dup` | somethingDup
diff --git a/tests/roots/test-productionlist/Dup2.rst b/tests/roots/test-productionlist/Dup2.rst
new file mode 100644
index 000000000..1d663757f
--- /dev/null
+++ b/tests/roots/test-productionlist/Dup2.rst
@@ -0,0 +1,5 @@
+Dup2
+====
+
+.. productionlist::
+	Dup: `Dup` | somethingDup
diff --git a/tests/roots/test-productionlist/P1.rst b/tests/roots/test-productionlist/P1.rst
new file mode 100644
index 000000000..6f9a863a7
--- /dev/null
+++ b/tests/roots/test-productionlist/P1.rst
@@ -0,0 +1,6 @@
+P1
+==
+
+.. productionlist:: P1
+	A: `A` | somethingA
+	B: `B` | somethingB
diff --git a/tests/roots/test-productionlist/P2.rst b/tests/roots/test-productionlist/P2.rst
new file mode 100644
index 000000000..e6c3bc144
--- /dev/null
+++ b/tests/roots/test-productionlist/P2.rst
@@ -0,0 +1,6 @@
+P2
+==
+
+.. productionlist:: P2
+	A: `A` | somethingA
+	B: `B` | somethingB
diff --git a/tests/roots/test-productionlist/conf.py b/tests/roots/test-productionlist/conf.py
new file mode 100644
index 000000000..a45d22e28
--- /dev/null
+++ b/tests/roots/test-productionlist/conf.py
@@ -0,0 +1 @@
+exclude_patterns = ['_build']
diff --git a/tests/roots/test-productionlist/firstLineRule.rst b/tests/roots/test-productionlist/firstLineRule.rst
new file mode 100644
index 000000000..30ea6e005
--- /dev/null
+++ b/tests/roots/test-productionlist/firstLineRule.rst
@@ -0,0 +1,5 @@
+FirstLineRule
+=============
+
+.. productionlist:: FirstLine: something
+	SecondLine: somethingElse
diff --git a/tests/roots/test-productionlist/index.rst b/tests/roots/test-productionlist/index.rst
new file mode 100644
index 000000000..61da59d55
--- /dev/null
+++ b/tests/roots/test-productionlist/index.rst
@@ -0,0 +1,26 @@
+.. toctree::
+
+	P1
+	P2
+	Bare
+	Dup1
+	Dup2
+	firstLineRule
+
+- A: :token:`A`
+- B: :token:`B`
+- P1:A: :token:`P1:A`
+- P1:B: :token:`P1:B`
+- P2:A: :token:`P1:A`
+- P2:B: :token:`P2:B`
+- Explicit title A, plain: :token:`MyTitle <A>`
+- Explicit title A, colon: :token:`My:Title <A>`
+- Explicit title P1:A, plain: :token:`MyTitle <P1:A>`
+- Explicit title P1:A, colon: :token:`My:Title <P1:A>`
+- Tilde A: :token:`~A`.
+- Tilde P1:A: :token:`~P1:A`.
+- Tilde explicit title P1:A: :token:`~MyTitle <P1:A>`
+- Tilde, explicit title P1:A: :token:`MyTitle <~P1:A>`
+- Dup: :token:`Dup`
+- FirstLine: :token:`FirstLine`
+- SecondLine: :token:`SecondLine`
diff --git a/tests/test_build_html.py b/tests/test_build_html.py
index 63a5948af..3350e56c2 100644
--- a/tests/test_build_html.py
+++ b/tests/test_build_html.py
@@ -222,7 +222,7 @@ def test_html4_output(app, status, warning):
          "[@class='reference internal']/code/span[@class='pre']", 'HOME'),
         (".//a[@href='#with']"
          "[@class='reference internal']/code/span[@class='pre']", '^with$'),
-        (".//a[@href='#grammar-token-try-stmt']"
+        (".//a[@href='#grammar-token2-_try-stmt']"
          "[@class='reference internal']/code/span", '^statement$'),
         (".//a[@href='#some-label'][@class='reference internal']/span", '^here$'),
         (".//a[@href='#some-label'][@class='reference internal']/span", '^there$'),
@@ -254,7 +254,7 @@ def test_html4_output(app, status, warning):
         (".//dl/dt[@id='term-boson']", 'boson'),
         # a production list
         (".//pre/strong", 'try_stmt'),
-        (".//pre/a[@href='#grammar-token-try1-stmt']/code/span", 'try1_stmt'),
+        (".//pre/a[@href='#grammar-token2-_try1-stmt']/code/span", 'try1_stmt'),
         # tests for ``only`` directive
         (".//p", 'A global substitution.'),
         (".//p", 'In HTML.'),
diff --git a/tests/test_domain_std.py b/tests/test_domain_std.py
index adde491c4..17057240c 100644
--- a/tests/test_domain_std.py
+++ b/tests/test_domain_std.py
@@ -8,11 +8,15 @@
     :license: BSD, see LICENSE for details.
 """
 
+import pytest
+
 from unittest import mock
 
 from docutils import nodes
 from docutils.nodes import definition, definition_list, definition_list_item, term
 
+from html5lib import HTMLParser
+
 from sphinx import addnodes
 from sphinx.addnodes import (
     desc, desc_addname, desc_content, desc_name, desc_signature, glossary, index
@@ -20,6 +24,7 @@ from sphinx.addnodes import (
 from sphinx.domains.std import StandardDomain
 from sphinx.testing import restructuredtext
 from sphinx.testing.util import assert_node
+from sphinx.util import docutils
 
 
 def test_process_doc_handle_figure_caption():
@@ -312,3 +317,56 @@ def test_multiple_cmdoptions(app):
     assert ('cmd', '--output') in domain.progoptions
     assert domain.progoptions[('cmd', '-o')] == ('index', 'cmdoption-cmd-o')
     assert domain.progoptions[('cmd', '--output')] == ('index', 'cmdoption-cmd-o')
+
+
+@pytest.mark.skipif(docutils.__version_info__ < (0, 13),
+                    reason='docutils-0.13 or above is required')
+@pytest.mark.sphinx(testroot='productionlist')
+def test_productionlist(app, status, warning):
+    app.builder.build_all()
+
+    warnings = warning.getvalue().split("\n");
+    assert len(warnings) == 2
+    assert warnings[-1] == ''
+    assert "Dup2.rst:4: WARNING: duplicate token description of Dup, other instance in Dup1" in warnings[0]
+
+    with (app.outdir / 'index.html').open('rb') as f:
+        etree = HTMLParser(namespaceHTMLElements=False).parse(f)
+    ul = list(etree.iter('ul'))[1]
+    cases = []
+    for li in list(ul):
+        assert len(list(li)) == 1
+        p = list(li)[0]
+        assert p.tag == 'p'
+        text = str(p.text).strip(' :')
+        assert len(list(p)) == 1
+        a = list(p)[0]
+        assert a.tag == 'a'
+        link = a.get('href')
+        assert len(list(a)) == 1
+        code = list(a)[0]
+        assert code.tag == 'code'
+        assert len(list(code)) == 1
+        span = list(code)[0]
+        assert span.tag == 'span'
+        linkText = span.text.strip()
+        cases.append((text, link, linkText))
+    assert cases == [
+        ('A', 'Bare.html#grammar-token2-_a', 'A'),
+        ('B', 'Bare.html#grammar-token2-_b', 'B'),
+        ('P1:A', 'P1.html#grammar-token2-p1_a', 'P1:A'),
+        ('P1:B', 'P1.html#grammar-token2-p1_b', 'P1:B'),
+        ('P2:A', 'P1.html#grammar-token2-p1_a', 'P1:A'),
+        ('P2:B', 'P2.html#grammar-token2-p2_b', 'P2:B'),
+        ('Explicit title A, plain', 'Bare.html#grammar-token2-_a', 'MyTitle'),
+        ('Explicit title A, colon', 'Bare.html#grammar-token2-_a', 'My:Title'),
+        ('Explicit title P1:A, plain', 'P1.html#grammar-token2-p1_a', 'MyTitle'),
+        ('Explicit title P1:A, colon', 'P1.html#grammar-token2-p1_a', 'My:Title'),
+        ('Tilde A', 'Bare.html#grammar-token2-_a', 'A'),
+        ('Tilde P1:A', 'P1.html#grammar-token2-p1_a', 'A'),
+        ('Tilde explicit title P1:A', 'P1.html#grammar-token2-p1_a', '~MyTitle'),
+        ('Tilde, explicit title P1:A', 'P1.html#grammar-token2-p1_a', 'MyTitle'),
+        ('Dup', 'Dup2.html#grammar-token2-_dup', 'Dup'),
+        ('FirstLine', 'firstLineRule.html#grammar-token2-_firstline', 'FirstLine'),
+        ('SecondLine', 'firstLineRule.html#grammar-token2-_secondline', 'SecondLine'),
+    ]