marker in their code. I'm not happy with the option-name that I chose.
Also, I know there are bugs related to the shebang line, and the borders of
##no critic -> ##use critic blocks.
theme=s
top:i
verbose=s
+ warn-about-useless-no-critic!
);
}
my @policies = $self->config->policies();
my @violations = map { _critique($_, $doc) } @policies;
+ # Warn about unecessary "## no critic" markers
+ if ( $self->config->warn_about_useless_no_critic() ) {
+ my @warnings = $doc->useless_no_critic_warnings(@violations);
+ for (@warnings) { warn "$_\n"; }
+ }
+
# Accumulate statistics
$self->statistics->accumulate( $doc, \@violations );
$self->{_color} = boolean_to_number( dor( $args{-color}, $options_processor->color() ) );
$self->{_criticism_fatal} =
boolean_to_number(dor( $args{'-criticism-fatal'}, $options_processor->criticism_fatal() ) );
+
+ $self->{_warn_about_useless_no_critic} =
+ boolean_to_number(dor( $args{'-warn-about-useless-no-critc'},
+ $options_processor->warn_about_useless_no_critic() ) );
}
$self->_validate_and_save_theme($args{-theme}, $errors);
#-----------------------------------------------------------------------------
+sub warn_about_useless_no_critic {
+ my $self = shift;
+ return $self->{_warn_about_useless_no_critic};
+}
+
+#-----------------------------------------------------------------------------
+
sub site_policy_names {
return Perl::Critic::PolicyFactory::site_policy_names();
}
Returns the value of the C<-criticsm-fatal> attribute for this Config.
+=item C< warn_about_useless_no_critic() >
+
+Returns the value of the C<-warn-about-useless-no-critic> attribute for this Config.
+
=back
=head1 SUBROUTINES
#-----------------------------------------------------------------------------
+sub useless_no_critic_warnings {
+ my ($self, @violations) = @_;
+
+
+ my %violation_lines = ();
+ for my $violation (@violations) {
+ my $line = $violation->location()->[0];
+ my $policy_name = $violation->policy();
+ $violation_lines{$policy_name}->{$line} = 1;
+ }
+
+
+ my @warnings = ();
+ my $file = $self->filename() || 'UNKNOWN';
+
+ my %disabled_lines = %{ $self->{_disabled_lines} };
+ for my $line (keys %disabled_lines) {
+ my %disabled_policies = %{ $disabled_lines{$line} };
+ for my $policy_name (keys %disabled_policies) {
+
+ if ($policy_name eq 'ALL' && not exists $violation_lines{$line}) {
+ push @warnings, qq{Useless disabling of all Policies in file "$file" at line $line.};
+ }
+ elsif (not $violation_lines{$line}->{$policy_name}) {
+ push @warnings, qq{Useless disabling of $policy_name in file "$file" at line $line.};
+ }
+ }
+ }
+
+ return @warnings;
+}
+
+#-----------------------------------------------------------------------------
+
sub _is_a_version_statement {
my (undef, $element) = @_;
been disabled at each line. Returns C<$self>.
-=item C<< line_is_disabled($line, $policy_name) >>
+=item C<< is_line_disabled($line, $policy_name) >>
Returns true if the given C<$policy_name> has been disabled for
at C<$line> in this document. Otherwise, returns false.
+=item C<< useless_no_critic_warnings(@violations) >>
+
+Given a list of violation objects that are assumed to have been found
+in this Document, returns a warning message for each line where a
+policy was disabled using a C<"##no critic"> pseudo-pragma, but
+no violation was actually found on that line. If multiple policies
+are disabled on a given line, then you'll get a warning message
+for each policy.
+
=back
$self->{_include} = [ words_from_string( $include ) ];
# Single-value defaults
- $self->{_force} = dor(delete $args{force}, $FALSE);
- $self->{_only} = dor(delete $args{only}, $FALSE);
+ $self->{_force} = dor(delete $args{force}, $FALSE);
+ $self->{_only} = dor(delete $args{only}, $FALSE);
$self->{_profile_strictness} =
dor(delete $args{'profile-strictness'}, $PROFILE_STRICTNESS_DEFAULT);
- $self->{_single_policy} = dor(delete $args{'single-policy'}, $EMPTY);
- $self->{_severity} = dor(delete $args{severity}, $SEVERITY_HIGHEST);
- $self->{_theme} = dor(delete $args{theme}, $EMPTY);
- $self->{_top} = dor(delete $args{top}, $FALSE);
- $self->{_verbose} = dor(delete $args{verbose}, $DEFAULT_VERBOSITY);
- $self->{_criticism_fatal} = dor(delete $args{'criticism-fatal'}, $FALSE);
- $self->{_pager} = dor(delete $args{pager}, $EMPTY);
+ $self->{_single_policy} = dor(delete $args{'single-policy'}, $EMPTY);
+ $self->{_severity} = dor(delete $args{severity}, $SEVERITY_HIGHEST);
+ $self->{_theme} = dor(delete $args{theme}, $EMPTY);
+ $self->{_top} = dor(delete $args{top}, $FALSE);
+ $self->{_verbose} = dor(delete $args{verbose}, $DEFAULT_VERBOSITY);
+ $self->{_criticism_fatal} = dor(delete $args{'criticism-fatal'}, $FALSE);
+ $self->{_warn_about_useless_no_critic} =
+ dor(delete $args{'warn_about_useless_no_critic'}, $FALSE);
+ $self->{_pager} = dor(delete $args{pager}, $EMPTY);
# If we're using a pager or not outputing to a tty don't use colors.
# Can't use IO::Interactive here because we /don't/ want to check STDIN.
#-----------------------------------------------------------------------------
+sub warn_about_useless_no_critic {
+ my ($self) = @_;
+ return $self->{_warn_about_useless_no_critic};
+}
+
+#-----------------------------------------------------------------------------
+
sub force {
my ($self) = @_;
return $self->{_force};
Returns the default C<criticism-fatal> setting (Either 1 or 0).
+=item C< warn_about_useless_no_critic() >
+
+Returns the default C<warn-about-useless-no-critic> setting (Either 1 or 0).
=back
--- /dev/null
+#!perl
+
+##############################################################################
+# $URL$
+# $Date$
+# $Author$
+# $Revision$
+##############################################################################
+
+use 5.006001;
+use strict;
+use warnings;
+
+use Test::More (tests => 1);
+use Perl::Critic::PolicyFactory (test => 1);
+use Perl::Critic::Document;
+use PPI::Document;
+
+#-----------------------------------------------------------------------------
+
+my $violation_free_code = <<'END_PERL';
+
+$foo = 0; ## this line is not disabled
+
+## no critic; #\
+$foo = 1; # |-> this block counts as 3 disabled lines
+## use critic; #/
+
+$foo = 2; ## no critic;
+
+$foo = 3; ## no critic (MagicNumbers)
+
+sub foo { ## no critic (ExcessComplexity, BuiltinHomonyms)
+ return 1;
+}
+
+$foo = 5; ## this line is not disabled
+
+END_PERL
+
+my $ppi_doc = PPI::Document->new(\$violation_free_code);
+my $doc = Perl::Critic::Document->new($ppi_doc);
+
+my @site_policies = Perl::Critic::PolicyFactory::site_policy_names();
+$doc->mark_disabled_lines(@site_policies);
+
+my @empty_violation_list = ();
+my @got_warnings = $doc->useless_no_critic_warnings(@empty_violation_list);
+is(scalar @got_warnings, 7, 'Got correct numer of useless-no-critic warnings.');
+
+#-----------------------------------------------------------------------------
