Maintain original fn argument names when fn is def'ined

[ikappaki]

Hi,

Can you please consider a patch to preserve function argument names when a single-arity function is defined using def? It partially addresses #1212.

As mentioned in the original feature request, an increasing number of Python libraries rely on the exact parameter names of decorated functions being preserved when defined with def. Some libraries, like FastAPI, offer alternative interfaces to bypass this requirement, but others, such as Anthropic's MCP server's resources API, do not, making them incompatible with Basilisp.

This patch aims to preserve the argument names of single-arity functions defined with def in Basilisp, preventing them from becoming globally unique. The original munge'd names will be retained.

Given that this is a major blocking issue for Python interop, I kindly ask you to consider this patch or suggest alternative solutions.

I have added a test to verify the new behavior, and the codebase passing provides a strong level of confidence in the solution.

---

Alternative options considered

remove global uniqueness of arguments

As described in the feature request, it’s not immediately clear why global uniqueness is required for function parameter names. However, when attempting to preserve the names for all function definitions (not just those who are defined with def, as in this patch), the basilisp.core namespace fails to compile. Therefore, this patch is a targeted solution that only maintains original argument names for def-defined functions.

The issue causing the failure in the basilisp.core namespace stems from the inline'ing of the first function in core.lpy. Any calls to (first coll) will return nil:

