From 9ffb93e7d6e4deafe7ee127df082d64f033cdc90 Mon Sep 17 00:00:00 2001 From: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com> Date: Wed, 9 Jul 2025 12:06:46 +0100 Subject: [PATCH 1/8] Support Rspec.configure ... config.include --- .gitignore | 2 + lib/solargraph/rspec/convention.rb | 17 ++- lib/solargraph/rspec/spec_helper_include.rb | 118 ++++++++++++++++++ spec/solargraph/rspec/convention_spec.rb | 60 +++++++++ .../rspec/spec_helper_include_spec.rb | 18 +++ 5 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 lib/solargraph/rspec/spec_helper_include.rb create mode 100644 spec/solargraph/rspec/spec_helper_include_spec.rb diff --git a/.gitignore b/.gitignore index bcc8d5f..92ae71e 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,5 @@ # rspec failure tracking .rspec_status + +.solargraph.yml \ No newline at end of file diff --git a/lib/solargraph/rspec/convention.rb b/lib/solargraph/rspec/convention.rb index 7fb8c38..256d66b 100644 --- a/lib/solargraph/rspec/convention.rb +++ b/lib/solargraph/rspec/convention.rb @@ -10,6 +10,7 @@ require_relative 'correctors/subject_method_corrector' require_relative 'correctors/context_block_methods_corrector' require_relative 'correctors/dsl_methods_corrector' +require_relative 'spec_helper_include' require_relative 'test_helpers' require_relative 'pin_factory' @@ -120,6 +121,8 @@ def local(source_map) pins = [] # @type [Array] namespace_pins = [] + # @type [Array] + extra_requires = ['rspec'] rspec_walker = SpecWalker.new(source_map: source_map, config: config) @@ -133,14 +136,26 @@ def local(source_map) rspec_walker.walk! pins += namespace_pins + begin + pins += SpecHelperInclude.instance.pins + extra_requires += SpecHelperInclude.instance.extra_requires + rescue StandardError => e + Solargraph.logger.error("[solargraph-rspec] [spec helper] Can't add pins: #{e}") + [] + end if pins.any? Solargraph.logger.debug( "[RSpec] added #{pins.map(&:inspect)} to #{source_map.filename}" ) end + if extra_requires.any? + Solargraph.logger.debug( + "[RSpec] added requires #{extra_requires} to #{source_map.filename}" + ) + end - Environ.new(requires: [], pins: pins) + Environ.new(requires: extra_requires, pins: pins) rescue StandardError, SyntaxError => e raise e if ENV['SOLARGRAPH_DEBUG'] diff --git a/lib/solargraph/rspec/spec_helper_include.rb b/lib/solargraph/rspec/spec_helper_include.rb new file mode 100644 index 0000000..00509d3 --- /dev/null +++ b/lib/solargraph/rspec/spec_helper_include.rb @@ -0,0 +1,118 @@ +module Solargraph + module Rspec + # RSpec.configure ... config.include handler, essentially + class SpecHelperInclude + COMMON_HELPER_FILES = [ + 'spec/spec_helper.rb', + 'spec/rails_helper.rb' + ] + + # @param node [::Parser::AST::Node] + # @param file [String] The name of the file this is module is defined in + # @param module_name [String] The name of the module to be included + INCLUDED_MODULE_DATA = Struct.new(:node, :file, :module_name) + + def self.instance + @instance ||= new + end + + def self.reset + @instance = nil + end + + # @return [Array] + def pins + ns = Solargraph::Pin::Namespace.new( + name: "RSpec::ExampleGroups", + ) + ns2 = Solargraph::Pin::Namespace.new( + name: "RSpec::Example", + ) + + included_modules.flat_map do |m| + [ + Solargraph::Pin::Reference::Include.new( + closure: ns, + name: m.module_name, + location: Solargraph::Location.new(m.file, Solargraph::Parser.node_range(m.node)) + ), + Solargraph::Pin::Reference::Include.new( + closure: ns2, + name: m.module_name, + location: Solargraph::Location.new(m.file, Solargraph::Parser.node_range(m.node)) + ), + ] + end + end + + def extra_requires + included_modules.map(&:file).uniq + Dir["spec/support/**/*.rb"] + end + + # @return [Array] + def included_modules + @included_modules ||= parse_included_modules + end + + private + + # @return [Array] + def parse_included_modules + modules = [] + + COMMON_HELPER_FILES.each do |f| + ast = Solargraph::Parser.parse(File.read(f), f) + modules += extract_included_modules(ast, f) + rescue Errno::ENOENT + # Ignore this error - no file means we can chill + rescue StandardError => e + Solargraph.logger.error("[solargraph-rspec] [spec helper] Can't read helper file '#{f}': #{e}") + end + + modules + end + + # Parses the modules that were included int he Rspec.configure (in common helper files) + # @param ast [Parser::AST::Node] + # @param file [String] + # + # @return [Array] + def extract_included_modules(ast, file) + walker = Walker.new(ast) + + # @type [Array] + included_modules = [] + + walker.on :block, [:send] do |node| + send_node = node.children[0] + send_receiver = send_node.children[0] + + next if send_receiver.type != :const || send_receiver.children[2] == :Rspec + next unless send_node.children[1] == :configure + # No args + next if node.children[1].children.empty? + + config_name = node.children[1].children[0].children[0] + config_walker = Walker.new(node) + config_walker.on :send, [:lvar, config_name] do |include_node| + next unless include_node.children[1] == :include + + mod_node = include_node.children[2] + next unless mod_node.is_a? ::Parser::AST::Node + next unless mod_node.type == :const + + included_modules << INCLUDED_MODULE_DATA.new( + include_node, file, SpecWalker::FullConstantName.from_ast(mod_node), + ) + end + + config_walker.walk + end + + walker.walk + + included_modules + end + end + end +end diff --git a/spec/solargraph/rspec/convention_spec.rb b/spec/solargraph/rspec/convention_spec.rb index c81090c..1dd562b 100644 --- a/spec/solargraph/rspec/convention_spec.rb +++ b/spec/solargraph/rspec/convention_spec.rb @@ -1149,4 +1149,64 @@ class MyClass; end end end end + + describe 'included modules' do + require 'parser' + + before do + allow(Solargraph::Parser).to receive(:node_range).and_return(Solargraph::Range.from_to(0, 0, 0, 1)) + allow_any_instance_of(Solargraph::Rspec::SpecHelperInclude).to receive(:parse_included_modules).and_return([ + Solargraph::Rspec::SpecHelperInclude::INCLUDED_MODULE_DATA.new( + ::Parser::AST::Node.new(:send), 'spec_helper.rb', 'HelperModule' + ) + ]) + + load_string 'spec_helper.rb', <<~RUBY + module HelperModule + def module_method + end + end + RUBY + + load_string filename, <<~RUBY + RSpec.describe SomeNamespace::Transaction, type: :model do + it 'example test' do + mo + end + + describe 'example group' do + let(:var) { mo } + + before do + mo + end + + it 'example test' do + mo + end + end + end + RUBY + end + + it 'should complete inside a top level example' do + expect(completion_at(filename, [2, 6])).to include('module_method') + end + + it 'should complete inside a let block' do + expect(completion_at(filename, [6, 18])).to include('module_method') + end + + it 'should complete inside a context block' do + expect(completion_at(filename, [8, 6])).to include('module_method') + end + + it 'should complete inside a hook' do + expect(completion_at(filename, [11, 8])).to include('module_method') + end + + it 'should complete a nested example' do + expect(completion_at(filename, [15, 8])).to include('module_method') + end + end end diff --git a/spec/solargraph/rspec/spec_helper_include_spec.rb b/spec/solargraph/rspec/spec_helper_include_spec.rb new file mode 100644 index 0000000..4f159f7 --- /dev/null +++ b/spec/solargraph/rspec/spec_helper_include_spec.rb @@ -0,0 +1,18 @@ +RSpec.describe Solargraph::Rspec::SpecHelperInclude do + describe '#extract_included_modules' do + it 'should pull included modules' do + ast = Solargraph::Parser.parse(%( + Rspec.configure do |config| + config.include ModuleName + config.example OtherVal + config.include(SubMod::Module) + end + )) + + # @type [Array] + modules = Solargraph::Rspec::SpecHelperInclude.instance.send(:extract_included_modules, ast, 'spec_helper.rb') + + expect(modules.map(&:module_name)).to eql(%w[ModuleName SubMod::Module]) + end + end +end From f4faacefd49eef147f995b08179a236fc1f7e506 Mon Sep 17 00:00:00 2001 From: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com> Date: Thu, 10 Jul 2025 03:04:27 +0100 Subject: [PATCH 2/8] Make tests pass --- lib/solargraph/rspec/spec_helper_include.rb | 22 +++++------ spec/solargraph/rspec/convention_spec.rb | 37 ++++++++++++++----- .../rspec/spec_helper_include_spec.rb | 2 + spec/support/solargraph_helpers.rb | 13 ++++++- 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/lib/solargraph/rspec/spec_helper_include.rb b/lib/solargraph/rspec/spec_helper_include.rb index 00509d3..c5690c9 100644 --- a/lib/solargraph/rspec/spec_helper_include.rb +++ b/lib/solargraph/rspec/spec_helper_include.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Solargraph module Rspec # RSpec.configure ... config.include handler, essentially @@ -5,8 +7,8 @@ class SpecHelperInclude COMMON_HELPER_FILES = [ 'spec/spec_helper.rb', 'spec/rails_helper.rb' - ] - + ].freeze + # @param node [::Parser::AST::Node] # @param file [String] The name of the file this is module is defined in # @param module_name [String] The name of the module to be included @@ -22,12 +24,8 @@ def self.reset # @return [Array] def pins - ns = Solargraph::Pin::Namespace.new( - name: "RSpec::ExampleGroups", - ) - ns2 = Solargraph::Pin::Namespace.new( - name: "RSpec::Example", - ) + ns = Solargraph::Pin::Namespace.new(name: 'RSpec::ExampleGroups') + ns2 = Solargraph::Pin::Namespace.new(name: 'RSpec::Example') included_modules.flat_map do |m| [ @@ -40,15 +38,15 @@ def pins closure: ns2, name: m.module_name, location: Solargraph::Location.new(m.file, Solargraph::Parser.node_range(m.node)) - ), + ) ] end end def extra_requires - included_modules.map(&:file).uniq + Dir["spec/support/**/*.rb"] + included_modules.map(&:file).uniq + Dir['spec/support/**/*.rb'] end - + # @return [Array] def included_modules @included_modules ||= parse_included_modules @@ -102,7 +100,7 @@ def extract_included_modules(ast, file) next unless mod_node.type == :const included_modules << INCLUDED_MODULE_DATA.new( - include_node, file, SpecWalker::FullConstantName.from_ast(mod_node), + include_node, file, SpecWalker::FullConstantName.from_ast(mod_node) ) end diff --git a/spec/solargraph/rspec/convention_spec.rb b/spec/solargraph/rspec/convention_spec.rb index 1dd562b..74ff7da 100644 --- a/spec/solargraph/rspec/convention_spec.rb +++ b/spec/solargraph/rspec/convention_spec.rb @@ -1154,29 +1154,44 @@ class MyClass; end require 'parser' before do - allow(Solargraph::Parser).to receive(:node_range).and_return(Solargraph::Range.from_to(0, 0, 0, 1)) - allow_any_instance_of(Solargraph::Rspec::SpecHelperInclude).to receive(:parse_included_modules).and_return([ - Solargraph::Rspec::SpecHelperInclude::INCLUDED_MODULE_DATA.new( - ::Parser::AST::Node.new(:send), 'spec_helper.rb', 'HelperModule' - ) - ]) - - load_string 'spec_helper.rb', <<~RUBY + Solargraph::Rspec::SpecHelperInclude.reset + + allow_any_instance_of(Solargraph::Rspec::SpecHelperInclude).to receive(:parse_included_modules).and_return( + [ + Solargraph::Rspec::SpecHelperInclude::INCLUDED_MODULE_DATA.new( + # What the fuck + Parser::AST::Node.new( + :send, [], { + location: Parser::Source::Map.new( + Parser::Source::Range.new( + Parser::Source::Buffer.new('name.rb', source: '1'), + 0, 1 + ) + ) + } + ), 'spec_helper.rb', 'HelperModule' + ) + ] + ) + + source_helper = parse_string File.expand_path('spec/spec_helper.rb'), <<~RUBY module HelperModule def module_method end end RUBY - load_string filename, <<~RUBY + source_main = parse_string filename, <<~RUBY RSpec.describe SomeNamespace::Transaction, type: :model do it 'example test' do mo end - describe 'example group' do + describe 'fake example group' do let(:var) { mo } + mo + before do mo end @@ -1187,6 +1202,8 @@ def module_method end end RUBY + + load_sources(source_helper, source_main) end it 'should complete inside a top level example' do diff --git a/spec/solargraph/rspec/spec_helper_include_spec.rb b/spec/solargraph/rspec/spec_helper_include_spec.rb index 4f159f7..5c20281 100644 --- a/spec/solargraph/rspec/spec_helper_include_spec.rb +++ b/spec/solargraph/rspec/spec_helper_include_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + RSpec.describe Solargraph::Rspec::SpecHelperInclude do describe '#extract_included_modules' do it 'should pull included modules' do diff --git a/spec/support/solargraph_helpers.rb b/spec/support/solargraph_helpers.rb index 097d485..52a09d4 100644 --- a/spec/support/solargraph_helpers.rb +++ b/spec/support/solargraph_helpers.rb @@ -2,11 +2,22 @@ module SolargraphHelpers def load_string(filename, str) - source = Solargraph::Source.load_string(str, filename) + source = parse_string(filename, str) api_map.map(source) # api_map should be defined in the spec source end + # Util method to parse (but NOT load) a string + # This method is mostly here for heredocs + # + # @param filename [String] + # @param str [String] The source code + # + # @return [Solargraph::Source] + def parse_string(filename, str) + Solargraph::Source.load_string(str, filename) + end + def load_sources(*sources) source_maps = sources.map { |s| Solargraph::SourceMap.map(s) } bench = Solargraph::Bench.new(source_maps: source_maps) From 66d55fd7e7a450a3acd9779a915a3e437a212342 Mon Sep 17 00:00:00 2001 From: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com> Date: Thu, 10 Jul 2025 15:17:24 +0100 Subject: [PATCH 3/8] Require solargraph 56 (remote ci test) --- gemfiles/default.gemfile.lock | 39 ++++++++++++++++++++--------------- solargraph-rspec.gemspec | 2 +- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/gemfiles/default.gemfile.lock b/gemfiles/default.gemfile.lock index dc35419..74be383 100644 --- a/gemfiles/default.gemfile.lock +++ b/gemfiles/default.gemfile.lock @@ -1,8 +1,8 @@ PATH remote: .. specs: - solargraph-rspec (0.4.1) - solargraph (~> 0.52, >= 0.52.0) + solargraph-rspec (0.5.2) + solargraph (~> 0.56, >= 0.56.0) GEM remote: https://rubygems.org/ @@ -79,7 +79,7 @@ GEM debug (1.10.0) irb (~> 1.10) reline (>= 0.3.8) - diff-lcs (1.6.0) + diff-lcs (1.6.2) docile (1.4.1) domain_name (0.6.20240107) drb (2.2.1) @@ -97,7 +97,7 @@ GEM pp (>= 0.6.0) rdoc (>= 4.0.0) reline (>= 0.4.2) - jaro_winkler (1.6.0) + jaro_winkler (1.6.1) json (2.10.2) kramdown (2.5.1) rexml (>= 3.3.9) @@ -134,9 +134,11 @@ GEM netrc (0.11.0) nokogiri (1.17.2-arm64-darwin) racc (~> 1.4) + nokogiri (1.17.2-x86_64-linux) + racc (~> 1.4) observer (0.1.2) optparse (0.6.0) - ostruct (0.6.1) + ostruct (0.6.2) parallel (1.26.3) parser (3.3.7.2) ast (~> 2.4.1) @@ -144,6 +146,7 @@ GEM pp (0.6.2) prettyprint prettyprint (0.2.0) + prism (1.4.0) profile-viewer (0.0.4) optparse webrick @@ -183,7 +186,7 @@ GEM zeitwerk (~> 2.6) rainbow (3.1.1) rake (13.2.1) - rbs (3.6.1) + rbs (3.9.4) logger rdoc (6.12.0) psych (>= 4.0.0) @@ -200,16 +203,16 @@ GEM reverse_markdown (3.0.0) nokogiri rexml (3.4.1) - rspec (3.13.0) + rspec (3.13.1) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.3) + rspec-core (3.13.5) rspec-support (~> 3.13.0) - rspec-expectations (3.13.3) + rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) - rspec-mocks (3.13.2) + rspec-mocks (3.13.5) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-rails (7.1.1) @@ -225,7 +228,7 @@ GEM rspec-expectations (~> 3.0) rspec-mocks (~> 3.0) sidekiq (>= 5, < 9) - rspec-support (3.13.2) + rspec-support (3.13.4) rubocop (1.74.0) json (~> 2.3) language_server-protocol (~> 3.17.0.2) @@ -258,20 +261,21 @@ GEM simplecov (~> 0.19) simplecov-html (0.13.1) simplecov_json_formatter (0.1.4) - solargraph (0.52.0) + solargraph (0.56.0) backport (~> 1.2) - benchmark + benchmark (~> 0.4) bundler (~> 2.0) diff-lcs (~> 1.4) - jaro_winkler (~> 1.6) + jaro_winkler (~> 1.6, >= 1.6.1) kramdown (~> 2.3) kramdown-parser-gfm (~> 1.1) logger (~> 1.6) observer (~> 0.1) ostruct (~> 0.6) parser (~> 3.0) - rbs (~> 3.0) - reverse_markdown (>= 2.0, < 4) + prism (~> 1.4) + rbs (~> 3.3) + reverse_markdown (~> 3.0) rubocop (~> 1.38) thor (~> 1.0) tilt (~> 2.0) @@ -279,7 +283,7 @@ GEM yard-solargraph (~> 0.1) stringio (3.1.5) thor (1.3.2) - tilt (2.6.0) + tilt (2.6.1) timeout (0.4.3) tzinfo (2.0.6) concurrent-ruby (~> 1.0) @@ -298,6 +302,7 @@ GEM PLATFORMS arm64-darwin-24 + x86_64-linux DEPENDENCIES actionmailer diff --git a/solargraph-rspec.gemspec b/solargraph-rspec.gemspec index ebf6109..23301a0 100644 --- a/solargraph-rspec.gemspec +++ b/solargraph-rspec.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{\Aexe/}) { |f| File.basename(f) } spec.require_paths = ['lib'] - spec.add_dependency 'solargraph', '~> 0.52', '>= 0.52.0' + spec.add_dependency 'solargraph', '~> 0.56', '>= 0.56.0' # For more information and examples about making a new gem, check out our # guide at: https://bundler.io/guides/creating_gem.html From 7c817e79e8e315ca335f337f936252db9c23c8db Mon Sep 17 00:00:00 2001 From: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com> Date: Sun, 13 Jul 2025 13:09:56 +0100 Subject: [PATCH 4/8] Fix testing --- .github/workflows/ruby.yml | 36 ++++++++++-------- .gitignore | 4 +- gemfiles/.bundle/config | 1 + gemfiles/default.gemfile.lock | 15 ++++---- spec/solargraph/rspec/convention_spec.rb | 47 ++++++++++++------------ 5 files changed, 55 insertions(+), 48 deletions(-) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index c46c106..8ed2b8b 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -15,6 +15,10 @@ on: permissions: contents: read +env: + SOLARGRAPH_CACHE: "${{ github.workspace }}/solargraph-rspec/gem-cache" + BUNDLE_GEMFILE: "${{ github.workspace }}/solargraph-rspec/gemfiles/default.gemfile" + jobs: test: name: Tests (ruby v${{ matrix.ruby-version }}) @@ -25,7 +29,6 @@ jobs: # FIXME: Why '3.0' is not working with Appraisal? # FIXME: Add 'head' https://github.com/lekemula/solargraph-rspec/pull/8/commits/3b52752b96e7f2ec01831406f8e5a51c91523187 ruby-version: ['3.1', '3.2', '3.3'] - steps: - name: Checkout solargraph-rspec uses: actions/checkout@v4 @@ -42,32 +45,33 @@ jobs: - name: Cache Ruby gems uses: actions/cache@v3 with: - path: solargraph-rspec/vendor/bundle - key: bundle-use-ruby-${{ matrix.os }}-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/solargraph-rspec.gemspec') }} - restore-keys: | - bundle-use-ruby-${{ matrix.os }}-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/solargraph-rspec.gemspec') }} + path: solargraph-rspec/gemfiles/vendor/bundle + key: bundle-use-ruby-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/Gemfile', 'solargraph-rspec/gemfiles/default.gemfile.lock') }} - name: Install dependencies run: | cd solargraph-rspec bundle config path vendor/bundle bundle install --jobs 4 --retry 3 - bundle exec appraisal install - - name: Run Rubocop - run: cd solargraph-rspec && bundle exec rubocop + # - name: Run Rubocop + # run: cd solargraph-rspec && bundle exec rubocop - - name: Set up yardocs - # yard gems caches the yardocs into /doc/.yardoc path, hence they should be cached by ruby gems cache - run: cd solargraph-rspec && bundle exec appraisal yard gems --verbose + - name: 'Apply solargraph debug patch' + run: |- + echo 'exclude: ["vendor/**/*", "gemfiles/**/*"]' > solargraph-rspec/.solargraph.yml + cd solargraph-rspec/gemfiles/vendor/bundle/ruby/${{ matrix.ruby-version }}.0/gems/solargraph-0.56.0 - - name: List all Yardoc constants and methods - run: | - cd solargraph-rspec - bundle exec yard list + awk '/Caching yardoc for/ { print "return nil unless Dir.exist?(gemspec.gem_dir)"; print; next }1' lib/solargraph/yardoc.rb > tmp.rb + mv tmp.rb lib/solargraph/yardoc.rb + + - name: Cache Docs + run: |- + cd solargraph-rspec + bundle exec solargraph gems --rebuild - name: Run tests - run: cd solargraph-rspec && bundle exec appraisal rspec --format progress + run: cd solargraph-rspec && bundle exec rspec --format progress - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v4 diff --git a/.gitignore b/.gitignore index 92ae71e..79e5a26 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,6 @@ # rspec failure tracking .rspec_status -.solargraph.yml \ No newline at end of file +.solargraph.yml +vendor +gemfiles/vendor diff --git a/gemfiles/.bundle/config b/gemfiles/.bundle/config index c127f80..b8c1796 100644 --- a/gemfiles/.bundle/config +++ b/gemfiles/.bundle/config @@ -1,2 +1,3 @@ --- BUNDLE_RETRY: "1" +BUNDLE_PATH: "vendor/bundle" diff --git a/gemfiles/default.gemfile.lock b/gemfiles/default.gemfile.lock index 74be383..a4a89b5 100644 --- a/gemfiles/default.gemfile.lock +++ b/gemfiles/default.gemfile.lock @@ -98,12 +98,12 @@ GEM rdoc (>= 4.0.0) reline (>= 0.4.2) jaro_winkler (1.6.1) - json (2.10.2) + json (2.12.2) kramdown (2.5.1) rexml (>= 3.3.9) kramdown-parser-gfm (1.1.0) kramdown (~> 2.0) - language_server-protocol (3.17.0.4) + language_server-protocol (3.17.0.5) lint_roller (1.1.0) logger (1.6.6) loofah (2.24.0) @@ -139,8 +139,8 @@ GEM observer (0.1.2) optparse (0.6.0) ostruct (0.6.2) - parallel (1.26.3) - parser (3.3.7.2) + parallel (1.27.0) + parser (3.3.8.0) ast (~> 2.4.1) racc pp (0.6.2) @@ -229,7 +229,7 @@ GEM rspec-mocks (~> 3.0) sidekiq (>= 5, < 9) rspec-support (3.13.4) - rubocop (1.74.0) + rubocop (1.78.0) json (~> 2.3) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.1.0) @@ -237,11 +237,12 @@ GEM parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 2.9.3, < 3.0) - rubocop-ast (>= 1.38.0, < 2.0) + rubocop-ast (>= 1.45.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.41.0) + rubocop-ast (1.45.1) parser (>= 3.3.7.2) + prism (~> 1.4) ruby-progressbar (1.13.0) securerandom (0.3.2) shoulda-matchers (6.4.0) diff --git a/spec/solargraph/rspec/convention_spec.rb b/spec/solargraph/rspec/convention_spec.rb index 74ff7da..a73de93 100644 --- a/spec/solargraph/rspec/convention_spec.rb +++ b/spec/solargraph/rspec/convention_spec.rb @@ -252,7 +252,6 @@ # NOTE: This spec depends on RSpec's YARDoc comments, if it fails try running: yard gems it 'completes RSpec::Matchers methods' do - pending('https://github.com/castwide/solargraph/pull/877') load_string filename, <<~RUBY RSpec.describe SomeNamespace::Transaction, type: :model do context 'some context' do @@ -324,7 +323,6 @@ def self.my_class_method end it 'completes RSpec DSL methods' do - pending('https://github.com/castwide/solargraph/pull/877') load_string filename, <<~RUBY RSpec.describe SomeNamespace::Transaction, type: :model do desc @@ -732,6 +730,29 @@ class MyClass; end end end + # TODO: Move back to helpers context method + describe 'rspec-mocks' do + it 'completes methods from rspec-mocks' do + load_string filename, <<~RUBY + RSpec.describe SomeNamespace::Transaction, type: :model do + let(:something) { double } + + it 'should do something' do + allow(something).to rec + allow(double).to receive_me + my_double = doub + my_double = inst + end + end + RUBY + + expect(completion_at(filename, [4, 26])).to include('receive') + expect(completion_at(filename, [5, 30])).to include('receive_message_chain') + expect(completion_at(filename, [6, 18])).to include('double') + expect(completion_at(filename, [7, 18])).to include('instance_double') + end + end + describe 'helpers' do before { pending('https://github.com/castwide/solargraph/pull/877') } @@ -849,28 +870,6 @@ class MyClass; end end end - describe 'rspec-mocks' do - it 'completes methods from rspec-mocks' do - load_string filename, <<~RUBY - RSpec.describe SomeNamespace::Transaction, type: :model do - let(:something) { double } - - it 'should do something' do - allow(something).to rec - allow(double).to receive_me - my_double = doub - my_double = inst - end - end - RUBY - - expect(completion_at(filename, [4, 26])).to include('receive') - expect(completion_at(filename, [5, 30])).to include('receive_message_chain') - expect(completion_at(filename, [6, 18])).to include('double') - expect(completion_at(filename, [7, 18])).to include('instance_double') - end - end - describe 'rspec-rails' do # A model spec is a thin wrapper for an ActiveSupport::TestCase # See: https://api.rubyonrails.org/v5.2.8.1/classes/ActiveSupport/Testing/Assertions.html From e5853bcf222978853d1da0aa63971a9289b04fcb Mon Sep 17 00:00:00 2001 From: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com> Date: Sun, 13 Jul 2025 14:12:24 +0100 Subject: [PATCH 5/8] WIP: FactoryBot/Girl --- lib/solargraph/rspec/convention.rb | 7 ++ lib/solargraph/rspec/factory_bot.rb | 137 ++++++++++++++++++++++ spec/solargraph/rspec/factory_bot_spec.rb | 89 ++++++++++++++ 3 files changed, 233 insertions(+) create mode 100644 lib/solargraph/rspec/factory_bot.rb create mode 100644 spec/solargraph/rspec/factory_bot_spec.rb diff --git a/lib/solargraph/rspec/convention.rb b/lib/solargraph/rspec/convention.rb index 256d66b..797148c 100644 --- a/lib/solargraph/rspec/convention.rb +++ b/lib/solargraph/rspec/convention.rb @@ -10,6 +10,7 @@ require_relative 'correctors/subject_method_corrector' require_relative 'correctors/context_block_methods_corrector' require_relative 'correctors/dsl_methods_corrector' +require_relative 'factory_bot' require_relative 'spec_helper_include' require_relative 'test_helpers' require_relative 'pin_factory' @@ -136,6 +137,12 @@ def local(source_map) rspec_walker.walk! pins += namespace_pins + pins += begin + FactoryBot.instance.pins + rescue StandardError => e + Solargraph.logger.error("[solargraph-rspec] [factory bot] Can't add pins: #{e}") + [] + end begin pins += SpecHelperInclude.instance.pins extra_requires += SpecHelperInclude.instance.extra_requires diff --git a/lib/solargraph/rspec/factory_bot.rb b/lib/solargraph/rspec/factory_bot.rb new file mode 100644 index 0000000..b3b5949 --- /dev/null +++ b/lib/solargraph/rspec/factory_bot.rb @@ -0,0 +1,137 @@ +module Solargraph + module Rspec + class FactoryBot + FACTORY_LOCATIONS = [ + 'factories.rb', + 'factories/**/*.rb', + 'test/factories.rb', + 'test/factories/**/*.rb', + 'spec/factories.rb', + 'spec/factories/**/*.rb' + ].freeze + + FactoryData = Struct.new( + :factory_names, + :model_class, + :traits, + :kwargs, + keyword_init: true + ) + + def self.instance + @instance ||= new + end + + def self.reset + @instance = nil + end + + def pins + [ + method_builder('create', :instance), + method_builder('fake_create', :instance), + method_builder('create', :class), + method_builder('fake_create', :class), + ] + end + + private + + def method_builder(name, scope) + method = Solargraph::Pin::Method.new( + name: name, + scope: scope, + closure: Solargraph::Pin::Namespace.new( + name: 'FactoryGirl::Syntax::Methods', + location: PinFactory.dummy_location('factories.rb') + ) + ) + + method.signatures = factories.map do |d| + Solargraph::Pin::Signature.new( + return_type: Solargraph::ComplexType.parse(d.model_class), + closure: method, + parameters: [ + Solargraph::Pin::Parameter.new( + name: 'name', + return_type: Solargraph::ComplexType.parse(*d.factory_names.map { |n| ":#{n}" }), + closure: method + ) + ] + ) + end + + method + end + + # @return [Array] + def factories + @factories ||= parse_factories + end + + def parse_factories + # @type [Array] + nodes = [] + + FACTORY_LOCATIONS.each do |pattern| + Dir.glob(pattern).each do |file| + nodes << Solargraph::Parser.parse(File.read(file), file) + rescue StandardError + Solargraph.logger.error("[solargraph-rspec] [factory bot] Can't read file #{file}") + end + end + + return [] if nodes.empty? + + extract_factories_from_ast(nodes) + end + + # @param ast [Parser::AST::Node] + def extract_factories_from_ast(ast) + walker = Walker.new(ast) + # @type [Array] + factories = [] + + walker.on :block, [:send, nil, :factory] do |ast| + factory_cfg = ast.children.first.children + next if factory_cfg.length < 3 + next unless factory_cfg[2].type == :sym + + # @type [Array] + factory_names = [factory_cfg[2].children[0]] + model_class = factory_names[0].to_s.split('_').collect(&:capitalize).join + + if factory_cfg.length > 3 && factory_cfg[3].type == :hash + factory_cfg[3].children.each do |pair| + case pair.children[0].children[0] + when :aliases + if pair.children[1].type == :array + pair.children[1].children.each do |n| + factory_names << n.children[0] if n.type == :sym + end + end + when :class + if pair.children[1].type == :str + model_class = pair.children[1].children[0] + elsif pair.children[1].type == :const + model_class = pair.children[1].children[1].to_s + end + end + end + end + + factories << FactoryData.new( + factory_names: factory_names, + model_class: model_class + # traits: , + # kwargs: , + ) + end + + walker.walk + + factories + end + end + end +end \ No newline at end of file diff --git a/spec/solargraph/rspec/factory_bot_spec.rb b/spec/solargraph/rspec/factory_bot_spec.rb new file mode 100644 index 0000000..462d1da --- /dev/null +++ b/spec/solargraph/rspec/factory_bot_spec.rb @@ -0,0 +1,89 @@ +RSpec.describe Solargraph::Rspec::FactoryBot do + describe '#extract_factories_from_ast' do + let(:ast) do + Solargraph::Parser.parse( + <<~RUBY + FactoryGirl.define do + # @param factory_arg [String] + # @param trans_three [Boolean] Some comment + # @param added_attr [Boolean] Comment :3 + factory :person, aliases: [:alias_name_here], class: Abc do + # @return [String] Example content + first_name 'John' + factory_arg 'Doe' + # @return [String] Example content + example_value { } + + sequence(:email) { |n| "email\#{n}@example.com" } + + add_attribute(:example) { 'Value' } + add_attribute(:added_attr) { true } + + association :parent, factory: :some_table + association :some_table + + to_create { } + after(:something) {} + before(:something) {} + callback(:something) {} + + transient do + # @return [String] + trans_one { 'value' } + # @return [Number] Comment line 1 + # Comment line 2 + trans_two 1 + trans_three true + end + + # Example comment + trait :some_trait do + after(:create) do + end + end + end + + # Some comment + factory :some_table do + add_attribute(:to_create) { 'string' } + end + end + RUBY + ) + end + let(:extracted) do + Solargraph::Rspec::FactoryBot.instance.send(:extract_factories_from_ast, ast) + end + + it 'interprets class from class: arg' do + + end + + it 'interprets class from factory name if theres no class: arg' do + end + + it 'parses aliases' do + end + + it 'gets all regular kw args' do + end + + it 'gets transient kw args' do + end + + it 'gets all the traits' do + end + + it 'ignores callback functions' do + end + + it 'parses add_attribute' do + end + + it 'parses associations' do + end + + it 'parses sequence' do + end + end +end From bce3518958670fd990450eea20502fd3f947b0dc Mon Sep 17 00:00:00 2001 From: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com> Date: Mon, 14 Jul 2025 12:45:20 +0100 Subject: [PATCH 6/8] Parse factory in a good way --- lib/solargraph/rspec/factory_bot.rb | 203 ++++++++++++++---- spec/solargraph/rspec/factory_bot_spec.rb | 244 ++++++++++++++++------ 2 files changed, 352 insertions(+), 95 deletions(-) diff --git a/lib/solargraph/rspec/factory_bot.rb b/lib/solargraph/rspec/factory_bot.rb index b3b5949..ea61f3e 100644 --- a/lib/solargraph/rspec/factory_bot.rb +++ b/lib/solargraph/rspec/factory_bot.rb @@ -10,12 +10,25 @@ class FactoryBot 'spec/factories/**/*.rb' ].freeze - FactoryData = Struct.new( - :factory_names, - :model_class, - :traits, - :kwargs, - keyword_init: true + ALWAYS_IGNORE = %i[after before callbacks to_create].freeze + SPECIAL_CALLBACKS = %i[add_attribute sequence association trait].freeze + + # @param factory_names [Array] Names & aliases. The first name is the "official" factory name + # @param model_class [String] The class that this factory should uses + # @param traits [Array] A list of trait names + # @param kwargs [Array] Any available kwargs + # @param docs [YARD::Docstring, nil] The parsed docs + FactoryData = Struct.new(:factory_names, :model_class, :traits, :kwargs, :docs, keyword_init: true) + + UnresolvedAssociation = Struct.new( + # @return [Symbol] The column name + :column, + # @return [Symbol] The factory from which this is being made + :source_factory, + # @return [Symbol] The factory to which this association associates to + :target_factory, + # @return [::Parser::AST::Node] The node that does the association + :node ) def self.instance @@ -31,12 +44,47 @@ def pins method_builder('create', :instance), method_builder('fake_create', :instance), method_builder('create', :class), - method_builder('fake_create', :class), + method_builder('fake_create', :class) ] end private + # @param factory [FactoryData] + # @param method [Solargraph::Pin::Method] + def signature_for_factory(factory, method) + sig = Solargraph::Pin::Signature.new( + return_type: Solargraph::ComplexType.parse(factory.model_class), + closure: method, + docstring: factory.docs, + parameters: [] + ) + + sig.parameters << Solargraph::Pin::Parameter.new( + name: 'name', + return_type: Solargraph::ComplexType.parse(*factory.factory_names.map { |n| ":#{n}" }), + closure: sig + ) + + unless factory.traits.empty? + sig.parameters << Solargraph::Pin::Parameter.new( + name: 'traits', + return_type: Solargraph::ComplexType.parse(*factory.traits.map { |n| ":#{n}" }), + closure: sig, + decl: :restarg + ) + end + sig.parameters += factory.kwargs.map do |n| + Solargraph::Pin::Parameter.new( + name: n.to_s, + closure: sig, + decl: :kwoptarg, + ) + end + + sig + end + def method_builder(name, scope) method = Solargraph::Pin::Method.new( name: name, @@ -47,19 +95,7 @@ def method_builder(name, scope) ) ) - method.signatures = factories.map do |d| - Solargraph::Pin::Signature.new( - return_type: Solargraph::ComplexType.parse(d.model_class), - closure: method, - parameters: [ - Solargraph::Pin::Parameter.new( - name: 'name', - return_type: Solargraph::ComplexType.parse(*d.factory_names.map { |n| ":#{n}" }), - closure: method - ) - ] - ) - end + method.signatures = factories.map { |f| signature_for_factory(f, method) } method end @@ -70,27 +106,52 @@ def factories end def parse_factories - # @type [Array] - nodes = [] + # @type [Array] + factories = [] + # @type [Array)>] + associations = [] FACTORY_LOCATIONS.each do |pattern| Dir.glob(pattern).each do |file| - nodes << Solargraph::Parser.parse(File.read(file), file) - rescue StandardError - Solargraph.logger.error("[solargraph-rspec] [factory bot] Can't read file #{file}") + src = Solargraph::Source.load_string(File.read(file), file) + out = extract_factories_from_source(src) + factories += out[0] + associations << [src, out[1]] unless out[1].empty? + rescue StandardError => e + Solargraph.logger.error("[solargraph-rspec] [factory bot] Can't read file #{file}: #{e}") end end - return [] if nodes.empty? + associations.each do |cfg| + cfg[1].each do |ass| + target = factories.find { |f| f.factory_names.include? ass.target_factory } + source = factories.find { |f| f.factory_names.first == ass.source_factory } + if target.nil? + # If we can't find target factory - either its bad indexing or truly undefined + # We should lean on the more tolerant side & give a warning in logs & just accept whatever comments say + # If no comments are present, then too bad ig + Solargraph.logger.warn("[solargraph-rspec] [factory bot] can't map association #{ass.source_factory}##{ass.column} (as #{ass.target_factory}) to any factory") + else + param = source.docs.tags.find { |t| t.tag_name == 'param' && t.name == ass.column.to_s } + if param.nil? + source.docs.add_tag YARD::Tags::Tag.new(:param, '', [target.model_class], ass.column.to_s) + else + param.types = [target.model_class] + end + end + end + end - extract_factories_from_ast(nodes) + factories end - # @param ast [Parser::AST::Node] - def extract_factories_from_ast(ast) - walker = Walker.new(ast) + # @param src [Solargraph::Source] + # @return [Array(Array, Array)] + def extract_factories_from_source(src) + walker = Walker.new(src.node) # @type [Array] factories = [] + unresolved_associations = [] walker.on :block, [:send, nil, :factory] do |ast| factory_cfg = ast.children.first.children @@ -120,18 +181,90 @@ def extract_factories_from_ast(ast) end end + kwargs = [] + traits = [] + comments = src.comments_for(ast) || '' + + unless ast.children[2].nil? + w = Walker.new(ast.children[2]) + + w.on :send do |ast| + col = ast.children[1] + next if ALWAYS_IGNORE.include? col + + if SPECIAL_CALLBACKS.include? col + # these lads need an arg or more + next if ast.children.length < 3 + next unless ast.children[2].type == :sym + + mod = col + col = ast.children[2].children.first + + if mod == :trait + # Traits can't have docs so + traits << col + next + elsif mod == :association + target_factory = col + if ast.children.last&.type == :hash # rubocop:disable Metrics/BlockNesting + factory_pair = ast.children.last.children.find do |n| + n.type == :pair && n.children[0].type == :sym && n.children[0].children[0] == :factory + end + target_factory = factory_pair.children[1].children[0] unless factory_pair.nil? # rubocop:disable Metrics/BlockNesting + end + + unresolved_associations << UnresolvedAssociation.new(col, factory_names.first, target_factory, ast) + end + end + + comment = comment_for_attribute(src, col, ast) + comments += "#{comment}\n" unless comment.nil? + kwargs << col + end + + w.walk + end + + # Fun fact: solargraph captures errors & guarantees a parser to be returned + docstring = Solargraph::Source.parse_docstring(comments).to_docstring + + return_tags = docstring.tags(:return) + unless return_tags.empty? + # goal is to keep comments but ignore types, so that we can have stuff like create_list + docstring.delete_tags(:return) + tag = return_tags.first + tag.types = nil + docstring.add_tag(tag) + end + factories << FactoryData.new( factory_names: factory_names, - model_class: model_class - # traits: , - # kwargs: , + model_class: model_class, + kwargs: kwargs, + traits: traits, + docs: docstring ) end walker.walk - factories + [factories, unresolved_associations] + end + + def comment_for_attribute(src, name, node) + comment = src.comments_for(node) + return nil if comment.nil? + + if comment.start_with?('@return ') + comment = comment[7..] + elsif comment.start_with?('@type ') + comment = comment[5..] + else + return nil + end + + "@param #{name}#{comment}" end end end -end \ No newline at end of file +end diff --git a/spec/solargraph/rspec/factory_bot_spec.rb b/spec/solargraph/rspec/factory_bot_spec.rb index 462d1da..2916b6b 100644 --- a/spec/solargraph/rspec/factory_bot_spec.rb +++ b/spec/solargraph/rspec/factory_bot_spec.rb @@ -1,89 +1,213 @@ RSpec.describe Solargraph::Rspec::FactoryBot do describe '#extract_factories_from_ast' do - let(:ast) do - Solargraph::Parser.parse( - <<~RUBY - FactoryGirl.define do - # @param factory_arg [String] - # @param trans_three [Boolean] Some comment - # @param added_attr [Boolean] Comment :3 - factory :person, aliases: [:alias_name_here], class: Abc do - # @return [String] Example content - first_name 'John' - factory_arg 'Doe' - # @return [String] Example content - example_value { } - - sequence(:email) { |n| "email\#{n}@example.com" } - - add_attribute(:example) { 'Value' } - add_attribute(:added_attr) { true } - - association :parent, factory: :some_table - association :some_table - - to_create { } - after(:something) {} - before(:something) {} - callback(:something) {} - - transient do - # @return [String] - trans_one { 'value' } - # @return [Number] Comment line 1 - # Comment line 2 - trans_two 1 - trans_three true - end + let(:source_code) do + <<~RUBY + FactoryGirl.define do + # Some factory + # + # @param factory_arg [String] + # @param trans_three [Boolean] Some comment + # @param added_attr [Boolean] Comment :3 + # @return [String] hehe I'm a fake evil comment >:) + factory :person, aliases: [:alias_name_here], class: Abc do + # @return [String] Example content + first_name 'John' + factory_arg 'Doe' + # @return [String] Example content + example_value { } - # Example comment - trait :some_trait do - after(:create) do - end - end + # Some comment + sequence(:seq_col) { |n| "email\#{n}@example.com" } + + # @return [String] Some example attribute comment + add_attribute(:example) { 'Value' } + add_attribute(:added_attr) { true } + + association :parent, factory: :some_table + # @return [SomeBadType] + association :some_table + + to_create { } + after(:something) {} + before(:something) {} + callback(:something) {} + + transient do + # @return [String] + trans_one { 'value' } + # @return [Number] Comment line 1 + # Comment line 2 + trans_two 1 + trans_three true end - # Some comment - factory :some_table do - add_attribute(:to_create) { 'string' } + trait :some_trait do + after(:create) do + end end end - RUBY - ) + + # Some comment + factory :some_table do + add_attribute(:to_create) { 'string' } + + association :ass, factory: :alias_name_here + association :person + end + + factory :other_table, class: 'Blah' do + end + end + RUBY end - let(:extracted) do - Solargraph::Rspec::FactoryBot.instance.send(:extract_factories_from_ast, ast) + + before do + Solargraph::Rspec::FactoryBot.reset + allow(Dir).to receive(:glob).and_return(['factories.rb']) + allow(File).to receive(:read).and_return(source_code) + end + + let(:factories) { Solargraph::Rspec::FactoryBot.instance.send(:factories) } + let(:pins) { Solargraph::Rspec::FactoryBot.instance.pins } + + # @return [Solargraph::Pin::Signature, nil] + def find_factory_sig(factory_name) + # @type [Solargraph::Pin::Method, nil] + met = pins.find { |p| p.is_a?(Solargraph::Pin::Method) && p.name == 'create' } + expect(met).not_to be_nil + + met.signatures.find { |p| p.parameters.first&.return_type.to_s.split(', ').include? ":#{factory_name}" } + end + + # @return [Solargraph::Pin::Parameter, nil] + def find_factory_arg(factory_name, param) + sig = find_factory_sig(factory_name) + expect(sig).not_to be_nil + + sig.parameters.find { |p| p.name == param.to_s } end it 'interprets class from class: arg' do - + expect(factories.first.model_class).to eql('Abc') + expect(factories.first.factory_names).to include(:person) end it 'interprets class from factory name if theres no class: arg' do + expect(factories[1].model_class).to eql('SomeTable') + expect(factories[1].factory_names).to include(:some_table) end it 'parses aliases' do + expect(factories[0].factory_names).to eql(%i[person alias_name_here]) end - it 'gets all regular kw args' do - end + describe 'getting kw args' do + it 'gets all regular kw args' do + # parses both these: + # column value + # column { value } + expect(factories[0].kwargs).to include(*%i[first_name factory_arg example_value]) + end + + it 'gets transient kw args' do + expect(factories[0].kwargs).to include(*%i[trans_one trans_two trans_three]) + end + + it 'ignores callback functions' do + %w[to_create after before callback].each do |cb| + expect(factories[0].kwargs).not_to include(cb), "Expected to ignore #{cb}, but didn't" + end + end + + it 'parses sequence' do + expect(factories[0].kwargs).to include(:seq_col) + end + + describe 'add_attribute' do + it 'parses attributes defined via add_attribute' do + expect(factories[0].kwargs).to include(:example, :added_attr) + expect(factories[1].kwargs).to include(:to_create) + end + end + + describe 'association' do + context 'when associated factory has a class: ' do + it 'understands the return type with factory: param' do + arg = find_factory_arg(:some_table, :ass) + expect(arg).not_to be_nil + expect(arg.return_type.to_s).to eql('Abc') + end - it 'gets transient kw args' do - end + it 'understands the return type without factory: param' do + arg = find_factory_arg(:some_table, :person) + expect(arg).not_to be_nil + expect(arg.return_type.to_s).to eql('Abc') + end - it 'gets all the traits' do - end + it 'understands the return type when factory: param is an alias' do + arg = find_factory_arg(:some_table, :ass) + expect(arg).not_to be_nil + expect(arg.return_type.to_s).to eql('Abc') + end - it 'ignores callback functions' do - end + it 'forces return_type' do + arg = find_factory_arg(:person, :some_table) + expect(arg).not_to be_nil + expect(arg.return_type.to_s).to eql('SomeTable') + end + end - it 'parses add_attribute' do - end + context 'when associated factory has no class: param' do + it 'understands the return type with factory: param' do + arg = find_factory_arg(:person, :parent) + expect(arg).not_to be_nil + expect(arg&.return_type.to_s).to eql('SomeTable') + end + + it 'understands the return type without factory: param' do + arg = find_factory_arg(:person, :some_table) + expect(arg).not_to be_nil + expect(arg&.return_type.to_s).to eql('SomeTable') + end + end + end + + describe 'documentation' do + it 'should understand top level @param docs' do + param = find_factory_arg(:person, :trans_three) + + expect(param).not_to be_nil + expect(param.return_type.to_s).to eql('Boolean') + expect(param.documentation).to eql('Some comment') + end + + it 'should force return type on top level factory' do + sig = find_factory_sig(:person) + + expect(sig).not_to be_nil + expect(sig.return_type.to_s).to eql('Abc') + end + + it 'should understand param level @return tags' do + param = find_factory_arg(:person, :trans_two) + + expect(param).not_to be_nil + expect(param.return_type.to_s).to eql('Number') + expect(param.documentation).to eql("Comment line 1\nComment line 2") + end + + it 'should understand docs for add_attribute' do + param = find_factory_arg(:person, :example) - it 'parses associations' do + expect(param).not_to be_nil + expect(param.return_type.to_s).to eql('String') + expect(param.documentation).to eql('Some example attribute comment') + end + end end - it 'parses sequence' do + it 'gets traits' do + expect(factories[0].traits).to include(*%i[some_trait]) end end end From 6e075d02c6397b7c5395ab34b71ac10f140137b3 Mon Sep 17 00:00:00 2001 From: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com> Date: Mon, 14 Jul 2025 14:24:37 +0100 Subject: [PATCH 7/8] FactoryBot & FactoryGirl support, list support --- lib/solargraph/rspec/convention.rb | 2 +- lib/solargraph/rspec/factory_bot.rb | 50 ++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/lib/solargraph/rspec/convention.rb b/lib/solargraph/rspec/convention.rb index 797148c..2d94f08 100644 --- a/lib/solargraph/rspec/convention.rb +++ b/lib/solargraph/rspec/convention.rb @@ -143,12 +143,12 @@ def local(source_map) Solargraph.logger.error("[solargraph-rspec] [factory bot] Can't add pins: #{e}") [] end + begin pins += SpecHelperInclude.instance.pins extra_requires += SpecHelperInclude.instance.extra_requires rescue StandardError => e Solargraph.logger.error("[solargraph-rspec] [spec helper] Can't add pins: #{e}") - [] end if pins.any? diff --git a/lib/solargraph/rspec/factory_bot.rb b/lib/solargraph/rspec/factory_bot.rb index ea61f3e..a5e137a 100644 --- a/lib/solargraph/rspec/factory_bot.rb +++ b/lib/solargraph/rspec/factory_bot.rb @@ -40,12 +40,25 @@ def self.reset end def pins - [ - method_builder('create', :instance), - method_builder('fake_create', :instance), - method_builder('create', :class), - method_builder('fake_create', :class) + namespaces = [ + Solargraph::Pin::Namespace.new( + name: 'FactoryGirl::Syntax::Methods', + location: PinFactory.dummy_location('spec/factories.rb') + ), + Solargraph::Pin::Namespace.new( + name: 'FactoryBot::Syntax::Methods', + location: PinFactory.dummy_location('spec/factories.rb') + ) ] + + namespaces.flat_map do |ns| + [ + build_method('create', ns), + build_method('build', ns), + build_list_method('create', ns), + build_list_method('build', ns) + ] + end end private @@ -85,14 +98,27 @@ def signature_for_factory(factory, method) sig end - def method_builder(name, scope) - method = Solargraph::Pin::Method.new( - name: name, - scope: scope, - closure: Solargraph::Pin::Namespace.new( - name: 'FactoryGirl::Syntax::Methods', - location: PinFactory.dummy_location('factories.rb') + def build_list_method(method_prefix, ns) + m = build_method("#{method_prefix}_list", ns) + m.signatures.each do |sig| + sig.parameters.insert( + 1, + Solargraph::Pin::Parameter.new( + name: 'amount', + closure: sig, + return_type: Solargraph::ComplexType.parse('Integer') + ) ) + end + + m + end + + def build_method(method_name, ns) + method = Solargraph::Pin::Method.new( + name: method_name, + scope: :instance, + closure: ns ) method.signatures = factories.map { |f| signature_for_factory(f, method) } From f66d47bb1b210600b5842ed85e132e1ba8f6204d Mon Sep 17 00:00:00 2001 From: ShadiestGoat <48590492+ShadiestGoat@users.noreply.github.com> Date: Wed, 16 Jul 2025 21:18:10 +0100 Subject: [PATCH 8/8] pause --- Gemfile | 1 + gemfiles/default.gemfile | 1 + gemfiles/default.gemfile.lock | 3 + lib/solargraph/rspec/convention.rb | 1 - lib/solargraph/rspec/factory_bot.rb | 63 ++++++++++++------- spec/factories.rb | 16 +++++ spec/solargraph/rspec/convention_spec.rb | 77 +++++++++++++++++++++++ spec/solargraph/rspec/factory_bot_spec.rb | 14 ++++- spec/spec_helper.rb | 3 + 9 files changed, 156 insertions(+), 23 deletions(-) create mode 100644 spec/factories.rb diff --git a/Gemfile b/Gemfile index 69379c2..a59643b 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,7 @@ gem 'simplecov-cobertura' # Code coverage group :third_party_plugin_tests do gem 'actionmailer' gem 'airborne' + gem 'factory_bot', '~> 6.5' gem 'rspec-rails' gem 'rspec-sidekiq' gem 'shoulda-matchers' diff --git a/gemfiles/default.gemfile b/gemfiles/default.gemfile index ced0edc..c77ff6f 100644 --- a/gemfiles/default.gemfile +++ b/gemfiles/default.gemfile @@ -16,6 +16,7 @@ gem "simplecov-cobertura" group :third_party_plugin_tests do gem "actionmailer" gem "airborne" + gem "factory_bot", "~> 6.5" gem "rspec-rails" gem "rspec-sidekiq" gem "shoulda-matchers" diff --git a/gemfiles/default.gemfile.lock b/gemfiles/default.gemfile.lock index a4a89b5..9139bdc 100644 --- a/gemfiles/default.gemfile.lock +++ b/gemfiles/default.gemfile.lock @@ -84,6 +84,8 @@ GEM domain_name (0.6.20240107) drb (2.2.1) erubi (1.13.1) + factory_bot (6.5.4) + activesupport (>= 6.1.0) globalid (1.2.1) activesupport (>= 6.1) hashdiff (1.1.2) @@ -311,6 +313,7 @@ DEPENDENCIES appraisal bundler debug + factory_bot (~> 6.5) profile-viewer pry-byebug rake diff --git a/lib/solargraph/rspec/convention.rb b/lib/solargraph/rspec/convention.rb index 2d94f08..1efb841 100644 --- a/lib/solargraph/rspec/convention.rb +++ b/lib/solargraph/rspec/convention.rb @@ -143,7 +143,6 @@ def local(source_map) Solargraph.logger.error("[solargraph-rspec] [factory bot] Can't add pins: #{e}") [] end - begin pins += SpecHelperInclude.instance.pins extra_requires += SpecHelperInclude.instance.extra_requires diff --git a/lib/solargraph/rspec/factory_bot.rb b/lib/solargraph/rspec/factory_bot.rb index a5e137a..6589e6b 100644 --- a/lib/solargraph/rspec/factory_bot.rb +++ b/lib/solargraph/rspec/factory_bot.rb @@ -17,7 +17,7 @@ class FactoryBot # @param model_class [String] The class that this factory should uses # @param traits [Array] A list of trait names # @param kwargs [Array] Any available kwargs - # @param docs [YARD::Docstring, nil] The parsed docs + # @param docs [YARD::Docstring] The parsed docs FactoryData = Struct.new(:factory_names, :model_class, :traits, :kwargs, :docs, keyword_init: true) UnresolvedAssociation = Struct.new( @@ -40,6 +40,8 @@ def self.reset end def pins + return [] if factories.empty? + namespaces = [ Solargraph::Pin::Namespace.new( name: 'FactoryGirl::Syntax::Methods', @@ -51,13 +53,17 @@ def pins ) ] - namespaces.flat_map do |ns| - [ - build_method('create', ns), - build_method('build', ns), - build_list_method('create', ns), - build_list_method('build', ns) - ] + # Tmp change: this is done for debug. + # In the end the scope should always be :instance + [:class, :instance].flat_map do |scope| + namespaces.flat_map do |ns| + [ + build_method('create', ns, scope), + build_method('build', ns, scope), + build_list_method('create', ns, scope), + build_list_method('build', ns, scope) + ] + end end end @@ -98,8 +104,8 @@ def signature_for_factory(factory, method) sig end - def build_list_method(method_prefix, ns) - m = build_method("#{method_prefix}_list", ns) + def build_list_method(method_prefix, ns, scope) + m = build_method("#{method_prefix}_list", ns, scope) m.signatures.each do |sig| sig.parameters.insert( 1, @@ -114,10 +120,10 @@ def build_list_method(method_prefix, ns) m end - def build_method(method_name, ns) + def build_method(method_name, ns, scope) method = Solargraph::Pin::Method.new( name: method_name, - scope: :instance, + scope: scope, closure: ns ) @@ -231,15 +237,7 @@ def extract_factories_from_source(src) traits << col next elsif mod == :association - target_factory = col - if ast.children.last&.type == :hash # rubocop:disable Metrics/BlockNesting - factory_pair = ast.children.last.children.find do |n| - n.type == :pair && n.children[0].type == :sym && n.children[0].children[0] == :factory - end - target_factory = factory_pair.children[1].children[0] unless factory_pair.nil? # rubocop:disable Metrics/BlockNesting - end - - unresolved_associations << UnresolvedAssociation.new(col, factory_names.first, target_factory, ast) + unresolved_associations << UnresolvedAssociation.new(col, factory_names.first, extract_association_name_from_ast(col, ast), ast) end end @@ -291,6 +289,29 @@ def comment_for_attribute(src, name, node) "@param #{name}#{comment}" end + + # @param col [Symbol] + # @param ast [::Parser::AST::Node] + def extract_association_name_from_ast(col, ast) + return col if ast.children.last&.type != :hash + + factory_pair = ast.children.last.children.find do |n| + n.type == :pair && n.children[0].type == :sym && n.children[0].children[0] == :factory + end + return col if factory_pair.nil? + + if factory_pair.children[1].type == :array + return col if factory_pair.children[1].children.empty? + + name = factory_pair.children[1].children.first + else + name = factory_pair.children[1] + end + + return col if name.type != :sym + + name.children[0] + end end end end diff --git a/spec/factories.rb b/spec/factories.rb new file mode 100644 index 0000000..b88583b --- /dev/null +++ b/spec/factories.rb @@ -0,0 +1,16 @@ +FactoryBot.define do + # Comment! + factory :user do + # @return [String] + example { 'String' } + end + + factory :fact do + name { "name :3" } + end +end + +class User + def user_method_only + end +end diff --git a/spec/solargraph/rspec/convention_spec.rb b/spec/solargraph/rspec/convention_spec.rb index a73de93..10a7428 100644 --- a/spec/solargraph/rspec/convention_spec.rb +++ b/spec/solargraph/rspec/convention_spec.rb @@ -1225,4 +1225,81 @@ def module_method expect(completion_at(filename, [15, 8])).to include('module_method') end end + + describe 'factory bot' do + before do + # step 1: we need to include the factory bot syntax in our spec_helper file + Solargraph::Rspec::SpecHelperInclude.reset + allow_any_instance_of(Solargraph::Rspec::SpecHelperInclude).to receive(:parse_included_modules).and_return( + [ + Solargraph::Rspec::SpecHelperInclude::INCLUDED_MODULE_DATA.new( + Parser::AST::Node.new( + :send, [], { + location: Parser::Source::Map.new( + Parser::Source::Range.new( + Parser::Source::Buffer.new('name.rb', source: '1'), + 0, 1 + ) + ) + } + ), 'spec_helper.rb', 'FactoryBot::Syntax::Methods' + ) + ] + ) + + # Step 2: we need to load a factory or 2 + Solargraph::Rspec::FactoryBot.reset + allow_any_instance_of(Solargraph::Rspec::FactoryBot).to receive(:factories).and_return( + [ + Solargraph::Rspec::FactoryBot::FactoryData.new( + factory_names: %i[user person], + model_class: 'User', + traits: %i[some_trait], + kwargs: %i[name last_name], + docs: YARD::Docstring.parser.to_docstring + ), + Solargraph::Rspec::FactoryBot::FactoryData.new( + factory_names: %i[post], + model_class: 'Post', + traits: %i[trait_a trait_b trait_c], + kwargs: %i[tags], + docs: YARD::Docstring.parser.to_docstring + ) + ] + ) + + load_string filename, <<~RUBY + RSpec.describe SomeNamespace::Transaction, type: :model do + describe 'fake example group' do + let(:let_user) { create(:user) } + + it 'example test' do + include FactoryBot::Syntax::Methods + let_user + crea + create(:user) + eg_person = create(:user) + eg_post = create(:post) + end + end + end + RUBY + end + + it 'should interpret create(:factory_name) result as a specific module' do + puts api_map.get_methods('FactoryBot::Syntax::Methods') + include FactoryBot::Syntax::Methods + + create(:user) + + abc = FactoryBot::Syntax::Methods.create(:user) + bac = create(:user) + + top_clip = api_map.clip_at(filename, [7, 10]) + # eg_clip = api_map.clip_at(filename, [13, 6]) + + puts "=========== infer top clip" + puts top_clip.complete.pins + end + end end diff --git a/spec/solargraph/rspec/factory_bot_spec.rb b/spec/solargraph/rspec/factory_bot_spec.rb index 2916b6b..49fa495 100644 --- a/spec/solargraph/rspec/factory_bot_spec.rb +++ b/spec/solargraph/rspec/factory_bot_spec.rb @@ -53,6 +53,7 @@ association :ass, factory: :alias_name_here association :person + association :arr_fact, factory: %i[person some_trait] end factory :other_table, class: 'Blah' do @@ -87,11 +88,16 @@ def find_factory_arg(factory_name, param) sig.parameters.find { |p| p.name == param.to_s } end - it 'interprets class from class: arg' do + it 'interprets class from class: arg (when const)' do expect(factories.first.model_class).to eql('Abc') expect(factories.first.factory_names).to include(:person) end + it 'interprets class from class: arg (when string)' do + expect(factories.last.model_class).to eql('Blah') + expect(factories.last.factory_names).to include(:other_table) + end + it 'interprets class from factory name if theres no class: arg' do expect(factories[1].model_class).to eql('SomeTable') expect(factories[1].factory_names).to include(:some_table) @@ -170,6 +176,12 @@ def find_factory_arg(factory_name, param) expect(arg&.return_type.to_s).to eql('SomeTable') end end + + it 'understand array factory: param' do + arg = find_factory_arg(:some_table, :arr_fact) + expect(arg).not_to be_nil + expect(arg&.return_type.to_s).to eql('Abc') + end end describe 'documentation' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ad61c99..2b10675 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,6 +14,9 @@ RSpec.configure do |config| config.include SolargraphHelpers + # Tmp: in the end, this should be removed as we are not a db project + config.include FactoryBot::Syntax::Methods + # Enable flags like --only-failures and --next-failure config.example_status_persistence_file_path = '.rspec_status'