Repository: domluna/JuliaFormatter.jl
Pull request #67 (2019-09-15 18:45:40)

 Handle Unicode comments better
Handle Unicode comments better - ref #65. Use Integer and AbstractString in for type/function definitions

odow: Why is it necessary to use `Integer` and `AbstractString`?
domluna: Best practice to use wider types? There was a mix of Int/Integer and AbstractString/String in type/function defs so I figured I'd normalize those to use the wider type. In practice I think this is only useful for the `val` type of a PTree, which is sometimes a `SubString` instead of a `String`. 
StefanKarpinski: The integer fields at least, I would leave as `Int`: they are indices into strings which should be `Int` for any kind of abstract string and guaranteed for `String`. Unless you expect the strings to be very large and you want to avoid transcoding, I would also just use the `String` and let various kinds of strings be converted to `String` everywhere (or most often, just used as-is since `String` is common). The general rule of thumb is that for dispatch, it's good to leave things nice and generic, only requiring what you need for methods to work, but for places like fields or data structures, use the most concrete type you can.

Patch:
diff --git a/src/JuliaFormatter.jl b/src/JuliaFormatter.jl
index 76d6e3c9..e5a4ffd9 100644
--- a/src/JuliaFormatter.jl
+++ b/src/JuliaFormatter.jl
@@ -17,19 +17,22 @@ struct Document
     # mapping the offset in the file to the raw literal
     # string and what lines it starts and ends at.
     lit_strings::Dict{Int,Tuple{Int,Int,String}}
-    comments::Dict{Int,String}
+    comments::Dict{Int,Tuple{Int,String}}
 
     # CSTParser does not detect semicolons.
     # It's useful to know where these are for
     # a few node types.
     semicolons::Set{Int}
 end
+
 function Document(text::AbstractString)
     ranges = UnitRange{Int}[]
     line_to_range = Dict{Int,UnitRange{Int}}()
     lit_strings = Dict{Int,Tuple{Int,Int,String}}()
-    comments = Dict{Int,String}()
+    comments = Dict{Int,Tuple{Int,String}}()
     semicolons = Set{Int}()
+    # dummy initial token
+    prev_tok = Tokens.Token()
 
     for t in CSTParser.Tokenize.tokenize(text)
         if t.kind === Tokens.WHITESPACE
@@ -55,32 +58,44 @@ function Document(text::AbstractString)
                 end
             end
         elseif t.kind === Tokens.COMMENT
+            ws = 0
+            if prev_tok.kind === Tokens.WHITESPACE
+                # Handles the case where the value of the
+                # WHITESPACE token is like " \n ".
+                i = findlast(c -> c == '\n', prev_tok.val)
+                i === nothing && (i = 1)
+                ws = count(c -> c == ' ', prev_tok.val[i:end])
+            end
             if t.startpos[1] == t.endpos[1]
-                comments[t.startpos[1]] = t.val
+                # Determine the number of spaces prior to a possible inline comment
+                comments[t.startpos[1]] = (ws, t.val)
             else
                 sl = t.startpos[1]
                 offset = t.startbyte
-                comment_offset = 1
+                cs = ""
                 for (i, c) in enumerate(t.val)
+                    cs *= c
                     if c == '\n'
                         s = length(ranges) > 0 ? last(ranges[end]) + 1 : 1
                         push!(ranges, s:offset+1)
-                        comments[sl] = t.val[comment_offset:i-1]
+                        comments[sl] = (ws, cs)
                         sl += 1
-                        comment_offset = i + 1
+                        cs = ""
                     end
                     offset += 1
                 end
                 # last comment
-                comments[sl] = t.val[comment_offset:end]
+                comments[sl] = (ws, cs)
             end
         elseif t.kind === Tokens.SEMICOLON
             push!(semicolons, t.startpos[1])
         end
+        prev_tok = t
     end
     for (l, r) in enumerate(ranges)
         line_to_range[l] = r
     end
+    # @info "" comments ranges
     Document(text, ranges, line_to_range, lit_strings, comments, semicolons)
 end
 
@@ -95,11 +110,10 @@ end
 State(doc, indent_size, margin) = State(doc, indent_size, 0, 1, 0, margin)
 
 @inline nspaces(s::State) = s.indent
-@inline getline(d::Document, line::Int) = d.text[d.line_to_range[line]]
-@inline hascomment(d::Document, line::Int) = haskey(d.comments, line)
-@inline has_semicolon(d::Document, line::Int) = line in d.semicolons
+@inline hascomment(d::Document, line::Integer) = haskey(d.comments, line)
+@inline has_semicolon(d::Document, line::Integer) = line in d.semicolons
 
-@inline function cursor_loc(s::State, offset::Int)
+@inline function cursor_loc(s::State, offset::Integer)
     for (l, r) in enumerate(s.doc.ranges)
         if offset in r
             return (l, offset - first(r) + 1, length(r))
@@ -140,7 +154,7 @@ 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, "")) && (return text)
+    occursin("nofmt", get(d.comments, 1, (0, ""))[2]) && (return text)
 
     s = State(d, indent, margin)
     t = pretty(x, s)
