StyLua icon indicating copy to clipboard operation
StyLua copied to clipboard

Sort requires

Open jeparlefrancais opened this issue 3 years ago • 6 comments

It would be really neat if stylua could re-order my requires in alphabetical order.

It should be an optional parameter because stylua can't assume that require does not have side effects, but typically it's good practice to avoid them, so generally it's fine 🙂

jeparlefrancais avatar Feb 08 '22 21:02 jeparlefrancais

I sort my requires alphabetically but it's easy to forget sometimes. This would enforce a style which would be great for collaborative projects. It would also be great if StyLua sorted services alphabetically.

nezuo avatar Feb 08 '22 22:02 nezuo

@JohnnyMorganz I'd like to tackle this issue if you don't mind, but I think I'll need some help to start. Can we discuss about the solution?

wesleimp avatar Feb 11 '22 14:02 wesleimp

@JohnnyMorganz I'd like to tackle this issue if you don't mind, but I think I'll need some help to start. Can we discuss about the solution?

@wesleimp I'm definitely open to contributions!

I think the first thing that needs to be discussed though is the potential impact of something like this. Sorting requires is quite invasive, and I know its something prettier does not involve itself with (black also doesn't do it, but I guess thats because there is python's isort).

Also, there are potential problems of correctness. Sometimes requires can be impure and as such the ordering has to be maintained (but to be fair, should we concern ourselves with this? It is most likely a sign of weird code). Similarly, requires which don't take a constant string and depend on previously defined variables also need to be considered. Will need to think about all other possible issues

It would also be great if StyLua sorted services alphabetically.

@nezuo This is a very Roblox-specific thing. Currently, stylua doesn't really have roblox-specific handling (apart from Luau, which is technically open for anyone to use). I do agree that ordering services would be quite nice (and also probably defining all services before all requires), but stylua has currently tried to be codebase-agnostic, and this would change that. Would need thinking about as well (the luau feature flag does not necessarily imply its a Roblox codebase)

JohnnyMorganz avatar Feb 11 '22 18:02 JohnnyMorganz

Sometimes requires can be impure and as such the ordering has to be maintained (but to be fair, should we concern ourselves with this? It is most likely a sign of weird code

I'm sure requiring files because of side-effects has their own use-cases. And re-ordering those side-effects would end up changing what the code was supposed to do.

Will need to think about all other possible issues

If stylua starts changing the codes in this way, what's preventing us from wanting to sort tables, function definitions, variable declarations and all sort of stuffs? That would result in chaos. Gotta draw the line somewhere.


I think this should be a part of LSP, not the formatter.

MunifTanjim avatar Jul 23 '22 12:07 MunifTanjim

If stylua starts changing the codes in this way, what's preventing us from wanting to sort tables, function definitions, variable declarations and all sort of stuffs? That would result in chaos. Gotta draw the line somewhere.

I don't think this should be a major concern. Drawing a line somewhere, its not hard; StyLua is an opinionated formatter. The only hard part in this is deciding on a place to draw the line that works best.

The most compelling issue for me is that of correctness. If we could guarantee imports were pure, there would be no problem sorting them. Since they aren't, it presents a major problem, and it should probably be done by a different tool.

LastTalon avatar Jul 23 '22 16:07 LastTalon

Having thought I while about this, I agree that this sort of feature probably belongs in a separate tool.

The added bloat to stylua is large, and I don't think it fits correctly into this project.

The main disadvantage here is that we lose out on some of the things stylua has already solved, mainly on the CLI side. Maybe I should move more of this stuff over into library code...

JohnnyMorganz avatar Jul 26 '22 21:07 JohnnyMorganz

Going to close this as not planned for now. I think it either belongs in another tool (like isort), or as part of a language server (maybe I'll add it to https://github.com/JohnnyMorganz/luau-lsp)

JohnnyMorganz avatar Feb 09 '23 19:02 JohnnyMorganz

For anyone interested, this functionality has been submitted to Luau Language Server and should hopefully be available in one of the next releases: https://github.com/JohnnyMorganz/luau-lsp/pull/294

JohnnyMorganz avatar Feb 11 '23 16:02 JohnnyMorganz

Following some external motivation, I've reconsidered again, and I think its actually fair to add this to stylua. Two things: it will be off by default (users subscribe into requires sorting, and if they do they accept that their requires are pure), and a lot simpler than originally planned (no need for excessive configuration).

The main basis is that:

  • We group consecutive requires into a "block", if we hit a line which is a non-require or empty, we close the old block and start a new one
  • Requires are sorted only within their block. They do not move around the file. This allows us to solve the assumption of depending on local variables (if there is a local variable in between requires, it would split them into two separate blocks, so a require will always be after any local variable it uses)

JohnnyMorganz avatar Feb 11 '23 20:02 JohnnyMorganz