ConstNodeInspector doesn’t try to resolve module namespaces

[wildmaples]

This is a copy-paste of @tomstuart's bug report, originally posted to https://github.com/Shopify/packwerk-old/issues/289

Describe the bug

When Packwerk::ConstNodeInspector finds a constant in a class or module definition, it uses the lexical nesting to fully-qualify that constant’s name. This is not always correct because a namespace may refer to a constant that has already been defined elsewhere, e.g. in another package.

This bug is a variation on #234: this time it’s a reference to an existing constant elsewhere in the enclosing namespace, rather than to the root namespace, which causes the problem.

To Reproduce

Say we have a pair of components, one and two, which define the modules A::B and A::C respectively:

% tree components
components
├── one
│   ├── a
│   │   └── b.rb
│   └── package.yml
└── two
    ├── a
    │   └── c.rb
    └── package.yml

Both package.yml files have enforce_dependencies and enforce_privacy set to true and no dependencies defined, so Packwerk should not allow either component to refer to the other.

In addition to defining A::B, the file one/a/b.rb also defines a nested class A::B::D:

module A
  module B
    class D
      def info
        'defined inside ::A::B by component one'
      end
    end
  end
end

And in addition to defining A::C, the file two/a/c.rb reopens that class and overwrites one of its methods:

module A
  module C
    class B::D
      def info
        'defined inside ::A::C by component two'
      end
    end
  end
end

It might not be immediately obvious that two is redefining A::B::D#info here, but it is, because the reference to B in class B::D gets resolved to the existing constant A::B, regardless of the definition occurring lexically inside module C. (This is what #234 is about.)

So when these two components are loaded, two refers to (and modifies) the class A::B::D from one:

% irb -Icomponents/one -Icomponents/two
>> require 'a/b'
=> true
>> require 'a/c'
=> true
>> A::B::D.new.info
=> "defined inside ::A::C by component two"
=> nil

packwerk check doesn’t detect this violation because it doesn’t realise that class B::D is a reference to A::B::D. It assumes it must refer to A::C::B::D because it appears inside module A; module C; …; end end, but Ruby constant resolution is more complex.

This doesn’t present an immediate problem within Shopify because we don’t use compact constant nesting (class B::D) in class definitions, but Packwerk may give incorrect results on external codebases.

Expected behavior

packwerk check should give an error like this:

Dependency violation: ::A::B::D belongs to 'components/one', but 'components/two' does not specify a dependency on 'components/one'.
Are we missing an abstraction?
Is the code making the reference, and the referenced constant, in the right packages?

Inference details: 'B::D' refers to ::A::B::D which seems to be defined in components/one/a/b.rb.

Version information

• Packwerk: v0.1.10
• Ruby: v2.6.6p146