diff --git a/src/pretty.jl b/src/pretty.jl
index c59612f0..877eb9ec 100644
--- a/src/pretty.jl
+++ b/src/pretty.jl
@@ -23,14 +23,14 @@ mutable struct PTree
     force_nest::Bool
 end
 
-PTree(x::CSTParser.EXPR, indent::Int) =
+PTree(x::CSTParser.EXPR, indent::Integer) =
     PTree(x.typ, -1, -1, indent, 0, nothing, PTree[], Ref(x), false)
 
-function PTree(x::CSTParser.EXPR, startline::Int, endline::Int, val::AbstractString)
+function PTree(x::CSTParser.EXPR, startline::Integer, endline::Integer, val::AbstractString)
     PTree(x.typ, startline, endline, 0, length(val), val, nothing, Ref(x), false)
 end
 
-function PTree(x::CSTParser.Head, startline::Int, endline::Int, val::AbstractString)
+function PTree(x::CSTParser.Head, startline::Integer, endline::Integer, val::AbstractString)
     PTree(x, startline, endline, 0, length(val), val, nothing, nothing, false)
 end
 
diff --git a/src/print.jl b/src/print.jl
index cac5ccdc..d3eae596 100644
--- a/src/print.jl
+++ b/src/print.jl
@@ -46,25 +46,24 @@ function print_tree(io::IOBuffer, x::PTree, s::State)
 end
 
 @inline function print_notcode(io, x, s)
-    prev_nl = true
     for l = x.startline:x.endline
-        v = getline(s.doc, l)
+        ws, v = get(s.doc.comments, l, (0, "\n"))
         v == "" && continue
         if l == x.endline && v[end] == '\n'
             v = v[1:prevind(v, end)]
         end
+        ws > 0 && write(io, repeat(" ", ws))
         write(io, v)
-        prev_nl = v == "\n" ? true : false
+        if l != x.endline && v[end] != '\n'
+            write(io, "\n")
+        end
     end
 end
 
 @inline function print_inlinecomment(io, x, s)
-    v = get(s.doc.comments, x.startline, "")
+    ws, v = get(s.doc.comments, x.startline, (0, ""))
     isempty(v) && return
-    v = getline(s.doc, x.startline)
-    idx = findlast(c -> c == '#', v)
-    idx === nothing && return
-    idx = findlast(c -> !isspace(c), v[1:prevind(v, idx)])
-    v = v[end] == '\n' ? v[nextind(v, idx):prevind(v, end)] : v[nextind(v, idx):end]
+    v = v[end] == '\n' ? v[nextind(v, 1):prevind(v, end)] : v
+    ws > 0 && write(io, repeat(" ", ws))
     write(io, v)
 end


