Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

:use is removed from ns declarations #152

Closed
pbaille opened this issue Nov 9, 2024 · 7 comments
Closed

:use is removed from ns declarations #152

pbaille opened this issue Nov 9, 2024 · 7 comments
Labels
bug Something isn't working namespace related to ns form parsing or printing v1 blocker Required for a v1.0.0 release

Comments

@pbaille
Copy link

pbaille commented Nov 9, 2024

I've been surprised by this behavior:

echo '(ns my.company.core (:use my-thing))' | standard-clj fix -
=> (ns my.company.core)

It removes the :use statement.
It seems a bit radical in my opinion, of course using :use is rarely justified but should it at least be replaced by a :refer :all ?

A part from that I really enjoy this formatter for now, so thank you :)

@oakmac
Copy link
Owner

oakmac commented Nov 9, 2024

I have been waiting for someone to open an issue about this 😉 I have "support use" as a TODO item on Issue #5 and I never got around to finishing it.

I am not sure of the best behavior for the formatter here. I consider the current behavior to be wrong / a bug. Two thoughts:

  • have the formatter error out when it encounters a use statement in the ns form and say "Standard Clojure Style does not support this. Please refactor using refer and try again."
  • Attempt to safely convert the use statement into valid require forms that work in the same way.

I think the second option is the better user experience, but I am not sure how challenging it will be to code up. The first option is not great, but better than the current behavior.

This comment on Issue #142 is relevant. It may seem odd that a formatter recommends the user to "please refactor", but presumably if the user is using this tool then they desire the end state for their code to be something produced by the tool, and I do not have plans to add use into the pretty-printed ns form.


A part from that I really enjoy this formatter for now, so thank you :)

Thank you 😁 🎉

@oakmac oakmac added bug Something isn't working v1 blocker Required for a v1.0.0 release namespace related to ns form parsing or printing labels Nov 9, 2024
@NoahTheDuke
Copy link

I do not have plans to add use into the pretty-printed ns form.

Why not? There is a lot of code that uses :use and some people still prefer it to :require (especially with clojure.test).

@oakmac
Copy link
Owner

oakmac commented Nov 10, 2024

Why not?

My main reasoning is that use is not recommended by how to ns and my general understanding is that require is preferred over use for basically all use cases. Is there a situation in which use is preferable to require in the ns form?

@NoahTheDuke
Copy link

It's not about preference, it's about supporting existing working code. (For example, Clojure core is filled with :use, and that won't ever change.)

It's okay if Standard is only for the subset of Clojure you like, but I think that should be made more clear.

@oakmac
Copy link
Owner

oakmac commented Nov 11, 2024

It's not about preference, it's about supporting existing working code. (For example, Clojure core is filled with :use, and that won't ever change.)

Some thoughts:

The current behavior is definitely a bug and needs to be fixed with either of the two options I outlined earlier. Option 1 is probably the easiest to code up right now, and Option 2 is a superior user experience.

My understanding is that :require is always preferred over :use inside the ns form. I have no expectation that Clojure core will stop supporting :use, but that does not mean it is recommended for newer code. The authors of ClojureScript decided not to include :use in the language, for example. To the degree that Standard Clojure Style can make a codebase better, that is the right direction to move towards.

Standard JS is a good reference point here. == and eval() are valid parts of the JavaScript language that will likely always be supported, but that does not mean they are recommended to be used. It is ok to say "This feature of a language was maybe not the best idea. As a team we are ok with not using it."

@NoahTheDuke
Copy link

NoahTheDuke commented Dec 2, 2024

I think deciding to not support formatting :use makes sense: it's out of favor, there are better ways to write it, etc. However, as seen in the original post, deleting it leads to broken code, which is bad imo. A formatter should not lead to broken code under any circumstances, regardless of what is or is not favored.

As an example, does standard also delete (use 'x.y.z) forms?

@oakmac
Copy link
Owner

oakmac commented Dec 28, 2024

With PR-171, Standard Clojure Style will now throw an error recommending to refactor using :require instead of silently deleting :use inside of the ns form.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working namespace related to ns form parsing or printing v1 blocker Required for a v1.0.0 release
Projects
None yet
Development

No branches or pull requests

3 participants