Re-enable support for .gem_rbs_collection directories #942
Conversation
|
This is going to be needed to modernize solargraph-rails to use the RBS collection types, as well as to include shims needed to typecheck solargraph at strict level. I have another PR incoming which tries harder to pick the best types between multiple method pins on the same path - will leave this as draft until that one's open, at which point it becomes a prereq. @castwide, is that the nature of the issue you had with the collection support originally? |
| @@ -34,6 +34,8 @@ jobs: | |||
| bundle exec solargraph config | |||
| yq -yi '.plugins += ["solargraph-rails"]' .solargraph.yml | |||
| yq -yi '.plugins += ["solargraph-rspec"]' .solargraph.yml | |||
| - name: Install gem types | |||
| run: bundle exec rbs collection install | |||
There was a problem hiding this comment.
Let's exercise the functionality in our own build
| @@ -47,7 +47,7 @@ class InvalidRubocopVersionError < RuntimeError; end | |||
| autoload :Parser, 'solargraph/parser' | |||
| autoload :RbsMap, 'solargraph/rbs_map' | |||
| autoload :GemPins, 'solargraph/gem_pins' | |||
| autoload :Cache, 'solargraph/cache' | |||
| autoload :PinCache, 'solargraph/pin_cache' | |||
There was a problem hiding this comment.
Not a fantastic name ("GemCache?"), but it helped me keep this distinct in my head from ApiMap::Cache
| @@ -178,15 +209,14 @@ def self.load directory | |||
| # @param directory [String] | |||
| # @param out [IO] The output stream for messages | |||
| # @return [ApiMap] | |||
| def self.load_with_cache directory, out = IO::NULL | |||
There was a problem hiding this comment.
IO::NULL resolved to "/dev/null", not an object which accepted puts()
| def deserialize_combined_pin_cache(gemspec) | ||
| unless combined_pins_in_memory[[gemspec.name, gemspec.version]].nil? | ||
| return combined_pins_in_memory[[gemspec.name, gemspec.version]] | ||
| end |
There was a problem hiding this comment.
We now cache the combined pins to disk. No perf effect with this PR, but I strongly suspect it matters with #946 where the combination logic is more complex.
| if lockfile_path.exist? | ||
| collection_config = RBS::Collection::Config.from_path lockfile_path | ||
| gem_config = collection_config.gem(library) | ||
| data = gem_config&.to_s |
There was a problem hiding this comment.
This is the key to cache invalidation when a user updates their RBS collection.
Yep! This is precisely how I hoped to leverage RBS collections instead of implementing our own mechanism for custom sig extensions. I appreciate your work consolidating variable source definitions, too. |
This is probably best released after or alongside #946 to make sure that RBS collection types don't make current pin results any worse.