From 54e05fa8aa4e196edc92a73970892ddbd0d64b31 Mon Sep 17 00:00:00 2001
From: Will McGugan <willmcgugan@gmail.com>
Date: Fri, 2 Sep 2022 11:01:30 +0100
Subject: [PATCH 1/3] validate identifiers

---
 sandbox/will/spacing.py     |  2 +-
 src/textual/css/tokenize.py |  6 +++--
 src/textual/dom.py          | 45 +++++++++++++++++++++++++++++++++++--
 tests/test_dom.py           | 22 +++++++++++++++++-
 4 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/sandbox/will/spacing.py b/sandbox/will/spacing.py
index 9e7cb8b0e..e68af4833 100644
--- a/sandbox/will/spacing.py
+++ b/sandbox/will/spacing.py
@@ -4,7 +4,7 @@ from textual.widgets import Static
 
 class SpacingApp(App):
     def compose(self):
-        yield Static()
+        yield Static(id="2332")
 
 
 app = SpacingApp(css_path="spacing.css")
diff --git a/src/textual/css/tokenize.py b/src/textual/css/tokenize.py
index 7cee24ce2..dbec369df 100644
--- a/src/textual/css/tokenize.py
+++ b/src/textual/css/tokenize.py
@@ -26,6 +26,8 @@ TOKEN = "[a-zA-Z][a-zA-Z0-9_-]*"
 STRING = r"\".*?\""
 VARIABLE_REF = r"\$[a-zA-Z0-9_\-]+"
 
