Add the ILookup interface (#410)

This commit adds the `ILookup` interface, which is inserted into the `IAssociative` hierarchy. `ILookup` adds `ILookup.val_at`, which returns only values from associative objects and `IAssociative.entry` is amended to return an `IMapEntry`. 

For #409 we need to apply the `ILookup` interface to `ReaderConditional` types. Before, Basilisp only had the `IAssociative.entry` which was always meant to return an `IMapEntry` (rather than merely a value, as it was doing).

Author: chrisrink10

src/basilisp/core.lpy

@@ -3376,7 +3376,14 @@
          (~'contains [~this-gs ~key-gs]
           (or (~field-kw-set ~key-gs)
               (contains? ~'_recmap ~key-gs)))
-         (~'entry [~this-gs ~key-gs ~'& args#]
+         (~'entry [~this-gs ~key-gs]
+          (cond
+            (contains? ~field-kw-set ~key-gs)
+            (map-entry ~key-gs (python/getattr ~this-gs (munge ~key-gs)))
+
+            (contains? ~'_recmap ~key-gs)
+            (map-entry ~key-gs (get ~'_recmap ~key-gs))))
+         (~'val-at [~this-gs ~key-gs ~'& args#]
           (let [[default#] args#]
             (cond
               (contains? ~field-kw-set ~key-gs)

src/basilisp/lang/compiler/analyzer.py

@@ -236,7 +236,7 @@ def _warn_unused_names(self):
                 code_loc = (
                     Maybe(entry.symbol.meta)
                     .map(
-                        lambda m: f": {m.entry(reader.READER_LINE_KW)}"  # type: ignore
+                        lambda m: f": {m.val_at(reader.READER_LINE_KW)}"  # type: ignore
                     )
                     .or_else_get("")
                 )
@@ -312,22 +312,22 @@ def filename(self) -> str:
     @property
     def warn_on_unused_names(self) -> bool:
         """If True, warn when local names are unused."""
-        return self._opts.entry(WARN_ON_UNUSED_NAMES, True)
+        return self._opts.val_at(WARN_ON_UNUSED_NAMES, True)
 
     @property
     def warn_on_shadowed_name(self) -> bool:
         """If True, warn when a name is shadowed in an inner scope.
 
         Implies warn_on_shadowed_var."""
-        return self._opts.entry(WARN_ON_SHADOWED_NAME, False)
+        return self._opts.val_at(WARN_ON_SHADOWED_NAME, False)
 
     @property
     def warn_on_shadowed_var(self) -> bool:
         """If True, warn when a def'ed Var name is shadowed in an inner scope.
 
         Implied by warn_on_shadowed_name. The value of warn_on_shadowed_name
         supersedes the value of this flag."""
-        return self.warn_on_shadowed_name or self._opts.entry(
+        return self.warn_on_shadowed_name or self._opts.val_at(
             WARN_ON_SHADOWED_VAR, False
         )
 
@@ -411,7 +411,7 @@ def put_new_symbol(  # pylint: disable=too-many-arguments
         ) and self.warn_on_shadowed_var:
             if self.current_ns.find(s) is not None:
                 logger.warning(f"name '{s}' shadows def'ed Var from outer scope")
-        if s.meta is not None and s.meta.entry(SYM_NO_WARN_WHEN_UNUSED_META_KEY, None):
+        if s.meta is not None and s.meta.val_at(SYM_NO_WARN_WHEN_UNUSED_META_KEY, None):
             warn_if_unused = False
         st.new_symbol(s, binding, warn_if_unused=warn_if_unused)
 
@@ -454,7 +454,7 @@ def _meta_getter(meta_kw: kw.Keyword) -> MetaGetter:
 
     def has_meta_prop(o: Union[IMeta, Var]) -> bool:
         return (  # type: ignore
-            Maybe(o.meta).map(lambda m: m.entry(meta_kw, None)).or_else_get(False)
+            Maybe(o.meta).map(lambda m: m.val_at(meta_kw, None)).or_else_get(False)
         )
 
     return has_meta_prop
@@ -632,7 +632,7 @@ def _def_ast(  # pylint: disable=too-many-branches,too-many-locals
     # where we directly set the Var meta for the running Basilisp instance
     # this causes problems since we'll end up getting something like
     # `(quote ([] [v]))` rather than simply `([] [v])`.
-    arglists_meta = def_meta.entry(ARGLISTS_KW)  # type: ignore
+    arglists_meta = def_meta.val_at(ARGLISTS_KW)  # type: ignore
     if isinstance(arglists_meta, llist.List):
         assert arglists_meta.first == SpecialForm.QUOTE
         var_meta = def_meta.update(  # type: ignore
@@ -649,7 +649,7 @@ def _def_ast(  # pylint: disable=too-many-branches,too-many-locals
     var = Var.intern_unbound(
         ns_sym,
         bare_name,
-        dynamic=def_meta.entry(SYM_DYNAMIC_META_KEY, False),  # type: ignore
+        dynamic=def_meta.val_at(SYM_DYNAMIC_META_KEY, False),  # type: ignore
         meta=var_meta,
     )
     descriptor = Def(
@@ -1176,7 +1176,7 @@ def _deftype_ast(  # pylint: disable=too-many-branches
             field_default = (
                 Maybe(field.meta)
                 .map(
-                    lambda m: m.entry(  # type: ignore
+                    lambda m: m.val_at(  # type: ignore
                         SYM_DEFAULT_META_KEY, __DEFTYPE_DEFAULT_SENTINEL
                     )
                 )
@@ -1594,12 +1594,12 @@ def _import_ast(  # pylint: disable=too-many-branches
                 raise AnalyzerException(
                     "import alias must take the form: [module :as alias]", form=f
                 )
-            module_name = f.entry(0)
+            module_name = f.val_at(0)
             if not isinstance(module_name, sym.Symbol):
                 raise AnalyzerException("Python module name must be a symbol", form=f)
-            if not AS == f.entry(1):
+            if not AS == f.val_at(1):
                 raise AnalyzerException("expected :as alias for Python import", form=f)
-            module_alias_sym = f.entry(2)
+            module_alias_sym = f.val_at(2)
             if not isinstance(module_alias_sym, sym.Symbol):
                 raise AnalyzerException("Python module alias must be a symbol", form=f)
             module_alias = module_alias_sym.name

src/basilisp/lang/compiler/generator.py

@@ -209,13 +209,13 @@ def filename(self) -> str:
     @property
     def use_var_indirection(self) -> bool:
         """If True, compile all variable references using Var.find indirection."""
-        return self._opts.entry(USE_VAR_INDIRECTION, False)
+        return self._opts.val_at(USE_VAR_INDIRECTION, False)
 
     @property
     def warn_on_var_indirection(self) -> bool:
         """If True, warn when a Var reference cannot be direct linked (iff
         use_var_indirection is False).."""
-        return not self.use_var_indirection and self._opts.entry(
+        return not self.use_var_indirection and self._opts.val_at(
             WARN_ON_VAR_INDIRECTION, True
         )
 
@@ -588,7 +588,7 @@ def __should_warn_on_redef(
     ctx: GeneratorContext, defsym: sym.Symbol, safe_name: str, def_meta: lmap.Map
 ) -> bool:
     """Return True if the compiler should emit a warning about this name being redefined."""
-    no_warn_on_redef = def_meta.entry(SYM_NO_WARN_ON_REDEF_META_KEY, False)
+    no_warn_on_redef = def_meta.val_at(SYM_NO_WARN_ON_REDEF_META_KEY, False)
     if no_warn_on_redef:
         return False
     elif safe_name in ctx.current_ns.module.__dict__:
@@ -597,7 +597,7 @@ def __should_warn_on_redef(
         var = ctx.current_ns.find(defsym)
         assert var is not None, f"Var {defsym} cannot be none here"
 
-        if var.meta is not None and var.meta.entry(SYM_REDEF_META_KEY):
+        if var.meta is not None and var.meta.val_at(SYM_REDEF_META_KEY):
             return False
         elif var.is_bound:
             return True
@@ -652,7 +652,7 @@ def _def_to_py_ast(  # pylint: disable=too-many-branches
 
     # If the Var is marked as dynamic, we need to generate a keyword argument
     # for the generated Python code to set the Var as dynamic
-    is_dynamic = def_meta.entry(SYM_DYNAMIC_META_KEY, False)
+    is_dynamic = def_meta.val_at(SYM_DYNAMIC_META_KEY, False)
     dynamic_kwarg = (
         [ast.keyword(arg="dynamic", value=ast.Constant(is_dynamic))]
         if is_dynamic

src/basilisp/lang/interfaces.py

@@ -7,7 +7,7 @@
 T = TypeVar("T")
 
 
-class IDeref(Generic[T]):
+class IDeref(Generic[T], ABC):
     __slots__ = ()
 
     @abstractmethod
@@ -29,7 +29,7 @@ def deref(
 # Python 3.6 and 3.7, which affects a few simple test assertions.
 # Since there is little benefit to this type being Generic, I'm leaving
 # it as is for now.
-class IExceptionInfo(Exception):
+class IExceptionInfo(Exception, ABC):
     __slots__ = ()
 
     @property
@@ -42,7 +42,7 @@ def data(self) -> "IPersistentMap":
 V = TypeVar("V")
 
 
-class IMapEntry(Generic[K, V]):
+class IMapEntry(Generic[K, V], ABC):
     __slots__ = ()
 
     @property
@@ -80,6 +80,14 @@ def seq(self) -> "ISeq[T]":
         raise NotImplementedError()
 
 
+class ILookup(Generic[K, V], ABC):
+    __slots__ = ()
+
+    @abstractmethod
+    def val_at(self, k: K, default: Optional[V] = None) -> Optional[V]:
+        raise NotImplementedError()
+
+
 class IPersistentCollection(ISeqable[T]):
     __slots__ = ()
 
@@ -93,7 +101,9 @@ def empty() -> "IPersistentCollection[T]":
         raise NotImplementedError()
 
 
-class IAssociative(Mapping[K, V], IPersistentCollection[IMapEntry[K, V]]):
+class IAssociative(
+    ILookup[K, V], Mapping[K, V], IPersistentCollection[IMapEntry[K, V]]
+):
     __slots__ = ()
 
     @abstractmethod
@@ -105,7 +115,7 @@ def contains(self, k: K) -> bool:
         raise NotImplementedError()
 
     @abstractmethod
-    def entry(self, k: K, default: Optional[V] = None) -> Optional[V]:
+    def entry(self, k: K) -> Optional[IMapEntry[K, V]]:
         raise NotImplementedError()
 
 

src/basilisp/lang/keyword.py

@@ -36,7 +36,7 @@ def __hash__(self):
 
     def __call__(self, m: IAssociative, default=None):
         try:
-            return m.entry(self, default)
+            return m.val_at(self, default)
         except AttributeError:
             return None
 

src/basilisp/lang/map.py

@@ -1,51 +1,20 @@
 from builtins import map as pymap
-from typing import Callable, Dict, Iterable, Mapping, Sequence, TypeVar, Union
+from typing import Callable, Dict, Iterable, Mapping, TypeVar, Union
 
-from pyrsistent import (  # noqa # pylint: disable=unused-import
-    PMap,
-    PVector,
-    pmap,
-    pvector,
-)
+from pyrsistent import PMap, pmap  # noqa # pylint: disable=unused-import
 
 from basilisp.lang.interfaces import ILispObject, IMapEntry, IMeta, IPersistentMap, ISeq
 from basilisp.lang.obj import map_lrepr as _map_lrepr
 from basilisp.lang.seq import sequence
-from basilisp.lang.vector import Vector
+from basilisp.lang.vector import MapEntry, Vector
 from basilisp.util import partition
 
 T = TypeVar("T")
 K = TypeVar("K")
 V = TypeVar("V")
 
 
-class MapEntry(IMapEntry[K, V], Vector[Union[K, V]]):
-    __slots__ = ()
-
-    def __init__(self, wrapped: "PVector[Union[K, V]]") -> None:
-        try:
-            if not len(wrapped) == 2:
-                raise ValueError("Vector arg to map conj must be a pair")
-        except TypeError as e:
-            raise TypeError(f"Cannot make map entry from {type(wrapped)}") from e
-
-        super().__init__(wrapped)
-
-    @property
-    def key(self) -> K:
-        return self[0]
-
-    @property
-    def value(self) -> V:
-        return self[1]
-
-    @staticmethod
-    def of(k: K, v: V) -> "MapEntry[K, V]":
-        return MapEntry(pvector([k, v]))
-
-    @staticmethod
-    def from_vec(v: Sequence[Union[K, V]]) -> "MapEntry[K, V]":
-        return MapEntry(pvector(v))
+_ENTRY_SENTINEL = object()
 
 
 class Map(ILispObject, IMeta, IPersistentMap[K, V]):
@@ -126,7 +95,13 @@ def dissoc(self, *ks):
                 pass
         return Map(m.persistent())
 
-    def entry(self, k, default=None):
+    def entry(self, k):
+        v = self._inner.get(k, _ENTRY_SENTINEL)
+        if v is _ENTRY_SENTINEL:
+            return None
+        return MapEntry.of(k, v)
+
+    def val_at(self, k, default=None):
         return self._inner.get(k, default)
 
     def update(self, *maps: Mapping[K, V]) -> "Map":

src/basilisp/lang/multifn.py

@@ -26,8 +26,8 @@ def __init__(
     def __call__(self, *args, **kwargs):
         key = self._dispatch(*args, **kwargs)
         method_cache = self.methods
-        method: Optional[Method] = Maybe(method_cache.entry(key, None)).or_else(
-            lambda: method_cache.entry(self._default, None)  # type: ignore
+        method: Optional[Method] = Maybe(method_cache.val_at(key, None)).or_else(
+            lambda: method_cache.val_at(self._default, None)  # type: ignore
         )
         if method:
             return method(*args, **kwargs)
@@ -49,8 +49,8 @@ def get_method(self, key: T) -> Optional[Method]:
         method_cache = self.methods
         # The 'type: ignore' comment below silences a spurious MyPy error
         # about having a return statement in a method which does not return.
-        return Maybe(method_cache.entry(key, None)).or_else(
-            lambda: method_cache.entry(self._default, None)  # type: ignore
+        return Maybe(method_cache.val_at(key, None)).or_else(
+            lambda: method_cache.val_at(self._default, None)  # type: ignore
         )
 
     @staticmethod
@@ -60,7 +60,7 @@ def __remove_method(m: lmap.Map, key: T) -> lmap.Map:
 
     def remove_method(self, key: T) -> Optional[Method]:
         """Remove the method defined for this key and return it."""
-        method = self.methods.entry(key, None)
+        method = self.methods.val_at(key, None)
         if method:
             self._methods.swap(MultiFunction.__remove_method, key)
         return method