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

What should be the preferred approach to correcting 'bareword' exceptions under strict-by-default? #188

Open
jkeenan opened this issue Aug 11, 2020 · 1 comment
Labels
question Further information is requested or policy needs discussion

Comments

@jkeenan
Copy link
Collaborator

jkeenan commented Aug 11, 2020

In this ticket I raise the question of which of two (or perhaps more) approaches we should take to modifying unit tests in the Perl core distribution test suite as we try to upgrade it to run with "strict-by-default".

Let's start by looking at a simple data file and a simple Perl 5 program.

$ cat file-to-be-tied
alpha
beta
gamma

$ cat tie.pl
#!/usr/bin/env perl

use Tie::File;

$filename = './file-to-be-tied';
tie @array, Tie::File, $filename or die "Unable to tie to $filename";

print "$_\n" for @array;

untie @array;
print "Finished\n";

We run the program as is with Perl 5.

$ perl tie.pl
alpha
beta
gamma
Finished

But now we notice that we failed to include a use strict; statement. Let's modify the file to do only that.

$ cat tie-strict.pl 
#!/usr/bin/env perl
use strict;
use Tie::File;

$filename = './file-to-be-tied';
tie @array, Tie::File, $filename or die "Unable to tie to $filename";

print "$_\n" for @array;

untie @array;
print "Finished\n";

Now, if I asked you to identify the changes that would be needed in this file to bring it into compliance with strict, the first thing that you would be likely to say is: "Change the global variables to 'my' variables."

Okay, let's do that.

$ cat $ cat tie-first-attempt-at-correction.pl 
#!/usr/bin/env perl
use strict;
use Tie::File;

my($filename, @array);

$filename = './file-to-be-tied';
tie @array, Tie::File, $filename or die "Unable to tie to $filename";

print "$_\n" for @array;

untie @array;
print "Finished\n";

Let's try to compile that and see what happens.

$ perl -c tie-first-attempt-at-correction.pl 
Bareword "Tie::File" not allowed while "strict subs" in use at tie-first-attempt-at-correction.pl line 8.
tie-first-attempt-at-correction.pl had compilation errors.

So scoping the variables was necessary but not sufficient to achieve strict-compliance. Let's try the following.

$ cat tie-second-attempt-at-correction.pl
#!/usr/bin/env perl
use strict;
use Tie::File;

my($filename, @array);

$filename = './file-to-be-tied';
{
    no strict 'subs';
    tie @array, Tie::File, $filename or die "Unable to tie to $filename";
}

print "$_\n" for @array;

untie @array;
print "Finished\n";

Let's try to run this.

$ perl tie-second-attempt-at-correction.pl
alpha
beta
gamma
Finished

The program and compiles and runs as intended.

That's cool, but we could have taken a different approach to fixing the 'Bareword ... not allowed while "strict subs" in use' problem. We could have done this.

$ cat tie-third-attempt-at-correction.pl 
#!/usr/bin/env perl
use strict;
use Tie::File;

my($filename, @array);

$filename = './file-to-be-tied';
tie @array, 'Tie::File', $filename or die "Unable to tie to $filename";

print "$_\n" for @array;

untie @array;
print "Finished\n";

In the above, rather than placing the offending statement -- unaltered -- in a block and relaxing strict 'subs' for the duration of that block, we alter the offending statement by quoting the bareword.

Let's run this.

$ perl tie-third-attempt-at-correction.pl 
alpha
beta
gamma
Finished

The program compiles and runs as intended.

So, even within Perl 5, there are different ways to bring code into compliance with strictures.

By now you can guess where I'm going with this. In upgrading the core distribution test suite to run with strict compliance as per Objective 1, we have choices as to how to accomplish that.

Consider this Perl 7 program.

$ cat tie.p7.pl 
#!/usr/bin/env perl

use Tie::File;

