Repository: domluna/JuliaFormatter.jl
Pull request #89 (2019-10-05 03:57:10)

Options - Always use `for in`
Adds an initial option `always_for_in` which always uses `in` for `for` loops, by default this will be false. I think this is a very reasonable one to have, some people REALLY like always using `in` and I don't want them not using the formatter solely because it doesn't support this.

TODO

- [x] update README
- [x] update GH action script

nickrobinson251: Maybe this is a sign that the default isn't quite right for most people?

domluna: Perhaps. It's somewhat related to #85 but there's been a `=` vs `in` saga #34 #70. It's no secret the default is a little controversial so at least now there will be an, in some cases, much more desired alternative.

Patch:
diff --git a/README.md b/README.md
index 0d565cec..b108712d 100644
--- a/README.md
+++ b/README.md
@@ -12,9 +12,30 @@ Width-sensitive formatter for Julia code. Inspired by gofmt and refmt.
 `JuliaFormatter` exports `format_text`, `format_file` and `format`:
 
 ```julia
-format_text(text::AbstractString; indent = 4, margin = 92)
-format_file(file::AbstractString; indent = 4, margin = 92, overwrite = true, verbose = false)
-format(paths...; indent = 4, margin = 92, overwrite = true, verbose = false)
+format_text(
+    text::AbstractString;
+    indent = 4,
+    margin = 92,
+    always_for_in = false,
+)
+
+format_file(
+    file::AbstractString;
+    indent = 4,
+    margin = 92,
+    overwrite = true,
+    verbose = false,
+    always_for_in = false,
+)
+
+format(
+    paths...;
+    indent = 4,
+    margin = 92,
+    overwrite = true,
+    verbose = false,
+    always_for_in = false,
+)
 ```
 
 The `text` argument to `format_text` is a string containing the code to be formatted; the formatted code is retuned as a new string. The `file` argument to `format_file` is the path of a file to be formatted. The `format` function is either called with a singe string to format if it is a `.jl` file or to recuse into looking for `.jl` files if it is a directory. It can also be called with a collection of such paths to iterate over.
@@ -27,6 +48,7 @@ the limit will be wrapped if possible. There are cases where lines cannot be wra
 and they will still end up wider than the requested margin.
 * `overwrite` - If the file should be overwritten by the formatted output. If set to false, the formatted version of file named `foo.jl` will be written to `foo_fmt.jl`.
 * `verbose` - Whether to print the name of the file being formatted along with relevant details to `stdout`.
+* `always_for_in` - Always use `in` keyword for `for` loops. This defaults to `false`.
 
 There is also a command-line tool `bin/format.jl` which can be invoked with `-i`/`--indent` and `-m`/`--margin` and with paths which will be passed to `format`.
 
@@ -182,7 +204,9 @@ end
 
 ### `for in` vs. `for =`
 
