Repository: Textualize/textual
Pull request: Validate ids
Fix #3954.

There was already a check taking place but the regex used was too lenient.

Problem statement:
It is possible to give a widget an ID that can't be queried back
It is possible to give a widget an ID that can't then be queried back; for example:

```python
from textual.app import App, ComposeResult
from textual.widgets import Label

class BadIDApp(App[None]):

    def compose(self) -> ComposeResult:
        yield Label("Hello, World!", id="foo&")

    def on_mount(self) -> None:
        self.notify(f"{self.query_one('#foo&')}")

if __name__ == "__main__":
    BadIDApp().run()
```

Perhaps we should validate widget IDs when they are assigned?

Hints from issue discussion:
We do this with class names, so it would be sensible to do it for IDs as well.

Commit message: Fix regular expression check for identifiers.

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d3f838be7d..42dcff82e1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - `SelectionList` option IDs are usable as soon as the widget is instantiated https://github.com/Textualize/textual/issues/3903
 - Fix issue with `Strip.crop` when crop window start aligned with strip end https://github.com/Textualize/textual/pull/3998
 - Fixed Strip.crop_extend https://github.com/Textualize/textual/pull/4011
+- ID and class validation was too lenient https://github.com/Textualize/textual/issues/3954
 
 
 ## [0.47.1] - 2023-01-05
diff --git a/src/textual/dom.py b/src/textual/dom.py
index 461e9acea7..92a9e6e18c 100644
--- a/src/textual/dom.py
+++ b/src/textual/dom.py
@@ -62,7 +62,7 @@
 
 from typing_extensions import Literal
 
-_re_identifier = re.compile(IDENTIFIER)
+_re_identifier = re.compile(f"^{IDENTIFIER}$")
 
 
 WalkMethod: TypeAlias = Literal["depth", "breadth"]
diff --git a/tests/test_dom.py b/tests/test_dom.py
index 6f06fb8091..ea14a25bfe 100644
--- a/tests/test_dom.py
+++ b/tests/test_dom.py
@@ -259,3 +259,24 @@ def test_walk_children_with_self_breadth(search):
     ]
 
     assert children == ["f", "e", "d", "c", "b", "a"]
+
+
+@pytest.mark.parametrize(
+    "identifier",
+    [
+        " bad",
+        "  terrible  ",
+        "worse!  ",
+        "&ampersand",
+        "amper&sand",
+        "ampersand&",
+        "2_leading_digits",
+        "água",  # water
+        "cão",  # dog
+        "@'/.23",
+    ],
+)
+def test_id_validation(identifier: str):
+    """Regression tests for https://github.com/Textualize/textual/issues/3954."""
+    with pytest.raises(BadIdentifier):
+        DOMNode(id=identifier)


Code review comments:
diff_hunk: @@ -62,7 +62,7 @@
 
 from typing_extensions import Literal
 
-_re_identifier = re.compile(IDENTIFIER)
+_re_identifier = re.compile(f"^{IDENTIFIER}$")
path: src/textual/dom.py
text: @user1:
Do we need to do this, or can we use `re.fullmatch` ?

@author:
I think we can, yeah.
Do you suspect it'll be faster, is that it?

@author:
Tests pass with `re.fullmatch` instead of the formatted string identifier.
Do you want me to update the PR with `fullmatch`?

@user1:
Simpler perhaps. Go with full match.

@author:
See 98e28f0.