falco
falco copied to clipboard
Feature/implement formatter
This PR aims to provide formatter feature in falco.
Usage (command)
I added a new fmt
subcommand.
## multiple files
falco fmt /path/to/defaut.vcl /path/to/module.vcl ...
## Supports glob pattern
falco fmt /path/to/**/*.vcl
falco outputs formatted result to stdout, but overwrites the input file by providing -w, --write
argument.
falco fmt -w /path/to/default.vcl
Configuration
Formatting rules could be customized in format
section .falco.yaml
configuration file:
...
format:
[some override configurations here]
...
Implemented formatting rules are described discussion Now I have tested with huge VCLs that are used for production, it seems fine.
And I put some comments on this PR, please take a look also.
Haven't started looking through the implementation yet but tested this against my production VCL and it produced a lot of problems. They all seem to stem from 4 types of errors in the formatted output.
Input:
sub boolfn BOOL {
return true;
}
sub vcl_recv {
if (
// foo
req.http.foo == "bar"
// bar
&& req.http.bar == "baz"
) {
esi;
} elseif (req.http.foo) {
esi;
}
}
Output:
sub boolfn { // <-- 1) return type removed
return (true); // <-- 2) parens added around return value
}
sub vcl_recv {
if (// foo req.http.foo == "bar" // bar && req.http.bar == "baz") { // <-- 3) multi-line conditional with line comments collapsed
esi;
} ( (req.http.foo) { // <-- 4) `elseif` replaced with `(`
esi;
}
}
These were the issues that produced errors, will look through the output for other non-error causing formatting issues when I have some more time.
Ah, these are some cases that I have not considered, thanks I will look and fix it.
Got a start on looking through the implementation this morning and will take a look at the updates as well when I can.
In the meantime I noticed a few formatting issues that don't cause syntax errors like the previous examples I shared.
Input:
sub unary_prefix_operator {
if (!req.http.foo) { esi; }
declare local var.i INTEGER;
set var.i = -5;
}
director unary_postfix_operator client {
.quorum = 20 %;
}
sub switch_formatting {
switch (req.http.foo) {
case "1":
esi;
break;
default: break; case "2": esi; break;
}
}
Formatted output:
sub unary_prefix_operator {
if (! req.http.foo) { <- space inserted between unary prefix operator and operand
esi;
}
declare local var.i INTEGER;
set var.i = - 5; <- space inserted
}
<- extra newline inserted between declarations, is this intended?
director unary_postfix_operator client {
.quorum = 20%; <- in comparison for the postfix operator no space inserted (which is what I would expect)
}
sub switch_formatting {
switch (req.http.foo) {
case "1": <- case indented
esi; <- not indented relative to case statement
break; <- indented correctly relative to case statement but too far relative to the previous line
default:
break;
case "2":
esi;
break;
}
}
I found a problem in PrefixExpression, I will fix it.
My preference is that case
statement should be indented but I will add Indent case labels
whether indent or not as you said in Discussion.
While doing some more looking at the results of formatting various real world VCL files I notice comments getting lost in a few places. Given how critical it is for a formatter to not lose anything I started looking in more detail at how comment attachment is currently handled and all the places comments can be placed.
This led to what I wrote up in #299 and the test PR #300.
I hope to have some free time this weekend to get back to reviewing this.
@richardmarshall Could you take a look #302 before? This PR improves comment dealing with more complexity.
This PR could be merged by fixing some comment issues. I will merge this in a few days so please make another PRs if you find any problems.