Repository: lordmauve/adventurelib

Issue:
Room lets you use exit as a direction even though it is a method
Whenever I do 'north', 'east', 'west' or 'south', the game crashes with an error:

```
Traceback (most recent call last):
  File "C:\Users\84367\Desktop\main.py", line 81, in <module>
    start()
  File "G:\Python\lib\site-packages\adventurelib.py", line 536, in start
    _handle_command(cmd)
  File "G:\Python\lib\site-packages\adventurelib.py", line 509, in _handle_command
    func(**args)
  File "C:\Users\84367\Desktop\main.py", line 53, in go
    room = current_room.exit(direction)
TypeError: 'NoneType' object is not callable
```

Discussion:
@coco7nut001: I'm using Python 3.10.1 Not sure if that helps@lordmauve: Please can you provide a minimal reproducible example?@coco7nut001: > Please can you provide a minimal reproducible example?
Oh it return error when I go in a customized direction like up-down
@coco7nut001: ```
from adventurelib import *
Room.add_direction('up', 'down')
Room.add_direction('enter', 'exit')

@when('north', direction='north')
@when('south', direction='south')
@when('east', direction='east')
@when('west', direction='west')
def go(direction):
    global current_room
    room = current_room.exit(direction)
    if room:
        current_room = room
        print(f'You go {direction}.')
        look()
tent = Room(...)
camp = Room(...)
river = Room(...)
camp.enter = tent
camp.down = river
```@lordmauve: The problem is that you are using `exit` as a direction, when it is also used as a method to get an `exit`. I think `adventurelib` should stop you using directions that conflict with methods in the Room class.@coco7nut001: Is there any way to get around that annoying thing?@lordmauve: Yes:

- Don't use `enter`/`exit` as your directions, use `enter`/`out` for example
- Bind the command `exit` to the direction name you use, so `out` if that's what you go with:
   ```python
   @when('exit', direction='out')
   ```@Vaporbook: I got snagged on this too ... turns out I had done a copypasta from the docs, so you might want to change this line: https://github.com/lordmauve/adventurelib/blob/master/doc/rooms.rst?plain=1#L209 ...

I'm grateful for this very cool library - having a lot of fun!

Solution patch:
diff --git a/adventurelib.py b/adventurelib.py
index fc14d68..812d392 100644
--- a/adventurelib.py
+++ b/adventurelib.py
@@ -143,10 +143,14 @@ class Room:
         for dir in (forward, reverse):
             if not dir.islower():
                 raise InvalidCommand(
-                    'Invalid direction %r: directions must be all lowercase.'
+                    'Invalid direction %r: directions must be all lowercase.' % dir
                 )
             if dir in Room._directions:
                 raise KeyError('%r is already a direction!' % dir)
+            if hasattr(Room, dir):
+                raise AttributeError(
+                    '%r is an existing attribute and cannot be used as direction name!' % dir
+                )
         Room._directions[forward] = reverse
         Room._directions[reverse] = forward
 
diff --git a/doc/rooms.rst b/doc/rooms.rst
index 39ec6b2..6bf2815 100644
--- a/doc/rooms.rst
+++ b/doc/rooms.rst
@@ -206,7 +206,7 @@ you give an opposite. You would typically do this at the top of the file,
 before you define any rooms::
 
     Room.add_direction('up', 'down')
-    Room.add_direction('enter', 'exit')
+    Room.add_direction('enter', 'out')
 
     tent = Room(...)
     camp = Room(...)


Test patch:
diff --git a/test_adventurelib.py b/test_adventurelib.py
index 12b1491..3bbd46b 100644
--- a/test_adventurelib.py
+++ b/test_adventurelib.py
@@ -1,13 +1,21 @@
-from unittest.mock import patch
-from contextlib import redirect_stdout, contextmanager
+import copy
+from contextlib import contextmanager, redirect_stdout
 from io import StringIO
+from unittest.mock import patch
 
 import pytest
 
 import adventurelib
-from adventurelib import Pattern, when, _handle_command, say, Room, Item, Bag
-
-orig_commands = adventurelib.commands[:]
+from adventurelib import (
+    Bag,
+    InvalidCommand,
+    Item,
+    Pattern,
+    Room,
+    _handle_command,
+    say,
+    when,
+)
 
 
 @contextmanager
@@ -21,88 +29,99 @@ def active_context(ctx):
         adventurelib.set_context(prev_ctx)
 
 
-def teardown():
-    """Reset the commands."""
+@pytest.fixture(autouse=True)
+def setup_commands():
+    orig_commands = adventurelib.commands[:]
+    yield
     adventurelib.commands[:] = orig_commands
 
 
