Skip to content

style: Enforce perlcritic rule ProhibitImplicitNewlines#7349

Draft
perlpunk wants to merge 1 commit into
os-autoinst:masterfrom
perlpunk:more-perlcritic6
Draft

style: Enforce perlcritic rule ProhibitImplicitNewlines#7349
perlpunk wants to merge 1 commit into
os-autoinst:masterfrom
perlpunk:more-perlcritic6

Conversation

@perlpunk
Copy link
Copy Markdown
Contributor

ValuesAndExpressions::ProhibitImplicitNewlines

@perlpunk perlpunk force-pushed the more-perlcritic6 branch 4 times, most recently from 83ad3e7 to 319ac95 Compare April 29, 2026 23:11
ValuesAndExpressions::ProhibitImplicitNewlines
@perlpunk perlpunk force-pushed the more-perlcritic6 branch from 319ac95 to e90fb2c Compare May 4, 2026 14:35
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 4, 2026

This pull request is now in conflicts. Could you fix it? 🙏

@perlpunk
Copy link
Copy Markdown
Contributor Author

perlpunk commented May 4, 2026

The problem with this rule is that it complains about signatures spanning multiple lines:

#   ::warning file=t/lib/OpenQA/Test/Utils.pm,line=553,col=14,title=Literal line breaks in a string - severity 3::[ValuesAndExpressions::ProhibitImplicitNewlines] See pages 60,61 of PBP
[23:16:41] xt/02-perlcritic.t ........ 
sub test_cmd (
    $cmd, $args,
    $expected = qr//,
    $test_msg = 'command line is correct',
    $exit_code = 0,
    $exit_code_msg = 'command exits successfully'
  )
{

The reasons are:

  • This is by default read as a prototype, not a signature
  • The class PPI::Token::Prototype inherits from PPI::Token::Quote::Literal surprisingly, so the perlcritic rule also considers those as quoted strings

There is an issue about it:

PPI_CUSTOM_FEATURE_INCLUDES='Mojo::Base: {signatures: perl}' perlcritic t/lib/OpenQA/Test/Utils.pm

With this it passes, but maybe this is not important enough for now.
It would also automatically pass when we switch to use v5.40 at some point.

So I would mark this as not-ready for now, and we can exclude the policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant