Skip to content

Add ivar support to flow sensitive typing - #19

Closed
apiology wants to merge 13 commits into
union_type_enforcementfrom
union_type_enforcement_ivars
Closed

apiology wants to merge 13 commits into
union_type_enforcementfrom
union_type_enforcement_ivars

Conversation

@apiology

@apiology apiology commented Nov 20, 2025 •

Copy link
Copy Markdown
Owner

Adds support for doing flow-sensitive typing in instance variables, not just local variables.

Note: This approach assumes single-threaded use for type-checking purposes - it's possible to be clever and subvert it, but as a way to reduce false positives in nil-checking, it seems very effective.

Builds on:

@apiology

Copy link
Copy Markdown
Owner Author

@copilot review

Copilot AI commented Nov 20, 2025

Copy link
Copy Markdown

@apiology I've opened a new pull request, #20, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for instance variables (ivars) to flow-sensitive typing. Previously, flow-sensitive typing only handled local variables (lvars), which limited type narrowing in conditional expressions. The implementation tracks instance variables similarly to local variables, enabling proper type inference for patterns like !@foo.nil? && @foo.upcase.

Key changes:

  • Extended FlowSensitiveTyping class to track and process instance variables alongside local variables
  • Modified node processors to thread an ivars array parameter alongside the existing locals parameter
  • Updated InstanceVariable chain resolution to use var_at_location for flow-sensitive type selection

Reviewed Changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
lib/solargraph/parser/flow_sensitive_typing.rb Core logic updated to handle ivars in addition to lvars for type narrowing
lib/solargraph/parser/node_processor.rb Added ivars parameter to process method signature and return type
lib/solargraph/parser/node_processor/base.rb Added ivars reader and updated constructor to accept ivars parameter
lib/solargraph/parser/parser_gem/class_methods.rb Modified map method to handle ivars in return value
lib/solargraph/parser/parser_gem/node_processors/*.rb Updated all node processors to thread ivars parameter through processing
lib/solargraph/source/chain/instance_variable.rb Updated resolve method to use var_at_location for flow-sensitive ivar resolution
lib/solargraph/library.rb Removed obsolete @sg-ignore comments for ivar handling
lib/solargraph/pin/*.rb Removed redundant methods and obsolete flow-sensitive typing comments
lib/solargraph/type_checker/rules.rb Updated todo count for flow-sensitive typing improvements
spec/type_checker/levels/strong_spec.rb Added comprehensive tests for ivar flow-sensitive typing
spec/type_checker/levels/alpha_spec.rb Removed pending tests that are now covered
spec/source/chain/instance_variable_spec.rb Updated tests to include location parameters
spec/parser/flow_sensitive_typing_spec.rb Added test for ivar nil check pattern
.rubocop_todo.yml Updated exclusions to reflect code cleanup

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/solargraph/parser/flow_sensitive_typing.rb
Comment thread lib/solargraph/parser/flow_sensitive_typing.rb
Comment thread lib/solargraph/parser/flow_sensitive_typing.rb
Comment thread lib/solargraph/parser/parser_gem/node_processors/masgn_node.rb Outdated
Copilot AI and others added 2 commits November 20, 2025 14:08
Co-authored-by: apiology <3681194+apiology@users.noreply.github.com>
Fix typo and add instance variable test coverage for flow sensitive typing
@apiology
apiology marked this pull request as ready for review November 27, 2025 15:26
@apiology apiology closed this Jan 31, 2026
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.

3 participants