troessner / reek

Code smell detector for Ruby
https://github.com/troessner/reek
MIT License
4k stars 279 forks source link

Fix load error with REXML document #1699

Closed bkuhlmann closed 4 weeks ago

bkuhlmann commented 1 year ago

Overview

Hello. πŸ‘‹ I stumbled across an issue where I can't include Reek as a gem unless RuboCop is also required. I can see that Reek::Report::XMLReport requires rexml/document but there appears to be an order of operation issue where REXML isn't loaded in time.

Steps to Recreate

You can run the following script to reproduce:

#! /usr/bin/env ruby
# frozen_string_literal: true

# Save as `snippet`, then `chmod 755 snippet`, and run as `./snippet`.

require "bundler/inline"

gemfile true do
  source "https://rubygems.org"
  gem "reek"
end

puts "Failed to make it here. 😒"

Upon running ./snippet, I'll get the following output:

Stack Dump ``` Fetching gem metadata from https://rubygems.org/........ Resolving dependencies... Using ast 2.4.2 Using bundler 2.4.6 Using kwalify 0.7.2 Using rainbow 3.1.1 Using parser 3.2.0.0 Using reek 6.1.4 :37:in `require': cannot load such file -- rexml/document (LoadError) from :37:in `require' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/gems/3.2.0/gems/reek-6.1.4/lib/reek/report/xml_report.rb:13:in `' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/gems/3.2.0/gems/reek-6.1.4/lib/reek/report/xml_report.rb:12:in `' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/gems/3.2.0/gems/reek-6.1.4/lib/reek/report/xml_report.rb:6:in `' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/gems/3.2.0/gems/reek-6.1.4/lib/reek/report/xml_report.rb:5:in `' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/gems/3.2.0/gems/reek-6.1.4/lib/reek/report.rb:6:in `require_relative' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/gems/3.2.0/gems/reek-6.1.4/lib/reek/report.rb:6:in `' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/gems/3.2.0/gems/reek-6.1.4/lib/reek.rb:8:in `require_relative' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/gems/3.2.0/gems/reek-6.1.4/lib/reek.rb:8:in `' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/runtime.rb:60:in `require' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/runtime.rb:60:in `block (2 levels) in require' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/runtime.rb:55:in `each' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/runtime.rb:55:in `block in require' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/runtime.rb:44:in `each' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/runtime.rb:44:in `require' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/inline.rb:66:in `block (2 levels) in gemfile' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/settings.rb:131:in `temporary' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/inline.rb:51:in `block in gemfile' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler.rb:418:in `block in with_unbundled_env' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler.rb:664:in `with_env' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler.rb:418:in `with_unbundled_env' from /Users/bkuhlmann/.cache/frum/versions/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/inline.rb:42:in `gemfile' from /Users/bkuhlmann/Engineering/Misc/snippet:8:in `
' ```

Environment

mvz commented 1 year ago

Thanks, @bkuhlmann. We probably missed this because RuboCop is part of the bundle in development.

hakanai commented 1 year ago

I found this independently on our project the other day but had forgotten to come here and report it. πŸ˜‚

Worked fine after adding rexml to our Gemfile, so that seems to be the only missing dependency.

fbuys commented 8 months ago

Hey @mvz this dependency was added: https://github.com/troessner/reek/pull/1703/files But it does not seem like it is included in the 6.1.4 release: https://github.com/troessner/reek/blob/v6.1.4/reek.gemspec We should probably release a new minor version with rexml included.

Also, we did not add anything about it to the CHANGELOG. So I wonder if we should add a note about it to the CHANGELOG.

mvz commented 4 weeks ago

In the mean time, Reek 6.2.0 has been released which includes the rexml dependency in the gemspec.

bkuhlmann commented 4 weeks ago

Thanks! Actually, you mean Reek 6.3.0 but understand what you mean. :wink:

mvz commented 4 weeks ago

No I meant 6.2.0 because it already includes the change :smile:

bkuhlmann commented 3 weeks ago

Oh, sorry, I should have double checked. I thought this functionality rolled out in 6.3.0 but it was 6.2.0. I stand corrected! 😊