Maintain original fn argument names when fn is def'ined (#1222)

Add support for a new metadata key `^:allow-unsafe-names` which can be applied to single arity functions to disable munging and generating globally safe parameter names. This enables support for using libraries such as FastAPI and others which depend on the raw Python parameter names of functions.

Author: ikappaki

CHANGELOG.md

@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Changed
+    * Single arity functions can be tagged with `^:allow-unsafe-names` to preserve their parameter names (#1212)
 
 ## [v0.3.7]
 ### Fixed 

docs/compiler.rst

@@ -205,4 +205,4 @@ The former can be configured via the environment variable ``BASILISP_USE_DEV_LOG
 .. code-block:: bash
 
    export BASILISP_USE_DEV_LOGGER=true
-   export BASILISP_LOGGING_LEVEL=INFO
+   export BASILISP_LOGGING_LEVEL=INFO

docs/pyinterop.rst

@@ -10,7 +10,7 @@ Basilisp features myriad options for interfacing with host Python code.
 Name Munging
 ------------
 
-Per Python's `PEP 8 naming conventions <https://www.python.org/dev/peps/pep-0008/#naming-conventions>`_, Python method and function names frequently use ``snake_case``.
+Per Python's `PEP 8 naming conventions <https://www.python.org/dev/peps/pep-0008/#naming-conventions>`_, Python method and function and parameter names frequently use ``snake_case``.
 Basilisp is certainly capable of reading ``snake_case`` names without any special affordance.
 However, Basilisp code (like many Lisps) tends to prefer ``kebab-case`` for word separation.
 
@@ -22,6 +22,14 @@ When compiled, a ``kebab-case`` identifier always becomes a ``snake_case`` ident
 
    The Basilisp compiler munges *all* unsafe Basilisp identifiers to safe Python identifiers, but other cases are unlikely to appear in standard Python interop usage.
 
+.. note::
+
+   By default, the compiler munges function parameter names and makes them globally unique by appending a monotically increasing number suffix to support function inlining. To enable interop with Python libraries that rely on preserved parameter names, you can use the ```^:allow-unsafe-names`` metadata key to retain the (munged) parameter names. This flag only applies to single-arity Basilisp functions.
+
+   .. code-block::
+
+      (defn ^:allow-unsafe-names afun [a b] ...)
+
 .. _python_builtins:
 
 Python Builtins

src/basilisp/lang/compiler/constants.py

@@ -37,6 +37,7 @@ class SpecialForm:
 
 SYM_ABSTRACT_META_KEY = kw.keyword("abstract")
 SYM_ABSTRACT_MEMBERS_META_KEY = kw.keyword("abstract-members")
+SYM_ALLOW_UNSAFE_NAMES_META_KEY = kw.keyword("allow-unsafe-names")
 SYM_ASYNC_META_KEY = kw.keyword("async")
 SYM_KWARGS_META_KEY = kw.keyword("kwargs")
 SYM_PRIVATE_META_KEY = kw.keyword("private")

src/basilisp/lang/compiler/generator.py

@@ -37,6 +37,7 @@
     INTERFACE_KW,
     OPERATOR_ALIAS,
     REST_KW,
+    SYM_ALLOW_UNSAFE_NAMES_META_KEY,
     SYM_DYNAMIC_META_KEY,
     SYM_REDEF_META_KEY,
     VAR_IS_PROTOCOL_META_KEY,
@@ -110,7 +111,7 @@
     ast_ClassDef,
     ast_FunctionDef,
 )
-from basilisp.lang.interfaces import IMeta, ISeq
+from basilisp.lang.interfaces import IMeta, IPersistentMap, ISeq
 from basilisp.lang.runtime import CORE_NS
 from basilisp.lang.runtime import NS_VAR_NAME as LISP_NS_VAR
 from basilisp.lang.runtime import BasilispModule, Var
@@ -649,6 +650,21 @@ def with_lineno_and_col(
     return with_lineno_and_col
 
 
+MetaNode = Union[Const, MapNode]
+
+
+def _is_allow_unsafe_names(fn_meta_node: Optional[MetaNode]) -> bool:
+    """Return True if the `fn_meta_node` has the meta key set to
+    retain functio parameter names.
+
+    """
+    return (
+        bool(fn_meta_node.form.val_at(SYM_ALLOW_UNSAFE_NAMES_META_KEY, False)) is True
+        if fn_meta_node is not None and isinstance(fn_meta_node.form, IPersistentMap)
+        else False
+    )
+
+
 def _is_dynamic(v: Var) -> bool:
     """Return True if the Var holds a value which should be compiled to a dynamic
     Var access."""
@@ -896,7 +912,9 @@ def _def_to_py_ast(  # pylint: disable=too-many-locals
         assert node.init is not None  # silence MyPy
         if node.init.op == NodeOp.FN:
             assert isinstance(node.init, Fn)
-            def_ast = _fn_to_py_ast(ctx, node.init, def_name=defsym.name)
+            def_ast = _fn_to_py_ast(
+                ctx, node.init, def_name=defsym.name, meta_node=node.meta
+            )
             is_defn = True
         elif (
             node.init.op == NodeOp.WITH_META
@@ -1622,9 +1640,6 @@ def _synthetic_do_to_py_ast(
     )
 
 
-MetaNode = Union[Const, MapNode]
-
-
 def __fn_name(ctx: GeneratorContext, s: Optional[str]) -> str:
     """Generate a safe Python function name from a function name symbol.
 
@@ -1640,16 +1655,27 @@ def __fn_name(ctx: GeneratorContext, s: Optional[str]) -> str:
 
 
 def __fn_args_to_py_ast(
-    ctx: GeneratorContext, params: Iterable[Binding], body: Do
+    ctx: GeneratorContext,
+    params: Iterable[Binding],
+    body: Do,
+    allow_unsafe_param_names: bool = True,
 ) -> tuple[list[ast.arg], Optional[ast.arg], list[ast.stmt], Iterable[PyASTNode]]:
-    """Generate a list of Python AST nodes from function method parameters."""
+    """Generate a list of Python AST nodes from function method parameters.
+
+    Parameter names are munged and modified to ensure global
+    uniqueness by default.  If `allow_unsafe_param_names` is set to
+    True, the original munged parameter names are retained instead.
+
+    """
     fn_args, varg = [], None
     fn_body_ast: list[ast.stmt] = []
     fn_def_deps: list[PyASTNode] = []
     for binding in params:
         assert binding.init is None, ":fn nodes cannot have binding :inits"
         assert varg is None, "Must have at most one variadic arg"
-        arg_name = genname(munge(binding.name))
+        arg_name = munge(binding.name)
+        if not allow_unsafe_param_names:
+            arg_name = genname(arg_name)
 
         arg_tag: Optional[ast.expr]
         if (
@@ -1786,8 +1812,14 @@ def __single_arity_fn_to_py_ast(  # pylint: disable=too-many-locals
                 sym.symbol(lisp_fn_name), py_fn_name, LocalType.FN
             )
 
+        # check if we should preserve the original parameter names
+        allow_unsafe_param_names = _is_allow_unsafe_names(meta_node)
+
         fn_args, varg, fn_body_ast, fn_def_deps = __fn_args_to_py_ast(
-            ctx, method.params, method.body
+            ctx,
+            method.params,
+            method.body,
+            allow_unsafe_param_names=allow_unsafe_param_names,
         )
         meta_deps, meta_decorators = __fn_meta(ctx, meta_node)
 

tests/basilisp/compiler_test.py

@@ -7186,3 +7186,57 @@ def test_yield_as_coroutine_with_multiple_yields(
         assert kw.keyword("coroutine-value") == state.deref()
         assert None is next(coro, None)
         assert kw.keyword("done") == state.deref()
+
+
+def test_defn_argument_names(lcompile: CompileFn):
+    # By default, function parameter names are made globally unique.
+    #
+    # For example, notice how defn generate parameter names in the
+    # pattern <parameter-name>_<monotonically-increasing-number> to
+    # ensure global uniqueness.
+    code = """
+    (defn test_dfn0 [a b] 5)
+    """
+    fvar = lcompile(code)
+    args = list(inspect.signature(fvar.deref()).parameters.keys())
+    assert all(
+        re.fullmatch(p, s) for p, s in zip([r"a_\d+", r"b_\d+"], args)
+    ), f"unexpected argument names {args}"
+
+    code = """
+    (defn ^:allow-unsafe-names test_dfn1a [a b] 5)
+    """
+    fvar = lcompile(code)
+    args = list(inspect.signature(fvar.deref()).parameters.keys())
+    assert args == ["a", "b"]
+
+    code = """
+    (defn ^{:allow-unsafe-names false} test_dfn1b [a b] 5)
+    """
+    fvar = lcompile(code)
+    args = list(inspect.signature(fvar.deref()).parameters.keys())
+    assert all(
+        re.fullmatch(p, s) for p, s in zip([r"a_\d+", r"b_\d+"], args)
+    ), f"unexpected argument names {args}"
+
+    code = """
+    (def ^:allow-unsafe-names test_dfn2 (fn [a b & c] 5))
+    """
+    fvar = lcompile(code)
+    args = list(inspect.signature(fvar.deref()).parameters.keys())
+    assert args == ["a", "b", "c"]
+
+    code = """
+    (def test_dfn3 ^:allow-unsafe-names (fn [a b & c] 5))
+    """
+    fvar = lcompile(code)
+    args = list(inspect.signature(fvar.deref()).parameters.keys())
+    assert args == ["a", "b", "c"]
+
+    code = """
+    ^{:kwargs :collect}
+    (defn ^:allow-unsafe-names test_dfn4 [a b {:as xyz}] 5)
+    """
+    fvar = lcompile(code)
+    args = list(inspect.signature(fvar.deref()).parameters.keys())
+    assert args == ["a", "b", "xyz"]