MarketSquare / robotframework-tidy

Robot Framework code formatter
https://robotidy.readthedocs.io/
Apache License 2.0
103 stars 17 forks source link

Transformers should be sorted by user-given order #9

Closed bhirsz closed 3 years ago

bhirsz commented 3 years ago

Provide a way to run code transformers in set order. Depending on the order could end up in different state (ie first aligning keywords to column, then splitting too long lines to multiline or other way around can produce different results).

Order is determined on order parameter (ascending, higher number will be executed later). Transformer with the same order are executed in order depending on transformer name. Internal transformers should have order starting from 1. It will allow external transformers to use 0 to run them before internal ones.

It should be settable using @transformer:

@transformer(order=3)

And also it should be possible to override it from cli:

--transform ImportantToRunFirst:order=0
mnojek commented 3 years ago

Do we really want a tool to transform a code differently based on a different order of the transformers? Shouldn't it produce the same code with the same transformers enabled?

bhirsz commented 3 years ago

The problem is that it's not that we want it but it will happen. For example we have following code:

*** Settings ***
Setup  Keyword

*** Keywords ***

and we run two transformers: DiscardEmptySections and AdjustWhitespace (temporary name). We want to have two lines seperation between sections and one empty line after last line. If we run it in this order we get:

*** Settings ***
Setup  Keyword

but if we execute it in reversed order:

*** Settings ***
Setup  Keyword

We have one line too much. That's why some of the transformers needs to run before others. Although I would of course also prefer for transformers to be independent.

mnojek commented 3 years ago

What about making some of the transformers more smart and if there is this DiscardEmptySections transformer removing an empty section than it should also adjust empty lines after removing it. I think that the code should not look like differently depending on the order of running the transformers. I just want to run it without thinking what should be done in which order to receive the code that I want. I want it to be deterministic and predictable. I can even let the transformer run several times if it's needed for the code to look the same after each execution.

bhirsz commented 3 years ago

Yes I agree that predictable and deterministic is better. I have few possible solution for more 'smart' solutions but I probably overthink the whole problem. We should get back to it after we develop more transformers too see how big the actual problem is.

And you can already change/ensure order by running robotidy multiple times:

robotidy --transform DiscardEmptySections
robotidy --transform AdjustWhitespace

We could also add run_after=[list_of_transfromers] (or run_before) in @transformer as way of ensuring some transformers is executed after others. Let's wait with choosing any approach until we get more transformers in place.

bhirsz commented 3 years ago

Closing this issue. Robotidy currently runs all transformers in some predefined order (which minimize chance that some transformer will undo work of the other transformer). If someone runs only selected transformers they're still run in the order of default transformers. For now there is no need for option to define order of transformer - and we still can just run robotidy with one transformer at a time if there is such need.