$filename = './file-to-be-tied';
tie @array, Tie::File, $filename or die "Unable to tie to $filename";

print "$_\n" for @array;

untie @array;
print "Finished\n";

This is, of course, exactly the same program as the first one in this ticket, tie.pl. But if we run it against a Perl 7 executable, we get:

$ p7 tie.p7.pl
Global symbol "$filename" requires explicit package name (did you forget to declare "my $filename"?) at tie.p7.pl line 5.
Global symbol "@array" requires explicit package name (did you forget to declare "my @array"?) at tie.p7.pl line 6.
Global symbol "$filename" requires explicit package name (did you forget to declare "my $filename"?) at tie.p7.pl line 6.
Global symbol "$filename" requires explicit package name (did you forget to declare "my $filename"?) at tie.p7.pl line 6.
Global symbol "@array" requires explicit package name (did you forget to declare "my @array"?) at tie.p7.pl line 8.
Global symbol "@array" requires explicit package name (did you forget to declare "my @array"?) at tie.p7.pl line 10.
Bareword "Tie::File" not allowed while "strict subs" in use at tie.p7.pl line 6.
Execution of tie.p7.pl aborted due to compilation errors.

Which, of course, is the same error output we would have gotten by running
perl -Mstrict tie.pl.

And, just as in Perl 5, we have two different ways of fixing the problem.
First:

$ cat tie.no-strict-subs.p7.pl 
#!/usr/bin/env perl

use Tie::File;

my($filename, @array);

$filename = './file-to-be-tied';
{
    no strict 'subs';
    tie @array, Tie::File, $filename or die "Unable to tie to $filename";
}

print "$_\n" for @array;

untie @array;
print "Finished\n";

$ perl tie.no-strict-subs.p7.pl 
alpha
beta
gamma
Finished

Second:

$ cat tie.quote-bareword.p7.pl 
#!/usr/bin/env perl

use Tie::File;

my($filename, @array);

$filename = './file-to-be-tied';
tie @array, 'Tie::File', $filename or die "Unable to tie to $filename";

print "$_\n" for @array;

untie @array;
print "Finished\n";

$ perl tie.quote-bareword.p7.pl 
alpha
beta
gamma
Finished

For the past five weeks of work on the test suite, I have usually taken the first approach to correction, i.e., I have been placing the offending statement unaltered into a code block and starting that code block off with no strict 'subs';. I did so for two reasons: (1) because that's what the error message prompts me to do; and (2) because I figured I'd get less pushback from the developer who originally wrote the code way back when in a no strict environment.

But now I'm thinking that the Perl 7 test suite code would be cleaner and easier for a newcomer to understand if I corrected that source code that gave rise to the "bareword" exception in the first place, rather than saying (in effect), "Let's revert to 1994 defaults for just this block."

In working on the test suite, we'll face this dilemma repeatedly. For example, when we get to Objective 3: Warnings by Default, we'll be faced with this situation repeatedly. For example, when getting an "uninitialized value" warning, do we resolve the problem by (1) placing the offending statement unaltered into a block and then opening that block with no warnings 'uninitialized';; or by (2) initializing the variable that was cited in the warning.

So, my immediate question for you is, in addressing 'bareword' exceptions now in Objective 1 (and assuming both approaches are feasible), should we prefer suspending strict 'subs' for a block or putting clothing on the bareword.

@atoomic, @toddr, @xsawyerx: I'd appreciate your feedback at your earliest opportunity, as we need to provide guidance for new recruits who may start looking at the code base as early as tomorrow (Aug 12).

Thank you very much.
Jim Keenan

@atoomic
Copy link
Owner

atoomic commented Aug 11, 2020

@jkeenan IMO tie @array, 'Tie::File', $filename is the correct way to fix the strict issue.

@jkeenan jkeenan added the question Further information is requested or policy needs discussion label Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested or policy needs discussion
Projects
None yet
Development

No branches or pull requests

2 participants