-If the RHS is a range, i.e. `1:10` then `for in` is converted to `for =`. Otherwise `for =` is converted to `for in`. See [this issue](https://github.com/domluna/JuliaFormatter.jl/issues/34) for the rationale and further explanation.
+By default if the RHS is a range, i.e. `1:10` then `for in` is converted to `for =`. Otherwise `for =` is converted to `for in`. See [this issue](https://github.com/domluna/JuliaFormatter.jl/issues/34) for the rationale and further explanation.
+
+Alternative to the above - setting `always_for_in` to `true`, i.e. `format_text(..., always_for_in = true)` will always convert `=` to `in` even if the RHS is a range.
 
 ### Trailing Commas
 
diff --git a/bin/format.jl b/bin/format.jl
index cb2e8c3c..efef17cd 100755
--- a/bin/format.jl
+++ b/bin/format.jl
@@ -29,6 +29,9 @@ Flags:
     -o, --overwrite
         Writes the formatted source to a new file where the original
         filename is suffixed with _fmt, i.e. `filename_fmt.jl`.
+    --always_for_in
+        Always replaces `=` with `in` for `for` loops.
+        For example, `for i = 1:10` will be transformed to `for i in 1:10`.
 """
 
 function parse_opts!(args::Vector{String})
@@ -50,10 +53,12 @@ function parse_opts!(args::Vector{String})
             opt = :help
         elseif arg == "-o" || arg == "--overwrite"
             opt = :overwrite
+        elseif arg == "--always_for_in"
+            opt = :always_for_in
         else
             error("invalid option $arg")
         end
-        if opt == :verbose || opt == :help
+        if opt in (:verbose, :help, :always_for_in)
             opts[opt] = true
             deleteat!(args, i)
         elseif opt == :overwrite
diff --git a/src/JuliaFormatter.jl b/src/JuliaFormatter.jl
index 0527ee8f..8892350e 100644
--- a/src/JuliaFormatter.jl
+++ b/src/JuliaFormatter.jl
@@ -106,8 +106,10 @@ mutable struct State
     offset::Int
     line_offset::Int
     margin::Int
+    always_for_in::Bool
 end
-State(doc, indent_size, margin) = State(doc, indent_size, 0, 1, 0, margin)
+State(doc, indent_size, margin; always_for_in = false) =
+    State(doc, indent_size, 0, 1, 0, margin, always_for_in)
 
 @inline nspaces(s::State) = s.indent
 @inline hascomment(d::Document, line::Integer) = haskey(d.comments, line)
@@ -130,8 +132,9 @@ include("print.jl")
 """
     format_text(
         text::AbstractString;
-        indent::Integer = 4,
-        margin::Integer = 92,
+        indent::Int = 4,
+        margin::Int = 92,
+        always_for_in::Bool = false,
     ) :: String
 
 Formats a Julia source passed in as a string, returning the formatted
@@ -140,11 +143,17 @@ code as another string. The formatting options are:
 - `indent` which defaults to 4 spaces
 - `margin` which defaults to 92 columns
 
+If `always_for_in` is true `=` is always replaced with `in` if part of a
+`for` loop condition.  For example, `for i = 1:10` will be transformed
+to `for i in 1:10`.
 """
-function format_text(text::AbstractString; indent::Integer = 4, margin::Integer = 92)
-    if isempty(text)
-        return text
-    end
+function format_text(
+    text::AbstractString;
+    indent::Int = 4,
+    margin::Int = 92,
+    always_for_in::Bool = false,
+)
+    isempty(text) && return text
 
     x, ps = CSTParser.parse(CSTParser.ParseState(text), true)
     ps.errored && error("Parsing error for input $text")
@@ -154,9 +163,9 @@ function format_text(text::AbstractString; indent::Integer = 4, margin::Integer
 
     d = Document(text)
     # If "nofmt" occurs in a comment on line 1 do not format
-    occursin("nofmt", get(d.comments, 1, (0, ""))[2]) && (return text)
+    occursin("nofmt", get(d.comments, 1, (0, ""))[2]) && return text
 
-    s = State(d, indent, margin)
+    s = State(d, indent, margin, always_for_in = always_for_in)
     t = pretty(x, s)
     hascomment(s.doc, t.endline) && (add_node!(t, InlineComment(t.endline), s))
     nest!(t, s)
@@ -190,6 +199,7 @@ end
         margin::Integer = 92,
         overwrite::Bool = true,
         verbose::Bool = false,
+        always_for_in::Bool = false,
     )
 
 Formats the contents of `filename` assuming it's a Julia source file.
@@ -205,6 +215,10 @@ be written to `foo_fmt.jl` instead.
 
 If `verbose` is `true` details related to formatting the file will be printed
 to `stdout`.
+
+If `always_for_in` is true `=` is always replaced with `in` if part of a
+`for` loop condition.  For example, `for i = 1:10` will be transformed
+to `for i in 1:10`.
 """
 function format_file(
     filename::AbstractString;
@@ -212,6 +226,7 @@ function format_file(
     margin::Integer = 92,
     overwrite::Bool = true,
     verbose::Bool = false,
+    always_for_in::Bool = false,
 )
     path, ext = splitext(filename)
     if ext != ".jl"
@@ -219,7 +234,7 @@ function format_file(
     end
     verbose && println("Formatting $filename with indent = $indent, margin = $margin")
     str = read(filename) |> String
-    str = format_text(str, indent = indent, margin = margin)
+    str = format_text(str, indent = indent, margin = margin, always_for_in = always_for_in)
     overwrite ? write(filename, str) : write(path * "_fmt" * ext, str)
     nothing
 end
@@ -231,6 +246,7 @@ end
         margin::Integer = 92,
         overwrite::Bool = true,
         verbose::Bool = false,
+        always_for_in::Bool = false,
     )
 
 Recursively descend into files and directories, formatting and `.jl`
diff --git a/src/pretty.jl b/src/pretty.jl
index de2a02f1..ada52d9e 100644
--- a/src/pretty.jl
+++ b/src/pretty.jl
@@ -885,10 +885,16 @@ end
 # for i = 1:10 body end
 #
 # https://github.com/domluna/JuliaFormatter.jl/issues/34
-function eq_to_in_normalization!(x)
+function eq_to_in_normalization!(x, always_for_in)
     if x.typ === CSTParser.BinaryOpCall
         op = x.args[2]
         arg2 = x.args[3]
+
+        if always_for_in
+            x.args[2].kind = Tokens.IN
+            return
+        end
+
         if op.kind === Tokens.EQ && !is_colon_op(arg2)
             x.args[2].kind = Tokens.IN
         elseif op.kind === Tokens.IN && is_colon_op(arg2)
@@ -896,7 +902,7 @@ function eq_to_in_normalization!(x)
         end
     elseif x.typ === CSTParser.Block || x.typ === CSTParser.InvisBrackets
         for a in x.args
-            eq_to_in_normalization!(a)
+            eq_to_in_normalization!(a, always_for_in)
         end
     end
 end
@@ -907,7 +913,7 @@ function p_loop(x, s)
     add_node!(t, pretty(x.args[1], s), s)
     add_node!(t, Whitespace(1), s)
     if x.args[1].kind === Tokens.FOR
-        eq_to_in_normalization!(x.args[2])
+        eq_to_in_normalization!(x.args[2], s.always_for_in)
     end
     add_node!(t, pretty(x.args[2], s), s, join_lines = true)
     s.indent += s.indent_size
@@ -1610,7 +1616,7 @@ function p_comprehension(x, s)
             add_node!(t, Whitespace(1), s)
             if a.kind === Tokens.FOR
                 for j = i+1:length(x)
-                    eq_to_in_normalization!(x.args[j])
+                    eq_to_in_normalization!(x.args[j], s.always_for_in)
                 end
             end
         elseif CSTParser.is_comma(a) && i < length(x) && !is_punc(x.args[i+1])


Test patch:
diff --git a/test/runtests.jl b/test/runtests.jl
index 3ccac23f..1bc81526 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -1,16 +1,18 @@
 # fmt
-import JuliaFormatter
+using JuliaFormatter
 using CSTParser
 using Test
 
-fmt1(s, i = 4, m = 80) = JuliaFormatter.format_text(s, indent = i, margin = m)
+fmt1(s, i, m, always_for_in) =
+    JuliaFormatter.format_text(s, indent = i, margin = m, always_for_in = always_for_in)
 
 # Verifies formatting the formatted text
 # results in the same output
-function fmt(s, i = 4, m = 80)
-    s1 = fmt1(s, i, m)
-    fmt1(s1, i, m)
+function fmt(s; i = 4, m = 80, always_for_in = false)
+    s1 = fmt1(s, i, m, always_for_in)
+    fmt1(s1, i, m, always_for_in)
 end
+fmt(s, i, m) = fmt(s; i = i, m = m)
 
 function run_pretty(text::String, print_width::Int)
     d = JuliaFormatter.Document(text)
@@ -160,6 +162,56 @@ end
         @test fmt(str_) == str
     end
 
+    @testset "always eq to in" begin
+        str_ = """
+        for i = 1:n
+            println(i)
+        end"""
+        str = """
+        for i in 1:n
+            println(i)
+        end"""
+        @test fmt(str_, always_for_in = true) == str
+        @test fmt(str, always_for_in = true) == str
+
+        str_ = """
+        for i = I1, j in I2
+            println(i, j)
+        end"""
+        str = """
+        for i in I1, j in I2
+            println(i, j)
+        end"""
+        @test fmt(str_, always_for_in = true) == str
+        @test fmt(str, always_for_in = true) == str
+
+        str_ = """
+        for i = 1:30, j = 100:-2:1
+            println(i, j)
+        end"""
+        str = """
+        for i in 1:30, j in 100:-2:1
+            println(i, j)
+        end"""
+        @test fmt(str_, always_for_in = true) == str
+        @test fmt(str, always_for_in = true) == str
+
+        str_ = "[(i,j) for i=I1,j=I2]"
+        str = "[(i, j) for i in I1, j in I2]"
+        @test fmt(str_, always_for_in = true) == str
+        @test fmt(str, always_for_in = true) == str
+
+        str_ = "((i,j) for i=I1,j=I2)"
+        str = "((i, j) for i in I1, j in I2)"
+        @test fmt(str_, always_for_in = true) == str
+        @test fmt(str, always_for_in = true) == str
+
+        str_ = "[(i, j) for i = 1:2:10, j = 100:-1:10]"
+        str = "[(i, j) for i in 1:2:10, j in 100:-1:10]"
+        @test fmt(str_, always_for_in = true) == str
+        @test fmt(str, always_for_in = true) == str
+    end
+
     @testset "tuples" begin
         @test fmt("a,b") == "a, b"
         @test fmt("a ,b") == "a, b"