Skip to content

At alpha typechecking level, warn on method calls off of nilable types - #803

Closed
apiology wants to merge 215 commits into
castwide:masterfrom
apiology:stricter_unions
Closed

At alpha typechecking level, warn on method calls off of nilable types#803
apiology wants to merge 215 commits into
castwide:masterfrom
apiology:stricter_unions

Conversation

@apiology

@apiology apiology commented Mar 20, 2025

Copy link
Copy Markdown
Contributor

Require that all unique types of a complex type offer the same method before it can match when "loose unions" mode is turned off.

Depends on:

Included to fix build:

Includes for convenience:

More specifically, require that all unique types of a complex type
offer the same method before it can match.
@apiology apiology changed the title At strong level, don't allow method calls off of nilable types At strong typechecking level, warn on method calls off of nilable types Mar 20, 2025
@apiology
apiology marked this pull request as draft March 21, 2025 13:14
@apiology

apiology commented Mar 21, 2025

Copy link
Copy Markdown
Contributor Author

Realistically, there's enough return if foo.nil? code out there that this will probably have to wait until we have flow-sensitive typing. If that end up being difficult, I may propose hiding this under a --level={alpha,beta} level or somesuch. Long term we may want a flag to turn on new checks introduced between major releases, to help keep folks from being broken when we introduce things? Not sure.

apiology added 26 commits April 25, 2025 07:35
```sh
$ SOLARGRAPH_ASSERTS=on bundle exec solargraph method_pin --rbs 'RuboCop::AST::ArrayNode#values'
def values: () -> Array
$ bundle exec solargraph help method_pin
Usage:
  solargraph method_pin [PATH]

Options:
  [--rbs], [--no-rbs], [--skip-rbs]           # Output the pin as RBS
                                              # Default: false
  [--typify], [--no-typify], [--skip-typify]  # Output the calculated return type of the pin from annotations
                                              # Default: false
  [--probe], [--no-probe], [--skip-probe]     # Output the calculated return type of the pin from annotations and inference
                                              # Default: false
  [--stack], [--no-stack], [--skip-stack]     # Show entire stack by including definitions in superclasses
                                              # Default: false

Describe a method pin
$
```
# ::Array<::Solargraph::ComplexType::UniqueType> does not match
# inferred type ::Array<::Proc> for Solargraph::ComplexType#map
def map(&block)
@items.map(&block)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I suspect we're not understanding the &block construct

if !api_map.loose_unions && pin_groups.any? { |pins| pins.empty? }
pin_groups = []
end
pins = pin_groups.flatten.uniq(&:path)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a key bit - sharpening our existing behavior when asked to require all union types to match the behavior.

expect(checker.problems.map(&:message))
.to eq(['Wrong argument type for #foo: a expected String, received String, nil'])
end
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the real win here - the ability to check for potential nil issues

@apiology

apiology commented Mar 2, 2026

Copy link
Copy Markdown
Contributor Author

Merged into v0.59 release branch

@apiology apiology closed this Mar 2, 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