diff --git a/spec/reproduction_41_spec.cr b/spec/reproduction_41_spec.cr new file mode 100644 index 0000000..abf11ed --- /dev/null +++ b/spec/reproduction_41_spec.cr @@ -0,0 +1,27 @@ +require "spec" +require "../src/crystalline/text_document" +require "lsp/base/range" + +describe Crystalline::TextDocument do + it "fixes #41: does not strip newlines during incremental updates" do + initial_content = "def foo\nend\n" + doc = Crystalline::TextDocument.new(URI.parse("file:///test.cr"), nil, initial_content) + doc.contents.should eq(initial_content) + + # Simulation: Append a comment at the end of the line (at the newline position) + # Line 0 is "def foo\n" (length 8) + # Character 8 is the \n. + range = LSP::Range.new( + start: LSP::Position.new(line: 0, character: 8), + end: LSP::Position.new(line: 0, character: 8) + ) + + # If .chomp was used, prefix would be "def foo" (stripping \n). + # Adding "# comment" would result in "def foo# commentend\n" (LOST newline). + # WITHOUT .chomp, prefix is "def foo\n". + # Result should be "def foo\n# commentend\n" + + doc.update_contents([{"# comment", range}], version: 2) + doc.contents.should eq("def foo\n# commentend\n") + end +end diff --git a/spec/reproduction_63_spec.cr b/spec/reproduction_63_spec.cr new file mode 100644 index 0000000..e6ba389 --- /dev/null +++ b/spec/reproduction_63_spec.cr @@ -0,0 +1,51 @@ +require "spec" +require "lsp/server" +require "../src/crystalline/requires" +require "../src/crystalline/*" + +class FakeServer < LSP::Server + def initialize + super(IO::Memory.new, IO::Memory.new) + @client_capabilities = LSP::ClientCapabilities.new + end +end + +describe Crystalline::Workspace do + it "fixes #63: range formatting does not add extra newlines" do + server = FakeServer.new + workspace = Crystalline::Workspace.new(server, "file:///tmp") + + file_uri = URI.parse("file:///tmp/test_paste.cr") + initial_content = "foo \"bar\"\n" + + workspace.open_document(LSP::DidOpenTextDocumentParams.new( + text_document: LSP::TextDocumentItem.new( + uri: file_uri.to_s, + language_id: "crystal", + version: 1, + text: initial_content + ) + )) + + # Range is the whole first line except the newline. + range = LSP::Range.new( + start: LSP::Position.new(line: 0, character: 0), + end: LSP::Position.new(line: 0, character: 9) + ) + + params = LSP::DocumentRangeFormattingParams.new( + text_document: LSP::TextDocumentIdentifier.new(uri: file_uri.to_s), + range: range, + options: LSP::FormattingOptions.new(tab_size: 2, insert_spaces: true) + ) + + result = workspace.format_document(params) + result.should_not be_nil + if result + formatted_text, _ = result + # Crystal.format("foo \"bar\"") normally returns "foo \"bar\"\n" + # Our fix should chomp it. + formatted_text.should eq("foo \"bar\"") + end + end +end diff --git a/spec/reproduction_76_spec.cr b/spec/reproduction_76_spec.cr new file mode 100644 index 0000000..526ba16 --- /dev/null +++ b/spec/reproduction_76_spec.cr @@ -0,0 +1,59 @@ +require "spec" +require "lsp/server" +require "../src/crystalline/requires" +require "../src/crystalline/*" + +class FakeServer < LSP::Server + def initialize + super(IO::Memory.new, IO::Memory.new) + @client_capabilities = LSP::ClientCapabilities.new + end +end + +describe Crystalline::Workspace do + it "fixes #76: does not trigger completion inside comments" do + server = FakeServer.new + workspace = Crystalline::Workspace.new(server, "file:///tmp") + + file_uri = URI.parse("file:///tmp/test_comment.cr") + # A file with a dot inside a comment + content = <<-CRYSTAL + # This is a comment. + puts 1 + CRYSTAL + + workspace.open_document(LSP::DidOpenTextDocumentParams.new( + text_document: LSP::TextDocumentItem.new( + uri: file_uri.to_s, + language_id: "crystal", + version: 1, + text: content + ) + )) + + # Try to trigger completion at the dot in the comment + # Line 0, character 19 (the dot at the end of "comment.") + pos = LSP::Position.new(line: 0, character: 19) + + result = workspace.completion(server, file_uri, pos, ".") + + # It should return nil early because it's in a comment. + result.should be_nil + + # Test case: # inside a string should NOT be a comment + content_with_string = <<-CRYSTAL + x = "this is # not a comment." + CRYSTAL + workspace.update_document(server, LSP::DidChangeTextDocumentParams.new( + text_document: LSP::VersionedTextDocumentIdentifier.new(uri: file_uri.to_s, version: 2), + content_changes: [ + LSP::DidChangeTextDocumentParams::TextDocumentContentChangeEvent.new( + text: content_with_string + ), + ] + )) + # Dot at end of string. Line 0, character 31. + pos_in_string = LSP::Position.new(line: 0, character: 31) + workspace.completion(server, file_uri, pos_in_string, ".") + end +end diff --git a/src/crystalline/analysis/analysis.cr b/src/crystalline/analysis/analysis.cr index c49f8df..75fc882 100644 --- a/src/crystalline/analysis/analysis.cr +++ b/src/crystalline/analysis/analysis.cr @@ -10,11 +10,17 @@ module Crystalline::Analysis end {% end %} - private def self.spawn_dedicated(*, name : String? = nil, &block) - fiber = Fiber.new(name, &block) - {% if flag?(:preview_mt) %} fiber.set_current_thread(@@dedicated_thread) {% end %} - fiber.enqueue - fiber + private def self.spawn_dedicated(*, name : String? = nil, &block : -> _) + captured_block = block + {% if flag?(:preview_mt) %} + fiber = Fiber.new(name, &captured_block) + fiber.set_current_thread(@@dedicated_thread) + fiber.enqueue + fiber + {% else %} + captured_block.call + Fiber.current + {% end %} end # Compile a target *file_uri*. diff --git a/src/crystalline/ext/boehm.cr b/src/crystalline/ext/boehm.cr index 0911252..4fa2f9e 100644 --- a/src/crystalline/ext/boehm.cr +++ b/src/crystalline/ext/boehm.cr @@ -1,7 +1,8 @@ module GC def self.init - # LibGC.set_free_space_divisor(10) - # LibGC.set_force_unmap_on_gcollect(1) + LibGC.set_free_space_divisor(10) + LibGC.set_force_unmap_on_gcollect(1) + LibGC.enable_incremental previous_def end end diff --git a/src/crystalline/progress.cr b/src/crystalline/progress.cr index 847893b..deef396 100644 --- a/src/crystalline/progress.cr +++ b/src/crystalline/progress.cr @@ -8,14 +8,26 @@ class Crystalline::Progress end def report(server, *, async = false, &cb : Proc(String?)) - create_request = LSP::WorkDoneProgressCreateRequest.new( - id: 0, - params: LSP::WorkDoneProgressCreateParams.new( - token: @token, - ), - ) + if server.client_capabilities.window.try &.work_done_progress + create_request = LSP::WorkDoneProgressCreateRequest.new( + id: 0, + params: LSP::WorkDoneProgressCreateParams.new( + token: @token, + ), + ) - create_request.on_response { + create_request.on_response { + if async + spawn { + report_callback(server, &cb) + } + else + report_callback(server, &cb) + end + } + + server.send(create_request) + else if async spawn { report_callback(server, &cb) @@ -23,9 +35,7 @@ class Crystalline::Progress else report_callback(server, &cb) end - } - - server.send(create_request) + end end def send_progress_start(server) diff --git a/src/crystalline/result_cache.cr b/src/crystalline/result_cache.cr index 4907fd3..b9e7d41 100644 --- a/src/crystalline/result_cache.cr +++ b/src/crystalline/result_cache.cr @@ -1,6 +1,6 @@ class Crystalline::ResultCache # A monotonic timestamp used to store the invalidation date. - @@reference_clock = Time.monotonic + @@reference_clock = Time.instant # A cache of compiler results with invalidation time, indexed by file name. @cache : Hash(String, {Crystal::Compiler::Result?, Time::Span?}) = Hash(String, {Crystal::Compiler::Result?, Time::Span?}).new @@ -42,6 +42,6 @@ class Crystalline::ResultCache # Return the current monotonic time. def monotonic_now - Time.monotonic - @@reference_clock + Time.instant - @@reference_clock end end diff --git a/src/crystalline/text_document.cr b/src/crystalline/text_document.cr index 411a50e..cfc1fa7 100644 --- a/src/crystalline/text_document.cr +++ b/src/crystalline/text_document.cr @@ -63,7 +63,7 @@ class Crystalline::TextDocument end private def partial_update(contents : String, range : LSP::Range, version : Number? = nil) - prefix = @inner_contents[range.start.line]?.try &.[...range.start.character].chomp || "" + prefix = @inner_contents[range.start.line]?.try &.[...range.start.character] || "" suffix = @inner_contents[range.end.line]?.try &.[range.end.character..]? || @inner_contents[range.end.line]? || "" replacement_lines = String.build { |str| str << prefix << contents << suffix diff --git a/src/crystalline/workspace.cr b/src/crystalline/workspace.cr index c02f195..4a35014 100644 --- a/src/crystalline/workspace.cr +++ b/src/crystalline/workspace.cr @@ -68,22 +68,52 @@ class Crystalline::Workspace def format_document(params : LSP::DocumentFormattingParams) : {String, TextDocument}? @opened_documents[params.text_document.uri]?.try { |document| - {Crystal.format(document.contents), document} + contents = document.contents + return if contents.blank? + formatted = Crystal.format(contents) + # Basic safety check: if formatting returned an empty string but the original wasn't empty, + # something went wrong. Also check for basic syntax validity of the result. + begin + Crystal::Parser.parse(formatted) + rescue e + LSP::Log.warn { "Formatting skipped for #{params.text_document.uri}: the result contains syntax errors. #{e.message}" } + return nil + end + {formatted, document} } rescue e - # swallow exceptions silently + LSP::Log.warn { "Formatting failed for #{params.text_document.uri}: #{e.message}" } + nil end def format_document(params : LSP::DocumentRangeFormattingParams) : {String, TextDocument}? @opened_documents[params.text_document.uri]?.try { |document| range = params.range - contents_lines = document.contents.lines(chomp: false)[range.start.line..range.end.line] - contents_lines[-1] = contents_lines.last[...range.end.character] if range.end.character > 0 - contents_lines[0] = contents_lines.first[range.start.character...] - {Crystal.format(contents_lines.join), document} + contents_lines = document.contents.lines(chomp: false)[range.start.line..range.end.line]? + return if contents_lines.nil? || contents_lines.empty? + + last_line = contents_lines.last + end_char = Math.min(range.end.character, last_line.size) + contents_lines[-1] = last_line[...end_char] + + first_line = contents_lines.first + start_char = Math.min(range.start.character, first_line.size) + contents_lines[0] = first_line[start_char...] + + target = contents_lines.join + return if target.blank? + + formatted = Crystal.format(target) + # For range formatting, we might not be able to parse the fragment alone, + # but we can check if it's empty. + # Also chomp the result because Crystal.format always adds a trailing newline. + return if formatted.blank? + + {formatted.chomp, document} } rescue e - # swallow exceptions silently + LSP::Log.warn { "Range formatting failed for #{params.text_document.uri}: #{e.message}" } + nil end # Run a top level semantic analysis to compute dependencies. @@ -159,7 +189,7 @@ class Crystalline::Workspace return cached_result unless cached_result.nil? && discard_nil_cached_result end - sync_channel = Channel(Crystal::Compiler::Result?).new + sync_channel = Channel(Crystal::Compiler::Result?).new(1) progress.report(server) do file_overrides = nil @@ -172,16 +202,36 @@ class Crystalline::Workspace @opened_documents.each { |uri_str, text_document| contents = text_overrides.try(&.[uri_str]?) || text_document.contents contents = fix_source(contents) + file_overrides[URI.parse(uri_str).decoded_path] = contents + } - if target_string == uri_str - # If the entry point itself needs to be loaded from memory. - sources = [ - Crystal::Compiler::Source.new(target.decoded_path, contents), - ] + if (doc = @opened_documents[target_string]?) + contents = text_overrides.try(&.[target_string]?) || doc.contents + sources = [Crystal::Compiler::Source.new(target.decoded_path, fix_source(contents))] + end + + # Fix for #67: if the project has a src/requires.cr file, add it as an additional source + # to force discovery of classes, without shifting line numbers of the entry point. + if project && (root_path = project.root_uri.decoded_path) + requires_path = Path[root_path, "src", "requires.cr"] + requires_path_s = requires_path.to_s + requires_uri = "file://#{requires_path}" + + if File.exists?(requires_path) + # Priority: text_overrides -> opened_documents -> filesystem + requires_contents = text_overrides.try(&.[requires_uri]?) || + @opened_documents[requires_uri]?.try(&.contents) || + File.read(requires_path) + + requires_contents = fix_source(requires_contents) + + # IMPORTANT: Do not add it to sources if it is already the target! + if sources && target_string != requires_uri + sources << Crystal::Compiler::Source.new(requires_path_s, requires_contents) + end + file_overrides[requires_path_s] = requires_contents end - file_path = URI.parse(uri_str).decoded_path - file_overrides[file_path] = contents - } + end end lib_path = project.try(&.default_lib_path) @@ -361,11 +411,52 @@ class Crystalline::Workspace nil end - def completion(server : LSP::Server, file_uri : URI, position : LSP::Position, trigger_character : String?) + def completion(server : LSP::Server, file_uri : URI, position : LSP::Position, trigger_character : String?) : LSP::CompletionList? text_document = @opened_documents[file_uri.to_s]? return unless text_document - # LSP::Log.info { "completion: #{trigger_character}"} + # Check if we are inside a comment. + current_line = text_document.contents.lines(chomp: false)[position.line]? + if current_line + # Convert UTF-16 character position to byte offset for the lexer. + byte_offset = 0 + char_offset = 0 + current_line.each_char do |char| + # In UTF-16, characters > 0xFFFF take 2 code units (surrogate pair). + u16_size = char.ord > 0xFFFF ? 2 : 1 + break if char_offset + u16_size > position.character + + byte_offset += char.bytesize + char_offset += u16_size + end + # Lexer column numbers are 1-based byte offsets. + lexer_column = byte_offset + 1 + + lexer = Crystal::Lexer.new(current_line) + begin + loop do + token = lexer.next_token + break if token.type.eof? + + if (loc = token.location) + break if loc.column_number > lexer_column + end + + if token.type.comment? + if (loc = token.location) + token_start = loc.column_number + # If the cursor is at or after the start of the comment. + if lexer_column >= token_start + return nil + end + end + end + end + rescue Crystal::SyntaxException + # Ignore syntax errors while typing + end + end + document_lines = fix_source(text_document.contents).lines(chomp: false) left_offset = 0 @@ -420,7 +511,6 @@ class Crystalline::Workspace result = self.compile( server, file_uri, - in_memory: true, discard_nil_cached_result: true, wants_doc: true, text_overrides: text_overrides,