Index Class.new and Module.new

[vinistock]

Initial step for #392

This PR starts handling Class.new and Module.new. The shared characteristic between both is that they create a new possible owner for definitions. That means, you can have methods and instance variables be associated with them.

However, neither of those produces of a new lexical scope, so constant references that occur inside of the block ignore these new classes and modules. Only the class and module keywords can produce new lexical scopes.

To account for this, I turned the definitions_stack into a nesting_stack where we can have two types of entries:

• LexicalScope: an owner definition that also needs to be accounted for constant references
• Owner: an owner that doesn't influence constant references

[vinistock]

• Index Class.new and Module.new #402 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Morriar]

What's the plan for anonymous ones like this:

class Foo
  Class.new do
    def foo; end
  end
end

I think we'll need to represent class/module defs without a name (or <ANONYMOUS>?) so we can push them onto the stack and avoid registering foo in the wrong parent.

Maybe the handling of Class.new & Module.new should also move to visit_call_node?

[st0012]

Can we add these tests to resolution?

    #[test]
    fn resolving_global_variable_alias_inside_method() {
        let mut context = GraphTest::new();
        context.index_uri("file:///foo.rb", {
            r"
            class Foo
              def setup
                alias $bar $baz
              end
            end
            "
        });
        context.resolve();

        // Global variable aliases should still be owned by Object, regardless of where defined
        let read_lock = context.graph().declarations().read().unwrap();
        let object = read_lock.get(&DeclarationId::from("Object")).unwrap();
        assert_members(object, &["$bar"]);
    }

    #[test]
    fn resolving_method_defined_inside_method() {
        let mut context = GraphTest::new();
        context.index_uri("file:///foo.rb", {
            r"
            class Foo
              def setup
                def inner_method; end
              end
            end
            "
        });
        context.resolve();

        let read_lock = context.graph().declarations().read().unwrap();
        let foo_class = read_lock.get(&DeclarationId::from("Foo")).unwrap();
        // inner_method should be owned by Foo, not by setup
        assert_members(foo_class, &["setup", "inner_method"]);
    }

    #[test]
    fn resolving_attr_accessors_inside_method() {
        let mut context = GraphTest::new();
        context.index_uri("file:///foo.rb", {
            r"
            class Foo
              def self.setup
                attr_reader :reader_attr
                attr_writer :writer_attr
                attr_accessor :accessor_attr
              end
            end
            "
        });
        context.resolve();

        let read_lock = context.graph().declarations().read().unwrap();
        let foo_singleton_class = read_lock.get(&DeclarationId::from("Foo::<Foo>")).unwrap();
        assert_members(foo_singleton_class, &["setup"]);
        let foo_class = read_lock.get(&DeclarationId::from("Foo")).unwrap();
        // All attr_* should be owned by Foo, not by setup
        assert_members(foo_class, &["reader_attr", "writer_attr", "accessor_attr"]);
    }

[st0012]

Will it be better to name it like current_lexical_owner_name_id?
Owner includes class/module/singleton class/anonymous class & module. Lexical owner excludes the last one.

[vinistock]

I think the nomenclature needs to be a touch different. We now have 3 types of nestings:

• LexicalScope: anything that uses the class or module keywords
• Owner: a namespace that can own things, but isn't a new lexical scope (e.g.: Class.new)
• Method: just to indicate we're inside a method, but it doesn't represent a new lexical scope and can't own anything

I think using "lexical owner" can be confusing because it kind of mixes the first two nesting variants. Personally, I would use lexical_scope and owner separately without mixing the two.

[st0012]

That's fair. Can this distinction go into the architecture doc?

Also TIL: nomenclature

[vinistock]

Yes! I'll add it to #423

[vinistock]

Added the test cases 👍.