gleam-lang / gleam

⭐️ A friendly language for building type-safe, scalable systems!
https://gleam.run
Apache License 2.0
17.48k stars 725 forks source link

Code action: Convert variable names to snake_case #2978

Closed jfmengels closed 2 months ago

jfmengels commented 5 months ago

Coming from other languages like Elm and JavaScript where the conventions are to have variables named in camel-case.

In Gleam, camel-case is forbidden and results in syntax errors, which I can very much understand as it should make for very consistent naming patterns. My force of habit is strong, and I spend quite some time converting the camel-case names I write to snake-case, which is distracting from my goal at hand.

A suggestion I have that could make that easier, is have the formatter automatically convert names to snake-case.

For instance, the formatter could change the code like this

fn doSomething() { todo }
-->
fn do_something() { todo }

The compiler error already suggests this name change in the error message.

Same idea for references to consts/functions in expressions.

If this can be done reliably, then I can't think of many downsides (but I may not be familiar enough with the syntax).

One downside I can imagine is that if you've never written Gleam, and you don't ever see the error message that variables should be snake_case, then it will feel quite jarring to see a rename. Once you know it, I think it will be helpful though.

jfmengels commented 5 months ago

Alternatively, this could be an LSP code action. Requires more manual work from the user but it would be less surprising.

Some editors sometimes have a "Convert to snake_case" command which solves the problem, but it's not necessarily discoverable. Took me a few days to discover mine had that. An LSP code action would be a lot more discoverable.

lpil commented 5 months ago

Sounds good to me! I think we would only want to do this for definitions and not for referencing variables and functions. What do you think?

We'll need to make the parser permit uppercase names have the analyser emit an error if one is used in a definition.

jfmengels commented 5 months ago

I could see it used for both definitions and references, but I do imagine this being more surprising or annoying for references. I think it would be reasonable to start only with definitions, and at some point to have a branch where it also does it for references, and try to see how it feels.

Although it should probably be tried out by beginners, as seasoned Gleam developers will likely not write camelcase already (unless they also often work with camelcase languages).

lpil commented 5 months ago

For references it could "correct" to a value that type checks but isn't the value that the programmer intended to use. I'd like to not have anything that could make the programmer's code silently do the wrong thing.

GearsDatapacks commented 3 months ago

I might try and tackle this one. Should I implement it in the formatter or in as an LSP action?

And should I take a look at what was suggested in #2998, and change this error to occur in the analyser?

GearsDatapacks commented 3 months ago

Thinking about this more, I think this should be an lsp action, since the job of the formatter isn't really to fix errors in code.

lpil commented 3 months ago

Yes I think so too!

GearsDatapacks commented 3 months ago

One question about the implementation of this: Should I unify the three name token types and only discriminate between them later, or keep them and just not report any errors until the analyser?

lpil commented 3 months ago

Keep them distinct please. The change to the codebase should be small.