Merge pull request #1307 from julia-vscode/fix-offset

Use index_at for get_offset

Project.toml

@@ -26,7 +26,7 @@ JSONRPC = "1.1"
 JuliaFormatter = "0.20.0, 0.21, 0.22, 0.23, 1"
 PrecompileTools = "1"
 StaticLint = "8.0"
-JuliaWorkspaces = "4.3"
+JuliaWorkspaces = "4.4"
 SymbolServer = "8"
 Tokenize = "0.5.10"
 URIs = "1.3"

src/document.jl

@@ -76,9 +76,6 @@ get_offset(doc::Document, line::Integer, character::Integer) = get_offset(doc._t
 get_offset(doc::Document, p::Position) = get_offset(doc, p.line, p.character)
 get_offset(doc::Document, r::Range) = get_offset(doc, r.start):get_offset(doc, r.stop)
 
-# get_offset, but correct
-get_offset3(args...) = index_at(args...) - 1
-
 index_at(doc::Document, pos, args...) = index_at(doc._text_document, pos, args...)
 
 get_position_from_offset(doc::Document, offset::Integer) = get_position_from_offset(doc._text_document, offset)

src/requests/completions.jl

@@ -41,7 +41,7 @@ end
 function textDocument_completion_request(params::CompletionParams, server::LanguageServerInstance, conn)
     state = let
         doc = getdocument(server, params.textDocument.uri)
-        offset = get_offset3(get_text_document(doc), params.position)
+        offset = get_offset(get_text_document(doc), params.position)
         rng = Range(doc, offset:offset)
         x = get_expr(getcst(doc), offset)
         using_stmts = server.completion_mode == :import ? get_preexisting_using_stmts(x, doc) : Dict()

src/requests/highlight.jl

@@ -1,7 +1,5 @@
 function textDocument_documentHighlight_request(params::DocumentHighlightParams, server::LanguageServerInstance, conn)
     doc = getdocument(server, params.textDocument.uri)
-    # This is ONLY here to test whether it also crashes when get_offset crashes
-    index_at(doc, params.position)
     offset = get_offset(doc, params.position)
     identifier = get_identifier(getcst(doc), offset)
     identifier !== nothing || return nothing

src/requests/signatures.jl

@@ -1,13 +1,6 @@
 function textDocument_signatureHelp_request(params::TextDocumentPositionParams, server::LanguageServerInstance, conn)
     doc = getdocument(server, params.textDocument.uri)
     sigs = SignatureInformation[]
-    # TODO The following call is just here for diagnostics
-    # We currently have crashes in the call to get_offset in crash reporting
-    # but they are fairly rare. So the idea here is to see whether we also get_expr
-    # crashes in index_at or not. If we still see crashes in get_offset after this here
-    # is merged, then the bug is simply in get_offset and we should migrate this function
-    # over to use index_at. If not, then there might still be a problem in the sync protocol.
-    index_at(get_text_document(doc), params.position)
     offset = get_offset(doc, params.position)
     x = get_expr(getcst(doc), offset)
 

src/textdocument.jl

@@ -49,10 +49,7 @@ get_language_id(doc::TextDocument) = doc._language_id
 Converts a 0-based `Position` that is UTF-16 encoded to a 1-based UTF-8
 encoded Julia string index.
 """
-function index_at(doc::TextDocument, p::Position, forgiving_mode=false)
-    line = p.line
-    character = p.character
-
+function index_at(doc::TextDocument, line::Integer, character::Integer, forgiving_mode=false)
     line_indices = get_line_indices(doc)
     text = get_text(doc)
 
@@ -85,9 +82,15 @@ function index_at(doc::TextDocument, p::Position, forgiving_mode=false)
         pos = nextind(text, pos)
     end
 
+    if character < 0
+        error("Invalid UTF-16 index supplied")
+    end
+
     return pos
 end
 
+index_at(doc::TextDocument, p::Position, args...) = index_at(doc, p.line, p.character, args...)
+
 function apply_text_edits(doc::TextDocument, edits, new_version)
     content = doc._content
 
@@ -220,30 +223,7 @@ This takes 0 based line/char inputs. Corresponding functions are available for
 Position and Range arguments, the latter returning a UnitRange{Int}.
 """
 function get_offset(doc::TextDocument, line::Integer, character::Integer)
-    c = ' '
-    line_offsets = get_line_offsets(doc)
-    io = IOBuffer(get_text(doc))
-    try
-        seek(io, line_offsets[line + 1])
-        while character > 0
-            c = read(io, Char)
-            character -= 1
-            if UInt32(c) >= 0x010000
-                character -= 1
-            end
-        end
-        if UInt32(c) < 0x0080
-            return position(io)
-        elseif UInt32(c) < 0x0800
-            return position(io) - 1
-        elseif UInt32(c) < 0x010000
-            return position(io) - 2
-        else
-            return position(io) - 3
-        end
-    catch err
-        throw(LSOffsetError("get_offset crashed. More diagnostics:\nline=$line\ncharacter=$character\nposition(io)=$(position(io))\nline_offsets='$line_offsets'\ntext='$(_obscure_text(get_text(doc)))'\n\noriginal_error=$(sprint(Base.display_error, err, catch_backtrace()))"))
-    end
+    return index_at(doc, line, character) - 1
 end
 get_offset(doc::TextDocument, p::Position) = get_offset(doc, p.line, p.character)
 get_offset(doc::TextDocument, r::Range) = get_offset(doc, r.start):get_offset(doc, r.stop)

test/test_document.jl

@@ -127,44 +127,44 @@ end
     @test sizeof(LanguageServer.get_text(doc)) == 6
     @test LanguageServer.get_offset(doc, 0, 0) == 0
     @test LanguageServer.get_position_from_offset(doc, 0) == (0, 0)
-    @test LanguageServer.get_offset(doc, 0, 1) == 1
+    @test LanguageServer.get_offset(doc, 0, 1) == 2
     @test LanguageServer.get_position_from_offset(doc, 1) == (0, 1)
-    @test LanguageServer.get_offset(doc, 0, 2) == 3
+    @test LanguageServer.get_offset(doc, 0, 2) == 4
     @test LanguageServer.get_position_from_offset(doc, 3) == (0, 2)
-    @test LanguageServer.get_offset(doc, 0, 3) == 5
+    @test LanguageServer.get_offset(doc, 0, 3) == 6
     @test LanguageServer.get_position_from_offset(doc, 5) == (0, 3)
 
     doc = LanguageServer.Document(TextDocument(uri"", "ࠀࠀࠀ", 0), false) # 0x0800
     @test sizeof(LanguageServer.get_text(doc)) == 9
     @test LanguageServer.get_offset(doc, 0, 0) == 0
     @test LanguageServer.get_position_from_offset(doc, 0) == (0, 0)
-    @test LanguageServer.get_offset(doc, 0, 1) == 1
-    @test LanguageServer.get_position_from_offset(doc, 1) == (0, 1)
-    @test LanguageServer.get_offset(doc, 0, 2) == 4
+    @test LanguageServer.get_offset(doc, 0, 1) == 3
+    @test LanguageServer.get_position_from_offset(doc, 2) == (0, 1)
+    @test LanguageServer.get_offset(doc, 0, 2) == 6
     @test LanguageServer.get_position_from_offset(doc, 4) == (0, 2)
-    @test LanguageServer.get_offset(doc, 0, 3) == 7
+    @test LanguageServer.get_offset(doc, 0, 3) == 9
     @test LanguageServer.get_position_from_offset(doc, 7) == (0, 3)
 
     doc = LanguageServer.Document(TextDocument(uri"", "𐐀𐐀𐐀", 0), false)
     @test sizeof(LanguageServer.get_text(doc)) == 12
     @test LanguageServer.get_offset(doc, 0, 0) == 0
     @test LanguageServer.get_position_from_offset(doc, 0) == (0, 0)
-    @test LanguageServer.get_offset(doc, 0, 2) == 1
+    @test LanguageServer.get_offset(doc, 0, 2) == 4
     @test LanguageServer.get_position_from_offset(doc, 1) == (0, 2)
-    @test LanguageServer.get_offset(doc, 0, 4) == 5
+    @test LanguageServer.get_offset(doc, 0, 4) == 8
     @test LanguageServer.get_position_from_offset(doc, 5) == (0, 4)
-    @test LanguageServer.get_offset(doc, 0, 6) == 9
+    @test LanguageServer.get_offset(doc, 0, 6) == 12
     @test LanguageServer.get_position_from_offset(doc, 9) == (0, 6)
 
     doc = LanguageServer.Document(TextDocument(uri"", "𐀀𐀀𐀀", 0), false) # 0x010000
     @test sizeof(LanguageServer.get_text(doc)) == 12
     @test LanguageServer.get_offset(doc, 0, 0) == 0
     @test LanguageServer.get_position_from_offset(doc, 0) == (0, 0)
-    @test LanguageServer.get_offset(doc, 0, 2) == 1
+    @test LanguageServer.get_offset(doc, 0, 2) == 4
     @test LanguageServer.get_position_from_offset(doc, 1) == (0, 2)
-    @test LanguageServer.get_offset(doc, 0, 4) == 5
+    @test LanguageServer.get_offset(doc, 0, 4) == 8
     @test LanguageServer.get_position_from_offset(doc, 5) == (0, 4)
-    @test LanguageServer.get_offset(doc, 0, 6) == 9
+    @test LanguageServer.get_offset(doc, 0, 6) == 12
     @test LanguageServer.get_position_from_offset(doc, 9) == (0, 6)
 end
 

test/test_shared_server.jl

@@ -10,6 +10,8 @@ function settestdoc(text)
 
     doc = LanguageServer.getdocument(server, uri"untitled:testdoc")
     LanguageServer.parse_all(doc, server)
+    LanguageServer.lint!(doc, server)
+    LanguageServer.semantic_pass(doc)
     doc
 end