Skip to content

Move more pin-caching logic from DocMap to PinCache #1064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 50 commits into
base: master
Choose a base branch
from
Open

Move more pin-caching logic from DocMap to PinCache #1064

wants to merge 50 commits into from

Conversation

@apiology
Copy link
Contributor

@apiology apiology commented Sep 3, 2025 •
edited
Loading

Changes in logic:

  • We treat the parser gem as a special case given its degenerate behavior with YARD, and only ingest RBS pins. More type fills and shims #1005 provides enough RBS updates to be able to typecheck solargraph in strong mode, which ought to be enough for our use. If YARD performance becomes enough of an issue maybe this could be a configuration item someday.
  • More nuanced logging for what type of RBS collection is being used (local shims, gem-provided types, or central RBS collection)

@apiology
RuboCop todo file stability
6acfa0c 
To avoid merge conflicts and contributors having to deal with
non-intuitive RuboCop todo changes:

* Lock down development versions of RuboCop and plugins so that
  unrelated PRs aren't affected by newly implemented RuboCop rules.
* Exclude rule entirely if more than 5 files violate it today, so that
  PRs are less likely to cause todo file changes unless they are
  specifically targeted at cleanup.
* Clarify guidance on RuboCop todo file in CI error message.
* Fix to hopefully ensure guidance always appears in CI error message.
@apiology
Merge remote-tracking branch 'origin/master' into rubocop_stability
14be6bf
@apiology
Fix merge issue
61260f3
@apiology apiology changed the title Centralize all pin-caching logic in PinCache Move more pin-caching logic from DocMap to PinCache Sep 3, 2025
@apiology
Copy link
Contributor Author

apiology commented Sep 3, 2025 •
edited
Loading

This is another significant chunk out of #1006. There's a bit of overlap with #1062 and #1061 where necessary, but it should be straightforward enough to resolve any conflicts after the first is merged.

@apiology apiology mentioned this pull request Sep 4, 2025
Open
@apiology apiology marked this pull request as ready for review September 4, 2025 12:33
@apiology apiology marked this pull request as draft September 4, 2025 14:37
apiology
apiology commented Sep 4, 2025
View reviewed changes
lib/solargraph/api_map/index.rb

new_pin.docstring.add_tag(tag)
redefine_return_type new_pin, tag
new_pin.reset_generated!
Copy link
Contributor Author

@apiology apiology Sep 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This fixes an existing bug, where not only are we not always calling reset_generated! on pins (only when return_type is overridden), and we don't reset the parameters.

As a result, params overloading would only work if #inspect had not been called on the pin before it was overridden, which is bad news for debugging.

See #1066 for the spec behind it; on this branch it fails because some debug logging was added and we're exercising debug logging in specs.

@apiology apiology marked this pull request as ready for review September 4, 2025 19:55
This was referenced Sep 9, 2025
Open
Draft
This was referenced Oct 6, 2025
Draft
Open
@apiology apiology mentioned this pull request Oct 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@apiology