Add compat for JuliaSyntax v1 by svchb · Pull Request #978 · JuliaEditorSupport/JuliaFormatter.jl

svchb · 2026-05-05T08:18:48Z

I kept the changes as minimal as possible.

svchb · 2026-05-05T11:21:36Z

@domluna all tests pass locally

…iaFormatter.jl into update_julia_syntax_v1

pfitzseb

Generally LGTM

svchb · 2026-05-19T11:43:43Z

@penelopeysm can you review this?

penelopeysm · 2026-05-19T12:15:41Z

I could, but honestly, it would take me a bit of time! I haven't really gotten into the codebase as much as I'd like, and have been quite busy IRL too. If you don't mind waiting, I could try to get to it over the weekend?

svchb · 2026-05-19T12:23:53Z

Sure no worries as long as we get this project moved forward again 🚂

pfitzseb

Ok, finally got a chance to take a proper look at this. It would generally be nice to get some more unit tests for the new utility functions if possible.

pfitzseb · 2026-05-19T17:32:33Z

+function is_short_function_def(cst::JuliaSyntax.GreenNode)
+    kind(cst) === K"function" && haschildren(cst) || return false
+    idx = findfirst(n -> !JuliaSyntax.is_whitespace(n), children(cst))
+    isnothing(idx) && return false
+    return kind(cst[idx]) !== K"function"
+end


Suggested change

function is_short_function_def(cst::JuliaSyntax.GreenNode)

kind(cst) === K"function" && haschildren(cst) || return false

idx = findfirst(n -> !JuliaSyntax.is_whitespace(n), children(cst))

isnothing(idx) && return false

return kind(cst[idx]) !== K"function"

end

is_short_function_def(cst::JuliaSyntax.GreenNode) = JuliaSyntax.has_flags(cst, JuliaSyntax.SHORT_FORM_FUNCTION_FLAG)

pfitzseb · 2026-05-20T13:40:21Z

+    elseif has_do_block_call(node)
+        p_do_call(style, node, s, ctx, lineage)


With this, does the

elseif k === K"do" p_do(style, node, s, ctx, lineage)

ever get called? Seems like only having one of them should be enough (and specifically, why can't you re-use the existing case here)?

That branch is still hit for macros like @m(x) do y; false; end. To be honest I didn't even know this was a valid syntactic construct until now

pfitzseb · 2026-05-20T13:41:51Z

+    return nothing
+end
+
+function is_source_operator(s::State, cst::JuliaSyntax.GreenNode, offset::Integer)


In theory this should be covered by the JuliaSyntax.is_operator_* calls no? Would be nice if we didn't have to look into the file for this.

pfitzseb · 2026-05-20T13:44:58Z

+function source_kind(s::State, cst::JuliaSyntax.GreenNode, offset::Integer)
+    span(cst) == 0 && return nothing
+    offset = Int(offset)
+    n = Int(span(cst))
+    val = getsrcval(s.doc, offset:(offset+n-1))
+    try
+        return JuliaSyntax.Kind(val)
+    catch
+        return nothing
+    end
+end
+
+function source_operator_kind(s::State, cst::JuliaSyntax.GreenNode, offset::Integer)
+    if JuliaSyntax.is_operator(cst) && !haschildren(cst)
+        return kind(cst)
+    elseif kind(cst) === K"Identifier" && !haschildren(cst)
+        k = source_kind(s, cst, offset)
+        if !isnothing(k) && JuliaSyntax.is_operator(k)
+            return k
+        end
+    end
+    return nothing
+end


We need this because of JuliaLang/JuliaSyntax.jl#591, right? Wonder whether this will get fixed upstream at some point...

I guess this is an intentional choice in JuliaSyntax

JuliaLang/JuliaSyntax.jl#474
JuliaLang/JuliaSyntax.jl#523

So I think we have to work around it somehow. As mentioned in that PR, JuliaSyntax.tokenize() has a flag to retain the information, but we aren't using that, and doing so would probably be a large refactor. So I'm happy to use this approach of re-parsing.

(I think the JuliaFormatter codebase needs more comments in general to explain things like this, but that's a separate issue.)

pfitzseb · 2026-05-20T13:48:49Z

+function source_op_kind(s::State, cst::JuliaSyntax.GreenNode, op_indices::Vector{Int})
+    opkind = op_kind(cst)
+    opkind !== K"None" && opkind !== K"Identifier" && return opkind
+
+    offset = Int(s.offset)
+    childs = children(cst)
+    for (i, c) in enumerate(childs)
+        if i in op_indices
+            k = source_operator_kind(s, c, offset)
+            if !isnothing(k) && !(kind(cst) === K"dotcall" && k === K".")
+                return k
+            end
+        end
+        offset += Int(span(c))
+    end
+    return opkind
+end
+
+function update_operator_indices(childs::Vector{JuliaSyntax.GreenNode{T}}) where {T}
+    args = findall(n -> !JuliaSyntax.is_whitespace(n), childs)
+    length(args) < 4 && return Int[]
+    return args[2:(end-1)]
+end


Seems like the operator should be easy enough to find by checking in which position it occurs (JS.is_[prefix|infix|suffix]_op_call) and then directly accessing that argument.

penelopeysm

Thanks @svchb and sorry it took a while! I'm familiar enough with parsing but I had to figure out all the exact data structures that JuliaFormatter and JuliaSyntax use.

I'm conscious that bumping compat for JuliaSyntax has been a longstanding issue on JuliaFormatter. So I think that we should just try to merge this in as long as it works. Although I think that there are quite a few things that ought to be cleaned up (especially with the helper functions that do similar things to JuliaSyntax), we can defer these.

However I still do have some questions / comments which hopefully won't hold things up too much.

penelopeysm · 2026-05-24T15:30:18Z

+    return is_source_operator(s, childs[op_arg], offset) ? op_arg : nothing
+end
+
+function is_source_prefix_op_call(cst::JuliaSyntax.GreenNode, s::State)


I think this entire function can just be replaced with JuliaSyntax.is_prefix_op_call(cst). I ran the test suite with this change and it all passes

penelopeysm · 2026-05-24T15:30:26Z

+function source_kind(s::State, cst::JuliaSyntax.GreenNode, offset::Integer)
+    span(cst) == 0 && return nothing
+    offset = Int(offset)
+    n = Int(span(cst))
+    val = getsrcval(s.doc, offset:(offset+n-1))
+    try
+        return JuliaSyntax.Kind(val)
+    catch
+        return nothing
+    end
+end
+
+function source_operator_kind(s::State, cst::JuliaSyntax.GreenNode, offset::Integer)
+    if JuliaSyntax.is_operator(cst) && !haschildren(cst)
+        return kind(cst)
+    elseif kind(cst) === K"Identifier" && !haschildren(cst)
+        k = source_kind(s, cst, offset)
+        if !isnothing(k) && JuliaSyntax.is_operator(k)
+            return k
+        end
+    end
+    return nothing
+end


I guess this is an intentional choice in JuliaSyntax

JuliaLang/JuliaSyntax.jl#474
JuliaLang/JuliaSyntax.jl#523

So I think we have to work around it somehow. As mentioned in that PR, JuliaSyntax.tokenize() has a flag to retain the information, but we aren't using that, and doing so would probably be a large refactor. So I'm happy to use this approach of re-parsing.

(I think the JuliaFormatter codebase needs more comments in general to explain things like this, but that's a separate issue.)

penelopeysm · 2026-05-24T15:35:47Z

+    elseif is_source_prefix_op_call(node, s)
+        p_unaryopcall(style, node, s, ctx, lineage)


Suggested change

elseif is_source_prefix_op_call(node, s)

p_unaryopcall(style, node, s, ctx, lineage)

elseif JuliaSyntax.is_prefix_op_call(node)

p_unaryopcall(style, node, s, ctx, lineage)

penelopeysm · 2026-05-24T15:48:27Z

+    elseif !isnothing(source_prefix_operator_index(node, s))
+        p_call(style, node, s, ctx, lineage)


I'm pretty sure that this dispatch is not necessary -- they all fall through to the other branches correctly. For example, I think the intent of this branch is to catch something like ">=(x)"? (Please correct me if I'm wrong.) However, this doesn't actually get picked up by is_unary (because, since JuliaSyntax v1, the kind of the first token is Identifier), so it falls through all the way to is_func_call, which is exactly what we want. Again, I checked with this change and the test suite passes.

Suggested change

elseif !isnothing(source_prefix_operator_index(node, s))

p_call(style, node, s, ctx, lineage)

penelopeysm · 2026-05-24T16:22:43Z

    t
 end

 function p_cartesian_iterator(


I think we should rename this function to p_iteration to avoid confusion (since there's no longer a cartesian iterator kind in JuliaSyntax v1, and this function handles all types of iterators regardless of whether there are multiple of them).

penelopeysm · 2026-05-24T17:20:29Z

 function is_prefix_op_call(x::JuliaSyntax.GreenNode)
    is_opcall(x) || return false
    haschildren(x) || return false

    idx = findfirst(!JuliaSyntax.is_whitespace, children(x))
    isnothing(idx) && return false

    return JuliaSyntax.is_operator(x[idx])
 end


This function is now dead code (and gives the wrong behaviour anyway with JuliaSyntax v1 because prefix ops are parsed as K"Identifier") so can be removed.

penelopeysm · 2026-05-24T18:07:49Z

    end

    from_colon = ctx.from_colon
+    from_typedef = ctx.from_typedef || any(x -> x[1] === K"where", lineage)


Tests seem to pass without this any(...) check. What's the rationale for adding it?

penelopeysm · 2026-05-24T18:31:51Z

    ctx::PrettyContext,
-    lineage::Vector{Tuple{JuliaSyntax.Kind,Bool,Bool}},
+    lineage::Vector{Tuple{JuliaSyntax.Kind,Bool,Bool}};
+    child_limit::Union{Int,Nothing} = nothing,


I think this keyword argument is pretending to be more general than it really needs to be. Looking at where it gets used, it really just means 'one before the do block, if a do block exists'. I'd suggest we could just pass in the do block index, which would be clearer, and would also avoid recalculating it inside this function.

penelopeysm · 2026-05-24T19:06:46Z

+            opkind = try
+                JuliaSyntax.Kind(op.val)
+            catch
+                metadata.op_kind
+            end


Kind(op.val) can't fail because op.val can only be one of in, = or ∈ so I think we can elide the try/catch.

penelopeysm · 2026-05-24T19:07:06Z

+                metadata.op_dotted,
+                metadata.is_standalone_shortcircuit,
+                metadata.is_short_form_function,
+                op.val == "=",


Suggested change

op.val == "=",

opkind === K"=",

svchb added 2 commits May 4, 2026 14:57

switch to juliasyntax v1

8259677

fix edge cases

5cf92e5

svchb mentioned this pull request May 5, 2026

Fix SciMLStyle regressions #979

Open

svchb added 3 commits May 12, 2026 09:33

Merge branch 'master' into update_julia_syntax_v1

bd1743f

fix tests

ba7fa18

Merge branch 'update_julia_syntax_v1' of https://github.com/svchb/Jul…

173ae64

…iaFormatter.jl into update_julia_syntax_v1

pfitzseb reviewed May 15, 2026

View reviewed changes

Comment thread src/styles/default/pretty.jl Outdated

svchb added 2 commits May 18, 2026 15:26

fix formatting

51a5740

Merge branch 'master' into update_julia_syntax_v1

b91baf6

svchb requested a review from pfitzseb May 18, 2026 13:26

pfitzseb reviewed May 20, 2026

View reviewed changes

davidanthoff mentioned this pull request May 23, 2026

Update to work with JuliaFormatter.jl v2 julia-vscode/LanguageServer.jl#1391

Open

Merge branch 'master' into update_julia_syntax_v1

530011b

penelopeysm requested changes May 24, 2026

View reviewed changes

penelopeysm linked an issue May 24, 2026 that may be closed by this pull request

Update JuliaSyntax.jl to v1 #948

Open

penelopeysm mentioned this pull request May 24, 2026

v2 chokes on @main #950

Open

penelopeysm linked an issue May 24, 2026 that may be closed by this pull request

v2 chokes on @main #950

Open

penelopeysm mentioned this pull request May 24, 2026

Precompilation fails with JuliaSyntax: unknown Kind name "core_@cmd" #928

Closed

		elseif has_do_block_call(node)
		p_do_call(style, node, s, ctx, lineage)

		elseif is_source_prefix_op_call(node, s)
		p_unaryopcall(style, node, s, ctx, lineage)

		elseif !isnothing(source_prefix_operator_index(node, s))
		p_call(style, node, s, ctx, lineage)

Conversation

svchb commented May 5, 2026

Uh oh!

svchb commented May 5, 2026

Uh oh!

pfitzseb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

svchb commented May 19, 2026

Uh oh!

penelopeysm commented May 19, 2026

Uh oh!

svchb commented May 19, 2026

Uh oh!

pfitzseb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

penelopeysm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants