Bug #34307 for PPI: Are PPI::Element::__insert_before() and PPI::Element::__insert

Thu Mar 20 20:34:17 2008 matisse [...] spamcop.net - Ticket created

Subject:

Are PPI::Element::__insert_before() and PPI::Element::__insert_after() really private?

In PPI::Element the documentation describes insert_before() and insert_after() but the actual code seems to name those methods with TWO leading underscores: sub __insert_before { my $self = shift; $self->parent->__insert_before_child( $self, @_ ); } sub __insert_after { my $self = shift; $self->parent->__insert_after_child( $self, @_ ); } What's up with that? Are the docs wrong? Should the method names be changed? Something else?

Fri Mar 21 02:47:51 2008 adamkennedybackup [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #34307] Are PPI::Element::__insert_before() and PPI::Element::__insert_after() really private?
Date:	Fri, 21 Mar 2008 17:41:10 +1100
To:	bug-PPI [...] rt.cpan.org
From:	Adam Kennedy <adamkennedybackup [...] gmail.com>

The source code contains two versions, the public insert_before, and the internal and extremely unsafe non-param-checking __insert_before. The public method is for the public, and checks params properly. The internal methods do the actual changes and don't check params at all. The exist to allow internal code to use an optimised path, rather than slowing it down by excessive param-checking. Adam K Matisse Enzer via RT wrote: Show quoted text

> Thu Mar 20 20:34:17 2008: Request 34307 was acted upon. > Transaction: Ticket created by MATISSE > Queue: PPI > Subject: Are PPI::Element::__insert_before() and PPI::Element::__insert_after() > really private? > Broken in: 1.201 > Severity: (no value) > Owner: Nobody > Requestors: matisse@spamcop.net > Status: new > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=34307 > > > > In PPI::Element the documentation describes insert_before() and > insert_after() but the actual code seems to name those methods with TWO > leading underscores: > > sub __insert_before { > my $self = shift; > $self->parent->__insert_before_child( $self, @_ ); > } > > sub __insert_after { > my $self = shift; > $self->parent->__insert_after_child( $self, @_ ); > } > > What's up with that? Are the docs wrong? Should the method names be > changed? Something else? >

Fri Mar 21 02:47:53 2008 The RT System itself - Status changed from 'new' to 'open'

Fri Mar 21 11:16:52 2008 matisse [...] spamcop.net - Correspondence added

On Fri Mar 21 02:47:51 2008, adamkennedybackup@gmail.com wrote: Show quoted text

> The source code contains two versions, the public insert_before, and > the > internal and extremely unsafe non-param-checking __insert_before. > > The public method is for the public, and checks params properly. > > The internal methods do the actual changes and don't check params at > all. The exist to allow internal code to use an optimised path, rather > than slowing it down by excessive param-checking.

Thanks for the explanation. I've attached a test script that shows the private method working where the public method fails. I suspect this is because the node i am inserting in the test isa 'PPI::Document', rather than some other type - but I don't know how I would create a node type that the public insert_before() would allow. So perhaps the documentation should mention this somehow? In my test script i do: my $new_node = PPI::Document->new( \$new_content ); where what I suppose what I should do (if it worked) would be: my $new_node = PPI::Statement->new( \$new_content );

use strict; use warnings; use Test::More; use PPI; my %test_cases = ( 'use Foo;' => 'use Matisse::Foo;', 'use Foo::Bar;' => 'use Matisse::Foo::Bar;', 'use Foo::Bar qw( Biff Baz Boffo );' => 'use Matisse::Foo::Bar qw( Biff Baz Boffo );', ); my $test_count = 2 * scalar keys %test_cases; Test::More::plan( tests => $test_count ); foreach my $orig_code ( sort keys %test_cases ) { my $expected_code = $test_cases{$orig_code}; my $method = 'insert_before'; my $got_code_using_public_insert_before = modify_node_using( $method, $orig_code ); Test::More::is( $got_code_using_public_insert_before, $expected_code, "Public $method '$orig_code'" ); } foreach my $orig_code ( sort keys %test_cases ) { my $expected_code = $test_cases{$orig_code}; my $method = '__insert_before'; my $got_code_using_private_insert_before = modify_node_using( $method, $orig_code ); Test::More::is( $got_code_using_private_insert_before, $expected_code, "Private $method '$orig_code'" ); } sub modify_node_using { my $method = shift; my $orig_code = shift; my $ppi_doc = PPI::Document->new( \$orig_code ); my $include = $ppi_doc->find_first('PPI::Statement::Include'); my $new_content = $include->content(); $new_content =~ s/(\s)Foo/$1Matisse::Foo/; my $new_node = PPI::Document->new( \$new_content ); $include->$method($new_node); $include->remove(); my $got_code = $ppi_doc->content(); return $got_code; }

Sun Mar 23 11:22:44 2008 matisse [...] spamcop.net - Correspondence added 30 min

I found a way to create a new 'PPI::Statement::Include' from a string. So, maybe this should be added to the docs, or maybe the new() for PPI::Element should be changed to handle this directly? my $code_string = 'use Foo::Bar::Baz;'; my $new_doc = PPI::Document->new( \$code_string ); my $new_include = $new_doc->find_first('PPI::Statement::Include'); # $new_include->isa('PPI::Statement::Include')

Sun Mar 23 11:22:45 2008 matisse [...] spamcop.net - Status changed from 'open' to 'resolved'

Sun Mar 23 22:42:07 2008 adamkennedybackup [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #34307] Are PPI::Element::__insert_before() and PPI::Element::__insert_after() really private?
Date:	Mon, 24 Mar 2008 13:35:30 +1100
To:	bug-PPI [...] rt.cpan.org
From:	Adam Kennedy <adamkennedybackup [...] gmail.com>

1. Create the document from the string. 2. Extract the statement you care about from that document. 3. Insert it into the other document. Matisse Enzer via RT wrote: Show quoted text

> Queue: PPI > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=34307 > > > On Fri Mar 21 02:47:51 2008, adamkennedybackup@gmail.com wrote: >

>> The source code contains two versions, the public insert_before, and >> the >> internal and extremely unsafe non-param-checking __insert_before. >> >> The public method is for the public, and checks params properly. >> >> The internal methods do the actual changes and don't check params at >> all. The exist to allow internal code to use an optimised path, rather >> than slowing it down by excessive param-checking. >>

> > Thanks for the explanation. > > I've attached a test script that shows the private method working where > the public method fails. > > I suspect this is because the node i am inserting in the test isa > 'PPI::Document', rather than some other type - but I don't know how I > would create a node type that the public insert_before() would allow. > > So perhaps the documentation should mention this somehow? > > In my test script i do: > > my $new_node = PPI::Document->new( \$new_content ); > > where what I suppose what I should do (if it worked) would be: > > my $new_node = PPI::Statement->new( \$new_content ); > >

Sun Mar 23 22:42:09 2008 The RT System itself - Status changed from 'resolved' to 'open'

Sun Feb 21 00:52:34 2010 adamk [...] cpan.org - Correspondence added

No changes to the code required, resolving this bug

Sun Feb 21 00:52:38 2010 adamk [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #34307 for PPI: Are PPI::Element::__insert_before() and PPI::Element::__insert_after() really private?

Preferred bug tracker