Perl-Critic / PPI

53 stars 44 forks source link

handle scoped namespaces #132

Open karenetheridge opened 9 years ago

karenetheridge commented 9 years ago

The documentation spells out limitations of PPI::Statement::Package - there is no current support for scoped namespaces or determining the current namespace at a current node.

karenetheridge commented 9 years ago

Here's an example to consider:

                    print "package here is ", __PACKAGE__, "\n";        # main
package Foo;
                    print "package here is ", __PACKAGE__, "\n";        # Foo

{
                    print "package here is ", __PACKAGE__, "\n";        # Foo
    package Alpha;
                    print "package here is ", __PACKAGE__, "\n";        # Alpha
}
                    print "package here is ", __PACKAGE__, "\n";        # Foo

package Bar;
                    print "package here is ", __PACKAGE__, "\n";        # Bar

What can we do in PPI to figure out the active namespace at each line? Currently the user needs to manually write a recursive iterator to keep track of the scopes. It would be very handy if ::Node objects had a ->namespace interface -- one common usecase would be to associate our $variable declarations with a package name (e.g. $VERSION, which several Dist::Zilla plugins would want to do.)

karenetheridge commented 9 years ago

Idea: while walking the node tree, we "just" need to make a note whenever we see a package statement and record what namespace it is... any later sibling node, or any child node, can be marked as part of this namespace.

adamkennedy commented 9 years ago

If by "marking" you mean actually add information to the tree, we can't because the information could be quite unstable (and it would be mean potentially a lot of extra references)

Can you perhaps clarify "mark"?

karenetheridge commented 9 years ago

Can you perhaps clarify "mark"?

Interface-wise, one should be able to do $node->namespace to find out the effective namespace this (sub)expression is active in.

As to how this would be implemented, I don't know! The easiest thing would be to add an extra field to the node hash, but it might be useful to instead have a reference to the node containing the 'package' statement (if so, they'd have to be weak references of course).

I think the instability you refer to is that nodes can be moved around, so their effective namespace might change -- so, whenever a node is moved, the recorded namespace for it and all children (recursively) would have to be stripped.

It should be sufficient to calculate the namespace lazily - it can be recorded whenever we traverse the tree, but otherwise, only calculated on request, so if a lot of nodes are being moved around, there isn't extra overhead.

I don't know enough about PPI guts to anticipate other problems. Perhaps others can, or it may simply be necessary to start implementing this to find out!

moregan commented 9 years ago

Is there anything special to using the term "namespace" as opposed to "package"? I can imagine having both $node->package (for a PPI::Statement::Package) and $node->package_name (for a string).