Prepending methods to concerns can be confusing

I learned this today while working on https://gitlab.com/gitlab-org/gitlab-ee/issues/6122, and I thought I might not be the only one confused by this.

Let's say you have some plain modules, and you prepend one module to another:

module Ee
  def foo
    super * 2
  end
end

module Ce
  prepend Ee
  
  def foo
    1
  end
end

class Unconcerned
  include Ce
end

Unconcerned.new.foo
#=> 2

I think this is clear.

Now let's say those aren't any old modules, but ActiveSupport::Concerns:

module EeConcern
  extend ActiveSupport::Concern

  def foo
    super * 2
  end
end

module CeConcern
  extend ActiveSupport::Concern
  prepend EeConcern
  
  def foo
    1
  end
end

class Concerned
  include CeConcern
end

Concerned.new.foo
#=> 1

Wait, what? This is because ActiveSupport::Concern defines included, which changes the order of resolution of includes: https://stackoverflow.com/a/40071159

We can work around that:

module EeConcernIncluded
  extend ActiveSupport::Concern

  included do
    prepend InstanceMethods
  end

  module InstanceMethods
    def foo
      super * 2
    end
  end
end

module CeConcernIncluded
  extend ActiveSupport::Concern
  prepend EeConcernIncluded
  
  def foo
    1
  end
end

class ConcernIncluded
  include CeConcernIncluded
end

ConcernIncluded.new.foo
#=> 2

But I'm not sure that's the right way to go. And, honestly, I'm not sure I fully understand why this happens, but I can show that it does. Here are the inheritance chains:

Unconcerned.ancestors.take(4)
#=> [Unconcerned, Ee, Ce, Object]
Concerned.ancestors.take(4)
#=> [Concerned, CeConcern, EeConcern, Object]
ConcernIncluded.ancestors.take(4)
#=> [EeConcernIncluded::InstanceMethods, ConcernIncluded, CeConcernIncluded, EeConcernIncluded]

I think as we move more code to EE, it might be worth seeing if we can add a cop for this sequence:

  • A concern.
  • That prepends another concern.
  • That defines some overrides.

As it probably won't work how we (at least, I) would expect.

(Or we could try to define fewer methods in modules rather than classes, but that ship may have sailed!)