+@pytest.fixture(autouse=True)
+def setup_room():
+    original_room = {}
+    for key, value in Room.__dict__.items():
+        if not key.startswith("__"):
+            try:
+                original_room[key] = copy.copy(value)
+            except TypeError:
+                original_room[key] = value
+    yield
+    for key in list(Room.__dict__.keys()):
+        if not key.startswith("__") and key not in original_room:
+            delattr(Room, key)
+    for key, value in original_room.items():
+        try:
+            setattr(Room, key, value)
+        except AttributeError:
+            pass
+
+
 def test_match():
-    matches = Pattern('apple').match(['apple'])
+    matches = Pattern("apple").match(["apple"])
     assert matches == {}
 
 
 def test_no_match_different_words():
-    matches = Pattern('look').match(['apple'])
+    matches = Pattern("look").match(["apple"])
     assert matches is None
 
 
 def test_no_match_extra_words():
-    matches = Pattern('look').match(['look', 'at', 'butler'])
+    matches = Pattern("look").match(["look", "at", "butler"])
     assert matches is None
 
 
 def test_multiple_words():
-    pat = Pattern('take apple')
-    matches = pat.match(['take', 'apple'])
+    pat = Pattern("take apple")
+    matches = pat.match(["take", "apple"])
     assert matches == {}
 
 
 def test_multiple_words_mismatch():
-    matches = Pattern('take apple').match(['take', 'tortoise'])
+    matches = Pattern("take apple").match(["take", "tortoise"])
     assert matches is None
 
 
 def test_multiple_words_mismatch_length():
-    matches = Pattern('take apple').match(['take', 'golden', 'apple'])
+    matches = Pattern("take apple").match(["take", "golden", "apple"])
     assert matches is None
 
 
 def test_capturing_match():
-    matches = Pattern('take ITEM').match(['take', 'apple'])
-    assert matches == {'item': 'apple'}
+    matches = Pattern("take ITEM").match(["take", "apple"])
+    assert matches == {"item": "apple"}
 
 
 def test_multiple_captures():
-    pat = Pattern('give ITEM to PERSON')
-    matches = pat.match(['give', 'apple', 'to', 'wizard'])
-    assert matches == {'item': 'apple', 'person': 'wizard'}
+    pat = Pattern("give ITEM to PERSON")
+    matches = pat.match(["give", "apple", "to", "wizard"])
+    assert matches == {"item": "apple", "person": "wizard"}
 
 
 def test_capturing_multiword():
-    matches = Pattern('take ITEM').match(['take', 'golden', 'apple'])
-    assert matches == {'item': 'golden apple'}
+    matches = Pattern("take ITEM").match(["take", "golden", "apple"])
+    assert matches == {"item": "golden apple"}
 
 
 def test_multiple_multiword_captures():
-    pat = Pattern('give ITEM to PERSON')
-    matches = pat.match(['give', 'golden', 'apple', 'to', 'evil', 'wizard'])
-    assert matches == {'item': 'golden apple', 'person': 'evil wizard'}
+    pat = Pattern("give ITEM to PERSON")
+    matches = pat.match(["give", "golden", "apple", "to", "evil", "wizard"])
+    assert matches == {"item": "golden apple", "person": "evil wizard"}
 
 
 def test_word_combinations():
     combos = Pattern.word_combinations(have=3, placeholders=2)
-    assert list(combos) == [
-        (2, 1),
-        (1, 2)
-    ]
+    assert list(combos) == [(2, 1), (1, 2)]
 
 
 def test_word_combinations_2():
     combos = Pattern.word_combinations(have=4, placeholders=2)
-    assert list(combos) == [
-        (3, 1),
-        (2, 2),
-        (1, 3)
-    ]
+    assert list(combos) == [(3, 1), (2, 2), (1, 3)]
 
 
 def test_word_combinations_3():
     combos = Pattern.word_combinations(have=4, placeholders=3)
-    assert list(combos) == [
-        (2, 1, 1),
-        (1, 2, 1),
-        (1, 1, 2)
-    ]
+    assert list(combos) == [(2, 1, 1), (1, 2, 1), (1, 1, 2)]
 
 
 def test_word_combinations_4():
@@ -120,110 +139,119 @@ def test_word_combinations_4():
 def test_register():
     called = False
 
-    @when('north')
+    @when("north")
     def func():
         nonlocal called
         called = True
 
     print(adventurelib.commands)
-    _handle_command('north')
+    _handle_command("north")
     assert called is True
 
 
 def test_register_args():
     args = None
 
-    @when('north', dir='north')
+    @when("north", dir="north")
     def func(dir):
         nonlocal args
         args = [dir]
-    _handle_command('north')
-    assert args == ['north']
 
