ProhibitLongLines does not specify failing line

[kentfredric]

Though the source seems to walk all lines to validate, it seems the line of reporting is hardcoded to line 1.

# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 1, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 1, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 1, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 1, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 1, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 1, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 1, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 1, near 'use 5.008;    # utf8'.  (Severity: 2)

Seems in the code you got

Readonly::Scalar my $LOCATION_VISUAL_COLUMN_NUMBER      => 2;
Readonly::Scalar my $LOCATION_LOGICAL_LINE_NUMBER       => 3;

Backwards

So you have

      $viol->_set_location([ $ln, 1, $ln, 1, $fn ]);

Where you should have

      $viol->_set_location([ $ln, 1, 1, $ln, $fn ]);

And indeed, changing this results in

# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 354, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 356, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 357, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 360, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 361, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 362, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 365, near 'use 5.008;    # utf8'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 368, near 'use 5.008;    # utf8'.  (Severity: 2)

And those lines are rather indicative.

The "near" context isn't incredibly helpful of course, and thats a common theme in Perl::Critic

After a bit of further jiggering, I worked out how to display the source line, seems the original code is a bit wonky and needs fixing or something:

Does weird stuff with PPI and statements which are obviously no use to POD:

https://metacpan.org/source/THALJEF/Perl-Critic-1.121/lib/Perl/Critic/Violation.pm#L300

And resolves that stuff at ->new() time: https://metacpan.org/source/THALJEF/Perl-Critic-1.121/lib/Perl/Critic/Violation.pm#L87

So, as a hack workaround:

{
  package Perl::Critic::Tics::Violation::VirtualPos;
{
  $Perl::Critic::Tics::Violation::VirtualPos::VERSION = '0.008';
}
  BEGIN {require Perl::Critic::Violation; our @ISA = 'Perl::Critic::Violation';}
  sub _set_location { my ($self, $pos) = @_; $self->{__PACKAGE__}{pos} = $pos; }
  sub _set_violation_line { my ($self, $line ) = @_; $self->{__PACKAGE__}{line} = $line; }
  sub location { $_[0]->{__PACKAGE__}{pos} }
  sub source { $_[0]->{__PACKAGE__}{line} }
}

And call

     $viol->_set_violation_line($lines[ $ln - 1 ]);

And this gives me the desired result:

# Perl::Critic found these violations in "blib/lib/Dist/Zilla/PluginBundle/Author/KENTNL.pm":
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 354, near 'this bundle advocates a new naming system for people who are absolutely convinced they want their C<Author-Centric> distribution uploaded to CPAN.'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 356, near 'As we have seen with Dist::Zilla there have been a slew of PluginBundles with CPANID's in their name, to the point that there is a copious amount of name-space pollution'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 357, near 'in the PluginBundle name-space, and more Author bundles than task-bundles, which was really what the name-space was designed for, and I'm petitioning you to help reduce'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 360, near 'From a CPAN testers perspective, the annoyance of lots of CPANID-dists is similar to the annoyance of the whole DPCHRIST:: subspace, and that if this pattern continues,'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 361, near 'it will mean for the testers who do not wish to test everyones personal modules, that they will have to work hard to avoid this. If DPCHRIST:: had used something like'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 362, near 'Author::DPCHRIST:: instead, I doubt so many people would be horrified by it, because you can just have a policy/rule that excludes ^Author::, and everyone else who goes'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 365, near 'Then we could probably rationally add that same restriction to the irc announce bots, the "recent modules" list and so-forth, and possibly even apply special indexing restrictions'.  (Severity: 2)
# [Tics::ProhibitLongLines] Line is over base length limit of 130 characters at line 368, near 'So, for the sake of cleanliness, semantics, and general global sanity, I ask you to join me with my Author:: naming policy to voluntarily segregate modules that are most'.  (Severity: 2)

[rjbs]

Could you provide this as a PR instead of hunks of code?