(.alter-meta #'first assoc :inline (fn [seq] `(basilisp.lang.runtime/first ~seq)))

Thus, it appears there may be a requirement for global uniqueness in parameter names when inline'ing function names. While I wasn’t able to fully understand the underlying cause, I’m mentioning this in case you might know more and possibly offer a more complete solution to remove the global uniqueness requirement for all function definitions.

Thanks

[ikappaki]

Hi @chrisrink10, would be able to share your thoughts on this patch please or suggest alternative approaches I might consider? It's currently a show stopper for using Python libraries that depend on function argument names, and there doesn't appear to be a suitable workaround in Basilisp. Thanks

[chrisrink10]

As described in the feature request, it’s not immediately clear why global uniqueness is required for function parameter names. However, when attempting to preserve the names for all function definitions (not just those who are defined with def, as in this patch), the basilisp.core namespace fails to compile. Therefore, this patch is a targeted solution that only maintains original argument names for def-defined functions.

The issue causing the failure in the basilisp.core namespace stems from the inline'ing of the first function in core.lpy. Any calls to (first coll) will return nil:

(.alter-meta #'first assoc :inline (fn [seq] `(basilisp.lang.runtime/first ~seq)))

Thus, it appears there may be a requirement for global uniqueness in parameter names when inline'ing function names. While I wasn’t able to fully understand the underlying cause, I’m mentioning this in case you might know more and possibly offer a more complete solution to remove the global uniqueness requirement for all function definitions.

The Python code generated by Basilisp for that inline (with your proposed change) is here:

@runtime_24._basilisp_fn(arities=(1,))
def __lisp_fn_105(seq):
    return seq(
        concat(
            list_(sym_27.symbol("first", ns="basilisp.lang.runtime", meta=None)),
            list_(seq),
        )
    )


Var_31.find_safe(sym_27.symbol("first", ns="basilisp.core")).alter_meta(
    assoc, kw_16.keyword_from_hash(8006032063624964968, "inline"), __lisp_fn_105
)

Note how the parameter name is seq. Basilisp is trying to generate code calling the function seq with a parameter named seq. Since there is only one namespace in Python modules, the seq parameter is shadowing the seq function and causing an error. If you disable inlining, it should work.

I have an idea about how to solve this but I need to do a bit more investigation first.

[chrisrink10]

I still have some doubts about whether allowing this as the global default is the right thing to do because of cases like the above I have not been able to fully consider or test. My preference is towards having a metadata option that enables non-uniquified argument names (something like ^:allow-unsafe-names) which can be applied to a function def.

FastAPI is a particularly painful case, but I doubt this happens in enough libraries to be the global default. Furthermore, even in the examples you give in the linked ticket, using FastAPI directly with Basilisp isn't especially convenient right now:

(defn root  {:async true
             :decorators [(.get app "/")]}
  []
  {"message" "Hi there"})

It seems like someone should make a little wrapper macro which enables doing something like this automatically:

(defn hello  {:async true
              :decorators [(.get app "/hello/{name}")]}
  [^f/Request r]
  (let [nm (aget (.-path-params r) "name")]
    {"hello" nm}))

;; Could be something like
(defroute ^:async hello 
  {:route "/hello/{name}" :app app}
  [name]
  {"hello" name})

However, if we want to make it the default, we would need to add the opposite metadata key which could be applied (call it ^:gen-safe-name or something) to force the compiler to generate safe names for arguments. We would need to use this new metadata key on any inline functions generated automatically by the compiler and set it manually for any manually defined inline functions (such as first).

[ikappaki]

I still have some doubts about whether allowing this as the global default is the right thing to do because of cases like the above I have not been able to fully consider or test. My preference is towards having a metadata option that enables non-uniquified argument names (something like ^:allow-unsafe-names) which can be applied to a function def.

I've introduced the ^:allow-unsafe-names key as recommended and updated the documentation to reference this change.

FastAPI is a particularly painful case, but I doubt this happens in enough libraries to be the global default. Furthermore, even in the examples you give in the linked ticket, using FastAPI directly with Basilisp isn't especially convenient right now:

There is indeed a workaround for FastAPI using a Request-tagged parameter, which can be streamlined further with a macro, as you suggested. However, there’s no apparent solution for other libraries that don’t provide alternative APIs to bypass function parameter names inspection, such as Anhtropic's MCP Server Resource API which I mentioned in the original PR comment. This issue was actually the reason I raised the PR in the first place.

However, if we want to make it the default, we would need to add the opposite metadata key which could be applied (call it ^:gen-safe-name or something) to force the compiler to generate safe names for arguments. We would need to use this new metadata key on any inline functions generated automatically by the compiler and set it manually for any manually defined inline functions (such as first).

I'm fine with the new ^:allow-unsage-names metadata key, let me know if or when you think we're confident enough to switch to the ^:gen-safe-name suggestion.

thanks

[amano-kenji]

• Why is function inlining done? Is it done for performance optimization? Or, is it functionally necessary?
• Can ^:allow-unsafe-names work for multi-arity functions? Does it disable function inlining as well?

[ikappaki]

• Why is function inlining done? Is it done for performance optimization? Or, is it functionally necessary?

It is done for increasing performance, see inlining.

* Can `^:allow-unsafe-names` work for multi-arity functions? Does it disable function inlining as well?

It currently applies only to single-arity functions for simplicity, to support interop with Python libraries that depend on function parameter names being preserved, and does not disable inlining. Multiarity is a Basilisp-specific feature and outside the scope of the original issue, but I’ll reconsider supporting it if a real use case arises.

Ideally, as discussed above, argument names should be preserved in all cases, but there are technical challenges to consider first with inline functions.

[amano-kenji]

• What is multi-arity? I thought it meant something like (defn a-function [a b c] (do-something)). I think python supports something like def a-function(a, b, c):.
• I think a function is not inlined if it is not annotated with :inline meta key. I theorize that when inlining is not done with :inline meta key, basilisp can avoid changing function argument names. Then, something like :^allow-unsafe-names is not necessary because you wouldn't inline a function used by FastAPI and MCP server resources API. I think the issue was caused because function argument names were changed in the absence of :inline meta key. If my theory is true, then I would eliminate :^allow-unsafe-names, avoid changing function argument names in the absence of :inline meta key, and document the fact that :inline meta key can break FastAPI or anything that requires function argument names to stay unchanged.

[chrisrink10]

• What is multi-arity? I thought it meant something like (defn a-function [a b c] (do-something)). I think python supports something like def a-function(a, b, c):.

Multi-arity functions are those functions which support multiple discrete numbers of arguments. For instance keyword defines two distinct arities:

Usage: (keyword name)
       (keyword ns name)

[chrisrink10]

• I think a function is not inlined if it is not annotated with :inline meta key. I theorize that when inlining is not done with :inline meta key, basilisp can avoid changing function argument names. Then, something like :^allow-unsafe-names is not necessary because you wouldn't inline a function used by FastAPI and MCP server resources API. I think the issue was caused because function argument names were changed in the absence of :inline meta key. If my theory is true, then I would eliminate :^allow-unsafe-names, avoid changing function argument names in the absence of :inline meta key, and document the fact that :inline meta key can break FastAPI or anything that requires function argument names to stay unchanged.

It isn't that a function has ^:inline but rather the function generated or attached as the ^:inline function to another function. Since it is not knowable if a function is the ^:inline function for another function, we cannot generically detect this case as you suggest.

The function I showed here is the ^:inline function attached to first.

[chrisrink10]

I would consider this feature to be provisional or experimental until the correct solution emerges.