Activate perlcritic policy Variables::ProhibitPackageVars

diff --git a/.perlcriticrc b/.perlcriticrc
index fc4d4cef470d47f85c47248ddc5f7ebe9903a478..11e6a0a3cb4b219b2297097cc4a60448ef7b5af2 100644 (file)
--- a/.perlcriticrc
+++ b/.perlcriticrc
@@ -46,10 +46,10 @@ verbose = %f: [%p] %m at line %l, column %c.\n
 [Subroutines::RequireArgUnpacking]
 short_subroutine_statements = 2
 
-# The advantages of 'use constant' greatly outweigh the
+# Completely Disagree: The advantages of 'use constant' greatly outweigh the
 # few disadvantages.  Perl::Tidy relies heavily on constants for efficient and
-# robust coding of array indexes and debug code, and to avoid autovivication
-# problems that would occur if hashes were used instead.
+# robust coding of array indexes and for compiling out debug code, and to
+# avoid autovivication problems that would occur if hashes were used instead.
 [-ValuesAndExpressions::ProhibitConstantPragma]
 
 # Completely Disagree: adding quotes on here doc terminators causes needless
@@ -57,7 +57,10 @@ short_subroutine_statements = 2
 # my editor uses color to make it clear if interpolation is in effect.
 [-ValuesAndExpressions::RequireQuotedHeredocTerminator]
 
-# Perlcritic doesn't know that ARGV in Perl::Tidy actually is localized
+# Perlcritic doesn't know that ARGV in Perl::Tidy actually is localized.
+# Localization of @ARGV could be avoided by calling GetOptionsFromArray
+# instead of GetOptions, but that is not available before perl 5.10, and
+# we want to continue supporting Perl 5.8.
 [-Variables::RequireLocalizedPunctuationVars]
 
 # Unfortunately the perlcritic coding for this policy is buggy when lines=n is
@@ -108,8 +111,8 @@ max_nests=9
 # substr.  So skip this one.
 [-BuiltinFunctions::ProhibitLvalueSubstr]
 
-# The Tokenizer.pm needs package variables
-[-Variables::ProhibitPackageVars]
+# OK to do this check:
+# [-Variables::ProhibitPackageVars]
 
 # These would be okay for new code, but do not change any debugged regular
 # expressions without good reason.  It is too easy to introduce a subtle error.

diff --git a/lib/Perl/Tidy/HtmlWriter.pm b/lib/Perl/Tidy/HtmlWriter.pm
index c190cd5ce6fbe52bff2f66cdad52d0d253208835..d7e45cc97f521fb2d0aceb232f91ddc7b87e0f1f 100644 (file)
--- a/lib/Perl/Tidy/HtmlWriter.pm
+++ b/lib/Perl/Tidy/HtmlWriter.pm
@@ -16,27 +16,38 @@ use constant EMPTY_STRING => q{};
 use constant SPACE        => q{ };
 
 # class variables
-use vars qw{
-  %html_color
-  %html_bold
-  %html_italic
-  %token_short_names
-  %short_to_long_names
-  $rOpts
-  $css_filename
-  $css_linkname
-  $missing_html_entities
-  $missing_pod_html
-};
+my (
+
+    # INITIALIZER: BEGIN block
+    $missing_html_entities,
+    $missing_pod_html,
+
+    # INITIALIZER: BEGIN block
+    %short_to_long_names,
+    %token_short_names,
+
+    # INITIALIZER: sub check_options
+    $rOpts,
+    $rOpts_html_entities,
+    $css_linkname,
+    %html_bold,
+    %html_color,
+    %html_italic,
+
+);
 
 # replace unsafe characters with HTML entity representation if HTML::Entities
 # is available
 #{ eval "use HTML::Entities"; $missing_html_entities = $@; }
 
 BEGIN {
+
+    $missing_html_entities = EMPTY_STRING;
     if ( !eval { require HTML::Entities; 1 } ) {
         $missing_html_entities = $EVAL_ERROR ? $EVAL_ERROR : 1;
     }
+
+    $missing_pod_html = EMPTY_STRING;
     if ( !eval { require Pod::Html; 1 } ) {
         $missing_pod_html = $EVAL_ERROR ? $EVAL_ERROR : 1;
     }
@@ -555,6 +566,7 @@ sub check_options {
     }
 
     # make sure user gives a file name after -css
+    $css_linkname = EMPTY_STRING;
     if ( defined( $rOpts->{'html-linked-style-sheet'} ) ) {
         $css_linkname = $rOpts->{'html-linked-style-sheet'};
         if ( $css_linkname =~ /^-/ ) {
@@ -578,21 +590,20 @@ sub check_options {
         # forgets to specify the style sheet, like this:
         #    perltidy -html -css myfile1.pl myfile2.pl
         # This would cause myfile1.pl to parsed as the style sheet by GetOpts
-        my $css_filename = $css_linkname;
-        unless ( -e $css_filename ) {
-            write_style_sheet_file($css_filename);
+        unless ( -e $css_linkname ) {
+            write_style_sheet_file($css_linkname);
         }
     }
-    $missing_html_entities = 1 unless $rOpts->{'html-entities'};
+    $rOpts_html_entities = $rOpts->{'html-entities'};
     return;
 } ## end sub check_options
 
 sub write_style_sheet_file {
 
-    my $css_filename = shift;
+    my $filename = shift;
     my $fh;
-    unless ( $fh = IO::File->new("> $css_filename") ) {
-        Perl::Tidy::Die("can't open $css_filename: $ERRNO\n");
+    unless ( $fh = IO::File->new("> $filename") ) {
+        Perl::Tidy::Die("can't open $filename: $ERRNO\n");
     }
     write_style_sheet_data($fh);
     close_object($fh);
@@ -1381,7 +1392,7 @@ sub markup_html_element {
 sub escape_html {
 
     my $token = shift;
-    if ($missing_html_entities) {
+    if ( $missing_html_entities || !$rOpts_html_entities ) {
         $token =~ s/\&/&/g;
         $token =~ s/\</&lt;/g;
         $token =~ s/\>/&gt;/g;