+IDENTIFIER = r"[a-zA-Z_\-][a-zA-Z0-9_\-]*"
+
 # Values permitted in variable and rule declarations.
 DECLARATION_VALUES = {
     "scalar": SCALAR,
@@ -44,8 +46,8 @@ DECLARATION_VALUES = {
 expect_root_scope = Expect(
     whitespace=r"\s+",
     comment_start=COMMENT_START,
-    selector_start_id=r"\#[a-zA-Z_\-][a-zA-Z0-9_\-]*",
-    selector_start_class=r"\.[a-zA-Z_\-][a-zA-Z0-9_\-]*",
+    selector_start_id=r"\#" + IDENTIFIER,
+    selector_start_class=r"\." + IDENTIFIER,
     selector_start_universal=r"\*",
     selector_start=r"[a-zA-Z_\-]+",
     variable_name=rf"{VARIABLE_REF}:",
diff --git a/src/textual/dom.py b/src/textual/dom.py
index a79058e78..9d4fc394d 100644
--- a/src/textual/dom.py
+++ b/src/textual/dom.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from inspect import getfile
+import re
 from typing import (
     cast,
     ClassVar,
@@ -27,6 +28,7 @@ from .css.constants import VALID_DISPLAY, VALID_VISIBILITY
 from .css.errors import StyleValueError, DeclarationError
 from .css.parse import parse_declarations
 from .css.styles import Styles, RenderStyles
+from .css.tokenize import IDENTIFIER
 from .css.query import NoMatchingNodesError
 from .message_pump import MessagePump
 from .timer import Timer
@@ -38,6 +40,32 @@ if TYPE_CHECKING:
     from .widget import Widget
 
 
+_re_identifier = re.compile(IDENTIFIER)
+
+
+class BadIdentifier(Exception):
+    """raised by check_identifiers."""
+
+
+def check_identifiers(description: str, *names: str) -> None:
+    """Validate identifier and raise an error if it fails.
+
+    Args:
+        description (str): Description of where identifier is used for error message.
+        names (list[str]): Identifiers to check.
+
+    Returns:
+        bool: True if the name is valid.
+    """
+    match = _re_identifier.match
+    for name in names:
+        if match(name) is None:
+            raise BadIdentifier(
+                f"{name!r} is invalid {description}; "
+                "identifiers must contain only letters, numbers, underscores, hyphens, and must not begin with a number."
+            )
+
+
 class DOMError(Exception):
     pass
 
@@ -75,9 +103,17 @@ class DOMNode(MessagePump):
         id: str | None = None,
         classes: str | None = None,
     ) -> None:
+        self._classes = set()
         self._name = name
-        self._id = id
-        self._classes: set[str] = set() if classes is None else set(classes.split())
+        self._id = None
+        if id is not None:
+            self.id = id
+
+        _classes = classes.split() if classes else []
+        check_identifiers("class name", *_classes)
+
+        self._classes.update(_classes)
+
         self.children = NodeList()
         self._css_styles: Styles = Styles(self)
         self._inline_styles: Styles = Styles(self)
@@ -248,6 +284,8 @@ class DOMNode(MessagePump):
             ValueError: If the ID has already been set.
 
         """
+        check_identifiers("id", new_id)
+
         if self._id is not None:
             raise ValueError(
                 f"Node 'id' attribute may not be changed once set (current id={self._id!r})"
@@ -723,6 +761,7 @@ class DOMNode(MessagePump):
             *class_names (str): CSS class names to add.
 
         """
+        check_identifiers("class name", *class_names)
         old_classes = self._classes.copy()
         self._classes.update(class_names)
         if old_classes == self._classes:
@@ -739,6 +778,7 @@ class DOMNode(MessagePump):
             *class_names (str): CSS class names to remove.
 
         """
+        check_identifiers("class name", *class_names)
         old_classes = self._classes.copy()
         self._classes.difference_update(class_names)
         if old_classes == self._classes:
@@ -755,6 +795,7 @@ class DOMNode(MessagePump):
             *class_names (str): CSS class names to toggle.
 
         """
+        check_identifiers("class name", *class_names)
         old_classes = self._classes.copy()
         self._classes.symmetric_difference_update(class_names)
         if old_classes == self._classes:
diff --git a/tests/test_dom.py b/tests/test_dom.py
index 1a597a383..a7ec46482 100644
--- a/tests/test_dom.py
+++ b/tests/test_dom.py
@@ -2,7 +2,7 @@ import pytest
 
 from textual.css.errors import StyleValueError
 from textual.css.query import NoMatchingNodesError
-from textual.dom import DOMNode
+from textual.dom import DOMNode, BadIdentifier
 
 
 def test_display_default():
@@ -55,3 +55,23 @@ def test_get_child_no_matching_child(parent):
 def test_get_child_only_immediate_descendents(parent):
     with pytest.raises(NoMatchingNodesError):
         parent.get_child(id="grandchild1")
+
+
+def test_validate():
+    with pytest.raises(BadIdentifier):
+        DOMNode(id="23")
+    with pytest.raises(BadIdentifier):
+        DOMNode(id=".3")
+    with pytest.raises(BadIdentifier):
+        DOMNode(classes="+2323")
+    with pytest.raises(BadIdentifier):
+        DOMNode(classes="foo 22")
+
+    node = DOMNode()
+    node.add_class("foo")
+    with pytest.raises(BadIdentifier):
+        node.add_class("1")
+    with pytest.raises(BadIdentifier):
+        node.remove_class("1")
+    with pytest.raises(BadIdentifier):
+        node.toggle_class("1")

From e3e81082759163ec8f06aca6e5975eb31c604a3e Mon Sep 17 00:00:00 2001
From: Will McGugan <willmcgugan@gmail.com>
Date: Fri, 2 Sep 2022 11:03:03 +0100
Subject: [PATCH 2/3] words

---
 src/textual/dom.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/textual/dom.py b/src/textual/dom.py
index 9d4fc394d..2065c34d1 100644
--- a/src/textual/dom.py
+++ b/src/textual/dom.py
@@ -61,8 +61,8 @@ def check_identifiers(description: str, *names: str) -> None:
     for name in names:
         if match(name) is None:
             raise BadIdentifier(
-                f"{name!r} is invalid {description}; "
-                "identifiers must contain only letters, numbers, underscores, hyphens, and must not begin with a number."
+                f"{name!r} is an invalid {description}; "
+                "identifiers must contain only letters, numbers, underscores, or hyphens, and must not begin with a number."
             )
 
 

From 6c82cfd67705a7902d90e44b14a07460733cef9a Mon Sep 17 00:00:00 2001
From: Will McGugan <willmcgugan@gmail.com>
Date: Fri, 2 Sep 2022 11:04:28 +0100
Subject: [PATCH 3/3] ws

---
 src/textual/dom.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/textual/dom.py b/src/textual/dom.py
index 2065c34d1..b22e93690 100644
--- a/src/textual/dom.py
+++ b/src/textual/dom.py
@@ -111,7 +111,6 @@ class DOMNode(MessagePump):
 
         _classes = classes.split() if classes else []
         check_identifiers("class name", *_classes)
-
         self._classes.update(_classes)
 
         self.children = NodeList()