metricfu / metric_fu

A fist full of code metrics
http://metricfu.github.com/metric_fu
MIT License
627 stars 96 forks source link

Ruby 3.0 yield in class syntax warning #319

Closed daveinman closed 3 years ago

daveinman commented 4 years ago

Running metric_fu version 4.13.0 from ruby version >= 2.7 we get:

/usr/local/lib/ruby/gems/2.7.0/gems/metric_fu-4.13.0/lib/metric_fu/loader.rb:37: warning: yield' in class syntax will not be supported from Ruby 3.0. [Feature #15575] /usr/local/lib/ruby/gems/2.7.0/gems/metric_fu-4.13.0/lib/metric_fu/loader.rb:54: warning:yield' in class syntax will not be supported from Ruby 3.0. [Feature #15575]

The following patch will fix these warnings in loader.rb:

--- /usr/local/lib/ruby/gems/2.7.0/gems/metric_fu-4.13.0/lib/metric_fu/loader.rb    2020-06-20 19:19:32.000000000 -0700
+++ /tmp/loader.rb  2020-06-20 19:19:17.000000000 -0700
@@ -33,14 +33,12 @@
     #     ::metrics_dir which returns the full path
     #     ::metrics_require which takes a block of files to require relative to the metrics_dir
     def create_dirs(klass)
-      class << klass
         Array(yield).each do |dir|
-          define_method("#{dir}_dir") do
-            File.join(lib_dir, dir)
+          klass.define_singleton_method("#{dir}_dir") do
+            File.join(klass.lib_dir, dir)
           end
-          module_eval(%(def #{dir}_require(&block); lib_require('#{dir}', &block); end),  __FILE__, __LINE__)
+          klass.module_eval(%(def self.#{dir}_require(&block); lib_require('#{dir}', &block); end),  __FILE__, __LINE__)
         end
-      end
     end

     # Adds method x_dir relative to the metric_fu artifact directory to the given klass
@@ -50,11 +48,9 @@
     # @example For the artifact sub-directory '_scratch', creates on the klass one method:
     #     ::scratch_dir (which notably has the leading underscore removed)
     def create_artifact_subdirs(klass)
-      class << klass
-        Array(yield).each do |dir|
-          define_method("#{dir.gsub(/[^A-Za-z0-9]/, '')}_dir") do
-            File.join(artifact_dir, dir)
-          end
+      Array(yield).each do |dir|
+        klass.define_singleton_method("#{dir.gsub(/[^A-Za-z0-9]/, '')}_dir") do
+          File.join(artifact_dir, dir)
         end
       end
     end

These two warnings are telling us that two methods are yielding after changing into the scope of another class:

class Foo
  def foo(klass)
    class << klass
      yield
...

My fix does does the yield without moving the body of the method into the scope of 'klass'.

meliborn commented 3 years ago

I guess that's why I'm getting /lib/metric_fu/loader.rb:37: Invalid yield (SyntaxError)

etagwerker commented 3 years ago

@meliborn @daveinman Thanks for reporting this issue and suggesting a solution!

I decided to fix it like this: https://github.com/metricfu/metric_fu/pull/321

What do you think?