Browse Source

Say that inlining a long single-use sub would overcomplicate its caller

The annotations said the body "would bury" the caller, which is an odd
way to put it.  The reason is that the caller would get harder to read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
troglodyne-bot 6 days ago
parent
commit
cd9cc4c144

+ 1 - 1
bin/favicon_mongler.pl

@@ -50,7 +50,7 @@ extension.  The real container is assembled with Imager afterwards.
 # and 1.x errors out on -e rather than quietly accepting it.  Ask the binary
 # which era it belongs to instead of guessing -- this script has already been
 # flipped between the two by hand twice (e23b678, then back in 5f0cdca).
-my sub export_flag ($bin) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 14 lines, which would bury the setup at file scope
+my sub export_flag ($bin) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 14 lines, which would overcomplicate the setup at file scope
 
     # Shelling out, on purpose and on the record.  system() cannot hand back
     # what the child printed, and reaching for a pipe open instead would only

+ 1 - 1
bin/tcms-useradd

@@ -88,7 +88,7 @@ my sub read_password ($prompt) {
 
 # What a brand new user's profile post looks like before we merge anything over
 # the top of it.  Mirrors what setup() builds for the very first user.
-my sub default_user_post {    ## no critic (ProhibitSingleUseLexicalSubs) -- 14 lines, which would bury main()
+my sub default_user_post {    ## no critic (ProhibitSingleUseLexicalSubs) -- 14 lines, which would overcomplicate main()
     return {
         data       => 'Default user',
         preview    => '/img/avatar/humm.gif',

+ 1 - 1
lib/TCMS.pm

@@ -120,7 +120,7 @@ sub build_routes {
         my $callback = delete $v->{callback};
 
         #XXX For now we will discard the tPSGI object.  We might want it later.
-        my $cb_wrap = sub {    ## no critic (ProhibitSingleUseLexicalSubs) -- 71 lines, which would bury build_routes()
+        my $cb_wrap = sub {    ## no critic (ProhibitSingleUseLexicalSubs) -- 71 lines, which would overcomplicate build_routes()
             my ( $tpsgi, $query ) = @_;
 
             confess "Cannot determine directory for logs!" unless $tpsgi->{log_dir};

+ 2 - 2
lib/Trog/Component.pm

@@ -50,7 +50,7 @@ our $error;
 # Resolved at call time rather than compile time, so that components are free to
 # use Trog::Renderer without cycling back through us.  require() memoizes in
 # %INC; %loaded only saves us the repeat eval and string munging.
-my sub load ($name) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 22 lines, which would bury _component()
+my sub load ($name) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 22 lines, which would overcomplicate _component()
     state %loaded;
 
     die "Component name must be given\n"   unless $name;
@@ -73,7 +73,7 @@ my sub load ($name) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 22 lines
     return $module;
 }
 
-my sub _component ( $name, $args ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 25 lines, which would bury component()
+my sub _component ( $name, $args ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 25 lines, which would overcomplicate component()
     my $module = load($name);
 
     die "Component '$name' recursed more than $MAX_DEPTH deep\n" if $depth >= $MAX_DEPTH;

+ 4 - 4
lib/Trog/Data/SQLite.pm

@@ -115,7 +115,7 @@ or two character search matches most of the site anyway.
 
 =cut
 
-my sub like_clause ($term) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 12 lines, which would bury where()
+my sub like_clause ($term) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 12 lines, which would overcomplicate where()
 
     if ( length($term) < 3 ) {
         my $escaped = $term;
@@ -130,7 +130,7 @@ my sub like_clause ($term) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 1
 
 # filter(), clause for clause.  Read the two side by side when changing either.
 # Returns nothing when no post can match, and then there is no query to run.
-my sub where ($query) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 77 lines, which would bury build_query()
+my sub where ($query) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 77 lines, which would overcomplicate build_query()
     my ( @where, @bind );
 
     # Which versions are in play.  Everything else filters within that.
@@ -208,7 +208,7 @@ my sub where ($query) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 77 lin
     return ( \@where, \@bind );
 }
 
-my sub build_query ($query) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 41 lines, which would bury read()
+my sub build_query ($query) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 41 lines, which would overcomplicate read()
     my $select = q{SELECT p.post_data, p.uuid, p.version, v.version_max, v.first_created, f.author AS first_author};
 
     my ( $where, $bind ) = where($query) or return;
@@ -297,7 +297,7 @@ sub read ( $, $query = {} ) {
     return \@posts;
 }
 
-my sub insert ( $dbh, $data ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 21 lines, which would bury write()
+my sub insert ( $dbh, $data ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 21 lines, which would overcomplicate write()
     my $insert = $dbh->prepare('INSERT INTO posts (post_data) VALUES (?)');
 
     foreach my $post (@$data) {

+ 2 - 2
lib/Trog/DataModule.pm

@@ -458,7 +458,7 @@ our %relation_schema = ( type => 'string' );
 # string-or-hashref dance every upload field would otherwise need.  Uploads are
 # a hashref on the way in from the browser and the href string they became on
 # the way back out of the datastore.
-my sub expand_pseudo_types ($root) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 26 lines, which would bury read_sidecar()
+my sub expand_pseudo_types ($root) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 26 lines, which would overcomplicate read_sidecar()
     my %pseudo = (
         upload   => \%upload_schema,
         relation => \%relation_schema,
@@ -574,7 +574,7 @@ sub include_schema_for ( $include = '' ) {
     return $cache{$generation}{$name};
 }
 
-my sub includes_of ($type) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 17 lines, which would bury includes_for()
+my sub includes_of ($type) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 17 lines, which would overcomplicate includes_for()
     my $path     = Trog::Themes::themed_file_in_dir( 'forms', "$type.tx", 'text/html', 1 );
     my $template = $path ? eval { File::Slurper::read_text($path) } : undef;
 

+ 3 - 3
lib/Trog/DataSource/DirIndex.pm

@@ -107,7 +107,7 @@ my sub within ( $path, $within ) {
 # Everything this is allowed to look at, or nothing.  Returns the absolute path
 # and the path relative to the tCMS root, since the first is what we read and
 # the second is what we log and watch.
-my sub resolve ($directory) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 46 lines, which would bury posts()
+my sub resolve ($directory) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 46 lines, which would overcomplicate posts()
     return undef unless $directory;
 
     # Before abs_path, which would happily resolve one for us.
@@ -154,7 +154,7 @@ my sub resolve ($directory) {    ## no critic (ProhibitSingleUseLexicalSubs) --
     return { absolute => $absolute, relative => $relative ne q{} ? "$root/$relative" : $root };
 }
 
-my sub human_size ($bytes) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 12 lines, which would bury post_for()
+my sub human_size ($bytes) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 12 lines, which would overcomplicate post_for()
     return '' unless defined $bytes;
     return "$bytes B" if $bytes < 1024;
 
@@ -167,7 +167,7 @@ my sub human_size ($bytes) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 1
     return "$bytes B";
 }
 
-my sub post_for ( $name, $dir, $series ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 76 lines, which would bury posts()
+my sub post_for ( $name, $dir, $series ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 76 lines, which would overcomplicate posts()
     my $absolute = "$dir->{absolute}/$name";
 
     # Resolved, so a symlink pointing out of the tree is left out rather than

+ 2 - 2
lib/Trog/DataSource/ProvisionedVirt.pm

@@ -104,7 +104,7 @@ my sub _recipe_path ( $config, $domain ) {
     return -f $path ? $path : undef;    ## no critic (ProhibitFiletest_f) -- is there a recipe for this guest
 }
 
-my sub with_recipe ( $post, $config ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 49 lines, which would bury posts()
+my sub with_recipe ( $post, $config ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 49 lines, which would overcomplicate posts()
     return $post if $post->{unreachable};
 
     my $domain = $post->{domain} // '';
@@ -373,7 +373,7 @@ The reason, when there is one, is what the person reads -- so it has to tell
 
 =cut
 
-my sub credentials (%args) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 58 lines, which would bury reprovision()
+my sub credentials (%args) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 58 lines, which would overcomplicate reprovision()
     my $user = $args{user} // '';
 
     my %credentials;

+ 1 - 1
lib/Trog/DataSource/Virt.pm

@@ -109,7 +109,7 @@ my @states = (
 # to be a different host without having to lie to Sys::Hostname.
 our $hostname;
 
-my sub hostname {    ## no critic (ProhibitSingleUseLexicalSubs) -- 14 lines, which would bury is_self()
+my sub hostname {    ## no critic (ProhibitSingleUseLexicalSubs) -- 14 lines, which would overcomplicate is_self()
     return $hostname if defined $hostname;
 
     local $@;

+ 11 - 11
lib/Trog/Routes/HTML.pm

@@ -447,7 +447,7 @@ sub feedback_from_cookie ($cookies) {
     return ( int($failure), URI::Escape::uri_unescape($message) );
 }
 
-my sub build_social_meta ( $query, $title ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 55 lines, which would bury index()
+my sub build_social_meta ( $query, $title ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 55 lines, which would overcomplicate index()
     return ( undef, undef, undef ) unless $query->{social_meta} && $query->{route} && $query->{domain};
 
     my $default_tags = $Theme::default_tags;
@@ -503,7 +503,7 @@ my sub build_social_meta ( $query, $title ) {    ## no critic (ProhibitSingleUse
     return ( $default_tags, $meta_desc, $meta_tags );
 }
 
-my sub get_series ( $edit = 0 ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 13 lines, which would bury index()
+my sub get_series ( $edit = 0 ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 13 lines, which would overcomplicate index()
     state $data;
     $data //= Trog::Data->new( Trog::Config::get() );
 
@@ -659,7 +659,7 @@ sub totp ($query) {
     );
 }
 
-my sub setup_initial_db ( $dat, $user, $display_name, $contact_email ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 72 lines, which would bury login()
+my sub setup_initial_db ( $dat, $user, $display_name, $contact_email ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 72 lines, which would overcomplicate login()
     $dat->add(
         {
             "aclname"    => "series",
@@ -1360,7 +1360,7 @@ sub post_delete ($query) {
 # Anything a type needs computed rather than merely fetched.  The sub is named
 # in the sidecar, so validate it the same way a post's own callback field is --
 # a sidecar naming an arbitrary sub is a privilege question, not a typo.
-my sub enrich_callback ( $post, $query ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 33 lines, which would bury enrich_post()
+my sub enrich_callback ( $post, $query ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 33 lines, which would overcomplicate enrich_post()
     my $meta = Trog::DataModule::type_meta_for( $post->{form} );
     my $sub  = $meta->{'x-tcms-post-type'}{enrich};
     return if !$sub || ref $sub;
@@ -1617,7 +1617,7 @@ my sub post2rss ( $rss, $url, $post ) {
     return 1;
 }
 
-my sub _rss ( $query, $subtitle, $posts ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 49 lines, which would bury posts()
+my sub _rss ( $query, $subtitle, $posts ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 49 lines, which would overcomplicate posts()
 
     require XML::RSS;
     my $rss  = XML::RSS->new( version => '2.0', stylesheet => '/styles/rss-style.xsl' );
@@ -2697,7 +2697,7 @@ point, works, and is only missing an optimization.
 
 =cut
 
-my sub wizard_reindex ( $name, $fields ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 22 lines, which would bury post_wizard_save()
+my sub wizard_reindex ( $name, $fields ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 22 lines, which would overcomplicate post_wizard_save()
     my %indexed = map { $_->{name} => 1 } grep { $_->{indexed} } @$fields;
 
     my $this_type = "$name.tx";
@@ -2732,7 +2732,7 @@ my sub wizard_fail ( $query, $message ) {
     );
 }
 
-my sub wizard_field_html ($field) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 23 lines, which would bury wizard_template()
+my sub wizard_field_html ($field) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 23 lines, which would overcomplicate wizard_template()
     my ( $n, $t, $l, $p ) = @$field{qw{name type label placeholder}};
     my $required = $field->{required} ? 'required ' : '';
 
@@ -2763,7 +2763,7 @@ a display half guarded by !$post.addpost, and an edit half guarded by $can_edit.
 
 =cut
 
-my sub wizard_template ( $query, $fields, $includes, $body_form, $datasource = '' ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 54 lines, which would bury post_wizard_save()
+my sub wizard_template ( $query, $fields, $includes, $body_form, $datasource = '' ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 54 lines, which would overcomplicate post_wizard_save()
     my $placeholder = kolon_safe( $query->{title_placeholder} ) || 'Iowa Man Destroys Moon';
 
     # NOT escaped, on purpose.  Letting an admin write template code is the
@@ -2955,7 +2955,7 @@ sub short_error ( $error = '' ) {
 # sidecar as the fallback rather than making it a second opinion.  The template
 # is what actually renders, so where the two disagree it is the template which
 # is telling the truth.
-my sub wizard_display_of ($form) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 26 lines, which would bury wizard_types()
+my sub wizard_display_of ($form) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 26 lines, which would overcomplicate wizard_types()
     my $path = Trog::Themes::themed_file_in_dir( 'forms', $form, 'text/html', 1 );
     return undef unless $path;
 
@@ -2983,7 +2983,7 @@ my sub wizard_display_of ($form) {    ## no critic (ProhibitSingleUseLexicalSubs
 }
 
 # The custom fields of a type, as rows the wizard would have submitted.
-my sub wizard_fields_of ( $schema, $meta, $includes = {} ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 72 lines, which would bury wizard_types()
+my sub wizard_fields_of ( $schema, $meta, $includes = {} ) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 72 lines, which would overcomplicate wizard_types()
     my $properties = $schema->{properties} // {};
     my %required   = map { $_ => 1 } @{ $schema->{required} // [] };
     my $relations  = $meta->{'x-tcms-relations'};
@@ -3068,7 +3068,7 @@ there is no wizard form that produces one.
 
 =cut
 
-my sub wizard_display_from_template ($form) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 33 lines, which would bury wizard_types()
+my sub wizard_display_from_template ($form) {    ## no critic (ProhibitSingleUseLexicalSubs) -- 33 lines, which would overcomplicate wizard_types()
     my $path = Trog::Themes::themed_file_in_dir( 'forms', $form, 'text/html', 1 );
     return '' unless $path;
 

+ 1 - 1
lib/Trog/Vault.pm

@@ -130,7 +130,7 @@ our $looked_further = 0;
 # The two places worth looking that are not the environment.  Deferred rather
 # than done at load, since both are files and this module is loaded by things
 # which have no interest in a vault.
-my sub look_further {    ## no critic (ProhibitSingleUseLexicalSubs) -- 21 lines, which would bury key()
+my sub look_further {    ## no critic (ProhibitSingleUseLexicalSubs) -- 21 lines, which would overcomplicate key()
     $looked_further = 1;
 
     my $dir = $ENV{CREDENTIALS_DIRECTORY};

+ 1 - 1
t/Trog-Routes-HTML-series-lifecycle.t

@@ -70,7 +70,7 @@ state sub snapshot {
     return %seen;
 }
 
-state sub copy_tree {    ## no critic (ProhibitSingleUseLexicalSubs) -- 18 lines, which would bury the BEGIN block
+state sub copy_tree {    ## no critic (ProhibitSingleUseLexicalSubs) -- 18 lines, which would overcomplicate the BEGIN block
     my ( $from, $to ) = @_;
     File::Path::make_path($to);
     File::Find::find(