From cb7a460a30cc2eb55606fcf59dc6b9508b59bbcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9nich=20Bon=20=C4=86iri=C4=87?= Date: Mon, 27 Apr 2026 18:02:36 -0600 Subject: [PATCH 1/5] fix: resolve critical deadlocks and infinite loops Assisted-by: Gemini --- spec/reproduction_76_spec.cr | 59 ++++++++++++++++++++++++++ src/crystalline/analysis/analysis.cr | 16 ++++--- src/crystalline/progress.cr | 30 +++++++++----- src/crystalline/workspace.cr | 62 ++++++++++++++++++++++------ 4 files changed, 139 insertions(+), 28 deletions(-) create mode 100644 spec/reproduction_76_spec.cr 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/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/workspace.cr b/src/crystalline/workspace.cr index c02f195..8f50a7b 100644 --- a/src/crystalline/workspace.cr +++ b/src/crystalline/workspace.cr @@ -159,7 +159,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 +172,13 @@ class Crystalline::Workspace @opened_documents.each { |uri_str, text_document| contents = text_overrides.try(&.[uri_str]?) || text_document.contents contents = fix_source(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), - ] - end - file_path = URI.parse(uri_str).decoded_path - file_overrides[file_path] = contents + file_overrides[URI.parse(uri_str).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 end lib_path = project.try(&.default_lib_path) @@ -361,11 +358,51 @@ 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 +457,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, From 15f1401d6943dd54ae3bf7ca39c361a302793467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9nich=20Bon=20=C4=86iri=C4=87?= Date: Mon, 27 Apr 2026 18:03:42 -0600 Subject: [PATCH 2/5] fix: modernize cache and tune GC Assisted-by: Gemini --- src/crystalline/ext/boehm.cr | 5 +++-- src/crystalline/result_cache.cr | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) 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/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 From 02de1fa3aa8af16edc098a6d699482291af3c5f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9nich=20Bon=20=C4=86iri=C4=87?= Date: Mon, 27 Apr 2026 18:03:53 -0600 Subject: [PATCH 3/5] fix: resolve sync drift and improve formatting safety Assisted-by: Gemini --- spec/reproduction_41_spec.cr | 27 +++++++++++++++++ spec/reproduction_63_spec.cr | 51 ++++++++++++++++++++++++++++++++ src/crystalline/text_document.cr | 2 +- src/crystalline/workspace.cr | 45 +++++++++++++++++++++++----- 4 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 spec/reproduction_41_spec.cr create mode 100644 spec/reproduction_63_spec.cr 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/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 8f50a7b..f8831e5 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. @@ -404,6 +434,7 @@ class Crystalline::Workspace end end + document_lines = fix_source(text_document.contents).lines(chomp: false) left_offset = 0 right_offset = 0 From 63cd5c472ff8360b6096ac1033ed75aa08d8ac39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9nich=20Bon=20=C4=86iri=C4=87?= Date: Mon, 27 Apr 2026 18:04:47 -0600 Subject: [PATCH 4/5] feat: automatic discovery of src/requires.cr Assisted-by: Gemini --- src/crystalline/workspace.cr | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/crystalline/workspace.cr b/src/crystalline/workspace.cr index f8831e5..4a35014 100644 --- a/src/crystalline/workspace.cr +++ b/src/crystalline/workspace.cr @@ -209,6 +209,29 @@ class Crystalline::Workspace 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 + end end lib_path = project.try(&.default_lib_path) From 207e360af2e96e03cc7900b9ed7501945b43d3ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9nich=20Bon=20=C4=86iri=C4=87?= Date: Mon, 27 Apr 2026 18:04:48 -0600 Subject: [PATCH 5/5] feat: add log-level and version CLI flags Assisted-by: Gemini --- src/crystalline.cr | 38 ++++++++++++++++++++++++++++++++++---- src/crystalline/main.cr | 15 +++++++++++---- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/crystalline.cr b/src/crystalline.cr index ae14682..4b053b0 100644 --- a/src/crystalline.cr +++ b/src/crystalline.cr @@ -1,9 +1,39 @@ require "./crystalline/requires" require "./crystalline/*" +require "option_parser" -if ARGV.includes?("--version") - puts(Crystalline::VERSION) - exit +log_level = ::Log::Severity::Warn + +OptionParser.parse do |parser| + parser.banner = "Usage: crystalline [options]" + + parser.on("-v", "--version", "Show version") do + puts Crystalline::VERSION + exit + end + + parser.on("-h", "--help", "Show help") do + puts parser + exit + end + + parser.on("-l LEVEL", "--log LEVEL", "Set log level (debug, info, warn, error). Default: warn") do |level| + log_level = case level.downcase + when "debug" then ::Log::Severity::Debug + when "info" then ::Log::Severity::Info + when "warn" then ::Log::Severity::Warn + when "error" then ::Log::Severity::Error + else + STDERR.puts "Invalid log level: #{level}" + exit 1 + end + end + + parser.invalid_option do |flag| + STDERR.puts "ERROR: #{flag} is not a valid option." + STDERR.puts parser + exit 1 + end end -Crystalline.init +Crystalline.init(log_level: log_level) diff --git a/src/crystalline/main.cr b/src/crystalline/main.cr index 8e44cb7..5c072e6 100644 --- a/src/crystalline/main.cr +++ b/src/crystalline/main.cr @@ -44,12 +44,19 @@ module Crystalline end end - def self.init(*, input : IO = STDIN, output : IO = STDOUT) + def self.init(*, input : IO = STDIN, output : IO = STDOUT, log_level : ::Log::Severity = :warn) + # Setup a temporary backend to STDERR so we can log early initialization errors. + # This MUST be done first to avoid STDOUT corruption. + ::Log.setup(log_level, ::Log::IOBackend.new(STDERR)) + EnvironmentConfig.run - {% if flag?(:debug) %} - ::Log.setup(:debug, LSP::Log.backend.not_nil!) - {% end %} server = LSP::Server.new(input, output, SERVER_CAPABILITIES) + + # Re-setup with the LSP backend once the server is initialized. + if (backend = LSP::Log.backend) + ::Log.setup(log_level, backend) + end + Controller.new(server) rescue ex LSP::Log.error(exception: ex) { %(#{ex.message || "Unknown error during init."}\n#{ex.backtrace.join('\n')}) }