Fixed - was not working for variables in last line of heredoc.

[vsespb]

It seems that heredoc() method excludes terminator line, so no need to pop() https://metacpan.org/pod/PPI::Token::HereDoc#heredoc

added test for the bug, also added additional test to extract_variables (just to make sure our regexp deals with newlines right)

[guillaumeaubert]

Hi Victor,

Thank you for your pull request, and in particular for including a test :)

You found a really interesting bug in how PPI's tokenizer handles heredoc blocks. Due to this bug, quoted heredoc blocks (such as <<"HERE" or <<'HERE') include the terminator line in the output of ->heredoc():

$VAR1 = [
          '     SELECT $field_name
',
          '     FROM $table
',
          '\'HERE\'
'
        ];

But unquoted heredoc blocks (such as <<HERE) do not:

$VAR1 = [
          '     SELECT $field_name
',
          '     FROM $table
'
        ];

I must have been testing with quoted heredocs when I wrote the code, hence the original line I had there. I submitted a pull request to @adamkennedy to fix the underlying inconsistency and match the documentation: https://github.com/adamkennedy/PPI/pull/71.

In the meantime, I'm going to merge your pull request, and add a conditional on top to handle the inconsistency. Once the pull request is merged in PPI's repository, I will have to also add a test for PPI's version in that code, or require a minimum version of PPI.

[vsespb]

a bit strange. I've tried to add new test (and I commented out pop() for testing and removed all other tests):

## name somename
## failures 0
## cut

my $heredoc = <<'SELECT'.$x;

SELECT

and it seems heredoc contains "SELECT" in last element if the .run file ends with 0 or 1 newline. and does not contain "SELECT" if .run file ends with 2+ newlines or there are other statements after heredoc.

and this was not related to quoting of select.

[guillaumeaubert]

I investigated further and found the issue - your patch was indeed correct, and I reverted my later modification. The issue turned out to be in the test cases, which I fixed with eef8806.

[guillaumeaubert]

Regarding your new SELECT test case above - I can reproduce it, but only if there is no newline after SELECT. Can you confirm it breaks with 1 newline as well for you?

[vsespb]

Can you confirm it breaks with 1 newline as well for you?

yes. I sent PR-17 as example. Also travis should confirm that it's failing.

It seems PPI problem indeed, but I think it's very low priority for your module. Affected: 1) here doc is last line in file (so, it's not even a subroutine!) 2) SELECT|UPDATE used as terminators 3) it's false positive which is not so bad as false negative

other cases probably can include heredocs terminators with sigils, which is just waiting for trouble anyway.

[guillaumeaubert]

Thank you for opening GH-17 to track this PPI issue! I'm going to follow up there.