Merge pull request #340 from pirj/fix-warning-on-method-override

Fix warning when overriding an exsiting method

README.md

@@ -405,7 +405,7 @@ simple.aasm(:work).current
 
 _AASM_ doesn't prohibit to define the same event in more than one state machine. The
 latest definition "wins" and overrides previous definitions. Nonetheless, a warning is issued:
-`SimpleMultipleExample: The aasm event name run is already used!`.
+`SimpleMultipleExample: overriding method 'run'!`.
 
 All _AASM_ class- and instance-level `aasm` methods accept a state machine selector.
 So, for example, to use inspection on a class level, you have to use

lib/aasm/base.rb

@@ -74,13 +74,10 @@ module AASM
     def state(name, options={})
       @state_machine.add_state(name, klass, options)
 
-      if klass.instance_methods.include?("#{name}?")
-        warn "#{klass.name}: The aasm state name #{name} is already used!"
-      end
-
-      aasm_name = @name
-      klass.send :define_method, "#{name}?", ->() do
-        aasm(:"#{aasm_name}").current_state == :"#{name}"
+      aasm_name = @name.to_sym
+      state = name.to_sym
+      safely_define_method klass, "#{name}?", -> do
+        aasm(aasm_name).current_state == state
       end
 
       unless klass.const_defined?("STATE_#{name.upcase}")
@@ -92,27 +89,24 @@ module AASM
     def event(name, options={}, &block)
       @state_machine.add_event(name, options, &block)
 
-      if klass.instance_methods.include?("may_#{name}?".to_sym)
-        warn "#{klass.name}: The aasm event name #{name} is already used!"
-      end
+      aasm_name = @name.to_sym
+      event = name.to_sym
 
       # an addition over standard aasm so that, before firing an event, you can ask
       # may_event? and get back a boolean that tells you whether the guard method
       # on the transition will let this happen.
-      aasm_name = @name
-
-      klass.send :define_method, "may_#{name}?", ->(*args) do
-        aasm(:"#{aasm_name}").may_fire_event?(:"#{name}", *args)
+      safely_define_method klass, "may_#{name}?", ->(*args) do
+        aasm(aasm_name).may_fire_event?(event, *args)
       end
 
-      klass.send :define_method, "#{name}!", ->(*args, &block) do
-        aasm(:"#{aasm_name}").current_event = :"#{name}!"
-        aasm_fire_event(:"#{aasm_name}", :"#{name}", {:persist => true}, *args, &block)
+      safely_define_method klass, "#{name}!", ->(*args, &block) do
+        aasm(aasm_name).current_event = :"#{name}!"
+        aasm_fire_event(aasm_name, event, {:persist => true}, *args, &block)
       end
 
-      klass.send :define_method, "#{name}", ->(*args, &block) do
-        aasm(:"#{aasm_name}").current_event = :"#{name}"
-        aasm_fire_event(:"#{aasm_name}", :"#{name}", {:persist => false}, *args, &block)
+      safely_define_method klass, name, ->(*args, &block) do
+        aasm(aasm_name).current_event = event
+        aasm_fire_event(aasm_name, event, {:persist => false}, *args, &block)
       end
     end
 
@@ -183,5 +177,13 @@ module AASM
       end
     end
 
+    def safely_define_method(klass, method_name, method_definition)
+      if klass.instance_methods.include?(method_name.to_sym)
+        warn "#{klass.name}: overriding method '#{method_name}'!"
+      end
+
+      klass.send(:define_method, method_name, method_definition)
+    end
+
   end
 end

spec/unit/override_warning_spec.rb

@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe 'warns when overrides a method' do
+  module Clumsy
+    def self.included base
+      base.send :include, AASM
+
+      base.aasm do
+        state :valid
+        event(:save) { }
+      end
+    end
+  end
+
+  describe 'state' do
+    class Base
+      def valid?; end
+    end
+    it do
+      expect { Base.send :include, Clumsy }.
+        to output(/Base: overriding method 'valid\?'!/).to_stderr
+    end
+  end
+
+  describe 'event' do
+    context 'may?' do
+      class Base
+        def may_save?; end
+        def save!; end
+        def save; end
+      end
+      let(:klass) { Base }
+      it do
+        expect { Base.send :include, Clumsy }.
+          to output(/Base: overriding method 'may_save\?'!/).to_stderr
+        expect { Base.send :include, Clumsy }.
+          to output(/Base: overriding method 'save!'!/).to_stderr
+        expect { Base.send :include, Clumsy }.
+          to output(/Base: overriding method 'save'!/).to_stderr
+      end
+    end
+  end
+end