Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#246 CHECK bareword handle parsed as scheduled block #247

Closed
wants to merge 1 commit into from

Conversation

trwyant
Copy link
Contributor

@trwyant trwyant commented Feb 7, 2020

The fix ensures that a the keyword for a prospective scheduled block is followed by '{' or ';'. Otherwise it is not parsed as a scheduled block.

I have run both modified and unmodified PPI against all of CPAN, looking for instances of PPI::Statement::Scheduled where the keyword is not followed by a block. All the differences were either irrelevant (stuff like temporary directory names, or non-Perl (i.e. filters)), or
statements like
open( CHECK, ... )
which disappeared when the new code was used.

followed by '{' or ';'. Otherwise it is not parsed as a scheduled block.

I have run both modified and unmodified PPI against all of CPAN, looking
for instances of PPI::Statement::Scheduled where the keyword is not
followed by a block. All the differences were either irrelevant (stuff
like temporary directory names, or non-Perl (i.e. filters)), or
statements like
    open( CHECK, ... )
which disappeared when the new code was used.
@trwyant
Copy link
Contributor Author

trwyant commented Sep 11, 2020

Any chance of this being accepted? Or rejected? I submitted it back in February, and then completely forgot about it until I got my cage rattled on the original Perl::Critic issue, to wit [https://github.com/Perl-Critic/Perl-Critic/issues/878].

@wchristian
Copy link
Member

Sorry, i'll try and have a look tomorrow. :)

@trwyant
Copy link
Contributor Author

trwyant commented Oct 3, 2020

Nudge?

@oalders
Copy link
Collaborator

oalders commented Jan 21, 2022

Closing and re-opening to trigger CI.

@oalders oalders closed this Jan 21, 2022
@oalders oalders reopened this Jan 21, 2022
Copy link
Collaborator

@oalders oalders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wchristian
Copy link
Member

This looks ok if the tests pass after rebasing.

The stray newline can probably go?

There should be an additional ticket to verify that this stuff behaves well for all of these: https://perldoc.perl.org/perlmod#BEGIN,-UNITCHECK,-CHECK,-INIT-and-END

@oalders
Copy link
Collaborator

oalders commented Jul 18, 2022

I have a rebased branch and will merge at the command line if the checks pass.

@oalders
Copy link
Collaborator

oalders commented Jul 18, 2022

Merged via 52004df. Thanks, @trwyant!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants