DOP-1148: Improve string-handling (#172)

* Import Protocol from typing_extensions to fix py3.7

* Bump to v0.4.11

* DOP-1148: Improve string handling

* DOP-1148: Update Changelog

* DOP-1148: Add writer feedback

* DOP-1148: Reconfigure string-handling failures

* DOP-1148: Remove uncallable diagnostics, add parser test

* DOP-1148: Correct formatting errors

Co-authored-by: Andrew Aldridge <[email protected]>

Author: mango-db

CHANGELOG.md

@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Support additional ordered list labels (DOP-1129)
 - Support ordered list "start" attribute (DOP-1210)
 
+### Changed
+
+- Improve string-handling in spec-parser (DOP-1148).
+
 ## [v0.4.11] - 2020-06-22
 
 ### Fixed

snooty/rstspec.toml

@@ -167,8 +167,8 @@ example = """.. include:: ${1:/includes/file.rst}
 [directive.literalinclude]
 help = """Include a file as a block of code."""
 inherit = "include"
-options.start-after = "linenos"
-options.end-before = "linenos"
+options.start-after = "string"
+options.end-before = "string"
 options.linenos = "flag"
 options.copyable = "boolean"
 options.language = "string"

snooty/specparser.py

@@ -108,13 +108,13 @@ class RefRoleType:
 VALIDATORS: Dict[PrimitiveType, Callable[[Any], Any]] = {
     PrimitiveType.integer: int,
     PrimitiveType.nonnegative_integer: docutils.parsers.rst.directives.nonnegative_int,
-    PrimitiveType.path: str,
+    PrimitiveType.path: util.option_string,
     PrimitiveType.uri: docutils.parsers.rst.directives.uri,
-    PrimitiveType.string: str,
+    PrimitiveType.string: util.option_string,
     PrimitiveType.length: docutils.parsers.rst.directives.length_or_percentage_or_unitless,
     PrimitiveType.boolean: util.option_bool,
     PrimitiveType.flag: util.option_flag,
-    PrimitiveType.linenos: str,
+    PrimitiveType.linenos: util.option_string,
 }
 
 #: Option types can be a primitive type (PrimitiveType), an enum

snooty/test_parser.py

@@ -384,6 +384,19 @@ def test_literalinclude() -> None:
     assert len(diagnostics) == 1
     assert isinstance(diagnostics[0], DocUtilsParseError)
 
+    # Test poorly specified arbitrary string option
+    page, diagnostics = parse_rst(
+        parser,
+        path,
+        """
+.. literalinclude:: /test_parser/includes/sample_code.py
+   :start-after:
+""",
+    )
+    page.finish(diagnostics)
+    assert len(diagnostics) == 1
+    assert isinstance(diagnostics[0], DocUtilsParseError)
+
 
 def test_include() -> None:
     path = ROOT_PATH.joinpath(Path("test.rst"))

snooty/test_util.py

@@ -4,22 +4,10 @@
 import time
 import watchdog.events
 from pathlib import Path, PurePath
-from typing import Callable, List, Tuple, Type
+from typing import List, Tuple
 from . import util
 
 
-def expect_exception(f: Callable[[], None], exception_ty: Type[Exception]) -> None:
-    """Assert that the given function f raises the given exception type."""
-    try:
-        f()
-    except exception_ty:
-        return
-    except Exception as err:
-        raise AssertionError(f"Expected {exception_ty.__name__}; got {err}")
-
-    raise AssertionError(f"Expected {exception_ty.__name__} to be raised")
-
-
 def test_reroot_path() -> None:
     relative, absolute = util.reroot_path(
         PurePath("/foo/bar/baz.rst"), PurePath("/foo/dir/test.txt"), Path("foo")
@@ -31,6 +19,34 @@ def test_reroot_path() -> None:
     )[0] == PurePath("foo/bar/baz.rst")
 
 
+def test_option_string() -> None:
+    assert util.option_string("Test") == "Test"
+    # No input or blank input should raise a ValueError
+    try:
+        util.option_string(" ")
+    except ValueError:
+        pass
+
+
+def test_option_bool() -> None:
+    assert util.option_bool("tRuE") == True
+    assert util.option_bool("FaLsE") == False
+    # No input or blank input should raise a ValueError
+    try:
+        util.option_bool(" ")
+    except ValueError:
+        pass
+
+
+def test_option_flag() -> None:
+    assert util.option_flag("") == True
+    # Specifying an argument should raise a ValueError
+    try:
+        util.option_flag("test")
+    except ValueError:
+        pass
+
+
 def test_get_files() -> None:
     assert set(util.get_files(PurePath("test_data"), (".toml",))) == {
         Path("test_data/snooty.toml"),

snooty/util.py

@@ -191,6 +191,16 @@ def __len__(self) -> int:
         return sum(len(w) for w in self.directories.values())
 
 
+def option_string(argument: Optional[str]) -> Optional[str]:
+    """
+    Check for a valid string option and return it. If no argument is given,
+    raise ``ValueError``.
+    """
+    if argument and argument.strip():
+        return argument
+    raise ValueError("Must supply string argument to option")
+
+
 def option_bool(argument: Optional[str]) -> bool:
     """
     Check for a valid boolean option return it. If no argument is given,
@@ -199,8 +209,7 @@ def option_bool(argument: Optional[str]) -> bool:
     if argument and argument.strip():
         output = docutils.parsers.rst.directives.choice(argument, ("true", "false"))
         return output == "true"
-    else:
-        return True
+    return True
 
 
 def option_flag(argument: Optional[str]) -> bool:
@@ -213,8 +222,7 @@ def option_flag(argument: Optional[str]) -> bool:
     """
     if argument and argument.strip():
         raise ValueError('no argument is allowed; "%s" supplied' % argument)
-    else:
-        return True
+    return True
 
 
 def split_domain(name: str) -> Tuple[str, str]:
@@ -227,7 +235,6 @@ def split_domain(name: str) -> Tuple[str, str]:
     parts = name.split(":", 1)
     if len(parts) == 1:
         return "", parts[0]
-
     return parts[0], parts[1]