+    _handle_command("north")
+    assert args == ["north"]
 
-@pytest.mark.parametrize('ctx,expected', [
-    (None, 'north'),
-    ('confused', 'south'),
-    ('confused.really', 'cauliflower'),
-])
+
+@pytest.mark.parametrize(
+    "ctx,expected",
+    [
+        (None, "north"),
+        ("confused", "south"),
+        ("confused.really", "cauliflower"),
+    ],
+)
 def test_register_context(ctx, expected):
     """The result of a command changes in different contexts."""
     cmd = None
 
     # Register out of order to test tie-breaking by context nesting depth
-    @when('north', dir='north')
-    @when('north', dir='cauliflower', context='confused.really')
-    @when('north', dir='south', context='confused')
+    @when("north", dir="north")
+    @when("north", dir="cauliflower", context="confused.really")
+    @when("north", dir="south", context="confused")
     def func(dir):
         nonlocal cmd
         cmd = dir
+
     with active_context(ctx):
-        _handle_command('north')
+        _handle_command("north")
     assert cmd == expected
 
 
 def test_register_match():
     args = None
 
-    @when('hit TARGET with WEAPON', verb='hit')
+    @when("hit TARGET with WEAPON", verb="hit")
     def func(target, weapon, verb):
         nonlocal args
         args = [target, weapon, verb]
-    _handle_command('hit dragon with glass sword')
-    assert args == ['dragon', 'glass sword', 'hit']
+
+    _handle_command("hit dragon with glass sword")
+    assert args == ["dragon", "glass sword", "hit"]
 
 
 def say_at_width(width, msg):
     buf = StringIO()
-    with patch('adventurelib.get_terminal_size', return_value=(width, 24)):
+    with patch("adventurelib.get_terminal_size", return_value=(width, 24)):
         with redirect_stdout(buf):
             say(msg)
     return buf.getvalue()
 
 
 def test_say_room():
-    """saw() will format input as strings."""
-    r = Room('You are standing in a hallway.')
+    """say() will format input as strings."""
+    r = Room("You are standing in a hallway.")
 
     buf = StringIO()
     with redirect_stdout(buf):
         say(r)
-    assert buf.getvalue() == 'You are standing in a hallway.\n'
+    assert buf.getvalue() == "You are standing in a hallway.\n"
 
 
 def test_say_wrap():
     """The say() function will print output wrapped to the terminal width."""
-    out = say_at_width(40, """
+    out = say_at_width(
+        40,
+        """
     This is a long sentence that the say command will wrap.
-    """)
-
-    assert out == (
-        "This is a long sentence that the say\n"
-        "command will wrap.\n"
+    """,
     )
 
+    assert out == ("This is a long sentence that the say\n" "command will wrap.\n")
+
 
 def test_say_wrap2():
     """The say() function will print output wrapped to the terminal width."""
-    out = say_at_width(20, """
+    out = say_at_width(
+        20,
+        """
     This is a long sentence that the say command will wrap.
-    """)
+    """,
+    )
 
     assert out == (
-        "This is a long\n"
-        "sentence that the\n"
-        "say command will\n"
-        "wrap.\n"
+        "This is a long\n" "sentence that the\n" "say command will\n" "wrap.\n"
     )
 
 
 def test_say_multiple_paragraph():
     """Paragraphs separated by a blank line will be wrapped separately."""