Test patch:
diff --git a/test/files/PProf.jl b/test/files/PProf.jl
index b54fabc0..9f1dbb26 100644
--- a/test/files/PProf.jl
+++ b/test/files/PProf.jl
@@ -45,7 +45,7 @@ Useful for the Strings table
 NOTE: We must use Int64 throughout this package (regardless of system word-size) b/c the
 proto file specifies 64-bit integers.
 """
-function _enter!(dict::OrderedDict{T,Int64}, key::T) where T
+function _enter!(dict::OrderedDict{T,Int64}, key::T) where {T}
     if haskey(dict, key)
         return dict[key]
     else
diff --git a/test/files/adjoint.jl b/test/files/adjoint.jl
index 75ab62b4..675b210e 100644
--- a/test/files/adjoint.jl
+++ b/test/files/adjoint.jl
@@ -42,10 +42,10 @@ function gradm(ex, mut = false)
     fargs = kw == nothing ? [cx, :($f::$T), args...] : [kw, cx, :($f::$T), args...]
     gradtuple = isclosure ? gradtuple0 : gradtuple1
     gradtuplekw = isclosure ? gradtuple2 : gradtuple3
-    adj = @q @inline ZygoteRules.adjoint($(fargs...)) where $(Ts...) = $(esc(body))
+    adj = @q @inline ZygoteRules.adjoint($(fargs...)) where {$(Ts...)} = $(esc(body))
     quote
         $adj
-        @inline function ZygoteRules._forward($cx, $f::$T, $(args...)) where $(Ts...)
+        @inline function ZygoteRules._forward($cx, $f::$T, $(args...)) where {$(Ts...)}
             y, _back = adjoint(__context__, $f, $(argnames...))
             $(mut ? nothing : :(back(::Nothing) = nothing))
             back(Δ) = $gradtuple(_back(Δ))
@@ -57,7 +57,7 @@ function gradm(ex, mut = false)
             kw,
             $f::$T,
             $(args...),
-        ) where $(Ts...)
+        ) where {$(Ts...)}
             y, _back = adjoint(__context__, $f, $(argnames...); kw...)
             $(mut ? nothing : :(back(::Nothing) = nothing))
             back(Δ) = $gradtuplekw(_back(Δ))
diff --git a/test/files/cppwrapper.jl b/test/files/cppwrapper.jl
index 4887e4bd..0474e653 100644
--- a/test/files/cppwrapper.jl
+++ b/test/files/cppwrapper.jl
@@ -35,16 +35,16 @@ function JLPolygon(x::CPolygon)
     JLPolygon(unsafe_wrap(Array, x.vertexlist, x.numberofvertices, own = true))
 end
 
-function JLFacet(f::CFacet{T}) where T
+function JLFacet(f::CFacet{T}) where {T}
     c_polys = unsafe_wrap(Array, f.polygonlist, f.numberofpolygons, own = true)
     holes = unsafe_wrap(Array, f.holelist, f.numberofholes, own = true)
     JLFacet{T}(JLPolygon.(c_polys), holes)
 end
 
-function convert_poly(x::Vector{T}) where T <: Union{
+function convert_poly(x::Vector{T}) where {T<:Union{
     NgonFace{N,Cint},
     NTuple{N,Cint},
-} where N
+}} where {N}
     return map(1:length(x)) do i
         CPolygon(pointer(x, i), N)
     end
@@ -219,7 +219,7 @@ function TetgenIO(
     edges = LineFace{Cint}[],
     edgemarkers = Cint[],
 
-) where T
+) where {T}
     TetgenIO(
         points,
         pointattributes,
@@ -292,7 +292,7 @@ function Base.convert(
     end...)
 end
 
-function Base.convert(::Type{TetgenIO}, io::CPPTetgenIO{T}) where T
+function Base.convert(::Type{TetgenIO}, io::CPPTetgenIO{T}) where {T}
     NSimplex = Int(io.numberofcorners)
     NAttributes = Int(io.numberofpointattributes)
     NMTr = Int(io.numberofpointmtrs)
@@ -339,7 +339,7 @@ function unsafe_array_convert(
     return Base.unsafe_convert(Ptr{CFacet{T}}, facets[3])
 end
 
-function unsafe_array_convert(P::Type{Ptr{T}}, x::Vector{T}) where T
+function unsafe_array_convert(P::Type{Ptr{T}}, x::Vector{T}) where {T}
     Base.unsafe_convert(P, x)
 end
 
@@ -393,7 +393,7 @@ end
 function Base.unsafe_convert(
     ::Type{CPPTetgenIO{T}},
     x::Tuple{CPPTetgenIO{T},Vector{Any}},
-) where T
+) where {T}
     return x[1]
 end
 
diff --git a/test/files/prettify_demo.jl b/test/files/prettify_demo.jl
index 7f8cac61..0b832936 100644
--- a/test/files/prettify_demo.jl
+++ b/test/files/prettify_demo.jl
@@ -17,7 +17,7 @@ function TetgenIO(
     trifacemarkers = Cint[],
     edges = LineFace{Cint}[],
     edgemarkers = Cint[],
-) where T
+) where {T}
     TetgenIO(
         points,
         pointattributes,
diff --git a/test/files/reverse.jl b/test/files/reverse.jl
index aeebbe2d..ea1e34fa 100644
--- a/test/files/reverse.jl
+++ b/test/files/reverse.jl
@@ -27,7 +27,7 @@ using Base: @get!
 
 @inline tuple_va(N, xs) = xs
 @inline tuple_va(N, x, xs...) = (x, tuple_va(N, xs...)...)
-@inline tuple_va(::Val{N}, ::Nothing) where N = ntuple(_ -> nothing, Val(N))
+@inline tuple_va(::Val{N}, ::Nothing) where {N} = ntuple(_ -> nothing, Val(N))
 
 iscall(x, m::Module, n::Symbol) = isexpr(x, :call) && x.args[1] == GlobalRef(m, n)
 
diff --git a/test/runtests.jl b/test/runtests.jl
index 53942081..64142765 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -1431,34 +1431,42 @@ end
         # Issue #51
         # NOTE: `str_` has extra whitespace after
         # keywords on purpose
-        str_ = """
-        begin 
-            # comment
-        end"""
+        str_ = "begin \n # comment\n end"
         str = """
         begin
-            # comment
+         # comment
         end"""
         @test fmt(str_) == str
 
-        str_ = """
-        try 
-            # comment
-        catch 
-            # comment
-        finally 
-            # comment
-        end"""
+        str_ = "try \n # comment\n catch \n # comment\n finally \n # comment\n end"
         str = """
         try
-            # comment
+         # comment
         catch
-            # comment
+         # comment
         finally
-            # comment
+         # comment
         end"""
         @test fmt(str_) == str
 
+        str = """a = "hello ##" # # # α"""
+        @test fmt(str) == str
+
+        # issue #65
+        str = "1 # α"
+        @test fmt(str) == str
+
+        str = "# α"
+        @test fmt(str) == str
+
+        str = """
+        #=
+        α
+        =#
+        x = 1
+        """
+        @test fmt(str) == str
+
     end
 
     @testset "pretty" begin