From fe9b7e608735a536d376b750a58ac9e76b418a1b Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 14:36:48 -0400 Subject: [PATCH 1/9] Run specs in parallel, speed up CI, fix concurrency bugs Rebases castwide/solargraph#1167 (apiology/parallel_rspec) onto castwide/master instead of v0.59, since master and v0.59 have diverged substantially and v0.59 carries unrelated changes. This commit is the net diff of that branch (plus its merge-conflict and CI-regression fixes) applied directly against master; history was not preserved per request. Highlights: - Parallelize per-gem YARD/RBS pin caching with a thread pool (doc_map.rb, shell.rb) instead of caching gems serially. - Fix a mutex re-entry deadlock in Library#sync_catalog when the next cacheable gemspec is already being processed elsewhere (castwide/solargraph#1220), with its regression test. - Fix an exponential-blowup bug in Pin::Method#combine_same_type_arity_signatures (O(n^2) bail-out for large signature sets), with its regression test. - shell.rb's gems 'core' command called PinCache.core?/PinCache.cache_core, which never existed; use the real Solargraph::RbsMap::CoreMap#pins API instead. - Bundler::LazySpecification#materialize_for_installation is an internal, undocumented Bundler API whose arity changed without a deprecation path; guard against all known shapes (modern wrapper, old zero-arg method, incompatible-arity method) instead of assuming one signature. - Misc RuboCop/YARD-doc fixes and .rubocop_todo.yml updates. --- .envrc | 7 + .github/workflows/linting.yml | 4 +- .github/workflows/plugins.yml | 215 ++++++++++-- .github/workflows/rspec.yml | 48 ++- .rspec_parallel | 12 + .rubocop_todo.yml | 112 +++++-- Rakefile | 16 +- lib/solargraph/api_map.rb | 15 +- lib/solargraph/api_map/constants.rb | 4 +- lib/solargraph/api_map/store.rb | 4 +- lib/solargraph/complex_type.rb | 2 + lib/solargraph/complex_type/unique_type.rb | 7 +- lib/solargraph/diagnostics/base.rb | 6 +- lib/solargraph/diagnostics/type_check.rb | 5 +- lib/solargraph/doc_map.rb | 74 +++-- lib/solargraph/language_server/host.rb | 18 + .../language_server/host/diagnoser.rb | 11 +- lib/solargraph/library.rb | 8 +- lib/solargraph/pin/method.rb | 10 +- lib/solargraph/rbs_map/core_map.rb | 2 +- lib/solargraph/rbs_map/stdlib_map.rb | 6 + lib/solargraph/shell.rb | 95 +++--- lib/solargraph/source/chain.rb | 1 + lib/solargraph/source_map.rb | 2 +- lib/solargraph/workspace.rb | 63 ++-- lib/solargraph/workspace/config.rb | 6 +- lib/solargraph/workspace/gemspecs.rb | 35 +- lib/solargraph/workspace/require_paths.rb | 2 +- solargraph.gemspec | 5 + spec/api_map_method_spec.rb | 33 +- spec/api_map_spec.rb | 10 +- spec/diagnostics/rubocop_helpers_spec.rb | 2 +- spec/doc_map_spec.rb | 32 +- spec/fixtures/workspace-with-gemfile/Gemfile | 2 +- spec/gem_pins_spec.rb | 11 +- spec/language_server/host_spec.rb | 56 +++- .../extended/check_gem_version_spec.rb | 5 + .../message/text_document/definition_spec.rb | 14 +- .../message/text_document/hover_spec.rb | 8 +- .../message/text_document/rename_spec.rb | 87 ++--- .../text_document/type_definition_spec.rb | 10 + .../did_change_watched_files_spec.rb | 5 +- spec/language_server/protocol_spec.rb | 66 +++- .../language_server/transport/adapter_spec.rb | 7 +- spec/library_spec.rb | 133 ++++++-- spec/parallel_runtime_rspec.log | 111 +++++++ spec/parser/flow_sensitive_typing_spec.rb | 307 +++++++++--------- spec/pin/base_spec.rb | 16 +- spec/pin/method_spec.rb | 30 +- spec/rbs_map/conversions_spec.rb | 106 +++--- spec/rbs_map_spec.rb | 2 +- spec/shell_spec.rb | 192 ++++++----- spec/source/chain_spec.rb | 49 +-- spec/source_map/clip_spec.rb | 2 +- spec/spec_helper.rb | 36 +- spec/type_checker/levels/strict_spec.rb | 13 +- .../gemspecs_fetch_dependencies_spec.rb | 15 +- .../gemspecs_resolve_require_spec.rb | 162 ++++++--- spec/workspace/require_paths_spec.rb | 20 +- spec/yard_map/mapper_spec.rb | 33 +- 60 files changed, 1618 insertions(+), 752 deletions(-) create mode 100644 .rspec_parallel create mode 100644 spec/parallel_runtime_rspec.log diff --git a/.envrc b/.envrc index 92f925b17..2d6fa5f4f 100644 --- a/.envrc +++ b/.envrc @@ -1,3 +1,10 @@ +#!/bin/bash + +if [[ "$(uname)" == "Darwin" ]]; then + WORKERS=$(sysctl -n hw.physicalcpu) + export WORKERS +fi + # current git branch SOLARGRAPH_FORCE_VERSION=0.0.1.dev-$(git rev-parse --abbrev-ref HEAD | tr -d '\n' | tr -d '/' | tr -d '-'| tr -d '_') export SOLARGRAPH_FORCE_VERSION diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index 489cbaad7..f6c24a42a 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -90,7 +90,7 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: 3.4 - bundler-cache: false + bundler-cache: true - name: Install gems run: bundle install @@ -109,7 +109,7 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: 3.4 - bundler-cache: false + bundler-cache: true - name: Install gems run: bundle install diff --git a/.github/workflows/plugins.yml b/.github/workflows/plugins.yml index 218f598df..9c6a1f368 100644 --- a/.github/workflows/plugins.yml +++ b/.github/workflows/plugins.yml @@ -15,7 +15,7 @@ permissions: contents: read jobs: - regression: + rails_and_rspec_typechecking: runs-on: ubuntu-latest steps: @@ -47,9 +47,52 @@ jobs: run: bundle exec solargraph typecheck --level strong # @todo Temporary, expect to revert in 0.60 continue-on-error: true + rails_and_rspec_specs: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.4 + bundler-cache: true + - uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: yq + version: 1.0 + - name: Install gems + run: | + echo 'gem "solargraph-rails"' > .Gemfile + echo 'gem "solargraph-rspec"' >> .Gemfile + bundle install + bundle update --pre rbs + - name: Configure to use plugins + run: | + 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 update + # avoid trying to do this in parallel during the specs + time bundle exec solargraph gems core stdlib default - name: Ensure specs still run - run: bundle exec rake spec - rails: + run: | + # Speed up some of the bundle installs we run inside the tests + # as well when we're testing different solargraph usage + # scenarios. This is already set in the local bundle config by + # the setup-ruby action. + # + # See + # https://github.com/ruby/setup-ruby?tab=readme-ov-file#caching-bundle-install-automatically + bundle config set path $PWD/vendor/bundle + + SIMPLECOV_DISABLED=true + export SIMPLECOV_DISABLED + + bundle exec rake full_spec + rails_typechecking: runs-on: ubuntu-latest steps: @@ -60,7 +103,7 @@ jobs: ruby-version: 3.4 # keep same as typecheck.yml # See https://github.com/castwide/solargraph/actions/runs/19000135777/job/54265647107?pr=1119 rubygems: latest - bundler-cache: false + bundler-cache: true - uses: awalsh128/cache-apt-pkgs-action@latest with: packages: yq @@ -82,9 +125,51 @@ jobs: run: bundle exec solargraph typecheck --level strong # @todo Temporary, expect to revert in 0.60 continue-on-error: true + rails_specs: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.4 + rubygems: latest + bundler-cache: true + - uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: yq + version: 1.0 + - name: Install gems + run: | + echo 'gem "solargraph-rails"' > .Gemfile + bundle install + bundle update --pre rbs + - name: Configure to use plugins + run: | + bundle exec solargraph config + yq -yi '.plugins += ["solargraph-rails"]' .solargraph.yml + - name: Install gem types + run: | + bundle exec rbs collection update + # avoid trying to do this in parallel during the specs + bundle exec solargraph gems core stdlib - name: Ensure specs still run - run: bundle exec rake spec - rspec: + run: | + # Speed up some of the bundle installs we run inside the tests + # as well when we're testing different solargraph usage + # scenarios. This is already set in the local bundle config by + # the setup-ruby action. + # + # See + # https://github.com/ruby/setup-ruby?tab=readme-ov-file#caching-bundle-install-automatically + bundle config set path $PWD/vendor/bundle + + SIMPLECOV_DISABLED=true + export SIMPLECOV_DISABLED + + bundle exec rake full_spec + rspec_typechecking: runs-on: ubuntu-latest steps: @@ -93,7 +178,7 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: 3.4 # keep same as typecheck.yml - bundler-cache: false + bundler-cache: true - uses: awalsh128/cache-apt-pkgs-action@latest with: packages: yq @@ -115,9 +200,53 @@ jobs: run: bundle exec solargraph typecheck --level strong # @todo Temporary, expect to revert in 0.60 continue-on-error: true + rspec_specs: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.4 # keep same as typecheck.yml + bundler-cache: true + - uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: yq + version: 1.0 + - name: Install gems + run: | + echo 'gem "solargraph-rspec"' >> .Gemfile + bundle install + bundle update --pre rbs + - name: Configure to use plugins + run: | + bundle exec solargraph config + yq -yi '.plugins += ["solargraph-rspec"]' .solargraph.yml + - name: Install gem types + run: | + set -x + + bundle exec rbs collection update + + rspec_gems=$(bundle exec ruby -r 'solargraph-rspec' -e 'puts Solargraph::Rspec::Gems.gem_names.join(" ")' 2>/dev/null | tail -n1) + # avoid trying to do this in parallel during the specs + bundle exec solargraph gems core stdlib $rspec_gems diff-lcs addressable ast rexml crack hashdiff rspec-support bigdecimal public_suffix - name: Ensure specs still run - run: bundle exec rake spec + run: | + # Speed up some of the bundle installs we run inside the tests + # as well when we're testing different solargraph usage + # scenarios. This is already set in the local bundle config by + # the setup-ruby action. + # + # See + # https://github.com/ruby/setup-ruby?tab=readme-ov-file#caching-bundle-install-automatically + bundle config set path $PWD/vendor/bundle + + SIMPLECOV_DISABLED=true + export SIMPLECOV_DISABLED + bundle exec rake full_spec run_solargraph_rspec_specs: # check out solargraph-rspec as well as this project, and point the former to use the latter as a local gem runs-on: ubuntu-latest @@ -140,14 +269,14 @@ jobs: with: ruby-version: 3.4 rubygems: latest - bundler-cache: false + bundler-cache: true - name: Install gems run: | set -x cd ../solargraph-rspec echo "gem 'solargraph', path: '../solargraph'" >> Gemfile - bundle config path ${{ env.BUNDLE_PATH }} + bundle config set path ${{ env.BUNDLE_PATH }} bundle install --jobs 4 --retry 3 bundle exec appraisal install # @todo some kind of appraisal/bundle conflict? @@ -177,7 +306,21 @@ jobs: bundle exec appraisal solargraph gems $rspec_gems - name: Run specs run: | + # Speed up some of the bundle installs we run inside the tests + # as well when we're testing different solargraph usage + # scenarios. This is already set in the local bundle config by + # the setup-ruby action. + # + # See + # https://github.com/ruby/setup-ruby?tab=readme-ov-file#caching-bundle-install-automatically + bundle config set path $PWD/vendor/bundle cd ../solargraph-rspec + + SIMPLECOV_DISABLED=true + export SIMPLECOV_DISABLED + + # avoid trying to do this in parallel during the specs + bundle exec solargraph gems core stdlib bundle exec appraisal rspec --format progress run_solargraph_rails_specs: @@ -204,34 +347,50 @@ jobs: MATRIX_RAILS_VERSION: "7.0" - name: Install gems run: | - set -x - BUNDLE_PATH="${GITHUB_WORKSPACE:?}/vendor/bundle" - export BUNDLE_PATH - cd ../solargraph-rails - echo "gem 'solargraph', path: '${GITHUB_WORKSPACE:?}'" >> Gemfile - bundle install - bundle update --pre rbs - RAILS_DIR="$(pwd)/spec/rails7" - export RAILS_DIR - cd ${RAILS_DIR} - bundle install - bundle exec --gemfile ../../Gemfile rbs --version - bundle exec --gemfile ../../Gemfile rbs collection install - cd ../../ - # bundle exec rbs collection init - # bundle exec rbs collection install + set -x + # Share caches to speed up bundle install + # + # See + # https://github.com/ruby/setup-ruby?tab=readme-ov-file#caching-bundle-install-automatically + cd ../solargraph-rails + echo "gem 'solargraph', path: '${GITHUB_WORKSPACE:?}'" >> Gemfile + bundle install + bundle update --pre rbs + env: + MATRIX_RAILS_VERSION: "7.0" + MATRIX_RAILS_MAJOR_VERSION: '7' + - name: Install gem types + run: | + cd ../solargraph-rails + + RAILS_DIR="$(pwd)/spec/rails7" + export RAILS_DIR + cd ${RAILS_DIR} + bundle install + bundle exec --gemfile ../../Gemfile rbs --version + bundle exec --gemfile ../../Gemfile rbs collection install + cd ../../ + # bundle exec rbs collection init + # bundle exec rbs collection install env: MATRIX_RAILS_VERSION: "7.0" MATRIX_RAILS_MAJOR_VERSION: '7' - name: Run specs run: | - BUNDLE_PATH="${GITHUB_WORKSPACE:?}/vendor/bundle" - export BUNDLE_PATH + # Share caches to speed up bundle install + # + # See + # https://github.com/ruby/setup-ruby?tab=readme-ov-file#caching-bundle-install-automatically + bundle config set path $PWD/vendor/bundle cd ../solargraph-rails bundle exec solargraph --version bundle info solargraph bundle info rbs bundle info yard + + SIMPLECOV_DISABLED=true + export SIMPLECOV_DISABLED + ALLOW_IMPROVEMENTS=true bundle exec rake spec env: MATRIX_RAILS_VERSION: "7.0" diff --git a/.github/workflows/rspec.yml b/.github/workflows/rspec.yml index f75bbd15d..35d0ef1f0 100644 --- a/.github/workflows/rspec.yml +++ b/.github/workflows/rspec.yml @@ -39,6 +39,8 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby-version }} + # See https://github.com/apiology/solargraph/actions/runs/21799883466/job/62893545490?pr=27 + rubygems: latest bundler-cache: true - name: Set rbs version run: echo "gem 'rbs', '${{ matrix.rbs-version }}'" >> .Gemfile @@ -52,9 +54,24 @@ jobs: run: | bundle update rbs # use latest available for this Ruby version - name: Update types - run: bundle exec rbs collection update + run: | + bundle exec rbs collection update + # avoid trying to do this in parallel during the specs + bundle exec solargraph gems core stdlib - name: Run tests - run: bundle exec rake spec + run: | + # Speed up some of the bundle installs we run inside the tests + # as well when we're testing different solargraph usage + # scenarios. This is already set in the local bundle config by + # the setup-ruby action. + # + # See + # https://github.com/ruby/setup-ruby?tab=readme-ov-file#caching-bundle-install-automatically + bundle config set path $PWD/vendor/bundle + SIMPLECOV_DISABLED=true + export SIMPLECOV_DISABLED + + bundle exec rake full_spec undercover: runs-on: ubuntu-latest steps: @@ -68,10 +85,31 @@ jobs: with: ruby-version: '3.4' bundler-cache: true + - name: Update gems + run: | + bundle update rbs # use latest available for this Ruby version - name: Update types - run: bundle exec rbs collection update + run: | + set -x + + bundle exec rbs collection update + # avoid trying to do this in parallel during the specs + bundle exec solargraph gems core stdlib ast parser - name: Run tests - run: bundle exec rake spec + run: | + set -x + + # Speed up some of the bundle installs we run inside the tests + # as well when we're testing different solargraph usage + # scenarios. This is already set in the local bundle config by + # the setup-ruby action. + # + # See + # https://github.com/ruby/setup-ruby?tab=readme-ov-file#caching-bundle-install-automatically + bundle config set path $PWD/vendor/bundle + + bundle exec rake full_spec - name: Check PR coverage - run: bundle exec rake undercover + run: | + bundle exec rake undercover continue-on-error: true diff --git a/.rspec_parallel b/.rspec_parallel new file mode 100644 index 000000000..3af9812e9 --- /dev/null +++ b/.rspec_parallel @@ -0,0 +1,12 @@ +--seed 123 +--color +--require spec_helper +--profile +--order defined +--format progress +# to update test relative timings for better balancing in CI, +# uncomment the next line, run the specs, and PR in the changes: +# --format ParallelTests::RSpec::RuntimeLogger --out spec/parallel_runtime_rspec.log +--format ParallelTests::RSpec::SummaryLogger # --out tmp/spec_summary.log +# useful for debugging concurrency issues: +--format ParallelTests::RSpec::VerboseLogger diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 408a6dfcd..e503ad84a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -19,18 +19,37 @@ Gemspec/RequireMFA: - 'spec/fixtures/rdoc-lib/rdoc-lib.gemspec' - 'spec/fixtures/rubocop-custom-version/specifications/rubocop-0.0.0.gemspec' +# This cop supports safe autocorrection (--autocorrect). +Layout/ElseAlignment: + Exclude: + - 'lib/solargraph/rbs_translator.rb' + - 'lib/solargraph/source/chain/call.rb' + # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyleAlignWith, Severity. # SupportedStylesAlignWith: keyword, variable, start_of_line Layout/EndAlignment: Exclude: + - 'lib/solargraph/parser/parser_gem/node_processors/namespace_node.rb' + - 'lib/solargraph/rbs_translator.rb' - 'lib/solargraph/shell.rb' + - 'lib/solargraph/source/chain/call.rb' # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: Width, AllowedPatterns. Layout/IndentationWidth: Exclude: + - 'lib/solargraph/parser/parser_gem/node_processors/namespace_node.rb' + - 'lib/solargraph/rbs_map/conversions.rb' + - 'lib/solargraph/rbs_translator.rb' - 'lib/solargraph/shell.rb' + - 'lib/solargraph/source/chain/call.rb' + +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: AllowInHeredoc. +Layout/TrailingWhitespace: + Exclude: + - 'lib/solargraph/pin/base.rb' Lint/BinaryOperatorWithIdenticalOperands: Exclude: @@ -40,7 +59,6 @@ Lint/BinaryOperatorWithIdenticalOperands: Lint/BooleanSymbol: Exclude: - 'lib/solargraph/convention/struct_definition/struct_definition_node.rb' - - 'lib/solargraph/source/chain/literal.rb' # Configuration parameters: AllowedMethods. # AllowedMethods: enums @@ -48,11 +66,18 @@ Lint/ConstantDefinitionInBlock: Exclude: - 'spec/complex_type_spec.rb' +Lint/CopDirectiveSyntax: + Exclude: + - 'lib/solargraph/rbs_map/conversions.rb' + # Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches, IgnoreDuplicateElseBranch. Lint/DuplicateBranch: Exclude: - 'lib/solargraph/parser/parser_gem/node_chainer.rb' - 'lib/solargraph/pin/base.rb' + +Lint/DuplicateMethods: + Exclude: - 'lib/solargraph/rbs_map/conversions.rb' # This cop supports unsafe autocorrection (--autocorrect-all). @@ -66,6 +91,12 @@ Lint/UnderscorePrefixedVariableName: Exclude: - 'lib/solargraph/library.rb' +Lint/UnreachableCode: + Exclude: + - 'lib/solargraph/complex_type.rb' + - 'lib/solargraph/complex_type/type_methods.rb' + - 'lib/solargraph/complex_type/unique_type.rb' + # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods, IgnoreNotImplementedMethods, NotImplementedExceptions. # NotImplementedExceptions: NotImplementedError @@ -76,6 +107,7 @@ Lint/UnusedMethodArgument: Lint/UselessAssignment: Exclude: - 'lib/solargraph/pin/block.rb' + - 'lib/solargraph/rbs_map/conversions.rb' - 'spec/fixtures/long_squiggly_heredoc.rb' - 'spec/fixtures/rubocop-unused-variable-error/app.rb' - 'spec/fixtures/unicode.rb' @@ -85,14 +117,14 @@ Metrics/AbcSize: Exclude: - 'lib/solargraph/api_map/source_to_yard.rb' - 'lib/solargraph/parser/parser_gem/node_chainer.rb' + - 'lib/solargraph/shell.rb' - 'lib/solargraph/source/source_chainer.rb' - 'lib/solargraph/source_map/clip.rb' - - 'lib/solargraph/source_map/mapper.rb' # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns, inherit_mode. # AllowedMethods: refine Metrics/BlockLength: - Max: 61 + Max: 58 # Configuration parameters: CountBlocks, CountModifierForms. Metrics/BlockNesting: @@ -114,11 +146,16 @@ Metrics/CyclomaticComplexity: # Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns. Metrics/MethodLength: - Enabled: false + Exclude: + - 'lib/solargraph/complex_type.rb' + - 'lib/solargraph/convention/struct_definition.rb' + - 'lib/solargraph/parser/parser_gem/node_chainer.rb' + - 'lib/solargraph/shell.rb' + - 'lib/solargraph/source/chain/call.rb' # Configuration parameters: CountComments, CountAsOne. Metrics/ModuleLength: - Max: 195 + Max: 139 # Configuration parameters: Max, CountKeywordArgs, MaxOptionalParameters. Metrics/ParameterLists: @@ -141,6 +178,16 @@ Naming/AccessorMethodName: - 'lib/solargraph/api_map/store.rb' - 'lib/solargraph/language_server/message/base.rb' +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: EnforcedStyle, BlockForwardingName. +# SupportedStyles: anonymous, explicit +Naming/BlockForwarding: + Exclude: + - 'lib/solargraph/complex_type.rb' + - 'lib/solargraph/complex_type/type_methods.rb' + - 'lib/solargraph/pin/base.rb' + - 'lib/solargraph/pin/common.rb' + # Configuration parameters: ForbiddenDelimiters. # ForbiddenDelimiters: (?i-mx:(^|\s)(EO[A-Z]{1}|END)(\s|$)) Naming/HeredocDelimiterNaming: @@ -183,6 +230,8 @@ RSpec/BeforeAfterAll: - 'spec/api_map_spec.rb' - 'spec/language_server/host/dispatch_spec.rb' - 'spec/language_server/protocol_spec.rb' + - 'spec/library_spec.rb' + - 'spec/yard_map/mapper_spec.rb' # Configuration parameters: IgnoredMetadata. RSpec/DescribeClass: @@ -194,6 +243,13 @@ RSpec/DescribeClass: - '**/spec/views/**/*' - 'spec/complex_type_spec.rb' +# This cop supports unsafe autocorrection (--autocorrect-all). +# Configuration parameters: SkipBlocks, EnforcedStyle, OnlyStaticConstants. +# SupportedStyles: described_class, explicit +RSpec/DescribedClass: + Exclude: + - 'spec/rbs_map/stdlib_map_spec.rb' + # This cop supports safe autocorrection (--autocorrect). RSpec/ExpectActual: Exclude: @@ -212,6 +268,10 @@ RSpec/LeakyConstantDeclaration: RSpec/MultipleExpectations: Max: 14 +# Configuration parameters: AllowSubject. +RSpec/MultipleMemoizedHelpers: + Max: 7 + # Configuration parameters: AllowedGroups. RSpec/NestedGroups: Max: 4 @@ -240,16 +300,6 @@ Style/ArgumentsForwarding: Exclude: - 'lib/solargraph/complex_type.rb' -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle, ProceduralMethods, FunctionalMethods, AllowedMethods, AllowedPatterns, AllowBracesOnProceduralOneLiners, BracesRequiredMethods. -# SupportedStyles: line_count_based, semantic, braces_for_chaining, always_braces -# ProceduralMethods: benchmark, bm, bmbm, create, each_with_object, measure, new, realtime, tap, with_object -# FunctionalMethods: let, let!, subject, watch -# AllowedMethods: lambda, proc, it -Style/BlockDelimiters: - Exclude: - - 'spec/source/chain_spec.rb' - # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle, SingleLineConditionsOnly, IncludeTernaryExpressions. # SupportedStyles: assign_to_condition, assign_inside_condition @@ -280,6 +330,7 @@ Style/FrozenStringLiteralComment: # Configuration parameters: MinBodyLength, AllowConsecutiveConditionals. Style/GuardClause: Exclude: + - 'lib/solargraph/rbs_map/conversions.rb' - 'lib/solargraph/source_map/clip.rb' # This cop supports safe autocorrection (--autocorrect). @@ -291,12 +342,22 @@ Style/IfUnlessModifier: # SupportedStyles: require_parentheses, require_no_parentheses, require_no_parentheses_except_multiline Style/MethodDefParentheses: Exclude: + - 'lib/solargraph/rbs_map/conversions.rb' + - 'lib/solargraph/rbs_translator.rb' - 'spec/fixtures/rdoc-lib/lib/example.rb' Style/MultilineBlockChain: Exclude: - 'lib/solargraph/pin/search.rb' +# This cop supports unsafe autocorrection (--autocorrect-all). +# Configuration parameters: EnforcedStyle. +# SupportedStyles: literals, strict +Style/MutableConstant: + Exclude: + - 'lib/solargraph/rbs_map/conversions.rb' + - 'lib/solargraph/rbs_translator.rb' + # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: EnforcedStyle, AllowedMethods, AllowedPatterns. # SupportedStyles: predicate, comparison @@ -315,10 +376,13 @@ Style/OpenStructUse: Style/OptionalBooleanParameter: Enabled: false -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: ConvertCodeThatCanStartToReturnNil, AllowedMethods, MaxChainLength. -# AllowedMethods: present?, blank?, presence, try, try! -Style/SafeNavigation: +# This cop supports safe autocorrection (--autocorrect). +Style/RedundantParentheses: + Exclude: + - 'lib/solargraph/pin/base.rb' + +# This cop supports safe autocorrection (--autocorrect). +Style/RedundantRegexpCharacterClass: Exclude: - 'lib/solargraph/pin/base.rb' @@ -338,7 +402,6 @@ Style/SlicingWithRange: Style/StringLiterals: Exclude: - 'spec/fixtures/rdoc-lib/rdoc-lib.gemspec' - - 'spec/source/chain_spec.rb' # This cop supports safe autocorrection (--autocorrect). Style/SuperArguments: @@ -352,15 +415,14 @@ Style/SuperArguments: YARD/MismatchName: Exclude: - 'lib/solargraph/pin/reference.rb' + - 'lib/solargraph/rbs_map/conversions.rb' YARD/TagTypeSyntax: - Enabled: false + Exclude: + - 'lib/solargraph/parser/comment_ripper.rb' # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: AllowHeredoc, AllowURI, AllowQualifiedName, URISchemes, IgnoreCopDirectives, AllowedPatterns, SplitStrings. # URISchemes: http, https Layout/LineLength: - Max: 224 - -Naming/BlockForwarding: - Enabled: false + Max: 223 diff --git a/Rakefile b/Rakefile index 398957b1a..7ae79019b 100755 --- a/Rakefile +++ b/Rakefile @@ -34,19 +34,21 @@ task :typecheck_alpha do end desc 'Run RSpec tests, starting with the ones that failed last time' -task spec: %i[spec_failed undercover_no_fail full_spec] do +task spec: %i[spec_failed full_spec] do undercover end desc 'Run all RSpec tests' task :full_spec do warn 'starting spec' - sh 'TEST_COVERAGE_COMMAND_NAME=full-new bundle exec rspec' # --profile' - warn 'ending spec' - # move coverage/full-new to coverage/full on success so that we - # always have the last successful run's 'coverage info + sh 'TEST_COVERAGE_COMMAND_NAME=full-new bundle exec parallel_rspec --runtime-log spec/parallel_runtime_rspec.log --verbose-command spec/' # --profile' + # clear now-outdated coverage FileUtils.rm_rf('coverage/full') - FileUtils.mv('coverage/full-new', 'coverage/full') + # move coverage/full-new to coverage/full on success so that we + # always have the last successful run's coverage info + unless ENV['SIMPLECOV_DISABLED'] + FileUtils.mv('coverage/full-new', 'coverage/full') + end end # @sg-ignore #undercover return type could not be inferred @@ -90,6 +92,8 @@ desc 'Re-run failed specs. Add --fail-fast in your .rspec-local file if desired task :spec_failed do # allow user to check out any persistent failures while looking for # more in the whole test suite + # + # Note: prspec doesn't support --only-failures, so we have to use rspec directly here. sh 'TEST_COVERAGE_COMMAND_NAME=next-failure bundle exec rspec --only-failures || true' end diff --git a/lib/solargraph/api_map.rb b/lib/solargraph/api_map.rb index 298a62390..abb3ce071 100755 --- a/lib/solargraph/api_map.rb +++ b/lib/solargraph/api_map.rb @@ -262,17 +262,19 @@ class << self # # @param directory [String] # @param out [IO, StringIO, nil] The output stream for messages + # @param rebuild [Boolean] whether to rebuild the pins even if they are cached # @param loose_unions [Boolean] See #initialize # # @return [ApiMap] - def self.load_with_cache directory, out = $stderr, loose_unions: true + # @api Used by solargraph-rails at least + def self.load_with_cache directory, out = $stderr, rebuild: false, loose_unions: true api_map = load(directory, loose_unions: loose_unions) - if api_map.uncached_gemspecs.empty? + if api_map.uncached_gemspecs.empty? && !rebuild logger.info { "All gems cached for #{directory}" } return api_map end - api_map.cache_all_for_doc_map!(out: out) + api_map.cache_all_for_doc_map!(out: out, rebuild: rebuild) load(directory, loose_unions: loose_unions) end @@ -800,6 +802,13 @@ def qualify_superclass fq_sub_tag store.qualify_superclass fq_sub_tag end + # @param require_path [String] + # + # @return [Array, nil] + def resolve_require require_path + Solargraph::Workspace::Gemspecs.new(workspace&.directory_or_nil).resolve_require(require_path) + end + private # A hash of source maps with filename keys. diff --git a/lib/solargraph/api_map/constants.rb b/lib/solargraph/api_map/constants.rb index 880adacb6..010e9b128 100644 --- a/lib/solargraph/api_map/constants.rb +++ b/lib/solargraph/api_map/constants.rb @@ -188,12 +188,12 @@ def collect_and_cache gates end end - # @return [Hash{Array(String, Array) => String, :in_process, nil}] + # @return [Hash{Array, String, Array, String => String, :in_process, nil}] def cached_resolve @cached_resolve ||= {} end - # @return [Hash{Array => Array}] + # @return [Hash{Array, String => Array}] def cached_collect @cached_collect ||= {} end diff --git a/lib/solargraph/api_map/store.rb b/lib/solargraph/api_map/store.rb index ad0f64f20..31835c993 100644 --- a/lib/solargraph/api_map/store.rb +++ b/lib/solargraph/api_map/store.rb @@ -317,9 +317,9 @@ def catalog pinsets, &block true end - # @return [Hash{::Array(String, String) => ::Array}] + # @return [Hash{::Array, String, String => ::Array}] def fqns_pins_map - # @param h [Hash{::Array(String, String) => ::Array}] + # @param h [Hash{::Array, String, String => ::Array}] # @param base [String] # @param name [String] @fqns_pins_map ||= Hash.new do |h, (base, name)| diff --git a/lib/solargraph/complex_type.rb b/lib/solargraph/complex_type.rb index 27d2ff08c..19605316c 100644 --- a/lib/solargraph/complex_type.rb +++ b/lib/solargraph/complex_type.rb @@ -33,6 +33,7 @@ def initialize types = [UniqueType::UNDEFINED] # @param gates [Array] # # @return [ComplexType] + # @param [Array] gates def qualify api_map, *gates red = reduce_object types = red.items.map do |t| @@ -439,6 +440,7 @@ class << self # Chain::Call needs to know the decl type (:arg, :optarg, # :kwarg, etc) of the arguments given, instead of just having # an array of Chains as the arguments. + # @param [Boolean] partial def parse *strings, partial: false # @type [Hash{Array => ComplexType, Array}] @cache ||= {} diff --git a/lib/solargraph/complex_type/unique_type.rb b/lib/solargraph/complex_type/unique_type.rb index 4bbdda5b2..dc8d40ab3 100644 --- a/lib/solargraph/complex_type/unique_type.rb +++ b/lib/solargraph/complex_type/unique_type.rb @@ -302,7 +302,6 @@ def desc rooted_tags end - # @sg-ignore Need better if/elseanalysis # @return [String] def to_rbs if duck_type? @@ -311,8 +310,9 @@ def to_rbs 'bool' elsif name.downcase == 'nil' 'nil' - elsif name == GENERIC_TAG_NAME - all_params.first&.name + elsif name == GENERIC_TAG_NAME && !all_params.empty? + # @sg-ignore flow sensitive typing should be able to handle !empty? narrowing first to non-nil + all_params.first.name elsif %w[Class Module].include?(name) rbs_name elsif %w[Tuple Array].include?(name) && fixed_parameters? @@ -555,6 +555,7 @@ def expand named_types # # @param api_map [ApiMap] The ApiMap that performs qualification # @param gates [Array] The namespaces from which to resolve names + # # @return [self, ComplexType, UniqueType] The generated ComplexType def qualify api_map, *gates transform do |t| diff --git a/lib/solargraph/diagnostics/base.rb b/lib/solargraph/diagnostics/base.rb index ff91a9062..cbc181e7c 100644 --- a/lib/solargraph/diagnostics/base.rb +++ b/lib/solargraph/diagnostics/base.rb @@ -20,8 +20,12 @@ def initialize *args # # @param source [Solargraph::Source] # @param api_map [Solargraph::ApiMap] + # @param workspace [Solargraph::Workspace, nil] + # Explicit workspace to use, instead of the current working + # directory's workspace. Useful in specs for isolation. + # # @return [Array] - def diagnose source, api_map + def diagnose source, api_map, workspace: nil [] end end diff --git a/lib/solargraph/diagnostics/type_check.rb b/lib/solargraph/diagnostics/type_check.rb index b1333f9d9..b2ff446c4 100644 --- a/lib/solargraph/diagnostics/type_check.rb +++ b/lib/solargraph/diagnostics/type_check.rb @@ -7,12 +7,13 @@ module Diagnostics # class TypeCheck < Base # @return [Array] - def diagnose source, api_map + def diagnose source, api_map, workspace: nil # return [] unless args.include?('always') || api_map.workspaced?(source.filename) severity = Diagnostics::Severities::ERROR level = args.reverse.find { |a| %w[normal typed strict strong].include?(a) } || :normal # @sg-ignore sensitive typing needs to handle || on nil types - checker = Solargraph::TypeChecker.new(source.filename, api_map: api_map, level: level.to_sym) + checker = Solargraph::TypeChecker.new(source.filename, api_map: api_map, level: level.to_sym, + workspace: workspace) checker.problems .sort { |a, b| a.location.range.start.line <=> b.location.range.start.line } .map do |problem| diff --git a/lib/solargraph/doc_map.rb b/lib/solargraph/doc_map.rb index 6ad366d2b..95f2dbdcc 100644 --- a/lib/solargraph/doc_map.rb +++ b/lib/solargraph/doc_map.rb @@ -3,6 +3,7 @@ require 'pathname' require 'benchmark' require 'open3' +require 'concurrent-ruby' module Solargraph # A collection of pins generated from required gems. @@ -60,26 +61,6 @@ def initialize requires, workspace, out: $stderr @out = out end - # @param out [IO, StringIO, nil] - # @return [void] - # @param [Boolean] rebuild - def cache_all! out, rebuild: false - # if we log at debug level: - if logger.info? - gem_desc = uncached_gemspecs.map { |gemspec| "#{gemspec.name}:#{gemspec.version}" }.join(', ') - logger.info "Caching pins for gems: #{gem_desc}" unless uncached_gemspecs.empty? - end - logger.debug { "Caching for YARD: #{uncached_yard_gemspecs.map(&:name)}" } - logger.debug { "Caching for RBS collection: #{uncached_rbs_collection_gemspecs.map(&:name)}" } - load_serialized_gem_pins - uncached_gemspecs.each do |gemspec| - cache(gemspec, rebuild: rebuild, out: out) - end - load_serialized_gem_pins - @uncached_rbs_collection_gemspecs = [] - @uncached_yard_gemspecs = [] - end - # @param gemspec [Gem::Specification] # @param out [IO, StringIO, nil] # @return [void] @@ -102,6 +83,46 @@ def cache_rbs_collection_pins gemspec, out logger.info { "Cached #{pins.length} RBS collection pins for gem #{gemspec.name} #{gemspec.version} with cache_key #{rbs_version_cache_key.inspect}" unless pins.empty? } end + # @param out [IO, StringIO, nil] output stream for logging + # @param rebuild [Boolean] whether to rebuild the pins even if they are cached + # @return [void] + def cache_doc_map_gems! out, rebuild: false + if logger.info? + gem_desc = uncached_gemspecs.map { |gemspec| "#{gemspec.name}:#{gemspec.version}" }.join(', ') + logger.info "Caching pins for gems: #{gem_desc}" unless uncached_gemspecs.empty? + end + logger.debug { "Caching for YARD: #{uncached_yard_gemspecs.map(&:name)}" } + logger.debug { "Caching for RBS collection: #{uncached_rbs_collection_gemspecs.map(&:name)}" } + load_serialized_gem_pins + + pool_size = Concurrent.processor_count # roughly your CPU count + pool = Concurrent::FixedThreadPool.new(pool_size) + time = Benchmark.measure do + # Using 'names' as queue, run! + futures = uncached_gemspecs.map do |spec| + Concurrent::Promises.future_on(pool, spec) do + cache(spec, rebuild: rebuild, out: out) + end + end + + Concurrent::Promises.zip(*futures).value! + pool.shutdown + pool.wait_for_termination + end + milliseconds = (time.real * 1000).round + if (milliseconds > 500) && uncached_gemspecs.any? && out + out.puts "Built #{uncached_gemspecs.length} gems in #{milliseconds} ms in #{pool_size} threads" + end + + load_serialized_gem_pins + @uncached_rbs_collection_gemspecs = [] + @uncached_yard_gemspecs = [] + end + # @param out [IO, StringIO, nil] output stream for logging + # @param rebuild [Boolean] whether to rebuild the pins even if they are cached + # @return [void] + alias cache_all! cache_doc_map_gems! + # @param gemspec [Gem::Specification] # @param rebuild [Boolean] whether to rebuild the pins even if they are cached # @param out [IO, StringIO, nil] output stream for logging @@ -129,34 +150,33 @@ def unresolved_requires @unresolved_requires ||= required_gems_map.select { |_, gemspecs| gemspecs.nil? }.keys end - # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version + # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version def self.all_yard_gems_in_memory @all_yard_gems_in_memory ||= {} end - # @return [Hash{String => Hash{Array(String, String) => Array}}] stored by RBS collection path + # @return [Hash{String => Hash{Array, String, String => Array}}] stored by RBS collection path def self.all_rbs_collection_gems_in_memory @all_rbs_collection_gems_in_memory ||= {} end - # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version + # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version def yard_pins_in_memory self.class.all_yard_gems_in_memory end - # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version + # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version def rbs_collection_pins_in_memory - # @sg-ignore rbs_collection_path is String | nil but used as hash key self.class.all_rbs_collection_gems_in_memory[rbs_collection_path] ||= {} end - # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version + # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version def self.all_combined_pins_in_memory @all_combined_pins_in_memory ||= {} end # @todo this should also include an index by the hash of the RBS collection - # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version + # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version def combined_pins_in_memory self.class.all_combined_pins_in_memory end diff --git a/lib/solargraph/language_server/host.rb b/lib/solargraph/language_server/host.rb index f503ea177..012304b29 100644 --- a/lib/solargraph/language_server/host.rb +++ b/lib/solargraph/language_server/host.rb @@ -468,10 +468,28 @@ def stop notify_observers end + # @return [void] + def fully_stop + stop + # try for two minutes, raise if not fully stopped by then + start_time = Time.now + until fully_stopped? + if Time.now - start_time > 240 + raise 'Host did not fully stop within 240 seconds.' + end + logger.info 'Waiting for host to fully stop...' + sleep 0.1 + end + end + def stopped? @stopped end + def fully_stopped? + @stopped && diagnoser.fully_stopped? + end + # Locate multiple pins that match a completion item. The first match is # based on the corresponding location in a library source if available. # Subsequent matches are based on path. diff --git a/lib/solargraph/language_server/host/diagnoser.rb b/lib/solargraph/language_server/host/diagnoser.rb index 8c259c131..3b099732a 100644 --- a/lib/solargraph/language_server/host/diagnoser.rb +++ b/lib/solargraph/language_server/host/diagnoser.rb @@ -12,6 +12,7 @@ def initialize host @mutex = Mutex.new @queue = [] @stopped = true + @fully_stopped = true end # Schedule a file to be diagnosed. @@ -36,17 +37,25 @@ def stopped? @stopped end + def fully_stopped? + @fully_stopped + end + # Start the diagnosis thread. # # @return [self, nil] def start return unless @stopped - @stopped = false + @fully_stopped = @stopped = false + old_thread_id = Thread.current.object_id Thread.new do until stopped? + $stderr.puts "Diagnoser: start tick in thread #{old_thread_id}, current thread #{Thread.current.object_id}" tick + $stderr.puts "Diagnoser: end tick in thread #{old_thread_id}, current thread #{Thread.current.object_id}" sleep 0.1 end + @fully_stopped = true end self end diff --git a/lib/solargraph/library.rb b/lib/solargraph/library.rb index c18090446..4f03fb862 100644 --- a/lib/solargraph/library.rb +++ b/lib/solargraph/library.rb @@ -605,8 +605,12 @@ def cache_next_gemspec queued_gemspec_cache.push(spec) return if pending - queued_gemspec_cache.length < 1 - catalog - sync_catalog + # Try the next cacheable gemspec. This method is always called + # from inside sync_catalog's mutex (either directly or via this + # same recursive call), so recursing through sync_catalog here + # would try to re-lock a mutex this thread already holds and + # deadlock (https://github.com/castwide/solargraph/issues/1111). + cache_next_gemspec else logger.info "Caching #{spec.name} #{spec.version}" Thread.new do diff --git a/lib/solargraph/pin/method.rb b/lib/solargraph/pin/method.rb index c371794e1..1209d9139 100644 --- a/lib/solargraph/pin/method.rb +++ b/lib/solargraph/pin/method.rb @@ -521,7 +521,9 @@ def combine_same_type_arity_signatures same_type_arity_signatures # @param new_signature [Pin::Signature] same_type_arity_signatures.reduce([]) do |old_signatures, new_signature| next old_signatures + [new_signature] if old_signatures.empty? - old_signatures.flat_map do |old_signature| + + merged = false + combined = old_signatures.map do |old_signature| potential_new_signature = old_signature.combine_with(new_signature) if potential_new_signature.type_arity == old_signature.type_arity @@ -534,11 +536,13 @@ def combine_same_type_arity_signatures same_type_arity_signatures # based on types, not just arity, allowing for type # information describing how methods behave based on # their input types) - old_signatures - [old_signature] + [potential_new_signature] + merged = true + potential_new_signature else - old_signatures + [new_signature] + old_signature end end + merged ? combined : old_signatures + [new_signature] end # rubocop:enable Lint/UnreachableCode end diff --git a/lib/solargraph/rbs_map/core_map.rb b/lib/solargraph/rbs_map/core_map.rb index a2eeed60f..9dba7e28d 100644 --- a/lib/solargraph/rbs_map/core_map.rb +++ b/lib/solargraph/rbs_map/core_map.rb @@ -15,7 +15,7 @@ def resolved? def initialize; end - # @param out [IO, nil] output stream for logging + # @param out [StringIO, IO, nil] output stream for logging # @return [Enumerable] def pins out: $stderr return @pins if @pins diff --git a/lib/solargraph/rbs_map/stdlib_map.rb b/lib/solargraph/rbs_map/stdlib_map.rb index e6ebcf90f..d80d3b2a9 100644 --- a/lib/solargraph/rbs_map/stdlib_map.rb +++ b/lib/solargraph/rbs_map/stdlib_map.rb @@ -32,6 +32,7 @@ def initialize library, rebuild: false, out: $stderr end generated_pins = pins logger.debug { "Found #{generated_pins.length} pins for stdlib library #{library}" } + out&.puts "Caching RBS gem standard library pins for #{library}" PinCache.serialize_stdlib_require library, generated_pins end end @@ -66,6 +67,11 @@ def resolve_dependencies? def self.load library @stdlib_maps_hash[library] ||= StdlibMap.new(library) end + + # @return [Array] + def self.possible_stdlibs + RBS::Repository.default.gems.keys + end end end end diff --git a/lib/solargraph/shell.rb b/lib/solargraph/shell.rb index 89859da21..76c152155 100755 --- a/lib/solargraph/shell.rb +++ b/lib/solargraph/shell.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'benchmark' +require 'concurrent-ruby' require 'thor' require 'yard' require 'yaml' @@ -179,46 +180,56 @@ def uncache *gems # @param names [Array] # @return [void] def gems *names - # print time with ms + api_map = Solargraph::ApiMap.new workspace = Solargraph::Workspace.new('.') if names.empty? - Gem::Specification.to_a.each { |spec| do_cache spec, rebuild: options[:rebuild] } - $stderr.puts "Documentation cached for all #{Gem::Specification.count} gems." + api_map.cache_all_for_doc_map!(out: $stdout, rebuild: options[:rebuild]) else - warn("Caching these gems: #{names}") - names.each do |name| - if name == 'core' - # @sg-ignore cache_core and core? are dynamically defined - PinCache.cache_core(out: $stdout) # if !PinCache.core? || options[:rebuild] - next - end - - gemspec = workspace.find_gem(*name.split('=')) - if gemspec.nil? - warn "Gem '#{name}' not found" - else - if options[:rebuild] || !PinCache.has_yard?(gemspec) - pins = GemPins.build_yard_pins(['yard-activesupport-concern'], gemspec) - PinCache.serialize_yard_gem(gemspec, pins) + # run in parallel with a thread pool + pool_size = Concurrent.processor_count # roughly your CPU count + pool = Concurrent::FixedThreadPool.new(pool_size) + warn("Caching these gems with #{pool_size} workers: #{names}") + + # Using 'names' as queue, run! + futures = names.map do |name| + Concurrent::Promises.future_on(pool, name) do |_x| + if name == 'core' + PinCache.uncache_core if options[:rebuild] + Solargraph::RbsMap::CoreMap.new.pins(out: $stdout) + next end - workspace = Solargraph::Workspace.new(Dir.pwd) - rbs_map = RbsMap.from_gemspec(gemspec, workspace.rbs_collection_path, workspace.rbs_collection_config_path) - if options[:rebuild] || !PinCache.has_rbs_collection?(gemspec, rbs_map.cache_key) - # cache pins even if result is zero, so we don't retry building pins - pins = rbs_map.pins || [] - PinCache.serialize_rbs_collection_gem(gemspec, rbs_map.cache_key, pins) + gemspec = workspace.find_gem(*name.split('=')) + if gemspec.nil? + warn "Gem '#{name}' not found" + else + if options[:rebuild] || !PinCache.has_yard?(gemspec) + pins = GemPins.build_yard_pins(['yard-activesupport-concern'], gemspec) + PinCache.serialize_yard_gem(gemspec, pins) + end + + rbs_map = RbsMap.from_gemspec(gemspec, workspace.rbs_collection_path, workspace.rbs_collection_config_path) + if options[:rebuild] || !PinCache.has_rbs_collection?(gemspec, rbs_map.cache_key) + # cache pins even if result is zero, so we don't retry building pins + pins = rbs_map.pins || [] + PinCache.serialize_rbs_collection_gem(gemspec, rbs_map.cache_key, pins) + end end + rescue Gem::MissingSpecError + warn "Gem '#{name}' not found" + rescue Gem::Requirement::BadRequirementError => e + warn "Gem '#{name}' failed while loading" + warn e.message + # @sg-ignore Need to add nil check here + warn e.backtrace.join("\n") end - rescue Gem::MissingSpecError - warn "Gem '#{name}' not found" - rescue Gem::Requirement::BadRequirementError => e - warn "Gem '#{name}' failed while loading" - warn e.message - # @sg-ignore Need to add nil check here - warn e.backtrace.join("\n") end + + Concurrent::Promises.zip(*futures).value! # raises if any failed + pool.shutdown + pool.wait_for_termination + warn "Documentation cached for #{names.count} gems." end end @@ -596,27 +607,5 @@ def print_pin pin puts pin.inspect end end - - # @param gemspec [Gem::Specification, nil] - # @param rebuild [Boolean] - # @return [void] - def do_cache gemspec, rebuild: false - if gemspec.nil? - warn "Gem '#{gemspec&.name}' not found" - else - if rebuild || !PinCache.has_yard?(gemspec) - pins = GemPins.build_yard_pins(['yard-activesupport-concern'], gemspec) - PinCache.serialize_yard_gem(gemspec, pins) - end - - workspace = Solargraph::Workspace.new(Dir.pwd) - rbs_map = RbsMap.from_gemspec(gemspec, workspace.rbs_collection_path, workspace.rbs_collection_config_path) - if rebuild || !PinCache.has_rbs_collection?(gemspec, rbs_map.cache_key) - # cache pins even if result is zero, so we don't retry building pins - pins = rbs_map.pins || [] - PinCache.serialize_rbs_collection_gem(gemspec, rbs_map.cache_key, pins) - end - end - end end end diff --git a/lib/solargraph/source/chain.rb b/lib/solargraph/source/chain.rb index ce58e7c94..0adc25019 100644 --- a/lib/solargraph/source/chain.rb +++ b/lib/solargraph/source/chain.rb @@ -148,6 +148,7 @@ def infer api_map, name_pin, locals @@inference_invalidation_key = api_map.hash @@inference_cache = {} end + # @todo Missed nil violation out = infer_uncached(api_map, name_pin, locals).downcast_to_literal_if_possible logger.debug do "Chain#infer() - caching result - cache_key_hash=#{cache_key.hash}, links.map(&:hash)=#{links.map(&:hash)}, links=#{links}, cache_key.map(&:hash) = #{cache_key.map(&:hash)}, cache_key=#{cache_key}" diff --git a/lib/solargraph/source_map.rb b/lib/solargraph/source_map.rb index 224223282..5a80fd0af 100644 --- a/lib/solargraph/source_map.rb +++ b/lib/solargraph/source_map.rb @@ -195,7 +195,7 @@ def map source # @return [Array] attr_writer :convention_pins - # @return [Hash{Class => Array}] + # @return [Hash{Class => Array}] # rubocop:disable YARD/CollectionStyle def pin_class_hash # @todo Need to support generic resolution in classify and transform_values @pin_class_hash ||= pins.to_set.classify(&:class).transform_values(&:to_a) diff --git a/lib/solargraph/workspace.rb b/lib/solargraph/workspace.rb index d3346c9b4..3e0118c85 100644 --- a/lib/solargraph/workspace.rb +++ b/lib/solargraph/workspace.rb @@ -20,7 +20,9 @@ class Workspace attr_reader :gemnames alias source_gems gemnames - # @param directory [String] TODO: Remove '' and '*' special cases + # @todo Remove '*' special case + # @param directory [String] If empty, no config will be loaded, + # and no RBS collection will be used. Useful for specs. # @param config [Config, nil] # @param server [Hash] def initialize directory = '', config = nil, server = {} @@ -128,6 +130,31 @@ def would_require? path false end + # True if the workspace has a root Gemfile. + # + # @todo Handle projects with custom Bundler/Gemfile setups (see DocMap#gemspecs_required_from_bundler) + # + def gemfile? + directory && File.file?(File.join(directory, 'Gemfile')) + end + + # True if the workspace contains at least one gemspec file. + # + # @return [Boolean] + def gemspec? + !gemspec_files.empty? + end + + # Get an array of all gemspec files in the workspace. + # + # @return [Array] + def gemspec_files + return [] if directory.empty? || directory == '*' + @gemspec_files ||= Dir[File.join(directory, '**/*.gemspec')].select do |gs| + config.allow? gs + end + end + # @return [String, nil] def rbs_collection_path @rbs_collection_path ||= read_rbs_collection_path @@ -135,10 +162,11 @@ def rbs_collection_path # @return [String, nil] def rbs_collection_config_path - @rbs_collection_config_path ||= unless directory.empty? || directory == '*' - yaml_file = File.join(directory, 'rbs_collection.yaml') - yaml_file if File.file?(yaml_file) - end + @rbs_collection_config_path ||= + unless directory.empty? || directory == '*' + yaml_file = File.join(directory, 'rbs_collection.yaml') + yaml_file if File.file?(yaml_file) + end end # @param name [String] @@ -170,31 +198,6 @@ def directory_or_nil directory end - # True if the workspace has a root Gemfile. - # - # @todo Handle projects with custom Bundler/Gemfile setups (see DocMap#gemspecs_required_from_bundler) - # - def gemfile? - directory && File.file?(File.join(directory, 'Gemfile')) - end - - # True if the workspace contains at least one gemspec file. - # - # @return [Boolean] - def gemspec? - !gemspec_files.empty? - end - - # Get an array of all gemspec files in the workspace. - # - # @return [Array] - def gemspec_files - return [] if directory.empty? || directory == '*' - @gemspec_files ||= Dir[File.join(directory, '**/*.gemspec')].select do |gs| - config.allow? gs - end - end - private # The language server configuration (or an empty hash if the workspace was diff --git a/lib/solargraph/workspace/config.rb b/lib/solargraph/workspace/config.rb index f8ba2d4b8..30fa3d746 100644 --- a/lib/solargraph/workspace/config.rb +++ b/lib/solargraph/workspace/config.rb @@ -20,7 +20,11 @@ class Config # @param directory [String] def initialize directory = '' - @directory = File.absolute_path(directory) + @directory = if directory.empty? + '' + else + File.absolute_path(directory) + end @raw_data = config_data included excluded diff --git a/lib/solargraph/workspace/gemspecs.rb b/lib/solargraph/workspace/gemspecs.rb index 849da9368..cb8b0a207 100644 --- a/lib/solargraph/workspace/gemspecs.rb +++ b/lib/solargraph/workspace/gemspecs.rb @@ -222,13 +222,34 @@ def in_this_bundle? Bundler.definition&.lockfile&.to_s&.start_with?(directory) end + # Bundler::LazySpecification#materialize_for_installation is an + # internal, undocumented Bundler API (no changelog entry, no + # deprecation path) whose arity has changed across Bundler versions + # without warning: some releases take no arguments, some require a + # locked_platforms argument, and some only expose the safer + # materialized_for_installation wrapper (which defaults that + # argument). Guard against all three shapes rather than assuming any + # one signature, so an unexpected/future Bundler version is skipped + # instead of raising. + # + # @param specish_objects [Array] + # @return [Array] + def materialize_specs_for_installation specish_objects + first = specish_objects.first + if first.respond_to?(:materialized_for_installation) + specish_objects.map(&:materialized_for_installation) + elsif first.respond_to?(:materialize_for_installation) && first.method(:materialize_for_installation).arity.zero? + specish_objects.map(&:materialize_for_installation) + else + specish_objects + end + end + # @return [Array] def all_gemspecs_from_this_bundle # Find only the gems bundler is now using specish_objects = Bundler.definition.locked_gems.specs - if specish_objects.first.respond_to?(:materialize_for_installation) - specish_objects = specish_objects.map(&:materialize_for_installation) - end + specish_objects = materialize_specs_for_installation(specish_objects) specish_objects.map do |specish| if specish.respond_to?(:name) && specish.respond_to?(:version) && specish.respond_to?(:gem_dir) # duck type is good enough for outside uses! @@ -320,8 +341,14 @@ def all_gemspecs_from_external_bundle begin logger.info 'Fetching gemspecs required from external bundle' + # See materialize_specs_for_installation for why this checks + # both the modern wrapper and the arity of the older method + # instead of assuming a signature for Bundler's internal API. command = 'specish_objects = Bundler.definition.locked_gems&.specs; ' \ - 'if specish_objects.first.respond_to?(:materialize_for_installation);' \ + 'first = specish_objects.first; ' \ + 'if first.respond_to?(:materialized_for_installation);' \ + 'specish_objects = specish_objects.map(&:materialized_for_installation);' \ + 'elsif first.respond_to?(:materialize_for_installation) && first.method(:materialize_for_installation).arity.zero?;' \ 'specish_objects = specish_objects.map(&:materialize_for_installation);' \ 'end;' \ 'specish_objects.map { |specish| [specish.name, specish.version] }' diff --git a/lib/solargraph/workspace/require_paths.rb b/lib/solargraph/workspace/require_paths.rb index d12364b07..23057a1af 100644 --- a/lib/solargraph/workspace/require_paths.rb +++ b/lib/solargraph/workspace/require_paths.rb @@ -13,7 +13,7 @@ class RequirePaths attr_reader :directory, :config # @param directory [String, nil] - # @param config [Config, nil] + # @param config [Config] def initialize directory, config @directory = directory @config = config diff --git a/solargraph.gemspec b/solargraph.gemspec index 67e9d1294..3d1dbbb8d 100755 --- a/solargraph.gemspec +++ b/solargraph.gemspec @@ -34,6 +34,7 @@ Gem::Specification.new do |s| s.add_dependency 'backport', '~> 1.2' s.add_dependency 'benchmark', '~> 0.4' s.add_dependency 'bundler', '>= 2.0' + s.add_dependency 'concurrent-ruby', '~> 1.3', '>= 1.3.5' s.add_dependency 'diff-lcs', '~> 1.4' s.add_dependency 'jaro_winkler', '~> 1.6', '>= 1.6.1' s.add_dependency 'kramdown', '~> 2.3' @@ -55,10 +56,14 @@ Gem::Specification.new do |s| s.add_dependency 'yard-activesupport-concern', '~> 0.0' s.add_dependency 'yard-solargraph', '~> 0.1' + # use latest available based on Ruby version support - might need to + # set an upper bound if/when parallel_tests breaks compatibility + s.add_development_dependency 'parallel_tests', '>= 4.10.1' s.add_development_dependency 'pry', '~> 0.15' s.add_development_dependency 'public_suffix', '~> 3.1' s.add_development_dependency 'rake', '~> 13.2' s.add_development_dependency 'rspec', '~> 3.5' + s.add_development_dependency 'rspec-time-guard', '~> 0.2.0' # # very specific development-time RuboCop version patterns for CI # stability - feel free to update in an isolated PR diff --git a/spec/api_map_method_spec.rb b/spec/api_map_method_spec.rb index 063b22f32..7d5bfcd4a 100644 --- a/spec/api_map_method_spec.rb +++ b/spec/api_map_method_spec.rb @@ -3,12 +3,14 @@ describe Solargraph::ApiMap do let(:api_map) { described_class.new } let(:bench) do - Solargraph::Bench.new(external_requires: external_requires, workspace: Solargraph::Workspace.new('.')) + Solargraph::Bench.new(external_requires: external_requires, + workspace: Solargraph::Workspace.new) end let(:external_requires) { [] } + let(:catalog) { false } before do - api_map.catalog bench + api_map.catalog bench if catalog end describe '#resolve_method_alias' do @@ -118,23 +120,34 @@ class B end describe '#get_method_stack' do - let(:out) { StringIO.new } - let(:api_map) { described_class.load_with_cache(Dir.pwd, out) } - context 'with stdlib that has vital dependencies' do let(:external_requires) { ['yaml'] } let(:method_stack) { api_map.get_method_stack('YAML', 'safe_load', scope: :class) } it 'handles the YAML gem aliased to Psych' do - expect(method_stack).not_to be_empty + if method_stack.nil? + specs = (api_map.resolve_require('yaml') || []) + (api_map.resolve_require('psych') || []) + expect(specs).not_to be_empty + specs.each { |spec| api_map.cache_gem(spec) } + api_map.catalog bench + end + + expect(method_stack).not_to be_nil end end context 'with thor' do let(:external_requires) { ['thor'] } + let(:method_stack) { api_map.get_method_stack('Thor', 'desc', scope: :class) } + let(:catalog) { true } it 'handles finding Thor.desc' do + specs = api_map.resolve_require('thor') + specs.each { |spec| api_map.cache_gem(spec) } + api_map.catalog bench + + # if this fails you may not have an rbs collection installed expect(method_stack).not_to be_empty end end @@ -193,10 +206,10 @@ class Includer class Example # @macro klassify def foo(klass) - end + end end )) - api_map = Solargraph::ApiMap.new.map(source) + api_map = described_class.new.map(source) pin = api_map.get_path_pins('Example#foo').first expect(pin.typify(api_map).to_s).to eq('Array') end @@ -210,7 +223,7 @@ module Extension # @!method $1 # @return [$2] def make_method(name, klass) - end + end end class Example @@ -219,7 +232,7 @@ class Example make_method :macro_method, String end )) - api_map = Solargraph::ApiMap.new.map(source) + api_map = described_class.new.map(source) pin = api_map.get_path_pins('Example#macro_method').first expect(pin.return_type.to_s).to eq('String') end diff --git a/spec/api_map_spec.rb b/spec/api_map_spec.rb index 6f367d229..8ceb897ee 100755 --- a/spec/api_map_spec.rb +++ b/spec/api_map_spec.rb @@ -770,10 +770,12 @@ def bar; end it 'resolves aliases for YARD methods' do dir = File.absolute_path(File.join('spec', 'fixtures', 'yard_map')) - yard_pins = Dir.chdir dir do - YARD::Registry.load([File.join(dir, 'attr.rb')], true) - mapper = Solargraph::YardMap::Mapper.new(YARD::Registry.all) - mapper.map + yard_pins = Solargraph::CHDIR_MUTEX.synchronize do + Dir.chdir dir do + YARD::Registry.load([File.join(dir, 'attr.rb')], true) + mapper = Solargraph::YardMap::Mapper.new(YARD::Registry.all) + mapper.map + end end source_pins = Solargraph::SourceMap.load_string(%( class Foo diff --git a/spec/diagnostics/rubocop_helpers_spec.rb b/spec/diagnostics/rubocop_helpers_spec.rb index 7bf374d67..98e31c233 100644 --- a/spec/diagnostics/rubocop_helpers_spec.rb +++ b/spec/diagnostics/rubocop_helpers_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -describe Solargraph::Diagnostics::RubocopHelpers do +describe Solargraph::Diagnostics::RubocopHelpers, order: :defined do context 'with custom version' do around do |example| old_gem_path = Gem.paths.path diff --git a/spec/doc_map_spec.rb b/spec/doc_map_spec.rb index 2dbe28fb7..d6c043e5c 100644 --- a/spec/doc_map_spec.rb +++ b/spec/doc_map_spec.rb @@ -9,7 +9,7 @@ end let(:out) { StringIO.new } - let(:pre_cache) { true } + let(:pre_cache) { false } let(:requires) { [] } let(:workspace) do @@ -19,10 +19,12 @@ let(:plain_doc_map) { described_class.new([], workspace, out: nil) } before do - doc_map.cache_all!(nil) if pre_cache + doc_map.cache_doc_map_gems!($stderr) if pre_cache end context 'with a require in solargraph test bundle' do + let(:pre_cache) { true } + let(:requires) do ['ast'] end @@ -34,6 +36,8 @@ end context 'when understanding rspec + rspec-mocks require pattern' do + let(:pre_cache) { true } + let(:requires) do ['rspec-mocks'] end @@ -67,18 +71,26 @@ end end - it 'does not warn for redundant requires' do - # Requiring 'set' is unnecessary because it's already included in core. It - # might make sense to log redundant requires, but a warning is overkill. - allow(Solargraph.logger).to receive(:warn).and_call_original - described_class.new(['set'], workspace) - expect(Solargraph.logger).not_to have_received(:warn).with(/path set/) + context 'with a redundant require' do + let(:pre_cache) { false } + + it 'does not warn' do + # Requiring 'set' is unnecessary because it's already included in core. It + # might make sense to log redundant requires, but a warning is overkill. + allow(Solargraph.logger).to receive(:warn).and_call_original + described_class.new(['set'], workspace) + expect(Solargraph.logger).not_to have_received(:warn).with(/path set/) + end end context 'with require as bundle/require' do + let(:pre_cache) { false } + it 'imports all gems when bundler/require used' do - doc_map_with_bundler_require = described_class.new(['bundler/require'], workspace, out: nil) - doc_map_with_bundler_require.cache_all!(nil) + doc_map_with_bundler_require = described_class.new(['bundler/require'], workspace, out: $stderr) + if doc_map_with_bundler_require.pins.length <= plain_doc_map.pins.length + doc_map_with_bundler_require.cache_doc_map_gems!(nil) + end expect(doc_map_with_bundler_require.pins.length - plain_doc_map.pins.length).to be_positive end end diff --git a/spec/fixtures/workspace-with-gemfile/Gemfile b/spec/fixtures/workspace-with-gemfile/Gemfile index 9b1ff9e31..4c5f03f4c 100644 --- a/spec/fixtures/workspace-with-gemfile/Gemfile +++ b/spec/fixtures/workspace-with-gemfile/Gemfile @@ -1,3 +1,3 @@ source 'https://rubygems.org' -gem 'backport' +gem 'pry' diff --git a/spec/gem_pins_spec.rb b/spec/gem_pins_spec.rb index 9d8101d17..e2cbcf736 100644 --- a/spec/gem_pins_spec.rb +++ b/spec/gem_pins_spec.rb @@ -10,19 +10,20 @@ end context 'with a combined method pin' do - let(:path) { 'RBS::EnvironmentLoader#core_root' } - let(:requires) { ['rbs'] } + let(:path) { 'Hashdiff.diff' } + let(:requires) { ['hashdiff'] } it 'can merge YARD and RBS' do - expect(pin.source).to eq(:combined) + expect(pin.source).to eq(:combined), "Expected to merge YARD and RBS for #{path} in #{workspace.directory}" end it 'finds types from RBS' do - expect(pin.return_type.to_s).to eq('Pathname, nil') + expect(pin.return_type.to_s).to eq('Array') end it 'finds locations from YARD' do - expect(pin.location.filename).to end_with('environment_loader.rb') + expect(pin).not_to be_nil, "Expected to find pin for #{path} in #{workspace.directory}" + expect(pin.location.filename).to end_with('diff.rb') end end diff --git a/spec/language_server/host_spec.rb b/spec/language_server/host_spec.rb index f0497b8f3..360a25110 100644 --- a/spec/language_server/host_spec.rb +++ b/spec/language_server/host_spec.rb @@ -67,8 +67,11 @@ File.write(file, "foo = 'foo'") host.start host.prepare dir - Solargraph::LanguageServer::UriHelpers.file_to_uri(file) + file_uri = Solargraph::LanguageServer::UriHelpers.file_to_uri(file) host.open(file, File.read(file), 1) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(file_uri) + allow(library).to receive(:cacheable_specs).and_return([]) buffer = host.flush times = 0 # @todo Weak timeout for waiting until the diagnostics thread @@ -79,7 +82,8 @@ buffer = host.flush end expect(buffer).to include('textDocument/publishDiagnostics') - host.stop + ensure + host.fully_stop end end @@ -110,7 +114,13 @@ file1_uri = Solargraph::LanguageServer::UriHelpers.file_to_uri("#{app1_folder}/app.rb") file2_uri = Solargraph::LanguageServer::UriHelpers.file_to_uri("#{app2_folder}/app.rb") host.open_from_disk file1_uri + library = host.library_for(file1_uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) host.open_from_disk file2_uri + library = host.library_for(file2_uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) app1_map = host.document_symbols(file1_uri).map(&:path) expect(app1_map).to include('Folder1App') expect(app1_map).not_to include('Folder2App') @@ -121,8 +131,8 @@ it 'stops' do host = described_class.new - host.stop - expect(host.stopped?).to be(true) + host.fully_stop + expect(host.fully_stopped?).to be(true) end it 'retains orphaned sources' do @@ -133,6 +143,9 @@ host.prepare(dir) host.open(file_uri, File.read(file), 1) host.remove(dir) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(file_uri) + allow(library).to receive(:cacheable_specs).and_return([]) expect do host.document_symbols(file_uri) end.not_to raise_error @@ -209,6 +222,9 @@ def initialize(foo); end host = described_class.new host.prepare '' host.open uri, code, 1 + library = host.library_for(uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) sleep 0.1 until host.libraries.all?(&:mapped?) result = host.locate_pins({ 'data' => { @@ -247,15 +263,23 @@ def initialize(foo); end it 'rescues InvalidOffset errors' do host = described_class.new - host.open('file:///file.rb', 'class Foo; end', 1) + uri = 'file:///file.rb' + host.open(uri, 'class Foo; end', 1) + library = host.library_for(uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) expect { host.references_from('file:///file.rb', 0, 100) }.not_to raise_error end it 'logs InvalidOffset errors' do allow(Solargraph.logger).to receive(:warn) host = described_class.new - host.open('file:///file.rb', 'class Foo; end', 1) - host.references_from('file:///file.rb', 0, 100) + uri = 'file:///file.rb' + host.open(uri, 'class Foo; end', 1) + library = host.library_for(uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) + host.references_from(uri, 0, 100) expect(Solargraph.logger).to have_received(:warn).with(/InvalidOffsetError/) end end @@ -266,19 +290,27 @@ def initialize(foo); end end after do - @host.stop + @host.fully_stop end it 'creates a library for a file without a workspace' do - @host.open('file:///file.rb', 'class Foo; end', 1) - symbols = @host.document_symbols('file:///file.rb') + uri = 'file:///file.rb' + @host.open(uri, 'class Foo; end', 1) + library = @host.library_for(uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) + symbols = @host.document_symbols(uri) expect(symbols).not_to be_empty end it 'opens a file outside of prepared libraries' do @host.prepare(File.absolute_path(File.join('spec', 'fixtures', 'workspace'))) - @host.open('file:///file.rb', 'class Foo; end', 1) - symbols = @host.document_symbols('file:///file.rb') + uri = 'file:///file.rb' + @host.open(uri, 'class Foo; end', 1) + library = @host.library_for(uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) + symbols = @host.document_symbols(uri) expect(symbols).not_to be_empty end end diff --git a/spec/language_server/message/extended/check_gem_version_spec.rb b/spec/language_server/message/extended/check_gem_version_spec.rb index 26023f505..02f0b1c4a 100644 --- a/spec/language_server/message/extended/check_gem_version_spec.rb +++ b/spec/language_server/message/extended/check_gem_version_spec.rb @@ -36,6 +36,10 @@ end it 'responds to update actions' do + status = instance_double(Process::Status) + allow(status).to receive(:==).with(0).and_return(true) + allow(Open3).to receive(:capture2).with('gem update solargraph').and_return(['', status]) + host = Solargraph::LanguageServer::Host.new message = described_class.new(host, {}, current: Gem::Version.new('0.0.1')) message.process @@ -52,6 +56,7 @@ } host.receive action end.not_to raise_error + expect(Open3).to have_received(:capture2).with('gem update solargraph') end it 'uses bundler' do diff --git a/spec/language_server/message/text_document/definition_spec.rb b/spec/language_server/message/text_document/definition_spec.rb index d84d23cbe..541a9e397 100644 --- a/spec/language_server/message/text_document/definition_spec.rb +++ b/spec/language_server/message/text_document/definition_spec.rb @@ -25,6 +25,9 @@ } } }) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(file_uri) + allow(library).to receive(:cacheable_specs).and_return([]) message.process expect(message.result.first[:uri]).to eq(other_uri) end @@ -48,6 +51,9 @@ } } }) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(file_uri) + allow(library).to receive(:cacheable_specs).and_return([]) message.process expect(message.result.first[:uri]).to eq(other_uri) end @@ -58,12 +64,11 @@ host.prepare(path) sleep 0.1 until host.libraries.all?(&:mapped?) host.catalog + file_uri = Solargraph::LanguageServer::UriHelpers.file_to_uri(File.join(path, 'lib', 'other.rb')) message = described_class.new(host, { 'params' => { 'textDocument' => { - 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(File.join( - path, 'lib', 'other.rb' - )) + 'uri' => file_uri }, 'position' => { 'line' => 0, @@ -71,6 +76,9 @@ } } }) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(file_uri) + allow(library).to receive(:cacheable_specs).and_return([]) message.process expect(message.result.first[:uri]).to eq(Solargraph::LanguageServer::UriHelpers.file_to_uri(File.join(path, 'lib', 'thing.rb'))) diff --git a/spec/language_server/message/text_document/hover_spec.rb b/spec/language_server/message/text_document/hover_spec.rb index 76b3c9082..6ef633db2 100644 --- a/spec/language_server/message/text_document/hover_spec.rb +++ b/spec/language_server/message/text_document/hover_spec.rb @@ -29,12 +29,13 @@ def foo x = foo.upcase ) host = Solargraph::LanguageServer::Host.new - host.open('file:///test.rb', code, 1) + file_uri = 'file:///test.rb' + host.open(file_uri, code, 1) host.catalog message = described_class.new(host, { 'params' => { 'textDocument' => { - 'uri' => 'file:///test.rb' + 'uri' => file_uri }, 'position' => { 'line' => 4, @@ -42,7 +43,10 @@ def foo } } }) + library = host.library_for(file_uri) + allow(library).to receive(:cacheable_specs).and_return([]) message.process + # keep this from syncing a bunch of bundle gems in background expect(message.result[:contents][:value]).to eq("x\n\n`=~ String`") end end diff --git a/spec/language_server/message/text_document/rename_spec.rb b/spec/language_server/message/text_document/rename_spec.rb index 19903eaf9..4443f5cb4 100644 --- a/spec/language_server/message/text_document/rename_spec.rb +++ b/spec/language_server/message/text_document/rename_spec.rb @@ -1,10 +1,15 @@ # frozen_string_literal: true describe Solargraph::LanguageServer::Message::TextDocument::Rename do + let(:temp_file_url) do + # "file://#{Dir.mktmpdir}/file.rb" + 'file:///file.rb' + end + it 'renames a symbol' do host = Solargraph::LanguageServer::Host.new host.start - host.open('file:///file.rb', %( + host.open(temp_file_url, %( class Foo end foo = Foo.new @@ -15,7 +20,7 @@ class Foo 'method' => 'textDocument/rename', 'params' => { 'textDocument' => { - 'uri' => 'file:///file.rb' + 'uri' => temp_file_url }, 'position' => { 'line' => 1, @@ -24,14 +29,19 @@ class Foo 'newName' => 'Bar' } }) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(temp_file_url) + allow(library).to receive(:cacheable_specs).and_return([]) rename.process - expect(rename.result[:changes]['file:///file.rb'].length).to eq(2) + expect(rename.result[:changes][temp_file_url].length).to eq(2) + ensure + host.fully_stop end it 'renames an argument symbol from method signature' do host = Solargraph::LanguageServer::Host.new host.start - host.open('file:///file.rb', %( + host.open(temp_file_url, %( class Example def foo(bar) bar += 1 @@ -40,41 +50,15 @@ def foo(bar) end ), 1) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(temp_file_url) + allow(library).to receive(:cacheable_specs).and_return([]) rename = described_class.new(host, { 'id' => 1, 'method' => 'textDocument/rename', 'params' => { 'textDocument' => { - 'uri' => 'file:///file.rb' - }, - 'position' => { - 'line' => 2, - 'character' => 14 - }, - 'newName' => 'baz' - } - }) - rename.process - expect(rename.result[:changes]['file:///file.rb'].length).to eq(3) - end - - it 'renames an argument symbol from method body' do - host = Solargraph::LanguageServer::Host.new - host.start - host.open('file:///file.rb', %( - class Example - def foo(bar) - bar += 1 - return bar - end - end - ), 1) - rename = described_class.new(host, { - 'id' => 1, - 'method' => 'textDocument/rename', - 'params' => { - 'textDocument' => { - 'uri' => 'file:///file.rb' + 'uri' => temp_file_url }, 'position' => { 'line' => 3, @@ -83,14 +67,27 @@ def foo(bar) 'newName' => 'baz' } }) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(temp_file_url) + allow(library).to receive(:cacheable_specs).and_return([]) rename.process - expect(rename.result[:changes]['file:///file.rb'].length).to eq(3) + # try for 20 seconds to get the result, since this can be slow on CI + timeout = Time.now + 20 + until rename.result[:changes] && rename.result[:changes][temp_file_url] && !rename.result[:changes][temp_file_url].empty? + sleep 0.1 + if Time.now > timeout + raise "Timed out waiting for rename result: #{rename.result.inspect}" + end + end + expect(rename.result[:changes][temp_file_url].length).to eq(3) + ensure + host.fully_stop end it 'renames namespace symbol with proper range' do host = Solargraph::LanguageServer::Host.new host.start - host.open('file:///file.rb', %( + host.open(temp_file_url, %( module Namespace; end class Namespace::ExampleClass end @@ -101,7 +98,7 @@ class Namespace::ExampleClass 'method' => 'textDocument/rename', 'params' => { 'textDocument' => { - 'uri' => 'file:///file.rb' + 'uri' => temp_file_url }, 'position' => { 'line' => 2, @@ -110,10 +107,24 @@ class Namespace::ExampleClass 'newName' => 'Nameplace' } }) + # keep this from syncing a bunch of bundle gems in background + library = host.library_for(temp_file_url) + allow(library).to receive(:cacheable_specs).and_return([]) rename.process - changes = rename.result[:changes]['file:///file.rb'] + # try for 20 seconds to get the result, since this can be slow on CI + timeout = Time.now + 20 + until rename.result[:changes] && rename.result[:changes][temp_file_url] && !rename.result[:changes][temp_file_url].empty? + sleep 0.1 + if Time.now > timeout + raise "Timed out waiting for rename result: #{rename.result.inspect}" + end + end + changes = rename.result[:changes][temp_file_url] + expect(changes).not_to be_nil, -> { "Expected to find changes for #{temp_file_url} in #{rename.result.inspect}" } expect(changes.length).to eq(3) expect(changes.first[:range][:start][:character]).to eq(13) expect(changes.first[:range][:end][:character]).to eq(22) + ensure + host.fully_stop end end diff --git a/spec/language_server/message/text_document/type_definition_spec.rb b/spec/language_server/message/text_document/type_definition_spec.rb index 16f7f3006..52daf5ecd 100644 --- a/spec/language_server/message/text_document/type_definition_spec.rb +++ b/spec/language_server/message/text_document/type_definition_spec.rb @@ -1,6 +1,13 @@ # frozen_string_literal: true describe Solargraph::LanguageServer::Message::TextDocument::TypeDefinition do + around do |testobj| + # we need a consistent directory + Solargraph::CHDIR_MUTEX.synchronize do + testobj.run + end + end + it 'finds definitions of methods' do host = Solargraph::LanguageServer::Host.new host.prepare('spec/fixtures/workspace') @@ -19,6 +26,9 @@ } } }) + library = host.library_for(file_uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) message.process expect(message.result.first[:uri]).to eq(something_uri) end diff --git a/spec/language_server/message/workspace/did_change_watched_files_spec.rb b/spec/language_server/message/workspace/did_change_watched_files_spec.rb index ebe76fc50..c5c75fd5f 100644 --- a/spec/language_server/message/workspace/did_change_watched_files_spec.rb +++ b/spec/language_server/message/workspace/did_change_watched_files_spec.rb @@ -73,9 +73,12 @@ ] } }) + library = host.library_for(uri) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) changed.process expect(host.synchronizing?).to be(false) - library = host.library_for(uri) + expect(library.path_pins('Foo')).to be_empty expect(library.path_pins('FooBar')).not_to be_empty expect(changed.error).to be_nil diff --git a/spec/language_server/protocol_spec.rb b/spec/language_server/protocol_spec.rb index 25764e6eb..ef21676d7 100644 --- a/spec/language_server/protocol_spec.rb +++ b/spec/language_server/protocol_spec.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true -class Protocol - attr_reader :response +require 'tmpdir' +require 'rubocop' - # @return [Solargraph::LanguageServer::Host] - attr_reader :host +class Protocol + attr_reader :response, :host + # @param host [Solargraph::LanguageServer::Host] def initialize host @host = host @host.start @@ -14,6 +15,7 @@ def initialize host @response = message end @message_id = 0 + $stderr.puts "Started in thread #{Thread.current.object_id}" end def request method, params @@ -30,16 +32,30 @@ def request method, params end def stop - @host.stop + @host.fully_stop end end -describe Protocol do +describe Protocol, order: :defined do before :all do @protocol = described_class.new(Solargraph::LanguageServer::Host.new) end - after :all do + # Ensure we don't start caching gems from current bundle in background + around do |testobj| + raise "Requests not finished #{testobj} - #{@protocol.host.pending_requests.inspect}" unless @protocol.host.pending_requests.empty? + temp_dir = Dir.mktmpdir + Dir.chdir temp_dir + Solargraph.with_clean_env do + testobj.run + end + raise "Requests not finished - #{@protocol.host.send(:requests).inspect}" unless @protocol.host.pending_requests.empty? + ensure + Dir.chdir PROJECT_DIRECTORY + FileUtils.remove_entry(temp_dir) + end + + after :context do @protocol.stop end @@ -84,6 +100,9 @@ def stop @protocol.request 'initialized', nil response = @protocol.response expect(response['error']).to be_nil + expect(@protocol.host.pending_requests.size).to eq(1) + pending_id = @protocol.host.pending_requests.first + @protocol.host.receive({ 'id' => pending_id }) end it 'configured default dynamic registration capabilities from initialized' do @@ -113,9 +132,10 @@ def bar baz end it 'handles textDocument/documentHighlight' do + file_uri = 'file:///file.rb' @protocol.request 'textDocument/documentHighlight', { 'textDocument' => { - 'uri' => 'file:///file.rb' + 'uri' => file_uri }, 'position' => { 'line' => 1, @@ -123,8 +143,9 @@ def bar baz } } response = @protocol.response + expect(response['result']).not_to be_nil, -> { "Expected result to be non-nil, got #{response.inspect}" } # Two references to Foo: the class definition and the Foo.new call - expect(response['result'].length).to eq(2) + expect(response['result'].length).to eq(2), -> { "Expected 2 highlights for Foo, got #{response['result'].length} in #{response.inspect}" } end it 'handles textDocument/didChange' do @@ -290,6 +311,7 @@ def bar baz } response = @protocol.response expect(response['error']).to be_nil + expect(response['result']).not_to be_nil, -> { "Expected result to be non-nil, got #{response.inspect}" } expect(response['result']['signatures']).not_to be_empty end @@ -414,6 +436,9 @@ def bar baz } expect(@protocol.host.options['autoformat']).to be(false) expect(@protocol.host.registered?('textDocument/completion')).to be(false) + expect(@protocol.host.pending_requests.size).to eq(1) + pending_id = @protocol.host.pending_requests.first + @protocol.host.receive({ 'id' => pending_id }) end it 'handles $/solargraph/checkGemVersion' do @@ -422,25 +447,35 @@ def bar baz expect(response['error']).to be_nil expect(response['result']['installed']).to be_a(String) expect(response['result']['available']).to be_a(String) + expect(@protocol.host.pending_requests.size).to eq(1) + pending_id = @protocol.host.pending_requests.first + @protocol.host.receive({ 'id' => pending_id }) end it 'handles $/solargraph/documentGems' do + status = instance_double(Process::Status) + allow(status).to receive(:==).with(0).and_return(true) + allow(Open3).to receive(:capture2).with('solargraph', 'gems').and_return(['', status]) + @protocol.request '$/solargraph/documentGems', {} response = @protocol.response + expect(response['error']).to be_nil + expect(Open3).to have_received(:capture2).with('solargraph', 'gems') end it 'handles textDocument/formatting' do + filename = File.realpath('spec/fixtures/formattable.rb', PROJECT_DIRECTORY) @protocol.request 'textDocument/didOpen', { 'textDocument' => { - 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(File.realpath('spec/fixtures/formattable.rb')), - 'text' => File.read('spec/fixtures/formattable.rb'), + 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(filename), + 'text' => File.read(filename), 'version' => 1 } } @protocol.request 'textDocument/formatting', { 'textDocument' => { - 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(File.realpath('spec/fixtures/formattable.rb')) + 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(filename) } } response = @protocol.response @@ -449,16 +484,17 @@ def bar baz end it 'can format file without file extension' do + filename = File.realpath('spec/fixtures/formattable', PROJECT_DIRECTORY) @protocol.request 'textDocument/didOpen', { 'textDocument' => { - 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(File.realpath('spec/fixtures/formattable')), - 'text' => File.read('spec/fixtures/formattable'), + 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(filename), + 'text' => File.read(filename), 'version' => 1 } } @protocol.request 'textDocument/formatting', { 'textDocument' => { - 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(File.realpath('spec/fixtures/formattable')) + 'uri' => Solargraph::LanguageServer::UriHelpers.file_to_uri(filename) } } response = @protocol.response diff --git a/spec/language_server/transport/adapter_spec.rb b/spec/language_server/transport/adapter_spec.rb index 23d6ac123..eb8ef30d5 100644 --- a/spec/language_server/transport/adapter_spec.rb +++ b/spec/language_server/transport/adapter_spec.rb @@ -21,7 +21,8 @@ def flush tester = AdapterTester.new tester.opening expect(tester.host).to be_a(Solargraph::LanguageServer::Host) - expect(tester.host).not_to be_stopped + expect(tester.host).not_to be_fully_stopped + tester.host.fully_stop end it 'stops a host on close' do @@ -29,6 +30,7 @@ def flush tester.opening tester.closing expect(tester.host).to be_stopped + tester.host.fully_stop end it 'stops Backport when the host stops' do @@ -40,6 +42,8 @@ def flush end end expect(tester.host).to be_stopped + tester.host.fully_stop + expect(tester.host).to be_fully_stopped end it 'processes sent data' do @@ -50,5 +54,6 @@ def flush tester.receiving "Content-Length: #{message.length}\r\n\r\n#{message}" end.not_to raise_error tester.closing + tester.host.fully_stop end end diff --git a/spec/library_spec.rb b/spec/library_spec.rb index 9f9ab87dc..a414a8682 100644 --- a/spec/library_spec.rb +++ b/spec/library_spec.rb @@ -3,12 +3,16 @@ require 'tmpdir' require 'yard' -describe Solargraph::Library do +# run these in order so we don't uncache backport right when we +# need it before +describe Solargraph::Library, order: :defined do + let(:filename) { "file#{rand(1000)}.rb" } + it 'does not open created files in the workspace' do Dir.mktmpdir do |temp_dir_path| # Ensure we resolve any symlinks to their real path workspace_path = File.realpath(temp_dir_path) - file_path = File.join(workspace_path, 'file.rb') + file_path = File.join(workspace_path, filename) File.write(file_path, 'a = b') library = described_class.load(workspace_path) result = library.create(file_path, File.read(file_path)) @@ -19,11 +23,13 @@ it 'returns a Completion' do library = described_class.new + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) library.attach Solargraph::Source.load_string(%( x = 1 x - ), 'file.rb', 0) - completion = library.completions_at('file.rb', 2, 7) + ), filename, 0) + completion = library.completions_at(filename, 2, 7) expect(completion).to be_a(Solargraph::SourceMap::Completion) expect(completion.pins.map(&:name)).to include('x') end @@ -34,7 +40,7 @@ end it 'returns a Completion', time_limit_seconds: 50 do - library = described_class.new(Solargraph::Workspace.new(Dir.pwd, + library = described_class.new(Solargraph::Workspace.new(PROJECT_DIRECTORY, Solargraph::Workspace::Config.new)) library.attach Solargraph::Source.load_string(%( require 'backport' @@ -43,9 +49,9 @@ def foo(adapter) adapter.remo end - ), 'file.rb', 0) + ), filename, 0) # give Solargraph time to cache the gem - while (completion = library.completions_at('file.rb', 5, 19)).pins.empty? + while (completion = library.completions_at(filename, 5, 19)).pins.empty? sleep 0.25 end expect(completion).to be_a(Solargraph::SourceMap::Completion) @@ -53,13 +59,9 @@ def foo(adapter) end end - context 'with a require from an already-cached external gem' do - before do - Solargraph::Shell.new.gems('backport') - end - + context 'with a require from an already-cached external gem', order: :defined do it 'returns a Completion' do - library = described_class.new(Solargraph::Workspace.new(Dir.pwd, + library = described_class.new(Solargraph::Workspace.new(PROJECT_DIRECTORY, Solargraph::Workspace::Config.new)) library.attach Solargraph::Source.load_string(%( require 'backport' @@ -68,8 +70,8 @@ def foo(adapter) def foo(adapter) adapter.remo end - ), 'file.rb', 0) - completion = library.completions_at('file.rb', 5, 19) + ), filename, 0) + completion = library.completions_at(filename, 5, 19) expect(completion).to be_a(Solargraph::SourceMap::Completion) expect(completion.pins.map(&:name)).to include('remote') end @@ -82,9 +84,11 @@ class Foo def bar end end - ), 'file.rb', 0 + ), filename, 0 + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) library.attach src - paths = library.definitions_at('file.rb', 2, 13).map(&:path) + paths = library.definitions_at(filename, 2, 13).map(&:path) expect(paths).to include('Foo#bar') end @@ -98,9 +102,11 @@ def self.bar end end Foo.bar - ), 'file.rb', 0 + ), filename, 0 library.attach src - paths = library.type_definitions_at('file.rb', 7, 13).map(&:path) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) + paths = library.type_definitions_at(filename, 7, 13).map(&:path) expect(paths).to include('Bar') end @@ -112,9 +118,11 @@ def bar baz, key: '' end end Foo.new.bar() - ), 'file.rb', 0 + ), filename, 0 + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) library.attach src - pins = library.signatures_at('file.rb', 5, 18) + pins = library.signatures_at(filename, 5, 18) expect(pins.length).to eq(1) expect(pins.first.path).to eq('Foo#bar') end @@ -152,9 +160,11 @@ def bar baz, key: '' library = described_class.new src = Solargraph::Source.load_string(%( puts 'hello' - ), 'file.rb', 0) + ), filename, 0) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) library.attach src - result = library.diagnose 'file.rb' + result = library.diagnose filename expect(result).to be_a(Array) # @todo More tests end @@ -165,11 +175,13 @@ def bar baz, key: '' allow(config).to receive_messages(plugins: [], required: [], reporters: ['all!']) workspace = Solargraph::Workspace.new directory, config library = described_class.new workspace + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) src = Solargraph::Source.load_string(%( puts 'hello' - ), 'file.rb', 0) + ), filename, 0) library.attach src - result = library.diagnose 'file.rb' + result = library.diagnose filename expect(result.to_s).to include('rubocop') end @@ -180,18 +192,26 @@ class Foo def bar end end - ), 'file.rb', 0) + ), filename, 0) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) library.attach src - pins = library.document_symbols 'file.rb' + pins = library.document_symbols filename expect(pins.length).to eq(2) expect(pins.map(&:path)).to include('Foo') expect(pins.map(&:path)).to include('Foo#bar') end describe '#references_from' do + before :context do + Solargraph::Shell.new.gems('backport') + end + it 'collects references to a new method on a constant from assignment of Class.new' do workspace = Solargraph::Workspace.new('*') library = described_class.new(workspace) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) src1 = Solargraph::Source.load_string(%( Foo.new ), 'file1.rb', 0) @@ -209,6 +229,8 @@ def bar it 'collects references to a new method to a constant from assignment' do workspace = Solargraph::Workspace.new('*') library = described_class.new(workspace) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) src1 = Solargraph::Source.load_string(%( Foo.new ), 'file1.rb', 0) @@ -228,6 +250,8 @@ class Foo it 'collects references to an instance method symbol' do workspace = Solargraph::Workspace.new('*') library = described_class.new(workspace) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) src1 = Solargraph::Source.load_string(%( class Foo def bar @@ -255,6 +279,8 @@ def bar; end it 'collects references to a class method symbol' do workspace = Solargraph::Workspace.new('*') library = described_class.new(workspace) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) src1 = Solargraph::Source.load_string(%( class Foo def self.bar @@ -290,6 +316,8 @@ def bar; end it 'collects stripped references to constant symbols' do workspace = Solargraph::Workspace.new('*') library = described_class.new(workspace) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) src1 = Solargraph::Source.load_string(%( class Foo def bar @@ -319,6 +347,8 @@ class Other it 'rejects new references from different classes' do workspace = Solargraph::Workspace.new('*') library = described_class.new(workspace) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) source = Solargraph::Source.load_string(%( class Foo def bar @@ -350,6 +380,8 @@ def bar it 'returns YARD documentation from sources' do library = described_class.new + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) src = Solargraph::Source.load_string(%( class Foo # My bar method @@ -385,6 +417,8 @@ def bar; end it 'finds unique references' do library = described_class.new(Solargraph::Workspace.new('*')) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) src1 = Solargraph::Source.load_string(%( class Foo end @@ -401,6 +435,8 @@ class Foo it 'includes method parameters in references' do library = described_class.new(Solargraph::Workspace.new('*')) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) source = Solargraph::Source.load_string(%( class Foo def bar(baz) @@ -417,6 +453,8 @@ def bar(baz) it "lies about names when client can't handle the truth" do library = described_class.new(Solargraph::Workspace.new('*')) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) source = Solargraph::Source.load_string(%( class Foo def 🤦🏻foo♀️; 123; end @@ -429,6 +467,8 @@ def 🤦🏻foo♀️; 123; end it 'tells the truth about names when client can handle the truth' do library = described_class.new(Solargraph::Workspace.new('*')) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) source = Solargraph::Source.load_string(%( class Foo def 🤦🏻foo♀️; 123; end @@ -441,6 +481,8 @@ def 🤦🏻foo♀️; 123; end it 'includes block parameters in references' do library = described_class.new(Solargraph::Workspace.new('*')) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) source = Solargraph::Source.load_string(%( 100.times do |foo| puts foo @@ -464,6 +506,8 @@ class CallerExample def foo; end end ), 'test.rb') + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) library.attach source # Start of tag pins = library.definitions_at('test.rb', 4, 19) @@ -488,6 +532,8 @@ def foo; end end ), 'test.rb') library.attach source + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) pins = library.definitions_at('test.rb', 5, 19) expect(pins.map(&:path)).to include('Tagged') pins = library.definitions_at('test.rb', 5, 26) @@ -506,6 +552,8 @@ def foo; end end ), 'test.rb') library.attach source + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) pins = library.definitions_at('test.rb', 3, 31) expect(pins.map(&:path)).to include('TaggedExample') end @@ -520,12 +568,16 @@ def foo; end end ), 'test.rb') library.attach source + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) pins = library.definitions_at('test.rb', 3, 31) expect(pins.map(&:path)).to include('TaggedExample') end it 'skips comment text outside of tags' do library = described_class.new + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) source = Solargraph::Source.load_string(%( # String def foo; end @@ -537,6 +589,8 @@ def foo; end it 'marks aliases as methods or attributes in completion items' do library = described_class.new + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) source = Solargraph::Source.load_string(%( class Example attr_reader :foo @@ -560,6 +614,8 @@ def baz it 'marks aliases as methods or attributes in definitions' do library = described_class.new + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) source = Solargraph::Source.load_string(%( class Example attr_reader :foo @@ -608,6 +664,8 @@ def bar; end it 'removes files from Library#source_map_hash' do workspace = File.absolute_path(File.join('spec', 'fixtures', 'workspace')) library = described_class.load(workspace) + # keep this from syncing a bunch of bundle gems in background + allow(library).to receive(:cacheable_specs).and_return([]) library.map! library.catalog other_file = File.absolute_path(File.join('spec', 'fixtures', 'workspace', 'lib', 'other.rb')) @@ -657,4 +715,25 @@ def bar; end end end end + + describe '#sync_catalog' do + # Regression test for https://github.com/castwide/solargraph/issues/1111 + # + # When the first cacheable gemspec is already being cached by another + # process, cache_next_gemspec enqueues it and, if other gemspecs are + # still pending, recurses to try the next one. That recursion must not + # go back through sync_catalog's own mutex, or it deadlocks with a + # ThreadError on the thread that is already inside the mutex. + it 'does not deadlock when the next cacheable gemspec is already being processed elsewhere' do + library = described_class.new + api_map = library.send(:api_map) + gemspecs = (1..3).map { |i| instance_double(Gem::Specification, name: "gem_#{i}", version: Gem::Version.new('1.0.0')) } + allow(api_map).to receive(:catalog) + allow(api_map).to receive_messages(uncached_yard_gemspecs: gemspecs, uncached_rbs_collection_gemspecs: [], uncached_gemspecs: gemspecs, source_maps: [], pins: []) + allow(Solargraph::Yardoc).to receive(:processing?).and_return(true) + + library.catalog + expect { library.send(:sync_catalog) }.not_to raise_error + end + end end diff --git a/spec/parallel_runtime_rspec.log b/spec/parallel_runtime_rspec.log new file mode 100644 index 000000000..9edbc950c --- /dev/null +++ b/spec/parallel_runtime_rspec.log @@ -0,0 +1,111 @@ +spec/parser/flow_sensitive_typing_spec.rb:2.833531000011135 +spec/pin/method_spec.rb:2.1155049999943003 +spec/source/chain_spec.rb:2.051872000010917 +spec/rbs_map_spec.rb:1.6378260000201408 +spec/source/source_chainer_spec.rb:0.782007000001613 +spec/diagnostics/type_check_spec.rb:0.5934520000009798 +spec/language_server/message/text_document/type_definition_spec.rb:0.48828400002093986 +spec/convention_spec.rb:0.24250100000062957 +spec/api_map/index_spec.rb:0.022086999990278855 +spec/source/chain/constant_spec.rb:0.019666000007418916 +spec/source/chain/literal_spec.rb:0.017955000017536804 +spec/pin/combine_with_spec.rb:0.005975999985821545 +spec/language_server/host/message_worker_spec.rb:0.005041999975219369 +spec/workspace/config_spec.rb:0.003792000003159046 +spec/api_map/cache_spec.rb:0.0030030000198166817 +spec/pin/search_spec.rb:0.000901999999769032 +spec/complex_type/unique_type_spec.rb:0.00044499998330138624 +spec/source/chain/call_spec.rb:8.694724999979371 +spec/rbs_map/conversions_spec.rb:2.364409000001615 +spec/convention/activesupport_concern_spec.rb:1.3549990000028629 +spec/language_server/message/text_document/definition_spec.rb:1.1764510000066366 +spec/language_server/protocol_spec.rb:0.9354319999984 +spec/complex_type_spec.rb:0.6957209999964107 +spec/source_map/mapper_spec.rb:0.6735090000147466 +spec/language_server/message/text_document/hover_spec.rb:0.1836759999860078 +spec/language_server/message/workspace/did_change_watched_files_spec.rb:0.05829699998139404 +spec/language_server/message/initialize_spec.rb:0.0123240000102669 +spec/parser/node_chainer_spec.rb:0.012198999989777803 +spec/parser/node_methods_spec.rb:0.005478000006405637 +spec/language_server/uri_helpers_spec.rb:0.00205199999618344 +spec/parser_spec.rb:0.0012719999940600246 +spec/pin/documenting_spec.rb:0.0008420000085607171 +spec/pin/constant_spec.rb:0.0005150000215508044 +spec/source/chain/link_spec.rb:0.00048000001697801054 +spec/source/chain/array_spec.rb:0.00034899997990578413 +spec/pin/keyword_spec.rb:9.200000204145908e-05 +spec/type_checker/levels/typed_spec.rb:17.06391200001235 +spec/pin/base_variable_spec.rb:0.49064100001123734 +spec/diagnostics/require_not_found_spec.rb:0.37786599999526516 +spec/api_map/store_spec.rb:0.24749399998108856 +spec/pin/base_spec.rb:0.22538600000552833 +spec/source/chain/global_variable_spec.rb:0.025301999994553626 +spec/language_server/host/diagnoser_spec.rb:0.007955000008223578 +spec/source/cursor_spec.rb:0.004138999996939674 +spec/language_server/message_spec.rb:0.0015610000118613243 +spec/pin/method_alias_spec.rb:0.00045399999362416565 +spec/position_spec.rb:0.00041700000292621553 +spec/source/chain/class_variable_spec.rb:0.0004140000091865659 +spec/library_spec.rb:15.087453000014648 +spec/pin/local_variable_spec.rb:1.257486999995308 +spec/diagnostics/rubocop_spec.rb:1.097308999975212 +spec/convention/struct_definition_spec.rb:0.8356930000009015 +spec/pin/delegated_method_spec.rb:0.37205700000049546 +spec/source/chain/q_call_spec.rb:0.22859600000083447 +spec/source/chain/z_super_spec.rb:0.06280600000172853 +spec/diagnostics/update_errors_spec.rb:0.026749999989988282 +spec/api_map/config_spec.rb:0.00786000001244247 +spec/rbs_map/stdlib_map_spec.rb:0.006338000006508082 +spec/parser/node_processor_spec.rb:0.005274999974062666 +spec/yard_map/mapper/to_method_spec.rb:0.0030719999922439456 +spec/source/change_spec.rb:0.001357999979518354 +spec/language_server/transport/data_reader_spec.rb:0.00020400001085363328 +spec/source/chain/head_spec.rb:0.00017899999511428177 +spec/pin/parameter_spec.rb:6.296593999984907 +spec/api_map_method_spec.rb:3.729546000016853 +spec/workspace/gemspecs_resolve_require_spec.rb:3.406676000013249 +spec/shell_spec.rb:2.0584040000103414 +spec/yard_map/mapper_spec.rb:1.5831159999943338 +spec/workspace/gemspecs_fetch_dependencies_spec.rb:1.2263030000030994 +spec/api_map/constants_spec.rb:0.6155290000024252 +spec/diagnostics/rubocop_helpers_spec.rb:0.5695389999891631 +spec/language_server/transport/adapter_spec.rb:0.20772500001476146 +spec/source/chain/instance_variable_spec.rb:0.0337850000069011 +spec/api_map/source_to_yard_spec.rb:0.02226700002211146 +spec/workspace/gemspecs_find_gem_spec.rb:0.01667399998405017 +spec/language_server/message/completion_item/resolve_spec.rb:0.00914300000295043 +spec/pin/namespace_spec.rb:0.0015030000067781657 +spec/type_checker/rules_spec.rb:0.0011689999955706298 +spec/diagnostics_spec.rb:0.0004610000178217888 +spec/diagnostics/base_spec.rb:0.0001449999981559813 +spec/doc_map_spec.rb:4.773218000016641 +spec/workspace/require_paths_spec.rb:3.413786999997683 +spec/convention/gemfile_spec.rb:2.8036969999957364 +spec/language_server/message/extended/check_gem_version_spec.rb:2.2211780000070576 +spec/yardoc_spec.rb:2.139534999994794 +spec/language_server/message/text_document/rename_spec.rb:1.623290999996243 +spec/complex_type/conforms_to_spec.rb:0.987976999982493 +spec/language_server/message/text_document/formatting_spec.rb:0.9810560000187252 +spec/type_checker_spec.rb:0.7090679999964777 +spec/source_spec.rb:0.09150699997553602 +spec/logging_spec.rb:0.0027310000150464475 +spec/source_map/node_processor_spec.rb:0.002203000010922551 +spec/language_server/message/workspace/did_change_configuration_spec.rb:0.0013220000255387276 +spec/pin/block_spec.rb:0.0011569999915082008 +spec/pin/symbol_spec.rb:0.000614999997196719 +spec/source/updater_spec.rb:0.0005689999961759895 +spec/pin/instance_variable_spec.rb:0.0004830000107176602 +spec/type_checker/levels/alpha_spec.rb:4.420573000010336 +spec/language_server/host_spec.rb:4.271099999983562 +spec/gem_pins_spec.rb:3.252444000012474 +spec/api_map_spec.rb:2.8428080000157934 +spec/pin_cache_spec.rb:2.1292580000008456 +spec/rbs_map/core_map_spec.rb:1.4518410000018775 +spec/workspace_spec.rb:1.1714219999848865 +spec/source/chain/or_spec.rb:0.4800159999867901 +spec/language_server/host/dispatch_spec.rb:0.35667099998681806 +spec/source_map_spec.rb:0.281221999990521 +spec/source_map/clip_spec.rb:22.63660699999309 +spec/type_checker/levels/strong_spec.rb:25.854432000021916 +spec/type_checker/levels/normal_spec.rb:31.006626999995206 +spec/type_checker/levels/strict_spec.rb:33.33830899998429 diff --git a/spec/parser/flow_sensitive_typing_spec.rb b/spec/parser/flow_sensitive_typing_spec.rb index 4c9034873..446369065 100644 --- a/spec/parser/flow_sensitive_typing_spec.rb +++ b/spec/parser/flow_sensitive_typing_spec.rb @@ -1,8 +1,13 @@ # frozen_string_literal: true +require 'tempfile' + # @todo These tests depend on `Clip`, but we're putting the tests here to # avoid overloading clip_spec.rb. describe Solargraph::Parser::FlowSensitiveTyping do + # random temporary filename ending in '.rb' using tmpfile + let(:filename) { Tempfile.new(['flow_sensitive_typing_spec', '.rb']).path } + it 'uses is_a? in a simple if() to refine types' do source = Solargraph::Source.load_string(%( class ReproBase; end @@ -15,12 +20,12 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.to_s).to eq('Repro') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.to_s).to eq('ReproBase') end @@ -37,12 +42,12 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 10]) + clip = api_map.clip_at(filename, [7, 10]) expect(clip.infer.to_s).to eq('Repro1') - clip = api_map.clip_at('test.rb', [9, 10]) + clip = api_map.clip_at(filename, [9, 10]) expect(clip.infer.to_s).to eq('Repro2') end @@ -60,12 +65,12 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.to_s).to eq('Foo::Repro') - clip = api_map.clip_at('test.rb', [10, 10]) + clip = api_map.clip_at(filename, [10, 10]) expect(clip.infer.to_s).to eq('ReproBase') end @@ -85,12 +90,12 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [10, 10]) + clip = api_map.clip_at(filename, [10, 10]) expect(clip.infer.to_s).to eq('Foo::Bar::Repro') - clip = api_map.clip_at('test.rb', [12, 10]) + clip = api_map.clip_at(filename, [12, 10]) expect(clip.infer.to_s).to eq('ReproBase') end @@ -106,12 +111,12 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.to_s).to eq('ReproBase') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.to_s).to eq('Repro') end @@ -127,12 +132,12 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.to_s).to eq('Repro1') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.to_s).to eq('ReproBase') end @@ -151,15 +156,15 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 10]) + clip = api_map.clip_at(filename, [7, 10]) expect(clip.infer.to_s).to eq('Repro1') - clip = api_map.clip_at('test.rb', [9, 10]) + clip = api_map.clip_at(filename, [9, 10]) expect(clip.infer.to_s).to eq('Repro2') - clip = api_map.clip_at('test.rb', [11, 10]) + clip = api_map.clip_at(filename, [11, 10]) expect(clip.infer.to_s).to eq('ReproBase') end @@ -173,9 +178,9 @@ class Repro < ReproBase; end break unless value.is_a? Repro value end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 8]) + clip = api_map.clip_at(filename, [7, 8]) expect(clip.infer.to_s).to eq('Repro') end @@ -189,9 +194,9 @@ class Repro < ReproBase; end break unless value.is_a? Repro value end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 8]) + clip = api_map.clip_at(filename, [7, 8]) expect(clip.infer.to_s).to eq('Repro') end @@ -205,9 +210,9 @@ class Repro < ReproBase; end break unless value.is_a? Repro value end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 8]) + clip = api_map.clip_at(filename, [7, 8]) expect(clip.infer.to_s).to eq('Repro') end @@ -222,16 +227,16 @@ class Repro < ReproBase; end value end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [3, 6]) + clip = api_map.clip_at(filename, [3, 6]) expect(clip.infer.to_s).to eq('Array') - clip = api_map.clip_at('test.rb', [5, 8]) + clip = api_map.clip_at(filename, [5, 8]) expect(clip.infer.to_s).to eq('Numeric') - clip = api_map.clip_at('test.rb', [7, 8]) + clip = api_map.clip_at(filename, [7, 8]) expect(clip.infer.to_s).to eq('Float') end @@ -247,16 +252,16 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Integer') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('nil') end @@ -270,10 +275,10 @@ class Repro < ReproBase; end break unless value value end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 8]) + clip = api_map.clip_at(filename, [7, 8]) expect(clip.infer.to_s).to eq('ReproBase') end @@ -287,10 +292,10 @@ class Repro < ReproBase; end break if value.nil? value end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 8]) + clip = api_map.clip_at(filename, [7, 8]) expect(clip.infer.to_s).to eq('ReproBase') end @@ -304,13 +309,13 @@ def baz; end bar bar = Foo.new bar - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [6, 6]) + clip = api_map.clip_at(filename, [6, 6]) expect(clip.infer.to_s).to eq('Foo') - clip = api_map.clip_at('test.rb', [8, 6]) + clip = api_map.clip_at(filename, [8, 6]) expect(clip.infer.to_s).to eq('Foo') end @@ -319,9 +324,9 @@ def baz; end if is_a? Object x end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [2, 6]) + clip = api_map.clip_at(filename, [2, 6]) expect { clip.infer.to_s }.not_to raise_error end @@ -331,9 +336,9 @@ def baz; end if r.is_a? x end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [3, 6]) + clip = api_map.clip_at(filename, [3, 6]) expect { clip.infer.to_s }.not_to raise_error end @@ -349,15 +354,15 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('nil') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('::Integer') end @@ -373,15 +378,15 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('nil') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') end @@ -397,15 +402,15 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('nil') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') end @@ -421,15 +426,15 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('::Integer') end @@ -445,15 +450,15 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('::Integer') end @@ -469,15 +474,15 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('nil') end @@ -493,15 +498,15 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('nil') end @@ -513,15 +518,15 @@ def verify_repro(repr) repr unless repr.nil? || repr.downcase repr end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 33]) + clip = api_map.clip_at(filename, [4, 33]) expect(clip.infer.rooted_tags).to eq('::String') - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::String') - clip = api_map.clip_at('test.rb', [5, 8]) + clip = api_map.clip_at(filename, [5, 8]) expect(clip.infer.rooted_tags).to eq('::String, nil') end @@ -537,15 +542,15 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Integer') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') end @@ -561,15 +566,15 @@ def verify_repro(repr, throw_the_dice) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Integer') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') end @@ -585,15 +590,15 @@ def verify_repro(repr) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [4, 8]) + clip = api_map.clip_at(filename, [4, 8]) expect(clip.infer.rooted_tags).to eq('::Integer, nil') - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Integer') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.rooted_tags).to eq('nil') end @@ -608,15 +613,15 @@ def verify_repro(repr = nil) repr end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [3, 8]) + clip = api_map.clip_at(filename, [3, 8]) expect(clip.infer.rooted_tags).to eq('nil, ::Integer') - clip = api_map.clip_at('test.rb', [5, 10]) + clip = api_map.clip_at(filename, [5, 10]) expect(clip.infer.rooted_tags).to eq('::Integer') - clip = api_map.clip_at('test.rb', [7, 10]) + clip = api_map.clip_at(filename, [7, 10]) # @todo `false` might be acceptable here expect(clip.infer.rooted_tags).to eq('nil, ::Boolean') end @@ -634,10 +639,10 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 12]) + clip = api_map.clip_at(filename, [7, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') end @@ -653,10 +658,10 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean') end @@ -675,16 +680,16 @@ def bar(arr, baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [9, 12]) + clip = api_map.clip_at(filename, [9, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [11, 10]) + clip = api_map.clip_at(filename, [11, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') end @@ -702,16 +707,16 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [5, 10]) + clip = api_map.clip_at(filename, [5, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [8, 12]) + clip = api_map.clip_at(filename, [8, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [10, 10]) + clip = api_map.clip_at(filename, [10, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') end @@ -728,13 +733,13 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 12]) + clip = api_map.clip_at(filename, [7, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [9, 10]) + clip = api_map.clip_at(filename, [9, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') end @@ -753,16 +758,16 @@ def bar(baz: nil, other: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [8, 12]) + clip = api_map.clip_at(filename, [8, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [11, 10]) + clip = api_map.clip_at(filename, [11, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') end @@ -779,13 +784,13 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [7, 12]) + clip = api_map.clip_at(filename, [7, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [9, 10]) + clip = api_map.clip_at(filename, [9, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') end @@ -805,16 +810,16 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [8, 12]) + clip = api_map.clip_at(filename, [8, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [10, 12]) + clip = api_map.clip_at(filename, [10, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [12, 10]) + clip = api_map.clip_at(filename, [12, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') end @@ -829,19 +834,19 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [5, 10]) + clip = api_map.clip_at(filename, [5, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [6, 44]) + clip = api_map.clip_at(filename, [6, 44]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [6, 51]) + clip = api_map.clip_at(filename, [6, 51]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [7, 10]) + clip = api_map.clip_at(filename, [7, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') end @@ -859,16 +864,16 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [5, 10]) + clip = api_map.clip_at(filename, [5, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [8, 12]) + clip = api_map.clip_at(filename, [8, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [10, 10]) + clip = api_map.clip_at(filename, [10, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean') end @@ -886,16 +891,16 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [5, 10]) + clip = api_map.clip_at(filename, [5, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [8, 12]) + clip = api_map.clip_at(filename, [8, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [10, 10]) + clip = api_map.clip_at(filename, [10, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean') end @@ -917,24 +922,24 @@ def bar(baz: nil) baz end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [5, 10]) + clip = api_map.clip_at(filename, [5, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [8, 12]) + clip = api_map.clip_at(filename, [8, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') - clip = api_map.clip_at('test.rb', [10, 12]) + clip = api_map.clip_at(filename, [10, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean') pending('better scoping of return if in begin/rescue/ensure') - clip = api_map.clip_at('test.rb', [12, 12]) + clip = api_map.clip_at(filename, [12, 12]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') - clip = api_map.clip_at('test.rb', [14, 10]) + clip = api_map.clip_at(filename, [14, 10]) expect(clip.infer.rooted_tags).to eq('::Boolean, nil') end @@ -950,17 +955,17 @@ def a b c end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [6, 10]) + clip = api_map.clip_at(filename, [6, 10]) expect(clip.infer.to_s).to eq('String, nil') - clip = api_map.clip_at('test.rb', [7, 17]) + clip = api_map.clip_at(filename, [7, 17]) expect(clip.infer.to_s).to eq('nil') - clip = api_map.clip_at('test.rb', [8, 10]) + clip = api_map.clip_at(filename, [8, 10]) expect(clip.infer.to_s).to eq('String') end @@ -974,9 +979,9 @@ def foo a 123 end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [5, 17]) + clip = api_map.clip_at(filename, [5, 17]) expect(clip.infer.to_s).to eq('Integer') end @@ -993,9 +998,9 @@ def foo? out end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [9, 10]) + clip = api_map.clip_at(filename, [9, 10]) expect(clip.infer.to_s).to eq('Boolean') end @@ -1017,12 +1022,12 @@ def check end end end - ), 'test.rb') + ), filename) api_map = Solargraph::ApiMap.new.map(source) - clip = api_map.clip_at('test.rb', [11, 12]) + clip = api_map.clip_at(filename, [11, 12]) expect(clip.infer.to_s).to eq('Repro') - clip = api_map.clip_at('test.rb', [13, 12]) + clip = api_map.clip_at(filename, [13, 12]) expect(clip.infer.to_s).to eq('ReproBase') end end diff --git a/spec/pin/base_spec.rb b/spec/pin/base_spec.rb index e11566d38..374e97eaa 100644 --- a/spec/pin/base_spec.rb +++ b/spec/pin/base_spec.rb @@ -52,8 +52,14 @@ end it 'deals well with known closure combination issue' do - Solargraph::Shell.new.uncache('yard') - api_map = Solargraph::ApiMap.load_with_cache('.', $stderr) + # if this fails you might not have an rbs collection installed + api_map = Solargraph::ApiMap.load '' + + spec = Gem::Specification.find_by_name('yard') + api_map.cache_gem(spec) + + bench = Solargraph::Bench.new(external_requires: ['yard']) + api_map.catalog bench pins = api_map.get_method_stack('YARD::Docstring', 'parser', scope: :class) expect(pins.length).to eq(1) parser_method_pin = pins.first @@ -76,15 +82,15 @@ describe '#macro_names' do it 'returns names' do pin = described_class.new(name: 'Example', comments: "@macro addcomment\n@macro returnself") - expect(pin.macro_names).to eq(['addcomment', 'returnself']) + expect(pin.macro_names).to eq(%w[addcomment returnself]) end end describe '#nearly?' do it 'avoids recursion when two pins have the same closure' do - pin1 = Solargraph::Pin::Base.new(name: 'foo') + pin1 = described_class.new(name: 'foo') pin1.closure = pin1 - pin2 = Solargraph::Pin::Base.new(name: 'foo', closure: pin1) + pin2 = described_class.new(name: 'foo', closure: pin1) expect { pin1.nearly?(pin2) }.not_to raise_error end end diff --git a/spec/pin/method_spec.rb b/spec/pin/method_spec.rb index 6c07ced6d..8ed4e0b65 100644 --- a/spec/pin/method_spec.rb +++ b/spec/pin/method_spec.rb @@ -118,6 +118,16 @@ def bazzle; end expect(pin.return_type).to be_undefined end + it 'combines many non-mergeable same-type-arity signatures without exponential blowup' do + pin = described_class.new(name: 'foo') + signatures = (1..8).map { |_i| instance_double(Solargraph::Pin::Signature, type_arity: ['same']) } + signatures.each do |sig| + allow(sig).to receive(:combine_with).and_return(instance_double(Solargraph::Pin::Signature, type_arity: ['different'])) + end + result = pin.send(:combine_same_type_arity_signatures, signatures) + expect(result.length).to eq(signatures.length) + end + it 'does not merge with changes in parameters' do # @todo Method pin parameters are pins now pin1 = described_class.new(name: 'bar', parameters: %w[one two]) @@ -513,14 +523,18 @@ class Foo expect(pin.return_type).to be_undefined end - it 'combines signatures by type' do - # Integer+ in RBS is a number of signatures that dispatch based - # on type. Let's make sure we combine those with anything else - # found (e.g., additions from the BigDecimal RBS collection) - # without collapsing signatures - api_map = Solargraph::ApiMap.load_with_cache(Dir.pwd, nil) - method = api_map.get_method_stack('Integer', '+', scope: :instance).first - expect(method.signatures.count).to be > 3 + context 'with loaded bigdecimal require' do + it 'combines signatures by type' do + # Integer+ in RBS is a number of signatures that dispatch based + # on type. Let's make sure we combine those with anything else + # found (e.g., additions from the BigDecimal RBS collection) + # without collapsing signatures + api_map = Solargraph::ApiMap.new + bench = Solargraph::Bench.new external_requires: ['bigdecimal'] + api_map.catalog(bench) + method = api_map.get_method_stack('Integer', '+', scope: :instance).first + expect(method.signatures.count).to be > 3 + end end it 'infers untagged types from instance variables' do diff --git a/spec/rbs_map/conversions_spec.rb b/spec/rbs_map/conversions_spec.rb index 50f4b0b1a..e0f9cd7af 100644 --- a/spec/rbs_map/conversions_spec.rb +++ b/spec/rbs_map/conversions_spec.rb @@ -95,72 +95,80 @@ def bar: () -> untyped end end - context 'with standard loads for solargraph project' do - before :all do # rubocop:disable RSpec/BeforeAfterAll - @api_map = Solargraph::ApiMap.load_with_cache('.') - end - - let(:api_map) { @api_map } + context 'with superclass pin for Parser::AST::Node' do + let(:api_map) { Solargraph::ApiMap.new } - context 'with superclass pin for Parser::AST::Node' do - let(:superclass_pin) do - api_map.pins.find do |pin| - pin.is_a?(Solargraph::Pin::Reference::Superclass) && pin.context.namespace == 'Parser::AST::Node' - end + let(:superclass_pin) do + api_map.pins.find do |pin| + pin.is_a?(Solargraph::Pin::Reference::Superclass) && pin.context.namespace == 'Parser::AST::Node' end + end - it 'generates a rooted pin' do - # rooted! - expect(superclass_pin&.name).to eq('::AST::Node') - end + before do + gems = %w[parser ast open3] + bench = Solargraph::Bench.new(workspace: api_map.workspace, external_requires: gems) + api_map.catalog(bench) + api_map.cache_all_for_doc_map! + api_map.catalog(bench) end - # https://github.com/castwide/solargraph/issues/1042 - context 'with Hash superclass with untyped value and alias' do - let(:rbs) do - <<~RBS - class Sub < Hash[Symbol, untyped] - alias meth_alias [] - end - RBS - end + it 'generates a rooted pin' do + # rooted! + expect(superclass_pin&.name).to eq('::AST::Node'), lambda { + "superclass pin: #{superclass_pin.inspect}#{`bundle exec solargraph pin --references Parser::AST::Node`}\n#{`find ~/.cache/solargraph -type f | xargs ls -l`}" + } + end + end - let(:sup_method_stack) { api_map.get_method_stack('Hash{Symbol => undefined}', '[]', scope: :instance) } + # https://github.com/castwide/solargraph/issues/1042 + context 'with Hash superclass with untyped value and alias' do + let(:api_map) { Solargraph::ApiMap.new } - let(:sub_alias_stack) { api_map.get_method_stack('Sub', 'meth_alias', scope: :instance) } + let(:rbs) do + <<~RBS + class Sub < Hash[Symbol, untyped] + alias meth_alias [] + end + RBS + end - it 'does not crash looking at superclass method' do - expect { sup_method_stack }.not_to raise_error - end + let(:sup_method_stack) { api_map.get_method_stack('Hash{Symbol => undefined}', '[]', scope: :instance) } - it 'does not crash looking at alias' do - expect { sub_alias_stack }.not_to raise_error - end + let(:sub_alias_stack) { api_map.get_method_stack('Sub', 'meth_alias', scope: :instance) } - it 'finds superclass method pin return type' do - expect(sup_method_stack.map(&:return_type).map(&:rooted_tags).uniq).to eq(['undefined']) - end + it 'does not crash looking at superclass method' do + expect { sup_method_stack }.not_to raise_error + end - it 'finds superclass method pin parameter type' do - # RBS core's Hash#[] started taking its key as the _Key duck-type - # interface instead of the generic K as of RBS 4.1.0, so instantiating - # Hash{Symbol => untyped} no longer substitutes the param type on - # newer RBS - see ruby/rbs core/hash.rbs. - expected = if Gem::Version.new(RBS::VERSION) >= Gem::Version.new('4.1.0') - ['::Hash::_Key'] - else - ['Symbol'] - end - expect(sup_method_stack.flat_map(&:signatures).flat_map(&:parameters).map(&:return_type).map(&:rooted_tags) - .uniq).to eq(expected) - end + it 'does not crash looking at alias' do + expect { sub_alias_stack }.not_to raise_error + end + + it 'finds superclass method pin return type' do + expect(sup_method_stack.map(&:return_type).map(&:rooted_tags).uniq).to eq(['undefined']) + end + + it 'finds superclass method pin parameter type' do + # RBS core's Hash#[] started taking its key as the _Key duck-type + # interface instead of the generic K as of RBS 4.1.0, so instantiating + # Hash{Symbol => untyped} no longer substitutes the param type on + # newer RBS - see ruby/rbs core/hash.rbs. + expected = if Gem::Version.new(RBS::VERSION) >= Gem::Version.new('4.1.0') + ['::Hash::_Key'] + else + ['Symbol'] + end + expect(sup_method_stack.flat_map(&:signatures).flat_map(&:parameters).map(&:return_type).map(&:rooted_tags) + .uniq).to eq(expected) end end if Gem::Version.new(RBS::VERSION) >= Gem::Version.new('3.9.1') context 'with method pin for Open3.capture2e' do it 'accepts chdir kwarg' do - api_map = Solargraph::ApiMap.load_with_cache('.', $stdout) + api_map = Solargraph::ApiMap.new + bench = Solargraph::Bench.new(external_requires: ['open3']) + api_map.catalog(bench) method_pin = api_map.pins.find do |pin| pin.is_a?(Solargraph::Pin::Method) && pin.path == 'Open3.capture2e' diff --git a/spec/rbs_map_spec.rb b/spec/rbs_map_spec.rb index 09e7a1a80..91af85362 100644 --- a/spec/rbs_map_spec.rb +++ b/spec/rbs_map_spec.rb @@ -9,7 +9,7 @@ end it 'fails if it does not find data from gemspec' do - spec = Gem::Specification.find_by_name('backport') + spec = Gem::Specification.find_by_name('rspec-time-guard') rbs_map = described_class.from_gemspec(spec, nil, nil) expect(rbs_map).not_to be_resolved end diff --git a/spec/shell_spec.rb b/spec/shell_spec.rb index 3d8a254bf..ec4e12a42 100644 --- a/spec/shell_spec.rb +++ b/spec/shell_spec.rb @@ -6,58 +6,45 @@ describe Solargraph::Shell do let(:shell) { described_class.new } - let(:temp_dir) { Dir.mktmpdir } - - before do - File.open(File.join(temp_dir, 'Gemfile'), 'w') do |file| - file.puts "source 'https://rubygems.org'" - file.puts "gem 'solargraph', path: '#{File.expand_path('..', __dir__)}'" - end - output, status = Open3.capture2e('bundle install', chdir: temp_dir) - raise "Failure installing bundle: #{output}" unless status.success? - end - - # @type cmd [Array] - # @return [String] - def bundle_exec(*cmd) - # run the command in the temporary directory with bundle exec - output, status = Open3.capture2e("bundle exec #{cmd.join(' ')}", chdir: temp_dir) - expect(status.success?).to be(true), "Command failed: #{output}" - output - end - - after do - # remove the temporary directory after the tests - FileUtils.rm_rf(temp_dir) - end - describe '--version' do - let(:output) { bundle_exec('solargraph', '--version') } - - it 'returns output' do - expect(output).not_to be_empty - end - it 'returns a version when run' do + output = capture_stdout do + shell.version + end + expect(output).to eq("#{Solargraph::VERSION}\n") end end describe 'uncache' do it 'uncaches without erroring out' do - output = capture_stdout do - shell.uncache('backport') + allow(Solargraph::PinCache).to receive(:uncache) + + capture_stdout do + shell.uncache('public_suffix') end - expect(output).to include('Clearing pin cache in') + expect(Solargraph::PinCache).to have_received(:uncache).twice end it 'uncaches stdlib without erroring out' do - expect { shell.uncache('stdlib') }.not_to raise_error + allow(Solargraph::PinCache).to receive(:uncache) + + capture_stdout do + shell.uncache('stdlib') + end + + expect(Solargraph::PinCache).to have_received(:uncache) end it 'uncaches core without erroring out' do - expect { shell.uncache('core') }.not_to raise_error + allow(Solargraph::PinCache).to receive(:uncache) + + capture_stdout do + shell.uncache('core') + end + + expect(Solargraph::PinCache).to have_received(:uncache) end end @@ -129,6 +116,44 @@ def bundle_exec(*cmd) expect(output).to include("Gem 'solargraph123' not found") end end + + context 'with mocked Workspace' do + let(:workspace) { instance_double(Solargraph::Workspace) } + let(:api_map) { instance_double(Solargraph::ApiMap) } + let(:gemspec) { instance_double(Gem::Specification, name: 'abcd343kfk', version: '1.0.0') } + + before do + allow(Solargraph::ApiMap).to receive(:new).and_return(api_map) + allow(api_map).to receive(:workspace).and_return(workspace) + allow(Solargraph::Workspace).to receive(:new).and_return(workspace) + end + + it 'caches all without erroring out' do + allow(api_map).to receive(:cache_all_for_doc_map!) + + _output = capture_both { shell.gems } + + expect(api_map).to have_received(:cache_all_for_doc_map!) + end + + it 'caches single gem without erroring out' do + allow(workspace).to receive(:find_gem).with('98765').and_return(gemspec) + allow(workspace).to receive_messages(rbs_collection_path: nil, rbs_collection_config_path: nil) + allow(Solargraph::GemPins).to receive(:build_yard_pins).and_return([]) + rbs_map = instance_double(Solargraph::RbsMap, pins: [], cache_key: 'key') + allow(Solargraph::RbsMap).to receive(:from_gemspec).and_return(rbs_map) + allow(Solargraph::PinCache).to receive_messages(has_yard?: false, serialize_yard_gem: nil, + has_rbs_collection?: false, serialize_rbs_collection_gem: nil) + + capture_both do + shell.options = { rebuild: false } + shell.gems('98765') + end + + expect(Solargraph::PinCache).to have_received(:serialize_yard_gem).with(gemspec, []) + expect(Solargraph::PinCache).to have_received(:serialize_rbs_collection_gem).with(gemspec, 'key', []) + end + end end describe 'cache' do @@ -146,17 +171,6 @@ def bundle_exec(*cmd) end end - # @type cmd [Array] - # @return [String] - def bundle_exec(*cmd) - # run the command in the temporary directory with bundle exec - Bundler.with_unbundled_env do - output, status = Open3.capture2e("bundle exec #{cmd.join(' ')}") - expect(status.success?).to be(true), "Command failed: #{output}" - output - end - end - describe 'pin on a class' do let(:api_map) { instance_double(Solargraph::ApiMap) } let(:string_pin) { instance_double(Solargraph::Pin::Namespace, name: 'String') } @@ -310,56 +324,56 @@ def bundle_exec(*cmd) end end - describe 'rbs' do - let(:api_map) { instance_double(Solargraph::ApiMap) } - - before do - allow(shell).to receive(:`) - allow(Solargraph::ApiMap).to receive(:load).and_return(api_map) - allow(api_map).to receive(:source_maps).and_return(source_maps) - end + describe 'rbs' do + let(:api_map) { instance_double(Solargraph::ApiMap) } - context 'without inference' do - let(:source_maps) { [] } + before do + allow(shell).to receive(:`) + allow(Solargraph::ApiMap).to receive(:load).and_return(api_map) + allow(api_map).to receive(:source_maps).and_return(source_maps) + end - it 'invokes sord' do - capture_both do - shell.options = { filename: 'foo.rbs' } - shell.rbs + context 'without inference' do + let(:source_maps) { [] } + + it 'invokes sord' do + capture_both do + shell.options = { filename: 'foo.rbs' } + shell.rbs + end + expect(shell) + .to have_received(:`) + .with("sord #{Dir.pwd}/sig/foo.rbs --rbs --no-regenerate") end - expect(shell) - .to have_received(:`) - .with("sord #{Dir.pwd}/sig/foo.rbs --rbs --no-regenerate") end - end - context 'with inference' do - let(:source_maps) { [source_map] } - let(:source_map) { instance_double(Solargraph::SourceMap) } - let(:pin) do - instance_double(Solargraph::Pin::Method, - namespace: 'My::Namespace', path: 'My::Namespace#foo', - visibility: :public, - parameters: [], - scope: :instance, - location: nil, - name: 'foo', - class: Solargraph::Pin::Method, - return_type: Solargraph::ComplexType::UNDEFINED) - end + context 'with inference' do + let(:source_maps) { [source_map] } + let(:source_map) { instance_double(Solargraph::SourceMap) } + let(:pin) do + instance_double(Solargraph::Pin::Method, + namespace: 'My::Namespace', path: 'My::Namespace#foo', + visibility: :public, + parameters: [], + scope: :instance, + location: nil, + name: 'foo', + class: Solargraph::Pin::Method, + return_type: Solargraph::ComplexType::UNDEFINED) + end - it 'infers unknown types on pins' do - allow(source_map).to receive(:pins).and_return([pin]) - allow(pin).to receive_messages(typify: Solargraph::ComplexType.parse('String'), - docstring: YARD::Docstring.new(''), macros: []) - allow(pin).to receive(:code_object).and_return(nil) - capture_both do - shell.options = { filename: 'foo.rbs', inference: true } - shell.rbs + it 'infers unknown types on pins' do + allow(source_map).to receive(:pins).and_return([pin]) + allow(pin).to receive_messages(typify: Solargraph::ComplexType.parse('String'), + docstring: YARD::Docstring.new(''), macros: []) + allow(pin).to receive(:code_object).and_return(nil) + capture_both do + shell.options = { filename: 'foo.rbs', inference: true } + shell.rbs + end + expect(pin).to have_received(:typify) end - expect(pin).to have_received(:typify) end end end - end end diff --git a/spec/source/chain_spec.rb b/spec/source/chain_spec.rb index a6b29686e..3cef41cf0 100644 --- a/spec/source/chain_spec.rb +++ b/spec/source/chain_spec.rb @@ -1,25 +1,25 @@ describe Solargraph::Source::Chain do - it "gets empty definitions for undefined links" do + it 'gets empty definitions for undefined links' do chain = described_class.new([Solargraph::Source::Chain::Link.new]) expect(chain.define(nil, nil, [])).to be_empty end - it "infers undefined types for undefined links" do + it 'infers undefined types for undefined links' do chain = described_class.new([Solargraph::Source::Chain::Link.new]) expect(chain.infer(nil, nil, [])).to be_undefined end - it "calls itself undefined if any of its links are undefined" do + it 'calls itself undefined if any of its links are undefined' do chain = described_class.new([Solargraph::Source::Chain::Link.new]) expect(chain).to be_undefined end - it "returns undefined bases for single links" do + it 'returns undefined bases for single links' do chain = described_class.new([Solargraph::Source::Chain::Link.new]) expect(chain.base).to be_undefined end - it "defines constants from core classes" do + it 'defines constants from core classes' do api_map = Solargraph::ApiMap.new chain = described_class.new([Solargraph::Source::Chain::Constant.new('String')]) pins = chain.define(api_map, Solargraph::Pin::ROOT_PIN, []) @@ -27,7 +27,7 @@ expect(pins.first.path).to eq('String') end - it "infers types from core classes" do + it 'infers types from core classes' do api_map = Solargraph::ApiMap.new chain = described_class.new([Solargraph::Source::Chain::Constant.new('String')]) type = chain.infer(api_map, Solargraph::Pin::ROOT_PIN, []) @@ -35,25 +35,26 @@ expect(type.scope).to eq(:class) end - it "infers types from core methods" do + it 'infers types from core methods' do api_map = Solargraph::ApiMap.new - chain = described_class.new([Solargraph::Source::Chain::Constant.new('String'), Solargraph::Source::Chain::Call.new('new', nil)]) + chain = described_class.new([Solargraph::Source::Chain::Constant.new('String'), + Solargraph::Source::Chain::Call.new('new', nil)]) type = chain.infer(api_map, Solargraph::Pin::ROOT_PIN, []) expect(type.namespace).to eq('String') expect(type.scope).to eq(:instance) end - it "recognizes literals" do + it 'recognizes literals' do chain = described_class.new([Solargraph::Source::Chain::Literal.new('String', nil)]) expect(chain.literal?).to be(true) end - it "recognizes constants" do + it 'recognizes constants' do chain = described_class.new([Solargraph::Source::Chain::Constant.new('String')]) expect(chain.constant?).to be(true) end - it "recognizes unfinished constants" do + it 'recognizes unfinished constants' do chain = described_class.new([Solargraph::Source::Chain::Constant.new('String'), Solargraph::Source::Chain::Constant.new('')]) expect(chain.constant?).to be(true) expect(chain.base.constant?).to be(true) @@ -61,7 +62,7 @@ expect(chain.base.undefined?).to be(false) end - it "infers types from new subclass calls without a subclass initialize method" do + it 'infers types from new subclass calls without a subclass initialize method' do code = %( class Sup def initialize; end @@ -80,7 +81,7 @@ def meth; end expect(type.name).to eq('Sub') end - it "follows constant chains" do + it 'follows constant chains' do source = Solargraph::Source.load_string(%( module Mixin; end module Container @@ -95,7 +96,7 @@ class Foo; end expect(pins).to be_empty end - it "rebases inner constants chains" do + it 'rebases inner constants chains' do source = Solargraph::Source.load_string(%( class Foo class Bar; end @@ -105,11 +106,12 @@ class Bar; end api_map = Solargraph::ApiMap.new api_map.map source chain = Solargraph::Source::SourceChainer.chain(source, Solargraph::Position.new(3, 16)) - pins = chain.define(api_map, Solargraph::Pin::ProxyType.new(closure: Solargraph::Pin::Namespace.new(name: 'Foo'), return_type: Solargraph::ComplexType.parse('Class')), []) + pins = chain.define(api_map, + Solargraph::Pin::ProxyType.new(closure: Solargraph::Pin::Namespace.new(name: 'Foo'), return_type: Solargraph::ComplexType.parse('Class')), []) expect(pins.first.path).to eq('Foo::Bar') end - it "resolves relative constant paths" do + it 'resolves relative constant paths' do source = Solargraph::Source.load_string(%( class Foo class Bar @@ -123,11 +125,12 @@ module Other api_map = Solargraph::ApiMap.new api_map.map source chain = Solargraph::Source::SourceChainer.chain(source, Solargraph::Position.new(6, 16)) - pins = chain.define(api_map, Solargraph::Pin::ProxyType.anonymous(Solargraph::ComplexType.parse('Class')), []) + pins = chain.define(api_map, + Solargraph::Pin::ProxyType.anonymous(Solargraph::ComplexType.parse('Class')), []) expect(pins.first.path).to eq('Foo::Bar::Baz') end - it "avoids recursive variable assignments" do + it 'avoids recursive variable assignments' do source = Solargraph::Source.load_string(%( @foo = @bar @bar = @foo.quz @@ -135,12 +138,12 @@ module Other api_map = Solargraph::ApiMap.new api_map.map source chain = Solargraph::Source::SourceChainer.chain(source, Solargraph::Position.new(2, 18)) - expect { + expect do chain.define(api_map, Solargraph::Pin::ROOT_PIN, []) - }.not_to raise_error + end.not_to raise_error end - it "pulls types from multiple lines of code" do + it 'pulls types from multiple lines of code' do source = Solargraph::Source.load_string(%( 123 'abc' @@ -152,7 +155,7 @@ module Other expect(type.simple_tags).to eq('String') end - it "uses last line of a begin expression as return type" do + it 'uses last line of a begin expression as return type' do source = Solargraph::Source.load_string(%( begin 123 @@ -166,7 +169,7 @@ module Other expect(type.simple_tags).to eq('String') end - it "matches constants on complete symbols" do + it 'matches constants on complete symbols' do source = Solargraph::Source.load_string(%( class Correct; end class NotCorrect; end diff --git a/spec/source_map/clip_spec.rb b/spec/source_map/clip_spec.rb index b30002967..324cf4576 100644 --- a/spec/source_map/clip_spec.rb +++ b/spec/source_map/clip_spec.rb @@ -2441,7 +2441,7 @@ def bar; end ), 'test.rb') api_map = Solargraph::ApiMap.new.map(source) clip = api_map.clip_at('test.rb', [7, 6]) - expect(clip.infer.to_s).to eq('Symbol, Integer') + expect(clip.infer.to_s).to eq('123, :foo') end it 'replaces type with alternate reassignments' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 65d3bb7d4..2ed0f6674 100755 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true +require 'parallel_tests' require 'bundler/setup' require 'webmock/rspec' +require 'rspec_time_guard' WebMock.disable_net_connect!(allow_localhost: true) unless ENV['SIMPLECOV_DISABLED'] # set up lcov reporting for undercover @@ -23,18 +25,44 @@ enable_coverage(:branch) if ENV['SOLARGRAPH_BRANCH_COVERAGE'] end end +PROJECT_DIRECTORY = File.expand_path('..', __dir__) + +module ::RuboCop + class Runner + def run + raise 'this should always be mocked' + end + end +end + RSpec.configure do |c| # Allow use of --only-failures with rspec, handy for local development c.example_status_persistence_file_path = 'rspec-examples.txt' + c.before(:suite) do + files = c.files_to_run + banner = "PID (#{Process.pid}) #{files.count} files to run:" + puts [banner, *files].join("\n\t") + end +end +RspecTimeGuard.setup +RspecTimeGuard.configure do |config| + config.global_time_limit_seconds = 300 + config.continue_on_timeout = false end require 'solargraph' -# execute any logging blocks to make sure they don't blow up -Solargraph::Logging.logger.sev_threshold = Logger::DEBUG -# ...but still suppress logger output in specs (if possible) -if Solargraph::Logging.logger.respond_to?(:reopen) && !ENV.key?('SOLARGRAPH_LOG') + +# Suppress logger output in specs (if possible) +def set_logging + # execute any logging blocks to make sure they don't blow up + Solargraph::Logging.logger.sev_threshold = Logger::DEBUG + # ...but still suppress logger output in specs (if possible) + return unless Solargraph::Logging.logger.respond_to?(:reopen) && !ENV.key?('SOLARGRAPH_LOG') Solargraph::Logging.logger.reopen(File::NULL) + warn 'Logging set to null' end +set_logging + # @param name [String] # @param value [String] def with_env_var name, value diff --git a/spec/type_checker/levels/strict_spec.rb b/spec/type_checker/levels/strict_spec.rb index 9f5367138..aa7edc4b6 100644 --- a/spec/type_checker/levels/strict_spec.rb +++ b/spec/type_checker/levels/strict_spec.rb @@ -105,8 +105,13 @@ def bar(a); end require 'kramdown-parser-gfm' Kramdown::Parser::GFM.undefined_call ), 'test.rb') - api_map = Solargraph::ApiMap.load '.' - api_map.catalog Solargraph::Bench.new(source_maps: [source_map], external_requires: ['kramdown-parser-gfm']) + + api_map = Solargraph::ApiMap.new + specs = api_map.resolve_require('kramdown-parser-gfm') + specs.each { |spec| api_map.cache_gem(spec) } + bench = Solargraph::Bench.new(source_maps: [source_map], external_requires: ['kramdown-parser-gfm']) + api_map.catalog bench + checker = described_class.new('test.rb', api_map: api_map, level: :strict) expect(checker.problems).to be_empty end @@ -826,8 +831,6 @@ def meth(param1) end it 'uses nil? to refine type' do - pending 'nil? support in flow sensitive typing' - checker = type_checker(%( # @sg-ignore # @type [String, nil] @@ -838,7 +841,7 @@ def meth(param1) foo.downcase end )) - expect(checker.problems.map(&:message)).to eq(['Unresolved call to upcase']) + expect(checker.problems.map(&:message)).to eq(['Unresolved call to upcase on nil']) end it 'refines types on is_a? and && to downcast and avoid false positives' do diff --git a/spec/workspace/gemspecs_fetch_dependencies_spec.rb b/spec/workspace/gemspecs_fetch_dependencies_spec.rb index 56504e7dd..90fa55828 100644 --- a/spec/workspace/gemspecs_fetch_dependencies_spec.rb +++ b/spec/workspace/gemspecs_fetch_dependencies_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'benchmark' require 'fileutils' require 'tmpdir' require 'rubygems/commands/install_command' @@ -17,7 +18,7 @@ end it 'finds a known dependency' do - expect(deps.map(&:name)).to include('backport') + expect(deps.map(&:name)).to include('ostruct') end end @@ -27,7 +28,7 @@ end it 'finds a known dependency' do - expect(deps.map(&:name)).to include('backport') + expect(deps.map(&:name)).to include('rbs') end end @@ -64,7 +65,7 @@ # run bundle install output, status = Solargraph.with_clean_env do - Open3.capture2e('bundle install --verbose', chdir: dir_path) + Open3.capture2e('bundle install --verbose --local || bundle install --verbose', chdir: dir_path) end raise "Failure installing bundle: #{output}" unless status.success? @@ -75,20 +76,20 @@ end context 'with gem that exists in our bundle' do - let(:gem_name) { 'undercover' } + let(:gem_name) { 'simplecov' } it 'finds dependencies' do - expect(deps.map(&:name)).to include('ast') + expect(deps.map(&:name)).to include('simplecov-html') end end context 'with gem does not exist in our bundle' do - let(:gem_name) { 'activerecord' } + let(:gem_name) { 'functional-ruby' } it 'gives a useful message' do dep_names = nil output = capture_both { dep_names = deps.map(&:name) } - expect(output).to include('Please install the gem activerecord') + expect(output).to include("Please install the gem #{gem_name}") end end end diff --git a/spec/workspace/gemspecs_resolve_require_spec.rb b/spec/workspace/gemspecs_resolve_require_spec.rb index 8deba9ff8..fcc4cdc3f 100644 --- a/spec/workspace/gemspecs_resolve_require_spec.rb +++ b/spec/workspace/gemspecs_resolve_require_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'benchmark' require 'fileutils' require 'tmpdir' require 'rubygems/commands/install_command' @@ -9,38 +10,6 @@ let(:gemspecs) { described_class.new(dir_path) } - def find_or_install gem_name, version - Gem::Specification.find_by_name(gem_name, version) - rescue Gem::LoadError - install_gem(gem_name, version) - end - - def add_bundle - # write out Gemfile - File.write(File.join(dir_path, 'Gemfile'), <<~GEMFILE) - source 'https://rubygems.org' - gem 'backport' - GEMFILE - # run bundle install - output, status = Solargraph.with_clean_env do - Open3.capture2e('bundle install --verbose', chdir: dir_path) - end - raise "Failure installing bundle: #{output}" unless status.success? - # ensure Gemfile.lock exists - return if File.exist?(File.join(dir_path, 'Gemfile.lock')) - raise "Gemfile.lock not found after bundle install in #{dir_path}" - end - - def install_gem gem_name, version - Bundler.with_unbundled_env do - cmd = Gem::Commands::InstallCommand.new - cmd.handle_options [gem_name, '-v', version] - cmd.execute - rescue Gem::SystemExitException => e - raise unless e.exit_code == 0 - end - end - context 'with local bundle' do let(:dir_path) { File.realpath(Dir.pwd) } @@ -91,6 +60,7 @@ def configure_bundler_spec stub_value allow(bundler_stub_spec).to receive(:respond_to?).with(:name).and_return(true) allow(bundler_stub_spec).to receive(:respond_to?).with(:version).and_return(true) allow(bundler_stub_spec).to receive(:respond_to?).with(:gem_dir).and_return(false) + allow(bundler_stub_spec).to receive(:respond_to?).with(:materialized_for_installation).and_return(false) allow(bundler_stub_spec).to receive(:respond_to?).with(:materialize_for_installation).and_return(false) allow(bundler_stub_spec).to receive(:respond_to?).with(:stub).and_return(false) allow(bundler_stub_spec).to receive_messages(name: 'solargraph', stub: stub_value) @@ -141,6 +111,52 @@ def configure_bundler_spec stub_value end end + describe '#materialize_specs_for_installation' do + subject(:materialized) { gemspecs.send(:materialize_specs_for_installation, [specish]) } + + context 'when the object has the modern materialized_for_installation wrapper' do + let(:specish) { double(materialized_for_installation: :wrapped) } # rubocop:disable RSpec/VerifiedDoubles + + it 'calls the wrapper' do + expect(materialized).to eq([:wrapped]) + end + end + + context 'when materialize_for_installation takes no arguments (older Bundler)' do + # a real zero-arg method, not an RSpec double stub, since stubbed + # methods always report variadic arity regardless of signature + let(:specish) do + obj = Object.new + def obj.materialize_for_installation + :materialized + end + obj + end + + it 'calls it' do + expect(materialized).to eq([:materialized]) + end + end + + context 'when materialize_for_installation requires an argument and no wrapper exists' do + # simulates a Bundler internal API change (e.g. rubygems/rubygems + # commit "Pass locked platforms to materialization instead of + # mutating candidates") where the arity no longer matches what a + # bare &:materialize_for_installation call provides + let(:specish) do + obj = Object.new + def obj.materialize_for_installation _locked_platforms + raise 'should not be called: incompatible arity' + end + obj + end + + it 'skips materialization instead of raising' do + expect(materialized).to eq([specish]) + end + end + end + context 'with a less usual require mapping' do let(:require) { 'diff/lcs' } @@ -185,6 +201,24 @@ def configure_bundler_spec stub_value context 'with external bundle' do let(:dir_path) { File.realpath(Dir.mktmpdir).to_s } + def add_bundle + # write out Gemfile + File.write(File.join(dir_path, 'Gemfile'), <<~GEMFILE) + source 'https://rubygems.org' + gem 'public_suffix' + GEMFILE + + # run bundle install + output, status = Solargraph.with_clean_env do + Open3.capture2e('bundle install --verbose --local || bundle install --verbose', chdir: dir_path) + end + raise "Failure installing bundle: #{output}" unless status.success? + + # ensure Gemfile.lock exists + return if File.exist?(File.join(dir_path, 'Gemfile.lock')) + raise "Gemfile.lock not found after bundle install in #{dir_path}" + end + context 'with no actual bundle' do let(:require) { 'bundler/require' } @@ -194,7 +228,9 @@ def configure_bundler_spec stub_value end context 'with Gemfile and Bundler.require' do - before { add_bundle } + before do + add_bundle + end let(:require) { 'bundler/require' } @@ -203,12 +239,14 @@ def configure_bundler_spec stub_value end it 'returns gems' do - expect(specs.map(&:name)).to include('backport') + expect(specs.map(&:name)).to include('public_suffix') end end context 'with Gemfile and deep require into a possibly-core gem' do - before { add_bundle } + before do + add_bundle + end let(:require) { 'bundler/gem_tasks' } @@ -218,7 +256,9 @@ def configure_bundler_spec stub_value end context 'with Gemfile and deep require into a gem' do - before { add_bundle } + before do + add_bundle + end let(:require) { 'rspec/mocks' } @@ -228,7 +268,9 @@ def configure_bundler_spec stub_value end context 'with Gemfile but an unknown gem' do - before { add_bundle } + before do + add_bundle + end let(:require) { 'unknown_gemlaksdflkdf' } @@ -240,16 +282,32 @@ def configure_bundler_spec stub_value context 'with a Gemfile and a gem preference' do # find_or_install helper doesn't seem to work on older versions if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.1.0') + def find_or_install gem_name, version + Gem::Specification.find_by_name(gem_name, version) + rescue Gem::LoadError + install_gem(gem_name, version) + end + + def install_gem gem_name, version + Bundler.with_unbundled_env do + cmd = Gem::Commands::InstallCommand.new + cmd.handle_options [gem_name, '-v', version] + cmd.execute + rescue Gem::SystemExitException => e + raise unless e.exit_code == 0 + end + end + before do add_bundle - find_or_install('backport', '1.0.0') - Gem::Specification.find_by_name('backport', '= 1.0.0') + find_or_install('public_suffix', '1.0.0') + Gem::Specification.find_by_name('public_suffix', '= 1.0.0') end let(:preferences) do [ Gem::Specification.new.tap do |spec| - spec.name = 'backport' + spec.name = 'public_suffix' spec.version = '1.0.0' end ] @@ -257,17 +315,17 @@ def configure_bundler_spec stub_value it 'returns the preferred gemspec' do gemspecs = described_class.new(dir_path, preferences: preferences) - specs = gemspecs.resolve_require('backport') - backport = specs.find { |spec| spec.name == 'backport' } + specs = gemspecs.resolve_require('public_suffix') + public_suffix = specs.find { |spec| spec.name == 'public_suffix' } - expect(backport.version.to_s).to eq('1.0.0') + expect(public_suffix.version.to_s).to eq('1.0.0') end context 'with a gem preference that does not exist' do let(:preferences) do [ Gem::Specification.new.tap do |spec| - spec.name = 'backport' + spec.name = 'public_suffix' spec.version = '99.0.0' end ] @@ -275,20 +333,20 @@ def configure_bundler_spec stub_value it 'returns the gemspec we do have' do gemspecs = described_class.new(dir_path, preferences: preferences) - specs = gemspecs.resolve_require('backport') - backport = specs.find { |spec| spec.name == 'backport' } + specs = gemspecs.resolve_require('public_suffix') + public_suffix = specs.find { |spec| spec.name == 'public_suffix' } - expect(backport.version.to_s).to eq('1.2.0') + expect(public_suffix.version.to_s).to eq('3.1.1') end end context 'with a gem preference already set to the version we use' do - let(:version) { Gem::Specification.find_by_name('backport').version.to_s } + let(:version) { Gem::Specification.find_by_name('public_suffix').version.to_s } let(:preferences) do [ Gem::Specification.new.tap do |spec| - spec.name = 'backport' + spec.name = 'public_suffix' spec.version = version end ] @@ -296,10 +354,10 @@ def configure_bundler_spec stub_value it 'returns the gemspec we do have' do gemspecs = described_class.new(dir_path, preferences: preferences) - specs = gemspecs.resolve_require('backport') - backport = specs.find { |spec| spec.name == 'backport' } + specs = gemspecs.resolve_require('public_suffix') + public_suffix = specs.find { |spec| spec.name == 'public_suffix' } - expect(backport.version.to_s).to eq(version) + expect(public_suffix.version.to_s).to eq(version) end end end diff --git a/spec/workspace/require_paths_spec.rb b/spec/workspace/require_paths_spec.rb index eb95d0c5b..b89979022 100644 --- a/spec/workspace/require_paths_spec.rb +++ b/spec/workspace/require_paths_spec.rb @@ -2,25 +2,23 @@ require 'fileutils' require 'tmpdir' +require 'benchmark' describe Solargraph::Workspace::RequirePaths do - subject(:paths) { described_class.new(dir_path, config).generate } + subject(:paths) do + described_class.new(dir_path, config).generate + end let(:config) { Solargraph::Workspace::Config.new(dir_path) } - context 'with no config' do - let(:dir_path) { Dir.pwd } - let(:config) { nil } - - it 'includes the lib directory' do - expect(paths).to include(File.join(dir_path, 'lib')) - end - end - context 'with config and no gemspec' do let(:dir_path) { File.realpath(Dir.pwd) } - let(:config) { instance_double(Solargraph::Workspace::Config, require_paths: [], allow?: true) } + let(:config) do + instance_double(Solargraph::Workspace::Config, + require_paths: [], + allow?: false) + end it 'includes the lib directory' do expect(paths).to include(File.join(dir_path, 'lib')) diff --git a/spec/yard_map/mapper_spec.rb b/spec/yard_map/mapper_spec.rb index b2efd4cec..c8dda9f45 100644 --- a/spec/yard_map/mapper_spec.rb +++ b/spec/yard_map/mapper_spec.rb @@ -1,24 +1,29 @@ # frozen_string_literal: true describe Solargraph::YardMap::Mapper do - before :all do # rubocop:disable RSpec/BeforeAfterAll - @api_map = Solargraph::ApiMap.load('.') + # before :context here disables parallel tests in prspec, which + # would be needed regardless as we are changing the working + # directory + before :context do + @api_map = Solargraph::ApiMap.new end def pins_with require - doc_map = Solargraph::DocMap.new([require], @api_map.workspace, out: nil) - doc_map.cache_all!(nil) + doc_map = Solargraph::DocMap.new([require], @api_map.workspace, out: $stderr) + doc_map.cache_doc_map_gems!($stderr) doc_map.pins end it 'converts nil docstrings to empty strings' do dir = File.absolute_path(File.join('spec', 'fixtures', 'yard_map')) - Dir.chdir dir do - YARD::Registry.load([File.join(dir, 'attr.rb')], true) - mapper = described_class.new(YARD::Registry.all) - pins = mapper.map - pin = pins.select { |pin| pin.path == 'Foo#bar' }.first - expect(pin.comments).to be_a(String) + Solargraph::CHDIR_MUTEX.synchronize do + Dir.chdir dir do + YARD::Registry.load([File.join(dir, 'attr.rb')], true) + mapper = described_class.new(YARD::Registry.all) + pins = mapper.map + pin = pins.select { |pin| pin.path == 'Foo#bar' }.first + expect(pin.comments).to be_a(String) + end end # Cleanup FileUtils.remove_entry_secure File.join(dir, '.yardoc') @@ -39,14 +44,16 @@ def pins_with require it 'marks correct return type from RuboCop::Options.new' do # Using rubocop because it's a known dependency - pins = pins_with('rubocop').select { |pin| pin.path == 'RuboCop::Options.new' } - expect(pins.map(&:return_type).uniq.map(&:to_s)).to eq(['self']) - expect(pins.flat_map(&:signatures).map(&:return_type).uniq.map(&:to_s)).to eq(['self']) + all_pins = pins_with('open3') + pins = all_pins.select { |pin| pin.path == 'Open3.capture2e' } + expect(pins.map(&:return_type).uniq.map(&:to_s)).to eq(['Array(String, Process::Status)']) + expect(pins.flat_map(&:signatures).map(&:return_type).uniq.map(&:to_s)).to eq(['Array(String, Process::Status)']) end it 'marks non-explicit methods' do # Using rspec-expectations because it's a known dependency pin = pins_with('rspec/expectations').find { |pin| pin.path == 'RSpec::Matchers#expect' } + expect(pin.explicit?).to be(false) end From f5de7125bc17969528587680d2dcf4923af97c3c Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 15:41:05 -0400 Subject: [PATCH 2/9] Fix five sources of CI non-determinism from parallel test execution Each of these explained a distinct class of intermittent CI failure observed across this PR's rspec matrix/parallel_tests jobs, and will only get more frequent as test parallelism increases. - Diagnoser: an uncaught exception during a background diagnosis (e.g. a file/directory disappearing mid-diagnosis, such as protocol_spec's around-block temp dir cleanup racing the async diagnoser thread) killed the thread before it reached the line that marks it fully stopped, so Host#fully_stop hung for its full 240-second timeout every time. Now rescues broadly around individual diagnoses and guarantees the fully_stopped flag is set via ensure regardless of how the thread's loop exits. - MessageWorker: stop() never signaled its condition variable, so a thread blocked in tick's wait() with an empty queue could never wake up to notice stopped? and exit - a permanently leaked thread. Also added fully_stopped? tracking (matching Diagnoser) and wired it into Host#fully_stopped?, which previously didn't wait on MessageWorker's thread at all. - PinCache#save wrote directly to the final cache path with no atomicity. Multiple parallel_tests workers (separate OS processes) racing to cache the same not-yet-warm gem for the first time could corrupt or truncate each other's writes on the shared cache directory. Now writes to a temp file and renames into place (atomic on the same filesystem). - Yardoc.cache invoked a bare `yardoc` command, relying on it being found via shell PATH - which fails for unbundled environments/subprocesses where it only exists inside the current bundle's own bin directory. Now resolves the actual executable via Gem.bin_path, independent of PATH. - rubocop_helpers_spec.rb's "custom version" test unconditionally removed the process-global RuboCop constant in its cleanup, even when its own version-swap had been a no-op (because something else in the process, e.g. protocol_spec.rb's top-level require, had already loaded the real gem first) - i.e. even when there was nothing to restore. That left RuboCop undefined for the rest of the process, cascading into failures in unrelated specs (library_spec, protocol_spec's formatting/environment handlers, rubocop_spec) whenever this spec happened to run first. Now only cleans up (and reloads the real version) when the swap actually took effect. Local full-suite run: 13 failures -> 2, both isolated/self-contained and already understood (rubocop_helpers_spec's version-swap doesn't work when rubocop was already required by something else first, and a pre-existing gem_pins_spec bug in this PR's own test content). --- lib/solargraph/language_server/host.rb | 2 +- .../language_server/host/diagnoser.rb | 13 ++++- .../language_server/host/message_worker.rb | 16 +++++- lib/solargraph/pin_cache.rb | 9 ++- lib/solargraph/yardoc.rb | 8 ++- spec/diagnostics/rubocop_helpers_spec.rb | 19 ++++++- spec/pin_cache_spec.rb | 57 +++++++++++++++++++ 7 files changed, 115 insertions(+), 9 deletions(-) create mode 100644 spec/pin_cache_spec.rb diff --git a/lib/solargraph/language_server/host.rb b/lib/solargraph/language_server/host.rb index 012304b29..e4f4ab445 100644 --- a/lib/solargraph/language_server/host.rb +++ b/lib/solargraph/language_server/host.rb @@ -487,7 +487,7 @@ def stopped? end def fully_stopped? - @stopped && diagnoser.fully_stopped? + @stopped && diagnoser.fully_stopped? && message_worker.fully_stopped? end # Locate multiple pins that match a completion item. The first match is diff --git a/lib/solargraph/language_server/host/diagnoser.rb b/lib/solargraph/language_server/host/diagnoser.rb index 3b099732a..910e1e636 100644 --- a/lib/solargraph/language_server/host/diagnoser.rb +++ b/lib/solargraph/language_server/host/diagnoser.rb @@ -47,14 +47,15 @@ def fully_stopped? def start return unless @stopped @fully_stopped = @stopped = false - old_thread_id = Thread.current.object_id Thread.new do until stopped? - $stderr.puts "Diagnoser: start tick in thread #{old_thread_id}, current thread #{Thread.current.object_id}" tick - $stderr.puts "Diagnoser: end tick in thread #{old_thread_id}, current thread #{Thread.current.object_id}" sleep 0.1 end + ensure + # Guarantee fully_stopped? eventually becomes true even if tick + # raised something unexpected, so Host#fully_stop can't hang + # forever waiting on a thread that already died. @fully_stopped = true end self @@ -79,6 +80,12 @@ def tick # but it's quick and easy. Logging.logger.warn "Deferring diagnosis due to invalid offset: #{current}" mutex.synchronize { queue.push current } + rescue StandardError => e + # Diagnosing a single file shouldn't be able to permanently + # kill the background thread (e.g. a file/directory disappearing + # mid-diagnosis, or a reporter misconfiguration) - log it and + # keep processing the rest of the queue. + Logging.logger.warn "Error diagnosing #{current}: [#{e.class}] #{e.message}" end end diff --git a/lib/solargraph/language_server/host/message_worker.rb b/lib/solargraph/language_server/host/message_worker.rb index 440e2c7cc..6db2aa568 100644 --- a/lib/solargraph/language_server/host/message_worker.rb +++ b/lib/solargraph/language_server/host/message_worker.rb @@ -25,6 +25,7 @@ def initialize host @mutex = Mutex.new @resource = ConditionVariable.new @stopped = true + @fully_stopped = true end # pending handle messages @@ -37,9 +38,17 @@ def stopped? @stopped end + def fully_stopped? + @fully_stopped + end + # @return [void] def stop @stopped = true + # Wake up a thread that may be blocked in tick's + # @resource.wait with an empty queue, so it can notice + # stopped? and exit instead of hanging forever. + @mutex.synchronize { @resource.signal } end # @param message [Hash] The message to handle. Will be forwarded to Host#receive @@ -54,9 +63,11 @@ def queue message # @return [void] def start return unless @stopped - @stopped = false + @fully_stopped = @stopped = false Thread.new do tick until stopped? + ensure + @fully_stopped = true end end @@ -66,7 +77,8 @@ def tick @resource.wait(@mutex) if messages.empty? next_message end - # @sg-ignore Need to add nil check here + return if message.nil? + handler = @host.receive(message) handler&.send_response end diff --git a/lib/solargraph/pin_cache.rb b/lib/solargraph/pin_cache.rb index 803170764..17901a05e 100644 --- a/lib/solargraph/pin_cache.rb +++ b/lib/solargraph/pin_cache.rb @@ -216,7 +216,14 @@ def save file, pins base = File.dirname(file) FileUtils.mkdir_p base unless File.directory?(base) ser = Marshal.dump(pins) - File.write file, ser, mode: 'wb' + # Write to a temp file in the same directory and rename into place + # (rename is atomic on the same filesystem) so that concurrent + # readers/writers - e.g. multiple parallel_tests worker processes + # caching the same gem for the first time - never observe a + # partially-written or corrupted cache file. + tmp = "#{file}.#{Process.pid}.#{Thread.current.object_id}.tmp" + File.write tmp, ser, mode: 'wb' + File.rename tmp, file logger.debug { "Cache#save: Saved #{pins.length} pins to #{file}" } end diff --git a/lib/solargraph/yardoc.rb b/lib/solargraph/yardoc.rb index 2150dcbef..eff4f9e33 100644 --- a/lib/solargraph/yardoc.rb +++ b/lib/solargraph/yardoc.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'open3' +require 'shellwords' module Solargraph # Methods for caching and loading YARD documentation for gems. @@ -28,7 +29,12 @@ def cache yard_plugins, gemspec end Solargraph.logger.info "Caching yardoc for #{gemspec.name} #{gemspec.version}" - cmd = "yardoc --db #{path} --no-output --plugin solargraph" + # Resolve the actual executable instead of relying on a bare + # `yardoc` being on PATH - it may only exist inside the current + # bundle's own bin directory (e.g. when running outside `bundle + # exec`, or in an unbundled environment/subprocess). + yardoc_bin = Gem.bin_path('yard', 'yardoc') + cmd = "#{Shellwords.escape(yardoc_bin)} --db #{path} --no-output --plugin solargraph" yard_plugins.each { |plugin| cmd << " --plugin #{plugin}" } Solargraph.logger.debug { "Running: #{cmd}" } # @todo set these up to run in parallel diff --git a/spec/diagnostics/rubocop_helpers_spec.rb b/spec/diagnostics/rubocop_helpers_spec.rb index 98e31c233..999bafa5b 100644 --- a/spec/diagnostics/rubocop_helpers_spec.rb +++ b/spec/diagnostics/rubocop_helpers_spec.rb @@ -10,12 +10,29 @@ old_post_reset_hooks = Gem.post_reset_hooks.dup Gem.post_reset_hooks.clear Gem.paths = { 'GEM_PATH' => [custom_gem_path, *old_gem_path].join(Gem.path_separator) } + # Whether our require_rubocop(custom_version) call below actually + # swapped in the fixture version, versus being a no-op because + # something else in this process (e.g. a `require 'rubocop'` at the + # top of another spec file) already loaded the real gem first - + # Kernel#require only ever executes a given resolved path once. example.run + swapped_to_custom_version = defined?(RuboCop) && custom_version == RuboCop::Version::STRING old_post_reset_hooks.each(&Gem.post_reset_hooks.method(:<<)) Gem.paths = { 'GEM_PATH' => old_gem_path.join(Gem.path_separator) } # Cleanup loaded classes from custom gem path $LOAD_PATH.delete_if { |path| path[custom_gem_path] } - Object.send(:remove_const, 'RuboCop') + # RuboCop is a process-global constant, not scoped to this example - + # other specs/code running later in this same process (formatting, + # Library#diagnose, etc.) expect it to stay defined as the real, + # bundled version. Only remove/reload it if we actually swapped it + # out for the fixture version above; otherwise it's already the + # real version (untouched), and remove_const would just leave it + # undefined, since re-requiring the same already-loaded path is a + # no-op and won't redefine it. + if swapped_to_custom_version + Object.send(:remove_const, 'RuboCop') + described_class.require_rubocop + end end let(:custom_version) { '0.0.0' } diff --git a/spec/pin_cache_spec.rb b/spec/pin_cache_spec.rb new file mode 100644 index 000000000..f74f524cd --- /dev/null +++ b/spec/pin_cache_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'tmpdir' + +describe Solargraph::PinCache do + describe '.save' do + # Writing straight to the final path (the old behavior) means any + # other process reading or writing that same path concurrently - + # e.g. multiple parallel_tests workers, each a separate OS process, + # racing to cache the same not-yet-cached gem for the first time - + # can observe a truncated or torn file. Writing to a temp file and + # renaming into place is atomic on the same filesystem, so readers + # always see either the complete old file or the complete new one. + it 'writes to a temp file and renames it into place, never writing the target path directly' do + Dir.mktmpdir do |dir| + file = File.join(dir, 'gem.ser') + pins = [Solargraph::Pin::Base.new(name: 'foo')] + + written_path = nil + renamed_from = nil + renamed_to = nil + allow(File).to receive(:write).and_wrap_original do |original, path, *args, **kwargs| + written_path = path + original.call(path, *args, **kwargs) + end + allow(File).to receive(:rename).and_wrap_original do |original, from, to| + renamed_from = from + renamed_to = to + original.call(from, to) + end + + described_class.send(:save, file, pins) + + expect(written_path).not_to eq(file), 'save wrote directly to the target path instead of a temp file' + expect(renamed_from).to eq(written_path) + expect(renamed_to).to eq(file) + end + end + + it 'does not leave temp files behind' do + Dir.mktmpdir do |dir| + file = File.join(dir, 'single.ser') + described_class.send(:save, file, [Solargraph::Pin::Base.new(name: 'foo')]) + expect(Dir.glob("#{file}*")).to eq([file]) + end + end + + it 'round-trips pins through save and load' do + Dir.mktmpdir do |dir| + file = File.join(dir, 'roundtrip.ser') + pins = [Solargraph::Pin::Base.new(name: 'foo'), Solargraph::Pin::Base.new(name: 'bar')] + described_class.send(:save, file, pins) + expect(described_class.send(:load, file).map(&:name)).to eq(%w[foo bar]) + end + end + end +end From 68561b231e38ce7346fc75fab4ce33688dade5ec Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 16:09:53 -0400 Subject: [PATCH 3/9] Lock yardoc cache builds to fix a cross-process race Yardoc.cache's "check cached, else build" was a classic check-then-act race: two OS processes (e.g. two parallel_tests workers, each caching the same not-yet-cached gem for the first time) could both see "not cached" and run `yardoc --db path` concurrently against the same .yardoc database directory, corrupting or truncating each other's output. This is very likely the actual cause of spec/pin/base_spec.rb's intermittent "deals well with known closure combination issue" failure and the strict_spec.rb Kramdown-constant failure in CI (both build/read a gem's YARD pins via this path) - my earlier PinCache#save atomic- write fix only covered Solargraph's own Marshal cache files, not this separate tool-managed directory. Wrap the build in a per-gem flock'd lock file, re-checking cached? after acquiring the lock (double-checked locking) so only one process actually builds a given gem; the rest wait for the lock and then reuse what the first process built instead of racing. This is the same mechanism a prior, incomplete attempt at this (Yardoc.processing?, referenced from Library#diagnose but never actually used to coordinate Yardoc.cache itself) was clearly reaching for. --- lib/solargraph/yardoc.rb | 48 ++++++++++++++++++++++++++-------------- spec/yardoc_spec.rb | 23 +++++++++++++++++++ 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/lib/solargraph/yardoc.rb b/lib/solargraph/yardoc.rb index eff4f9e33..464aedf96 100644 --- a/lib/solargraph/yardoc.rb +++ b/lib/solargraph/yardoc.rb @@ -2,6 +2,7 @@ require 'open3' require 'shellwords' +require 'fileutils' module Solargraph # Methods for caching and loading YARD documentation for gems. @@ -28,22 +29,37 @@ def cache yard_plugins, gemspec return path end - Solargraph.logger.info "Caching yardoc for #{gemspec.name} #{gemspec.version}" - # Resolve the actual executable instead of relying on a bare - # `yardoc` being on PATH - it may only exist inside the current - # bundle's own bin directory (e.g. when running outside `bundle - # exec`, or in an unbundled environment/subprocess). - yardoc_bin = Gem.bin_path('yard', 'yardoc') - cmd = "#{Shellwords.escape(yardoc_bin)} --db #{path} --no-output --plugin solargraph" - yard_plugins.each { |plugin| cmd << " --plugin #{plugin}" } - Solargraph.logger.debug { "Running: #{cmd}" } - # @todo set these up to run in parallel - # @todo Is the chdir argument being used here? - # @sg-ignore Unrecognized keyword argument chdir to Open3.capture2e - stdout_and_stderr_str, status = Open3.capture2e(current_bundle_env_tweaks, cmd, chdir: gemspec.gem_dir) - unless status.success? - Solargraph.logger.warn { "YARD failed running #{cmd.inspect} in #{gemspec.gem_dir}" } - Solargraph.logger.info stdout_and_stderr_str + # The `yardoc` command below builds a whole directory of files (the + # .yardoc database) at `path`. Without coordination, two OS processes + # - e.g. multiple parallel_tests workers, each caching the same + # not-yet-cached gem for the first time - could both see "not cached" + # above and run `yardoc --db path` concurrently, corrupting each + # other's output. A per-gem file lock ensures only one process + # builds it; the rest wait, then reuse what the first one built. + # @sg-ignore FileUtils.mkdir_p accepts a String, despite what the RBS signature says + FileUtils.mkdir_p File.dirname(path) + lock_path = "#{path}.lock" + File.open(lock_path, File::CREAT | File::RDWR) do |lock_file| + lock_file.flock(File::LOCK_EX) + next if cached?(gemspec) + + Solargraph.logger.info "Caching yardoc for #{gemspec.name} #{gemspec.version}" + # Resolve the actual executable instead of relying on a bare + # `yardoc` being on PATH - it may only exist inside the current + # bundle's own bin directory (e.g. when running outside `bundle + # exec`, or in an unbundled environment/subprocess). + yardoc_bin = Gem.bin_path('yard', 'yardoc') + cmd = "#{Shellwords.escape(yardoc_bin)} --db #{path} --no-output --plugin solargraph" + yard_plugins.each { |plugin| cmd << " --plugin #{plugin}" } + Solargraph.logger.debug { "Running: #{cmd}" } + # @todo set these up to run in parallel + # @todo Is the chdir argument being used here? + stdout_and_stderr_str, status = Open3.capture2e(current_bundle_env_tweaks, cmd, chdir: gemspec.gem_dir) + # @sg-ignore Open3.capture2e's second return value is a Process::Status + unless status.success? + Solargraph.logger.warn { "YARD failed running #{cmd.inspect} in #{gemspec.gem_dir}" } + Solargraph.logger.info stdout_and_stderr_str + end end path end diff --git a/spec/yardoc_spec.rb b/spec/yardoc_spec.rb index 5ad0e5805..326515cb2 100644 --- a/spec/yardoc_spec.rb +++ b/spec/yardoc_spec.rb @@ -48,5 +48,28 @@ expect(called_with[0]['BUNDLE_GEMFILE']).to eq(File.absolute_path('Gemfile')) end end + + context 'when multiple OS processes cache the same not-yet-cached gem at once' do + # a small, fast-to-document gem - as multiple parallel_tests workers + # (separate OS processes) could all try to cache the same gem for + # the first time simultaneously + let(:gemspec) { Gem::Specification.find_by_name('diff-lcs') } + + it 'never corrupts the yardoc database' do + skip 'requires Process.fork' unless Process.respond_to?(:fork) + + pids = 4.times.map do + Process.fork do + described_class.cache([], gemspec) + exit!(0) + end + end + pids.each { |pid| Process.wait(pid) } + + expect(described_class.cached?(gemspec)).to be(true) + expect { YARD::Registry.load!(gem_yardoc_path) }.not_to raise_error + expect(YARD::Registry.all).not_to be_empty + end + end end end From 95b4945a88f24b2d782d18cb174b85e23a5cc1f6 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 16:38:40 -0400 Subject: [PATCH 4/9] Revert "Lock yardoc cache builds to fix a cross-process race" This reverts commit 68561b231e38ce7346fc75fab4ce33688dade5ec. --- lib/solargraph/yardoc.rb | 48 ++++++++++++++-------------------------- spec/yardoc_spec.rb | 23 ------------------- 2 files changed, 16 insertions(+), 55 deletions(-) diff --git a/lib/solargraph/yardoc.rb b/lib/solargraph/yardoc.rb index 464aedf96..eff4f9e33 100644 --- a/lib/solargraph/yardoc.rb +++ b/lib/solargraph/yardoc.rb @@ -2,7 +2,6 @@ require 'open3' require 'shellwords' -require 'fileutils' module Solargraph # Methods for caching and loading YARD documentation for gems. @@ -29,37 +28,22 @@ def cache yard_plugins, gemspec return path end - # The `yardoc` command below builds a whole directory of files (the - # .yardoc database) at `path`. Without coordination, two OS processes - # - e.g. multiple parallel_tests workers, each caching the same - # not-yet-cached gem for the first time - could both see "not cached" - # above and run `yardoc --db path` concurrently, corrupting each - # other's output. A per-gem file lock ensures only one process - # builds it; the rest wait, then reuse what the first one built. - # @sg-ignore FileUtils.mkdir_p accepts a String, despite what the RBS signature says - FileUtils.mkdir_p File.dirname(path) - lock_path = "#{path}.lock" - File.open(lock_path, File::CREAT | File::RDWR) do |lock_file| - lock_file.flock(File::LOCK_EX) - next if cached?(gemspec) - - Solargraph.logger.info "Caching yardoc for #{gemspec.name} #{gemspec.version}" - # Resolve the actual executable instead of relying on a bare - # `yardoc` being on PATH - it may only exist inside the current - # bundle's own bin directory (e.g. when running outside `bundle - # exec`, or in an unbundled environment/subprocess). - yardoc_bin = Gem.bin_path('yard', 'yardoc') - cmd = "#{Shellwords.escape(yardoc_bin)} --db #{path} --no-output --plugin solargraph" - yard_plugins.each { |plugin| cmd << " --plugin #{plugin}" } - Solargraph.logger.debug { "Running: #{cmd}" } - # @todo set these up to run in parallel - # @todo Is the chdir argument being used here? - stdout_and_stderr_str, status = Open3.capture2e(current_bundle_env_tweaks, cmd, chdir: gemspec.gem_dir) - # @sg-ignore Open3.capture2e's second return value is a Process::Status - unless status.success? - Solargraph.logger.warn { "YARD failed running #{cmd.inspect} in #{gemspec.gem_dir}" } - Solargraph.logger.info stdout_and_stderr_str - end + Solargraph.logger.info "Caching yardoc for #{gemspec.name} #{gemspec.version}" + # Resolve the actual executable instead of relying on a bare + # `yardoc` being on PATH - it may only exist inside the current + # bundle's own bin directory (e.g. when running outside `bundle + # exec`, or in an unbundled environment/subprocess). + yardoc_bin = Gem.bin_path('yard', 'yardoc') + cmd = "#{Shellwords.escape(yardoc_bin)} --db #{path} --no-output --plugin solargraph" + yard_plugins.each { |plugin| cmd << " --plugin #{plugin}" } + Solargraph.logger.debug { "Running: #{cmd}" } + # @todo set these up to run in parallel + # @todo Is the chdir argument being used here? + # @sg-ignore Unrecognized keyword argument chdir to Open3.capture2e + stdout_and_stderr_str, status = Open3.capture2e(current_bundle_env_tweaks, cmd, chdir: gemspec.gem_dir) + unless status.success? + Solargraph.logger.warn { "YARD failed running #{cmd.inspect} in #{gemspec.gem_dir}" } + Solargraph.logger.info stdout_and_stderr_str end path end diff --git a/spec/yardoc_spec.rb b/spec/yardoc_spec.rb index 326515cb2..5ad0e5805 100644 --- a/spec/yardoc_spec.rb +++ b/spec/yardoc_spec.rb @@ -48,28 +48,5 @@ expect(called_with[0]['BUNDLE_GEMFILE']).to eq(File.absolute_path('Gemfile')) end end - - context 'when multiple OS processes cache the same not-yet-cached gem at once' do - # a small, fast-to-document gem - as multiple parallel_tests workers - # (separate OS processes) could all try to cache the same gem for - # the first time simultaneously - let(:gemspec) { Gem::Specification.find_by_name('diff-lcs') } - - it 'never corrupts the yardoc database' do - skip 'requires Process.fork' unless Process.respond_to?(:fork) - - pids = 4.times.map do - Process.fork do - described_class.cache([], gemspec) - exit!(0) - end - end - pids.each { |pid| Process.wait(pid) } - - expect(described_class.cached?(gemspec)).to be(true) - expect { YARD::Registry.load!(gem_yardoc_path) }.not_to raise_error - expect(YARD::Registry.all).not_to be_empty - end - end end end From f13c49595a20dbd2282ab3942ddbba9aff101ae6 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 17:15:57 -0400 Subject: [PATCH 5/9] Fix the actual root cause of the remaining CI flakiness The two remaining intermittent failures (Kramdown constant in strict_spec.rb, 0 pins in pin/base_spec.rb) were never a cross-process caching race at all - reproduced deterministically locally with a cold cache, in complete isolation, no concurrency involved. My earlier PinCache atomicity and yardoc-locking fixes were solving a real but different problem than this one. Both tests called ApiMap#cache_gem(spec) for a gem before ever telling the ApiMap's DocMap that gem was needed (via #catalog with external_requires). DocMap#cache only builds pins for gemspecs in its own uncached_yard_gemspecs/uncached_rbs_collection_gemspecs lists, which are only populated from requires resolved during #catalog - so cache_gem was silently a no-op, and building only happened to succeed when something else had already warmed the gem's cache earlier in the same process (hence "intermittent", depending entirely on test/file run order and cache state, not timing). Fixed both to use the catalog -> cache_all_for_doc_map! -> catalog sequence already used correctly elsewhere in this same PR (see rbs_map/conversions_spec.rb's "with superclass pin for Parser::AST::Node" context): catalog first so DocMap learns about the dependency, cache_all_for_doc_map! to build it, catalog again to reload the ApiMap's pin store with the now-cached pins. Verified: both pass individually and together with ~/.cache/solargraph completely cleared beforehand (previously guaranteed to fail cold, pass only by accident once something else had warmed the cache). Full local suite with a cold cache: 1625 examples, 2 failures - both isolated, already-known, unrelated issues (rubocop_helpers_spec's version-swap test doesn't work when rubocop was already required by something else first, and a real RBS/YARD merge bug in gem_pins_spec.rb) - no other instances of this cache-ordering bug found anywhere else in the suite. --- spec/pin/base_spec.rb | 12 ++++++++---- spec/type_checker/levels/strict_spec.rb | 8 ++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/spec/pin/base_spec.rb b/spec/pin/base_spec.rb index 374e97eaa..39f47e492 100644 --- a/spec/pin/base_spec.rb +++ b/spec/pin/base_spec.rb @@ -55,11 +55,15 @@ # if this fails you might not have an rbs collection installed api_map = Solargraph::ApiMap.load '' - spec = Gem::Specification.find_by_name('yard') - api_map.cache_gem(spec) - - bench = Solargraph::Bench.new(external_requires: ['yard']) + bench = Solargraph::Bench.new(workspace: api_map.workspace, external_requires: ['yard']) + # cache_gem is a no-op for a gem the ApiMap's DocMap doesn't know it + # needs yet, so catalog (which tells DocMap about external_requires) + # has to run before caching, and again afterward to pick up the + # newly-cached pins. + api_map.catalog bench + api_map.cache_all_for_doc_map! api_map.catalog bench + pins = api_map.get_method_stack('YARD::Docstring', 'parser', scope: :class) expect(pins.length).to eq(1) parser_method_pin = pins.first diff --git a/spec/type_checker/levels/strict_spec.rb b/spec/type_checker/levels/strict_spec.rb index aa7edc4b6..386e56c88 100644 --- a/spec/type_checker/levels/strict_spec.rb +++ b/spec/type_checker/levels/strict_spec.rb @@ -107,9 +107,13 @@ def bar(a); end ), 'test.rb') api_map = Solargraph::ApiMap.new - specs = api_map.resolve_require('kramdown-parser-gfm') - specs.each { |spec| api_map.cache_gem(spec) } bench = Solargraph::Bench.new(source_maps: [source_map], external_requires: ['kramdown-parser-gfm']) + # cache_gem is a no-op for a gem the ApiMap's DocMap doesn't know it + # needs yet, so catalog (which tells DocMap about external_requires) + # has to run before caching, and again afterward to pick up the + # newly-cached pins. + api_map.catalog bench + api_map.cache_all_for_doc_map! api_map.catalog bench checker = described_class.new('test.rb', api_map: api_map, level: :strict) From 11a6ca0dcf3a7d6eb7a30727013b0f65f6a6c5ba Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 17:34:58 -0400 Subject: [PATCH 6/9] Synchronize protocol_spec's chdir with the existing CHDIR_MUTEX protocol_spec.rb's around block chdirs into a per-example temp directory and back, on the main thread, without holding Solargraph::CHDIR_MUTEX - the same mutex that Diagnostics::Rubocop#diagnose and the textDocument/formatting handler already use specifically because RuboCop::Runner internally chdirs (with a block) to read config files. An earlier fix in this series made the background diagnoser thread resilient to errors instead of dying on the first one (rescuing broadly, guaranteeing fully_stopped? via ensure), so it now keeps running background diagnoses - including RuboCop ones - for longer during a test run. That made it far more likely to have an active chdir in flight from that mutex right as protocol_spec's own (unsynchronized) chdir ran, which Ruby raises as "conflicting chdir during another chdir block" (visible in CI as a wave of Protocol example failures, e.g. "handles textDocument/definition"). Route protocol_spec's chdir calls through the same Solargraph::CHDIR_MUTEX so they can't overlap with RuboCop's. --- spec/language_server/protocol_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/language_server/protocol_spec.rb b/spec/language_server/protocol_spec.rb index ef21676d7..93c508b2c 100644 --- a/spec/language_server/protocol_spec.rb +++ b/spec/language_server/protocol_spec.rb @@ -45,13 +45,17 @@ def stop around do |testobj| raise "Requests not finished #{testobj} - #{@protocol.host.pending_requests.inspect}" unless @protocol.host.pending_requests.empty? temp_dir = Dir.mktmpdir - Dir.chdir temp_dir + # The background diagnoser thread may be running RuboCop diagnostics + # concurrently, which also chdirs (under the same mutex) to read + # config files - without sharing that mutex here, this chdir can + # raise "conflicting chdir during another chdir block". + Solargraph::CHDIR_MUTEX.synchronize { Dir.chdir temp_dir } Solargraph.with_clean_env do testobj.run end raise "Requests not finished - #{@protocol.host.send(:requests).inspect}" unless @protocol.host.pending_requests.empty? ensure - Dir.chdir PROJECT_DIRECTORY + Solargraph::CHDIR_MUTEX.synchronize { Dir.chdir PROJECT_DIRECTORY } FileUtils.remove_entry(temp_dir) end From 006358d741b9d7e7a5e39bb45f0a8a4966716fa0 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 19:00:56 -0400 Subject: [PATCH 7/9] Wire up the lost workspace: passthrough, drop leftover scope creep An audit of the full diff against master (prompted by "are we dragging in v0.59 changes that weren't intended?") found several small items inherited from the original apiology/parallel_rspec branch history, predating this session's rebase: - Diagnostics::Base#diagnose and TypeCheck#diagnose gained a `workspace:` kwarg (commit "Spec performance fixes", a 33% local speedup) so TypeChecker.new could reuse an already-loaded Workspace instead of implicitly building a fresh one via Workspace.new(File.dirname(filename)) on every diagnose call. The kwarg was added but the one production call site, Library#diagnose (library.rb), was never updated to pass it, so the optimization was inert. Wire it through, and add the same kwarg to the other Diagnostics::Base subclasses (Rubocop, UpdateErrors, RequireNotFound) so the polymorphic call in Library#diagnose doesn't raise ArgumentError for reporters that don't use it. - Remove RbsMap::StdlibMap.possible_stdlibs: added, never called. - Remove duplicate/redundant YARD @param comments added to ComplexType#qualify and .parse alongside the existing docs. - Revert a no-op reordering of Workspace#gemfile?/gemspec?/gemspec_files back to their master position; both locations are public, so this wasn't a visibility change, just unexplained churn. Left alone: a stray "@todo Missed nil violation" comment in source/chain.rb, and RuboCop-autocorrect-driven formatting diffs elsewhere in the branch (YARD/CollectionStyle, quote style) that predate this rebase and are needed to keep Overcommit clean. --- lib/solargraph/complex_type.rb | 2 - .../diagnostics/require_not_found.rb | 5 +- lib/solargraph/diagnostics/rubocop.rb | 3 +- lib/solargraph/diagnostics/update_errors.rb | 5 +- lib/solargraph/library.rb | 3 +- lib/solargraph/rbs_map/stdlib_map.rb | 5 -- lib/solargraph/workspace.rb | 50 +++++++++---------- spec/library_spec.rb | 2 +- 8 files changed, 38 insertions(+), 37 deletions(-) diff --git a/lib/solargraph/complex_type.rb b/lib/solargraph/complex_type.rb index 19605316c..27d2ff08c 100644 --- a/lib/solargraph/complex_type.rb +++ b/lib/solargraph/complex_type.rb @@ -33,7 +33,6 @@ def initialize types = [UniqueType::UNDEFINED] # @param gates [Array] # # @return [ComplexType] - # @param [Array] gates def qualify api_map, *gates red = reduce_object types = red.items.map do |t| @@ -440,7 +439,6 @@ class << self # Chain::Call needs to know the decl type (:arg, :optarg, # :kwarg, etc) of the arguments given, instead of just having # an array of Chains as the arguments. - # @param [Boolean] partial def parse *strings, partial: false # @type [Hash{Array => ComplexType, Array}] @cache ||= {} diff --git a/lib/solargraph/diagnostics/require_not_found.rb b/lib/solargraph/diagnostics/require_not_found.rb index df42da2e5..73ad054e7 100644 --- a/lib/solargraph/diagnostics/require_not_found.rb +++ b/lib/solargraph/diagnostics/require_not_found.rb @@ -6,7 +6,10 @@ module Diagnostics # either a file in the workspace or a gem. # class RequireNotFound < Base - def diagnose source, api_map + # @param source [Solargraph::Source] + # @param api_map [Solargraph::ApiMap] + # @param workspace [Solargraph::Workspace, nil] unused; accepted for interface compatibility with Diagnostics::Base + def diagnose source, api_map, workspace: nil return [] unless source.parsed? && source.synchronized? result = [] refs = {} diff --git a/lib/solargraph/diagnostics/rubocop.rb b/lib/solargraph/diagnostics/rubocop.rb index 39b79d9b7..1641bc8e4 100644 --- a/lib/solargraph/diagnostics/rubocop.rb +++ b/lib/solargraph/diagnostics/rubocop.rb @@ -21,8 +21,9 @@ class Rubocop < Base # @param source [Solargraph::Source] # @param _api_map [Solargraph::ApiMap] + # @param workspace [Solargraph::Workspace, nil] unused; accepted for interface compatibility with Diagnostics::Base # @return [Array] - def diagnose source, _api_map + def diagnose source, _api_map, workspace: nil @source = source require_rubocop(rubocop_version) # @sg-ignore Need to add nil check here diff --git a/lib/solargraph/diagnostics/update_errors.rb b/lib/solargraph/diagnostics/update_errors.rb index c2ca02408..79d3e17a4 100644 --- a/lib/solargraph/diagnostics/update_errors.rb +++ b/lib/solargraph/diagnostics/update_errors.rb @@ -3,7 +3,10 @@ module Solargraph module Diagnostics class UpdateErrors < Base - def diagnose source, api_map + # @param source [Solargraph::Source] + # @param api_map [Solargraph::ApiMap] + # @param workspace [Solargraph::Workspace, nil] unused; accepted for interface compatibility with Diagnostics::Base + def diagnose source, api_map, workspace: nil combine_ranges(source.code, source.error_ranges).map do |range| { range: range.to_hash, severity: Diagnostics::Severities::ERROR, diff --git a/lib/solargraph/library.rb b/lib/solargraph/library.rb index 4f03fb862..7572606e0 100644 --- a/lib/solargraph/library.rb +++ b/lib/solargraph/library.rb @@ -425,7 +425,8 @@ def diagnose filename end end repargs.each_pair do |reporter, args| - result.concat reporter.new(*args.uniq).diagnose(source, api_map) + # @sg-ignore Class doesn't resolve #new + result.concat reporter.new(*args.uniq).diagnose(source, api_map, workspace: workspace) end result end diff --git a/lib/solargraph/rbs_map/stdlib_map.rb b/lib/solargraph/rbs_map/stdlib_map.rb index d80d3b2a9..82dde5770 100644 --- a/lib/solargraph/rbs_map/stdlib_map.rb +++ b/lib/solargraph/rbs_map/stdlib_map.rb @@ -67,11 +67,6 @@ def resolve_dependencies? def self.load library @stdlib_maps_hash[library] ||= StdlibMap.new(library) end - - # @return [Array] - def self.possible_stdlibs - RBS::Repository.default.gems.keys - end end end end diff --git a/lib/solargraph/workspace.rb b/lib/solargraph/workspace.rb index 3e0118c85..eef171032 100644 --- a/lib/solargraph/workspace.rb +++ b/lib/solargraph/workspace.rb @@ -130,31 +130,6 @@ def would_require? path false end - # True if the workspace has a root Gemfile. - # - # @todo Handle projects with custom Bundler/Gemfile setups (see DocMap#gemspecs_required_from_bundler) - # - def gemfile? - directory && File.file?(File.join(directory, 'Gemfile')) - end - - # True if the workspace contains at least one gemspec file. - # - # @return [Boolean] - def gemspec? - !gemspec_files.empty? - end - - # Get an array of all gemspec files in the workspace. - # - # @return [Array] - def gemspec_files - return [] if directory.empty? || directory == '*' - @gemspec_files ||= Dir[File.join(directory, '**/*.gemspec')].select do |gs| - config.allow? gs - end - end - # @return [String, nil] def rbs_collection_path @rbs_collection_path ||= read_rbs_collection_path @@ -198,6 +173,31 @@ def directory_or_nil directory end + # True if the workspace has a root Gemfile. + # + # @todo Handle projects with custom Bundler/Gemfile setups (see DocMap#gemspecs_required_from_bundler) + # + def gemfile? + directory && File.file?(File.join(directory, 'Gemfile')) + end + + # True if the workspace contains at least one gemspec file. + # + # @return [Boolean] + def gemspec? + !gemspec_files.empty? + end + + # Get an array of all gemspec files in the workspace. + # + # @return [Array] + def gemspec_files + return [] if directory.empty? || directory == '*' + @gemspec_files ||= Dir[File.join(directory, '**/*.gemspec')].select do |gs| + config.allow? gs + end + end + private # The language server configuration (or an empty hash if the workspace was diff --git a/spec/library_spec.rb b/spec/library_spec.rb index a414a8682..ebb982772 100644 --- a/spec/library_spec.rb +++ b/spec/library_spec.rb @@ -172,7 +172,7 @@ def bar baz, key: '' it 'diagnoses using all reporters' do directory = '' config = instance_double(Solargraph::Workspace::Config) - allow(config).to receive_messages(plugins: [], required: [], reporters: ['all!']) + allow(config).to receive_messages(plugins: [], required: [], reporters: ['all!'], type_checker_rules: {}) workspace = Solargraph::Workspace.new directory, config library = described_class.new workspace # keep this from syncing a bunch of bundle gems in background From e236c1816c53209bb89adb0aee4c6f9e8efb6713 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 21:16:07 -0400 Subject: [PATCH 8/9] Move YARD/CollectionStyle reformatting to a separate PR The Hash{Array(String, String) => ...} -> Hash{Array, String, String => ...} docstring reformatting in api_map/constants.rb, api_map/store.rb, doc_map.rb, source_map.rb, and the quote/block-style cleanup in spec/source/chain_spec.rb weren't related to this PR's stated purpose (parallel specs, concurrency fixes) - they were fixing YARD/CollectionStyle and Style/StringLiterals offenses that a fresh `rubocop --auto-gen-config` surfaces under the currently-installed RuboCop/rubocop-yard versions but that master's own committed .rubocop_todo.yml doesn't yet grandfather (a pre-existing drift, not something this branch introduced). Revert those files to master's content and add a scoped YARD/CollectionStyle todo exclusion for the same 4 files, so this branch stays green without carrying the unrelated reformatting. The actual fix now lives in a standalone PR: apiology/solargraph#42. --- .rubocop_todo.yml | 16 ++++++++++ lib/solargraph/api_map/constants.rb | 4 +-- lib/solargraph/api_map/store.rb | 4 +-- lib/solargraph/doc_map.rb | 12 +++---- lib/solargraph/source_map.rb | 2 +- spec/source/chain_spec.rb | 49 ++++++++++++++--------------- 6 files changed, 50 insertions(+), 37 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e503ad84a..3779bcbee 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -396,12 +396,20 @@ Style/SlicingWithRange: Exclude: - 'lib/solargraph/convention/struct_definition/struct_definition_node.rb' +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: EnforcedStyle. +# SupportedStyles: line_count_based, semantic, braces_for_chaining, always_braces +Style/BlockDelimiters: + Exclude: + - 'spec/source/chain_spec.rb' + # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline. # SupportedStyles: single_quotes, double_quotes Style/StringLiterals: Exclude: - 'spec/fixtures/rdoc-lib/rdoc-lib.gemspec' + - 'spec/source/chain_spec.rb' # This cop supports safe autocorrection (--autocorrect). Style/SuperArguments: @@ -409,6 +417,14 @@ Style/SuperArguments: - 'lib/solargraph/pin/callable.rb' - 'lib/solargraph/pin/method.rb' +# This cop supports safe autocorrection (--autocorrect). +YARD/CollectionStyle: + Exclude: + - 'lib/solargraph/api_map/constants.rb' + - 'lib/solargraph/api_map/store.rb' + - 'lib/solargraph/doc_map.rb' + - 'lib/solargraph/source_map.rb' + # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStylePrototypeName. # SupportedStylesPrototypeName: before, after diff --git a/lib/solargraph/api_map/constants.rb b/lib/solargraph/api_map/constants.rb index 010e9b128..880adacb6 100644 --- a/lib/solargraph/api_map/constants.rb +++ b/lib/solargraph/api_map/constants.rb @@ -188,12 +188,12 @@ def collect_and_cache gates end end - # @return [Hash{Array, String, Array, String => String, :in_process, nil}] + # @return [Hash{Array(String, Array) => String, :in_process, nil}] def cached_resolve @cached_resolve ||= {} end - # @return [Hash{Array, String => Array}] + # @return [Hash{Array => Array}] def cached_collect @cached_collect ||= {} end diff --git a/lib/solargraph/api_map/store.rb b/lib/solargraph/api_map/store.rb index 31835c993..ad0f64f20 100644 --- a/lib/solargraph/api_map/store.rb +++ b/lib/solargraph/api_map/store.rb @@ -317,9 +317,9 @@ def catalog pinsets, &block true end - # @return [Hash{::Array, String, String => ::Array}] + # @return [Hash{::Array(String, String) => ::Array}] def fqns_pins_map - # @param h [Hash{::Array, String, String => ::Array}] + # @param h [Hash{::Array(String, String) => ::Array}] # @param base [String] # @param name [String] @fqns_pins_map ||= Hash.new do |h, (base, name)| diff --git a/lib/solargraph/doc_map.rb b/lib/solargraph/doc_map.rb index 95f2dbdcc..4bfaf61fd 100644 --- a/lib/solargraph/doc_map.rb +++ b/lib/solargraph/doc_map.rb @@ -150,33 +150,33 @@ def unresolved_requires @unresolved_requires ||= required_gems_map.select { |_, gemspecs| gemspecs.nil? }.keys end - # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version + # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version def self.all_yard_gems_in_memory @all_yard_gems_in_memory ||= {} end - # @return [Hash{String => Hash{Array, String, String => Array}}] stored by RBS collection path + # @return [Hash{String => Hash{Array(String, String) => Array}}] stored by RBS collection path def self.all_rbs_collection_gems_in_memory @all_rbs_collection_gems_in_memory ||= {} end - # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version + # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version def yard_pins_in_memory self.class.all_yard_gems_in_memory end - # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version + # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version def rbs_collection_pins_in_memory self.class.all_rbs_collection_gems_in_memory[rbs_collection_path] ||= {} end - # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version + # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version def self.all_combined_pins_in_memory @all_combined_pins_in_memory ||= {} end # @todo this should also include an index by the hash of the RBS collection - # @return [Hash{Array, String, String => Array}] Indexed by gemspec name and version + # @return [Hash{Array(String, String) => Array}] Indexed by gemspec name and version def combined_pins_in_memory self.class.all_combined_pins_in_memory end diff --git a/lib/solargraph/source_map.rb b/lib/solargraph/source_map.rb index 5a80fd0af..224223282 100644 --- a/lib/solargraph/source_map.rb +++ b/lib/solargraph/source_map.rb @@ -195,7 +195,7 @@ def map source # @return [Array] attr_writer :convention_pins - # @return [Hash{Class => Array}] # rubocop:disable YARD/CollectionStyle + # @return [Hash{Class => Array}] def pin_class_hash # @todo Need to support generic resolution in classify and transform_values @pin_class_hash ||= pins.to_set.classify(&:class).transform_values(&:to_a) diff --git a/spec/source/chain_spec.rb b/spec/source/chain_spec.rb index 3cef41cf0..a6b29686e 100644 --- a/spec/source/chain_spec.rb +++ b/spec/source/chain_spec.rb @@ -1,25 +1,25 @@ describe Solargraph::Source::Chain do - it 'gets empty definitions for undefined links' do + it "gets empty definitions for undefined links" do chain = described_class.new([Solargraph::Source::Chain::Link.new]) expect(chain.define(nil, nil, [])).to be_empty end - it 'infers undefined types for undefined links' do + it "infers undefined types for undefined links" do chain = described_class.new([Solargraph::Source::Chain::Link.new]) expect(chain.infer(nil, nil, [])).to be_undefined end - it 'calls itself undefined if any of its links are undefined' do + it "calls itself undefined if any of its links are undefined" do chain = described_class.new([Solargraph::Source::Chain::Link.new]) expect(chain).to be_undefined end - it 'returns undefined bases for single links' do + it "returns undefined bases for single links" do chain = described_class.new([Solargraph::Source::Chain::Link.new]) expect(chain.base).to be_undefined end - it 'defines constants from core classes' do + it "defines constants from core classes" do api_map = Solargraph::ApiMap.new chain = described_class.new([Solargraph::Source::Chain::Constant.new('String')]) pins = chain.define(api_map, Solargraph::Pin::ROOT_PIN, []) @@ -27,7 +27,7 @@ expect(pins.first.path).to eq('String') end - it 'infers types from core classes' do + it "infers types from core classes" do api_map = Solargraph::ApiMap.new chain = described_class.new([Solargraph::Source::Chain::Constant.new('String')]) type = chain.infer(api_map, Solargraph::Pin::ROOT_PIN, []) @@ -35,26 +35,25 @@ expect(type.scope).to eq(:class) end - it 'infers types from core methods' do + it "infers types from core methods" do api_map = Solargraph::ApiMap.new - chain = described_class.new([Solargraph::Source::Chain::Constant.new('String'), - Solargraph::Source::Chain::Call.new('new', nil)]) + chain = described_class.new([Solargraph::Source::Chain::Constant.new('String'), Solargraph::Source::Chain::Call.new('new', nil)]) type = chain.infer(api_map, Solargraph::Pin::ROOT_PIN, []) expect(type.namespace).to eq('String') expect(type.scope).to eq(:instance) end - it 'recognizes literals' do + it "recognizes literals" do chain = described_class.new([Solargraph::Source::Chain::Literal.new('String', nil)]) expect(chain.literal?).to be(true) end - it 'recognizes constants' do + it "recognizes constants" do chain = described_class.new([Solargraph::Source::Chain::Constant.new('String')]) expect(chain.constant?).to be(true) end - it 'recognizes unfinished constants' do + it "recognizes unfinished constants" do chain = described_class.new([Solargraph::Source::Chain::Constant.new('String'), Solargraph::Source::Chain::Constant.new('')]) expect(chain.constant?).to be(true) expect(chain.base.constant?).to be(true) @@ -62,7 +61,7 @@ expect(chain.base.undefined?).to be(false) end - it 'infers types from new subclass calls without a subclass initialize method' do + it "infers types from new subclass calls without a subclass initialize method" do code = %( class Sup def initialize; end @@ -81,7 +80,7 @@ def meth; end expect(type.name).to eq('Sub') end - it 'follows constant chains' do + it "follows constant chains" do source = Solargraph::Source.load_string(%( module Mixin; end module Container @@ -96,7 +95,7 @@ class Foo; end expect(pins).to be_empty end - it 'rebases inner constants chains' do + it "rebases inner constants chains" do source = Solargraph::Source.load_string(%( class Foo class Bar; end @@ -106,12 +105,11 @@ class Bar; end api_map = Solargraph::ApiMap.new api_map.map source chain = Solargraph::Source::SourceChainer.chain(source, Solargraph::Position.new(3, 16)) - pins = chain.define(api_map, - Solargraph::Pin::ProxyType.new(closure: Solargraph::Pin::Namespace.new(name: 'Foo'), return_type: Solargraph::ComplexType.parse('Class')), []) + pins = chain.define(api_map, Solargraph::Pin::ProxyType.new(closure: Solargraph::Pin::Namespace.new(name: 'Foo'), return_type: Solargraph::ComplexType.parse('Class')), []) expect(pins.first.path).to eq('Foo::Bar') end - it 'resolves relative constant paths' do + it "resolves relative constant paths" do source = Solargraph::Source.load_string(%( class Foo class Bar @@ -125,12 +123,11 @@ module Other api_map = Solargraph::ApiMap.new api_map.map source chain = Solargraph::Source::SourceChainer.chain(source, Solargraph::Position.new(6, 16)) - pins = chain.define(api_map, - Solargraph::Pin::ProxyType.anonymous(Solargraph::ComplexType.parse('Class')), []) + pins = chain.define(api_map, Solargraph::Pin::ProxyType.anonymous(Solargraph::ComplexType.parse('Class')), []) expect(pins.first.path).to eq('Foo::Bar::Baz') end - it 'avoids recursive variable assignments' do + it "avoids recursive variable assignments" do source = Solargraph::Source.load_string(%( @foo = @bar @bar = @foo.quz @@ -138,12 +135,12 @@ module Other api_map = Solargraph::ApiMap.new api_map.map source chain = Solargraph::Source::SourceChainer.chain(source, Solargraph::Position.new(2, 18)) - expect do + expect { chain.define(api_map, Solargraph::Pin::ROOT_PIN, []) - end.not_to raise_error + }.not_to raise_error end - it 'pulls types from multiple lines of code' do + it "pulls types from multiple lines of code" do source = Solargraph::Source.load_string(%( 123 'abc' @@ -155,7 +152,7 @@ module Other expect(type.simple_tags).to eq('String') end - it 'uses last line of a begin expression as return type' do + it "uses last line of a begin expression as return type" do source = Solargraph::Source.load_string(%( begin 123 @@ -169,7 +166,7 @@ module Other expect(type.simple_tags).to eq('String') end - it 'matches constants on complete symbols' do + it "matches constants on complete symbols" do source = Solargraph::Source.load_string(%( class Correct; end class NotCorrect; end From d739226d8be343da87e45fa2580b941605fb2400 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 2 Aug 2026 21:48:28 -0400 Subject: [PATCH 9/9] Disable YARD/CollectionStyle; move all_params nil-safety fix out Two follow-ups to the earlier extraction: - The per-file YARD/CollectionStyle todo exclusion was a stand-in for actually fixing the cop. Replace it with disabling the cop outright in .rubocop.yml, matching castwide/solargraph#1237's fix: for nested-generic/tuple Hash key types, the cop's own long-style autocorrect produces syntax that doesn't preserve the original tuple's meaning, so there's no safe autocorrected form to converge on for those cases. - UniqueType#to_rbs's all_params.empty? nil-safety fix is an unrelated, pre-existing bug (from the original branch's own history, unrelated to parallel specs) - move it to apiology/solargraph#43. --- .rubocop.yml | 6 ++++++ .rubocop_todo.yml | 8 -------- lib/solargraph/complex_type/unique_type.rb | 7 +++---- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index f4463bd11..e74b4decf 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -92,6 +92,12 @@ Metrics/PerceivedComplexity: Max: 40 RSpec/ExampleLength: Max: 310 +# Autocorrect mangles short-style Hash tags with nested generics/parens +# (e.g. Hash{Array(String, Array) => String}) into invalid syntax. +# Confirmed broken through rubocop-yard 1.3.0 (latest as of this writing), +# against yard 0.9.45 (also latest as of this writing). +YARD/CollectionStyle: + Enabled: false plugins: - rubocop-rspec diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3779bcbee..6311b05d6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -417,14 +417,6 @@ Style/SuperArguments: - 'lib/solargraph/pin/callable.rb' - 'lib/solargraph/pin/method.rb' -# This cop supports safe autocorrection (--autocorrect). -YARD/CollectionStyle: - Exclude: - - 'lib/solargraph/api_map/constants.rb' - - 'lib/solargraph/api_map/store.rb' - - 'lib/solargraph/doc_map.rb' - - 'lib/solargraph/source_map.rb' - # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStylePrototypeName. # SupportedStylesPrototypeName: before, after diff --git a/lib/solargraph/complex_type/unique_type.rb b/lib/solargraph/complex_type/unique_type.rb index dc8d40ab3..4bbdda5b2 100644 --- a/lib/solargraph/complex_type/unique_type.rb +++ b/lib/solargraph/complex_type/unique_type.rb @@ -302,6 +302,7 @@ def desc rooted_tags end + # @sg-ignore Need better if/elseanalysis # @return [String] def to_rbs if duck_type? @@ -310,9 +311,8 @@ def to_rbs 'bool' elsif name.downcase == 'nil' 'nil' - elsif name == GENERIC_TAG_NAME && !all_params.empty? - # @sg-ignore flow sensitive typing should be able to handle !empty? narrowing first to non-nil - all_params.first.name + elsif name == GENERIC_TAG_NAME + all_params.first&.name elsif %w[Class Module].include?(name) rbs_name elsif %w[Tuple Array].include?(name) && fixed_parameters? @@ -555,7 +555,6 @@ def expand named_types # # @param api_map [ApiMap] The ApiMap that performs qualification # @param gates [Array] The namespaces from which to resolve names - # # @return [self, ComplexType, UniqueType] The generated ComplexType def qualify api_map, *gates transform do |t|