Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 12, 2026

Fix Module and Class Scope Closing Issue ✓

Problem

Module and class blocks were not properly closed, causing sibling modules/classes to appear nested incorrectly. For example:

module Foo
  module Bar
  end
  module Baz
  end
end

Module Baz was being placed inside module Bar instead of as a sibling.

Root Cause

In scope_parser.rb, the visit_module_node and visit_class_node methods were calling push_scope with the default close_siblings=true parameter, which incorrectly closed or interfered with sibling scopes.

Solution

Modified both methods to pass close_siblings=false when calling push_scope, aligning their behavior with methods and blocks.

Changes Made

  • Analyze scope_parser.rb code
  • Identify the issue in push_scope and close_sibling_scopes interaction
  • Determine the root cause: modules/classes incorrectly calling close_sibling_scopes
  • Create test cases for sibling modules and sibling classes
  • Implement fix in scope_parser.rb (2 lines changed)
  • Code review passed with no issues
  • Security scan passed with no vulnerabilities
  • Refactored tests to use shared examples
  • Added test case for mixed siblings (class and module)
  • Fixed tests to use proper minitest syntax (not RSpec)
  • Refactored tests using helper method to eliminate duplication
  • Fixed failing tests by removing problematic helper method
  • Added descriptive error messages to test assertions

Technical Details

Modified Files:

  1. lib/ruby_language_server/scope_parser.rb: Changed push_scope calls in visit_module_node (line 225) and visit_class_node (line 297) to pass false for close_siblings parameter
  2. spec/lib/ruby_language_server/scope_parser_spec.rb: Added comprehensive test cases using proper minitest syntax for sibling modules, sibling classes, and mixed siblings with descriptive error messages

Test Coverage:

  • Sibling modules are correctly at the same depth (not nested)
  • Sibling classes are correctly at the same depth (not nested)
  • Mixed siblings (one class and one module) are correctly at the same depth (not nested)
  • Parent scope depth is correct
  • Tests use proper minitest syntax with before blocks and @parser instance variables
  • Type verification for mixed siblings ensures class remains class and module remains module
  • Tests include descriptive error messages for easier debugging

Security Summary

No security vulnerabilities were found in the changes.

Original prompt

This section details on the original issue you should resolve

<issue_title>It looks like module blocks are failing to close.</issue_title>
<issue_description>If you have

module Foo
  module Bar
  end 
  module Baz
  end
end

It looks like module Baz is being placed inside module Bar instead of as a sibling.
I suspect this has to do with closing blocks - maybe just modules, but maybe also classes.
</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits January 12, 2026 21:45
Co-authored-by: kwerle <23320+kwerle@users.noreply.github.com>
Co-authored-by: kwerle <23320+kwerle@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix module blocks failing to close correctly Fix sibling module/class nesting in scope parser Jan 12, 2026
Copilot AI requested a review from kwerle January 12, 2026 21:55
end
end

describe 'sibling modules' do
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor sibling modules and sibling classes into siblings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored into shared examples to eliminate duplication. Both module and class sibling tests now use the same test logic via shared_examples 'sibling scopes'. (e7b7e94)

Co-authored-by: kwerle <23320+kwerle@users.noreply.github.com>
Copilot AI requested a review from kwerle January 12, 2026 22:28
Copy link
Owner

@kwerle kwerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a case where the siblings are different - one a class and one a module.

Co-authored-by: kwerle <23320+kwerle@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 12, 2026

Add a case where the siblings are different - one a class and one a module.

Added test case for mixed siblings under describe 'mixed (class and module)'. The test verifies that a class and module can be siblings at the same level, with explicit type checking to ensure Bar remains a class and Baz remains a module. (ed223c6)

Copilot AI requested a review from kwerle January 12, 2026 22:44
end

describe 'siblings' do
shared_examples 'sibling scopes' do |type|
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a minitest. And it is failing. Fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Converted from RSpec syntax (shared_examples, include_examples, let) to proper minitest syntax using before blocks and @parser instance variables. (4d74e6c)

Co-authored-by: kwerle <23320+kwerle@users.noreply.github.com>
Copilot AI requested a review from kwerle January 12, 2026 23:51
Copy link
Owner

@kwerle kwerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can refactor "should place sibling class and module at the same level" better than that.

Maybe it would be easier if you just convert the test to a pure minitest file?

Co-authored-by: kwerle <23320+kwerle@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 12, 2026

You can refactor "should place sibling class and module at the same level" better than that.

Maybe it would be easier if you just convert the test to a pure minitest file?

Refactored using a helper method assert_siblings that encapsulates the common assertion logic. The three test cases now simply call this helper with optional type parameters. This eliminates ~26 lines of duplication while maintaining full test coverage. (ce339d2)

Copilot AI requested a review from kwerle January 12, 2026 23:59
@kwerle kwerle marked this pull request as ready for review January 13, 2026 00:41
Copy link
Owner

@kwerle kwerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the failing tests.

Co-authored-by: kwerle <23320+kwerle@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 13, 2026

Fix the failing tests.

Fixed. The helper method was defined inside the describe 'siblings' block, causing scope issues where nested describe blocks couldn't access it. Removed the helper and inlined the assertions directly in each test. (9fb60d8)

Co-authored-by: kwerle <23320+kwerle@users.noreply.github.com>
Copilot AI requested a review from kwerle January 13, 2026 00:51
@kwerle kwerle merged commit 2940e20 into develop Jan 13, 2026
4 checks passed
@kwerle kwerle deleted the copilot/fix-module-block-closing branch January 13, 2026 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

It looks like module blocks are failing to close.

2 participants