-    out = say_at_width(40, """
+    out = say_at_width(
+        40,
+        """
     This is a long sentence that the say command will wrap.
 
     And this is a second paragraph that is separately wrapped.
-    """)
+    """,
+    )
 
     assert out == (
         "This is a long sentence that the say\n"
@@ -236,10 +264,13 @@ def test_say_multiple_paragraph():
 
 def test_say_multiline_paragraph():
     """We can wrap a sentence written in multiple input lines."""
-    out = say_at_width(40, """
+    out = say_at_width(
+        40,
+        """
     This is a long sentence that the say command will wrap,
     and this clause is indented to match.
-    """)
+    """,
+    )
 
     assert out == (
         "This is a long sentence that the say\n"
@@ -248,18 +279,18 @@ def test_say_multiline_paragraph():
     )
 
 
-@patch('random.randrange', return_value=0)
+@patch("random.randrange", return_value=0)
 def test_bag_get_random(randrange):
     """We can select an item from a bag at random."""
-    bag = Bag(map(Item, 'abc'))
+    bag = Bag(map(Item, "abc"))
     assert bag.get_random() == list(bag)[0]
     randrange.assert_called_once_with(3)
 
 
-@patch('random.randrange', return_value=1)
+@patch("random.randrange", return_value=1)
 def test_bag_get_random2(randrange):
     """We can select an item from a bag at random."""
-    bag = Bag(map(Item, 'abc'))
+    bag = Bag(map(Item, "abc"))
     assert bag.get_random() == list(bag)[1]
     randrange.assert_called_once_with(3)
 
@@ -270,10 +301,10 @@ def test_empty_bag_get_random():
     assert bag.get_random() is None
 
 
-@patch('random.randrange', return_value=0)
+@patch("random.randrange", return_value=0)
 def test_bag_take_random(randrange):
     """We can select and remove an item from a bag at random."""
-    bag = Bag(map(Item, 'abc'))
+    bag = Bag(map(Item, "abc"))
     items = list(bag)
     assert bag.take_random() == items[0]
     assert bag == Bag(items[1:])
@@ -282,48 +313,42 @@ def test_bag_take_random(randrange):
 
 def test_bag_find():
     """We can find items in a bag by name, case insensitively."""
-    name, *aliases = ['Name', 'UPPER ALIAS', 'lower alias']
+    name, *aliases = ["Name", "UPPER ALIAS", "lower alias"]
     named_item = Item(name, *aliases)
-    appellative_item = Item('appellation', *aliases)
-    nameless_item = Item('noname', 'none at all')
+    appellative_item = Item("appellation", *aliases)
+    nameless_item = Item("noname", "none at all")
     bag = Bag({named_item, appellative_item, nameless_item})
 
-    assert bag.find('name') is named_item
-    assert bag.find('Name') is named_item
-    assert bag.find('NAME') is named_item
-    assert bag.find('appellation') is appellative_item
-    assert bag.find('upper alias') in {named_item, appellative_item}
-    assert bag.find('LOWER ALIAS') in {named_item, appellative_item}
-    assert not bag.find('other')
+    assert bag.find("name") is named_item
+    assert bag.find("Name") is named_item
+    assert bag.find("NAME") is named_item
+    assert bag.find("appellation") is appellative_item
+    assert bag.find("upper alias") in {named_item, appellative_item}
+    assert bag.find("LOWER ALIAS") in {named_item, appellative_item}
+    assert not bag.find("other")
 
 
-@pytest.mark.parametrize(
-    'current_context',
-    ['foo', 'foo.bar', 'foo.bar.baz']
-)
+@pytest.mark.parametrize("current_context", ["foo", "foo.bar", "foo.bar.baz"])
 def test_match_context(current_context):
     """We can match contexts."""
-    assert adventurelib._match_context('foo', current_context)
+    assert adventurelib._match_context("foo", current_context)
 
 
 @pytest.mark.parametrize(
-    'current_context',
-    [None, 'bar', 'bar.foo'],
+    "current_context",
+    [None, "bar", "bar.foo"],
 )
 def test_no_match_context(current_context):
     """A context doesn't match if it is not "within" the pattern context."""
-    assert not adventurelib._match_context('foo', current_context)
+    assert not adventurelib._match_context("foo", current_context)
 
 
 def test_match_context_none():
     """The current context matches if the pattern context is None."""
-    assert adventurelib._match_context(None, 'foo.bar')
+    assert adventurelib._match_context(None, "foo.bar")
 
 
-@pytest.mark.parametrize(
-    'context',
-    [None, 'foo', 'foo.bar']
-)
+@pytest.mark.parametrize("context", [None, "foo", "foo.bar"])
 def test_validate_context(context):
     """We can validate valid contexts."""
     adventurelib._validate_context(context)
@@ -370,5 +395,35 @@ def test_validate_pattern_double_ident():
     with pytest.raises(adventurelib.InvalidCommand) as exc:
         Pattern("take I with I")
     err = str(exc.value)
-    assert err == "Invalid command 'take I with I'"\
-                  " Identifiers may only be used once"
+    assert err == "Invalid command 'take I with I'" " Identifiers may only be used once"
+
+
+def test_room_add_direction_not_lowercase():
+    """Directions must be lowercase."""
+    room = Room("Test Room")
+    with pytest.raises(
+        InvalidCommand,
+        match="Invalid direction 'Up': directions must be all lowercase.",
+    ):
+        room.add_direction("Up", "down")
+
+
+def test_room_add_direction_already_added():
+    """Directions must be unique."""
+    room = Room("Test Room")
+    room.add_direction("up", "down")
+    with pytest.raises(
+        KeyError,
+        match="'up' is already a direction!",
+    ):
+        room.add_direction("up", "down")
+
+
+def test_room_add_direction_is_attribute():
+    """Directions must not be named as existing attributes."""
+    room = Room("Test Room")
+    with pytest.raises(
+        AttributeError,
+        match="'exit' is an existing attribute and cannot be used as direction name!",
+    ):
+        room.add_direction("up", "exit")