NAME

Perl::Critic::Policy::Subroutines::RequireSubOrder - Place subroutines in dependency order

DESCRIPTION

Subroutine dependencies are easier to manage when they are defined before they are called. By organizing subroutines with the lower-level helpers appearing first (at the top), and the calling subroutines after (at the bottom), the code will be easier to understand and maintain. This follows the common pattern of indicating module and helper imports first:

use Helpers qw/some functions/;
sub one { return some()+functions() }
sub two { return one() }
... script that calls two()
exit();

The reverse order, referred to here as backwards, does function because Perl's compile step can (usually) find the subroutine definitions:

... script that calls two()
exit();
sub two { return one() }
sub one { return some()+functions() }
use Helpers qw/some functions/;

The inside-out order may, therefore, be the most confusing but is commonly encountered:

use Helpers qw/some functions/;
... script that calls two()
exit();
sub two { return one() }
sub one { return some()+functions() }

This policy enforces the first form, similar to a topological sort, with dependency groups appearing first and their calling subroutines appearing later.

Motivation

State and Globals

A common pattern is to declare an outer my variable as a file or package-level state variable that adheres to the subroutine(s) where it's used:

my $value=1;
sub one { return $value }

sub main { return one() }

print main(),"\n";
exit();

Suppose, however, that the "main program" is given priority at the top of the script:

print main(),"\n";
exit();

my $value=1;
sub one { return $value }

sub main { return one() }

This compiles but gives a runtime warning, "Use of uninitialized value". RequireSubOrder will report that these subroutines (eg main) need to be moved before their callers.

Note that in scripts with large support functions, related failures can be difficult to identify when not following the packages-dependencies-script pattern.

Optional parentheses

As noted in perlsub, parentheses are optional "if predeclared/imported". This works:

sub one { return $_[0]+$_[1] }
sub two { return one 2,3 }

This does not, however:

sub two { return one 2,3 }
sub one { return $_[0]+$_[1] }

As noted in perldiag, the compiler can guess the intent and suggests "Do you need to predeclare one?", but the issue may depend on a missing import instead of a declaration or definition. RequireSubOrder catches this issue early by insisting on the first form.

Prototypes

Subroutine calls can enforce declared prototypes:

sub one($$) { return $_[0]+$_[1] }
sub two { return one(2,3,4) }

-> Too many arguments for main::one

Backwards declaration cannot, however:

sub two { return one(2,3,4) }
sub one($$) { return $_[0]+$_[1] }
print two(),"\n";

-> 5

Inlining

Declared subroutines may be inlined:

sub one() {1}
sub two { return one() }
print two(),"\n"

This may be inspected with B::Deparse:

sub one () {
  1;
}
sub two {
  return 1;
}
print two(), "\n";

With backwards definitions:

sub two { return one() }
sub one() {1}
print two(),"\n"

The code still runs and prints "1", but it is not inlined:

sub two {
  return &one();
}
print two(), "\n";
sub one () { 1 }

Supported behaviors

* Supports forward declarations.

* Supports inline package Pkg; ...; package main; ... declarations.

* Supports {package Pkg; ...} declarations inside blocks.

* Supports package Pkg {...} block declarations.

* Supports package-prefixed calls.

* Reports cycles.

* Reports definition-after-use in scripts.

* Ignores undeclared functions, which are assumed to be imported.

CONFIGURATION

All dependencies

By default, only the latest dependency is given for a misplaced subroutine to appear after. To report all names a subroutine must appear after:

[Subroutines::RequireSubOrder]
all = 1

Backwards ordering

The default ordering is topological for the reasons provided above. To instead apply a model where "all the dependent calls made by the subroutine appear after", with the expectation that "I shall responsibly forward declare all functions+prototypes":

[Subroutines::RequireSubOrder]
backwards = 1

Todo

* Consider sorting of _private versus public names.

* Consider alphabetical ordering within the same group.

BUGS

* Obviously!

* Lexical subroutines may misreport. Needs investigation.