diff --git a/modules/Bio/EnsEMBL/BaseAlignFeature.pm b/modules/Bio/EnsEMBL/BaseAlignFeature.pm index 335e52704556b2d6dc19015675e243224fb1ba6d..16aa70c9ad935f65f93cc32f716953c04e68c6db 100644 --- a/modules/Bio/EnsEMBL/BaseAlignFeature.pm +++ b/modules/Bio/EnsEMBL/BaseAlignFeature.pm @@ -623,7 +623,7 @@ sub _parse_features { my $hstrand = $f[0]->hstrand; my $slice = $f[0]->slice(); my $hslice = $f[0]->hslice(); - my $name = $slice->name() if($slice); + my $name = $slice ? $slice->name() : undef; my $hname = $f[0]->hseqname; my $score = $f[0]->score; my $percent = $f[0]->percent_id; diff --git a/modules/Bio/EnsEMBL/ChainedAssemblyMapper.pm b/modules/Bio/EnsEMBL/ChainedAssemblyMapper.pm index b673bb1d36cbeef2ed168198743371033687b753..119f2d6c9e7af130b942c8029c07279c47274d7e 100644 --- a/modules/Bio/EnsEMBL/ChainedAssemblyMapper.pm +++ b/modules/Bio/EnsEMBL/ChainedAssemblyMapper.pm @@ -70,11 +70,6 @@ normal assembly mapper. =cut - -my $FIRST = 'first'; -my $MIDDLE = 'middle'; -my $LAST = 'last'; - package Bio::EnsEMBL::ChainedAssemblyMapper; use strict; @@ -86,6 +81,10 @@ use Bio::EnsEMBL::Mapper::RangeRegistry; use Bio::EnsEMBL::Utils::Exception qw(throw deprecate); use Scalar::Util qw(weaken); +my $FIRST = 'first'; +my $MIDDLE = 'middle'; +my $LAST = 'last'; + #2^20 = approx 10^6 my $CHUNKFACTOR = 20; diff --git a/modules/Bio/EnsEMBL/DB/ExternalFeatureFactoryI.pm b/modules/Bio/EnsEMBL/DB/ExternalFeatureFactoryI.pm index 74481cfd653202fbdd95dc6df171a2ce58b4416e..8c549dd6c606245440c4373cb2bb971ff279fcaf 100755 --- a/modules/Bio/EnsEMBL/DB/ExternalFeatureFactoryI.pm +++ b/modules/Bio/EnsEMBL/DB/ExternalFeatureFactoryI.pm @@ -131,6 +131,10 @@ above. =cut package Bio::EnsEMBL::DB::ExternalFeatureFactoryI; + +use strict; +use warnings; + use Bio::EnsEMBL::External::ExternalFeatureAdaptor; use vars qw(@ISA); diff --git a/modules/Bio/EnsEMBL/DBFile/FileAdaptor.pm b/modules/Bio/EnsEMBL/DBFile/FileAdaptor.pm index 89b40066b5fbeed445dbf6392ec0f1ff0902f251..fe9a1aab2905b73d914f06a56ca17b2d6969b0b6 100755 --- a/modules/Bio/EnsEMBL/DBFile/FileAdaptor.pm +++ b/modules/Bio/EnsEMBL/DBFile/FileAdaptor.pm @@ -126,8 +126,7 @@ sub open_file{ throw("Cannot perform open with unsupported operator:\t${file_op}${filepath}"); } - my $fh; - my $success = open($fh, "${file_op}${filepath}"); + my $success = open my $fh, $file_op, $filepath; #$fh will be still be GLOB on fail #These warn instead of throw/die to allow diff --git a/modules/Bio/EnsEMBL/DBSQL/BaseAdaptor.pm b/modules/Bio/EnsEMBL/DBSQL/BaseAdaptor.pm index d2f514093bb4ad3f1e1a685d3ac33197897ae7f0..634dac750e5287a1ab59d80dbeadf12ad4d90dbf 100755 --- a/modules/Bio/EnsEMBL/DBSQL/BaseAdaptor.pm +++ b/modules/Bio/EnsEMBL/DBSQL/BaseAdaptor.pm @@ -1003,7 +1003,7 @@ sub get_dumped_data { my $data = shift; $data =~ s/\n|\r|\f|\\//g; - return eval ($data); + return eval ($data); ## no critic } diff --git a/modules/Bio/EnsEMBL/DBSQL/GeneAdaptor.pm b/modules/Bio/EnsEMBL/DBSQL/GeneAdaptor.pm index 2d390514b70cdbf18f38198a8cfc58b359b7d3b8..c1c67ea1b8d5d4654ed6acdcc88b0b9fa6f204b4 100644 --- a/modules/Bio/EnsEMBL/DBSQL/GeneAdaptor.pm +++ b/modules/Bio/EnsEMBL/DBSQL/GeneAdaptor.pm @@ -515,12 +515,11 @@ sub fetch_all_by_Slice_and_external_dbname_link { my $dbe_adaptor = $self->db()->get_DBEntryAdaptor(); my %linked_genes; - foreach $external_db_id (@external_db_ids) { - my @linked_genes = $dbe_adaptor->list_gene_ids_by_external_db_id($external_db_id); - - foreach my $gene_id (@linked_genes) { - $linked_genes{$gene_id} = 1; - } + foreach my $local_external_db_id (@external_db_ids) { + my @linked_genes = $dbe_adaptor->list_gene_ids_by_external_db_id($local_external_db_id); + foreach my $gene_id (@linked_genes) { + $linked_genes{$gene_id} = 1; + } } # Get all the genes on the slice. @@ -1878,9 +1877,11 @@ sub fetch_all_by_exon_supporting_evidence { throw("feature type must be dna_align_feature or protein_align_feature"); } - my $anal_from = ", analysis a " if ($analysis); - my $anal_where = "AND a.analysis_id = f.analysis_id AND a.analysis_id=? " - if ($analysis); + my ($anal_from, $anal_where); + if($analysis) { + $anal_from = ", analysis a "; + $anal_where = "AND a.analysis_id = f.analysis_id AND a.analysis_id=? "; + } my $sql = qq( SELECT DISTINCT(g.gene_id) @@ -1943,9 +1944,11 @@ sub fetch_all_by_transcript_supporting_evidence { throw("feature type must be dna_align_feature or protein_align_feature"); } - my $anal_from = ", analysis a " if ($analysis); - my $anal_where = "AND a.analysis_id = f.analysis_id AND a.analysis_id=? " - if ($analysis); + my ($anal_from, $anal_where); + if($analysis) { + $anal_from = ", analysis a "; + $anal_where = "AND a.analysis_id = f.analysis_id AND a.analysis_id=? "; + } my $sql = qq( SELECT DISTINCT(g.gene_id) diff --git a/modules/Bio/EnsEMBL/External/BlastAdaptor.pm b/modules/Bio/EnsEMBL/External/BlastAdaptor.pm index 7288ccc5d1415b58219d2a737312c52f3e9c2922..1f44c7c865382e497158dbb5581cda6f00db2ad8 100644 --- a/modules/Bio/EnsEMBL/External/BlastAdaptor.pm +++ b/modules/Bio/EnsEMBL/External/BlastAdaptor.pm @@ -847,9 +847,9 @@ sub get_all_SearchFeatures { sub dynamic_use { my( $self, $classname ) = @_; my( $parent_namespace, $module ) = $classname =~/^(.*::)(.*?)$/; - no strict 'refs'; + no strict 'refs'; ## no critic return 1 if $parent_namespace->{$module.'::'}; # return if already used - eval "require $classname"; + eval "require $classname"; ## no critic if($@) { warn "DrawableContainer: failed to use $classname\nDrawableContainer: $@"; return 0; diff --git a/modules/Bio/EnsEMBL/IdMapping/ExonScoreBuilder.pm b/modules/Bio/EnsEMBL/IdMapping/ExonScoreBuilder.pm index e35a31432e2f287ef84362c76d2487b7959fb708..96d96b5bca19a8015621a1c9b4d411502cd22b9d 100644 --- a/modules/Bio/EnsEMBL/IdMapping/ExonScoreBuilder.pm +++ b/modules/Bio/EnsEMBL/IdMapping/ExonScoreBuilder.pm @@ -343,6 +343,7 @@ sub sort_exons { my $self = shift; my $exons = shift; + ## no critic return sort { ( $a->[0]->common_sr_name cmp $b->[0]->common_sr_name ) || ( $a->[1] <=> $b->[1] ) @@ -497,7 +498,7 @@ sub run_exonerate { $self->conf()->param('lsf_opt_exonerate') ); local *BSUB; - open( BSUB, $bsub_cmd ) + open( BSUB, $bsub_cmd ) ## no critic or $self->logger->error("Could not open open pipe to bsub: $!\n"); print BSUB $exonerate_job; @@ -688,11 +689,11 @@ sub parse_exonerate_results { $num_files++; - open( F, '<', "$dump_path/$file" ); + open( my $fh, '<', "$dump_path/$file" ); my $threshold = $self->conf->param('exonerate_threshold') || 0.5; - while (<F>) { + while (<$fh>) { $num_lines++; chomp; diff --git a/modules/Bio/EnsEMBL/IdMapping/StableIdGenerator/AnophelesGambiae.pm b/modules/Bio/EnsEMBL/IdMapping/StableIdGenerator/AnophelesGambiae.pm index d0ed7ab172929acfc30704031c2f496ee3ef5670..6f86f0b600e70c3ef20ccbd276ce3bbc3898f110 100644 --- a/modules/Bio/EnsEMBL/IdMapping/StableIdGenerator/AnophelesGambiae.pm +++ b/modules/Bio/EnsEMBL/IdMapping/StableIdGenerator/AnophelesGambiae.pm @@ -18,7 +18,7 @@ =cut -package Bio::EnsEMBL::IdMapping::StableIdGenerator::AedesAegypti; +package Bio::EnsEMBL::IdMapping::StableIdGenerator::AnophelesGambiae; # Package that implements incrementing and verification of Aedes aegypti # stable IDs as used by the VectorBase project. diff --git a/modules/Bio/EnsEMBL/IdMapping/SyntenyFramework.pm b/modules/Bio/EnsEMBL/IdMapping/SyntenyFramework.pm index 24c2461e57a5178b305262ae50148628a4bb6753..659b683e97b9ecdb9f2351b971ce00fdd674904a 100644 --- a/modules/Bio/EnsEMBL/IdMapping/SyntenyFramework.pm +++ b/modules/Bio/EnsEMBL/IdMapping/SyntenyFramework.pm @@ -390,7 +390,7 @@ sub rescore_gene_matrix_lsf { $self->logger->debug("$cmd\n\n"); local *BSUB; - open( BSUB, $bsub_cmd ) + open( BSUB, $bsub_cmd ) ## no critic or $self->logger->error("Could not open open pipe to bsub: $!\n"); print BSUB $cmd; diff --git a/modules/Bio/EnsEMBL/IndividualSlice.pm b/modules/Bio/EnsEMBL/IndividualSlice.pm index 8a08a2433c7a15b95e962a9a7406b11ce5ff71f1..7d17892d7986175473cdf3183e8e91284f3b1a1b 100644 --- a/modules/Bio/EnsEMBL/IndividualSlice.pm +++ b/modules/Bio/EnsEMBL/IndividualSlice.pm @@ -169,7 +169,8 @@ sub seq { # sort edits in reverse order to remove complication of # adjusting downstream edits - my @allele_features_ordered = sort {$b->start() <=> $a->start() || $b->end() <=> $a->end()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); + my @allele_features_ordered; + @allele_features_ordered = sort {$b->start() <=> $a->start() || $b->end() <=> $a->end()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); foreach my $af (@allele_features_ordered){ $af->apply_edit($reference_sequence); #change, in the reference sequence, the af @@ -343,7 +344,8 @@ sub mapper{ my $mapper = Bio::EnsEMBL::Mapper->new('Slice','IndividualSlice'); #align with Slice #get all the VariationFeatures in the Individual Slice, from start to end in the Slice - my @allele_features_ordered = sort {$a->start() <=> $b->start() || $b->end() <=> $a->end()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); + my @allele_features_ordered; + @allele_features_ordered = sort {$a->start() <=> $b->start() || $b->end() <=> $a->end()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); my $start_slice = 1; my $end_slice; @@ -485,7 +487,8 @@ sub subseq { #apply all differences to the reference sequence # sort edits in reverse order to remove complication of # adjusting downstream edits - my @allele_features_ordered = sort {$b->start() <=> $a->start() || $b->end() <=> $a->end()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); + my @allele_features_ordered; + @allele_features_ordered = sort {$b->start() <=> $a->start() || $b->end() <=> $a->end()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); my $af_start; my $af_end; foreach my $af (@allele_features_ordered){ diff --git a/modules/Bio/EnsEMBL/PaddedSlice.pm b/modules/Bio/EnsEMBL/PaddedSlice.pm index 60307851adce4c7337ad4f390e7c8da5af7b99f4..747f869f4f65397d96efe6de019a4e3b2b0995d5 100644 --- a/modules/Bio/EnsEMBL/PaddedSlice.pm +++ b/modules/Bio/EnsEMBL/PaddedSlice.pm @@ -36,6 +36,9 @@ is carried out using C<subseq()> calls. package Bio::EnsEMBL::PaddedSlice; +use strict; +use warnings; + use Bio::EnsEMBL::Utils::Argument qw/rearrange/; use Bio::EnsEMBL::Utils::Scalar qw/assert_ref assert_strand/; use base qw/Bio::EnsEMBL::Utils::Proxy/; @@ -162,11 +165,11 @@ sub subseq { #Return if we were upstream of overlap if($start < $parent_start && $end < $parent_start) { - return N x (( $end - $start )+1); + return 'N' x (( $end - $start )+1); } #Return if we were downstream of overlap if($start > $parent_end && $end > $parent_end) { - return N x (( $end - $start )+1); + return 'N' x (( $end - $start )+1); } my $prefix = ''; @@ -174,11 +177,11 @@ sub subseq { my $subslice_start = ($start - $parent_start)+1; my $subslice_end = ($end - $parent_start) + 1; if($start < $parent_start) { - $prefix = N x ($parent_start - $start); + $prefix = 'N' x ($parent_start - $start); $subslice_start = 1; } if($end > $parent_end) { - $suffix = N x ($end - $parent_end); + $suffix = 'N' x ($end - $parent_end); $subslice_end = (($parent_end - $parent_start)+1); } diff --git a/modules/Bio/EnsEMBL/Pipeline/ChecksumGenerator.pm b/modules/Bio/EnsEMBL/Pipeline/ChecksumGenerator.pm index 430a905e87497bee9bea6ee86c7bf31d4ef4046e..133a2e495737ec8441b54e08cfdf9912c64fbf6d 100644 --- a/modules/Bio/EnsEMBL/Pipeline/ChecksumGenerator.pm +++ b/modules/Bio/EnsEMBL/Pipeline/ChecksumGenerator.pm @@ -135,7 +135,7 @@ sub checksum { sub permissions { my ($self, $file) = @_; - my $mode = 0666; + my $mode = 0666; ## no critic chmod($mode, $file) or $self->throw("Cannot perform the chmod to mode $mode for file $file"); return; } diff --git a/modules/Bio/EnsEMBL/Pipeline/Flatfile/FindDirs.pm b/modules/Bio/EnsEMBL/Pipeline/Flatfile/FindDirs.pm index d68e72f9e7f30f78b4519479793c3c1ecaa4a82c..d71bd68c3ae33dd4a78abde175ca31863ae1c721 100644 --- a/modules/Bio/EnsEMBL/Pipeline/Flatfile/FindDirs.pm +++ b/modules/Bio/EnsEMBL/Pipeline/Flatfile/FindDirs.pm @@ -37,7 +37,7 @@ Allowed parameters are: =cut -package Bio::EnsEMBL::Pipeline::FASTA::FindDirs; +package Bio::EnsEMBL::Pipeline::Flatfile::FindDirs; use strict; use warnings; diff --git a/modules/Bio/EnsEMBL/Pipeline/Production/PepStats.pm b/modules/Bio/EnsEMBL/Pipeline/Production/PepStats.pm index d4bfc50936a3fb359984167b012d67bb6b3e42b9..3177908f0c84fd1cb8d6861fb1c1e5819fa04565 100644 --- a/modules/Bio/EnsEMBL/Pipeline/Production/PepStats.pm +++ b/modules/Bio/EnsEMBL/Pipeline/Production/PepStats.pm @@ -71,11 +71,11 @@ sub store_attrib { sub run_pepstats { my ($self, $tmpfile) = @_; my $PEPSTATS = $self->param('binpath') . '/bin/pepstats'; - open(OUT, "$PEPSTATS -filter < $tmpfile 2>&1 |"); - my @lines = <OUT>; + open(my $fh, "$PEPSTATS -filter < $tmpfile 2>&1 |"); ## no critic + my @lines = <$fh>; my $attribs = {}; my $tid; - close(OUT); + close($fh); foreach my $line (@lines) { if ($line =~ /PEPSTATS of ([^ ]+)/) { @@ -130,7 +130,7 @@ sub dump_translation { my $helper = $dba->dbc()->sql_helper(); my $dbtype = $self->param('dbtype'); my $ta = Bio::EnsEMBL::Registry->get_adaptor($self->param('species'), $dbtype, 'translation'); - open(TMP, "> $tmpfile"); + open(my $fh, '>', $tmpfile); my $sql = q{ SELECT tl.translation_id FROM translation tl, transcript tr, seq_region s, coord_system cs @@ -146,9 +146,9 @@ sub dump_translation { if ($peptide_seq !~ /\n$/) { $peptide_seq .= "\n"; } - print TMP ">$dbid\n$peptide_seq"; + print $fh ">$dbid\n$peptide_seq"; } - close(TMP); + close($fh); } ## end sub dump_translation 1; diff --git a/modules/Bio/EnsEMBL/ProjectionSegment.pm b/modules/Bio/EnsEMBL/ProjectionSegment.pm index e7c90a7e948730f3c75d24b4ccf300e25f1c4aa9..205717c052e9783cdd55764afe918f81709efd62 100644 --- a/modules/Bio/EnsEMBL/ProjectionSegment.pm +++ b/modules/Bio/EnsEMBL/ProjectionSegment.pm @@ -50,6 +50,9 @@ $segment->from_start(), $segement->from_end(), $segment->to_Slice(). package Bio::EnsEMBL::ProjectionSegment; +use strict; +use warnings; + # # WARNING: THIS CLASS IS REPRESENTED BY A BLESSED ARRAY REFERENCE # NOT A HASH REFERENCE diff --git a/modules/Bio/EnsEMBL/Registry.pm b/modules/Bio/EnsEMBL/Registry.pm index 19ff9a40cea1cb4c57d9b0e853fd17b5decec682..b7a0d35c27c2224b59b770ee37dd96ee928a3ffb 100644 --- a/modules/Bio/EnsEMBL/Registry.pm +++ b/modules/Bio/EnsEMBL/Registry.pm @@ -355,7 +355,7 @@ sub load_all { $adaptor, $section ); } - my $test_eval = eval "require $adaptor"; + my $test_eval = eval "require $adaptor"; ## no critic if ($@ or (!$test_eval)) { die($@) } $adaptor->new(%adaptor_args); @@ -370,7 +370,7 @@ sub load_all { if($NEW_EVAL) { require Bio::EnsEMBL::Utils::IO; my $contents = Bio::EnsEMBL::Utils::IO::slurp($config_file); - $test_eval = eval $contents; + $test_eval = eval $contents; ## no critic } else { $test_eval = eval { require($config_file) }; @@ -1024,7 +1024,7 @@ sub get_adaptor { my $dba = $registry_register{_SPECIES}{$species}{ lc($group) }{'_DB'}; my $module = $ret; - my $test_eval = eval "require $module"; + my $test_eval = eval "require $module"; ## no critic if ($@ or (!$test_eval)) { warning("'$module' cannot be found.\nException $@\n"); return; @@ -1856,7 +1856,7 @@ sub load_registry_from_db { # Variation - my $test_eval = eval "require Bio::EnsEMBL::Variation::DBSQL::DBAdaptor"; + my $test_eval = eval "require Bio::EnsEMBL::Variation::DBSQL::DBAdaptor"; ## no critic if ($@or (!$test_eval)) { # Ignore variations as code required not there for this if ($verbose) { @@ -1938,7 +1938,7 @@ sub load_registry_from_db { } ## end foreach my $multidb (@variation_multidbs) } - my $func_eval = eval "require Bio::EnsEMBL::Funcgen::DBSQL::DBAdaptor"; + my $func_eval = eval "require Bio::EnsEMBL::Funcgen::DBSQL::DBAdaptor"; ## no critic if ($@ or (!$func_eval)) { if ($verbose) { # Ignore funcgen DBs as code required not there for this @@ -2023,7 +2023,7 @@ sub load_registry_from_db { my @compara_dbs = grep { /^ensembl_compara/ } @dbnames; if (@compara_dbs) { - my $comp_eval = eval "require Bio::EnsEMBL::Compara::DBSQL::DBAdaptor"; + my $comp_eval = eval "require Bio::EnsEMBL::Compara::DBSQL::DBAdaptor"; ## no critic if ($@ or (!$comp_eval)) { # Ignore Compara as code required not there for this if ($verbose) { diff --git a/modules/Bio/EnsEMBL/StrainSlice.pm b/modules/Bio/EnsEMBL/StrainSlice.pm index a34efdb27d098dd6afb800ddcea941ee7055d89e..724b6dcb02de1f22cb1d21199b71fca1c1a73c97 100644 --- a/modules/Bio/EnsEMBL/StrainSlice.pm +++ b/modules/Bio/EnsEMBL/StrainSlice.pm @@ -264,23 +264,25 @@ sub seq { #apply all differences to the reference sequence #first, in case there are any indels, create the new sequence (containing the '-' bases) - # sort edits in reverse order to remove complication of + # sort edits in reverse order to remove complication of # adjusting downstream edits - my @indels_ordered = sort {$b->start() <=> $a->start()} @{$self->{'alignIndels'}} if (defined $self->{'alignIndels'}); + my @indels_ordered; + @indels_ordered = sort {$b->start() <=> $a->start()} @{$self->{'alignIndels'}} if (defined $self->{'alignIndels'}); foreach my $vf (@indels_ordered){ - $vf->apply_edit($reference_sequence); #change, in the reference sequence, the vf + $vf->apply_edit($reference_sequence); #change, in the reference sequence, the vf } #need to find coverage information if diffe # sort edits in reverse order to remove complication of # adjusting downstream edits - my @variation_features_ordered = sort {$b->start() <=> $a->start()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); + my @variation_features_ordered; + @variation_features_ordered = sort {$b->start() <=> $a->start()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); foreach my $vf (@variation_features_ordered){ - $vf->apply_edit($reference_sequence); #change, in the reference sequence, the vf + $vf->apply_edit($reference_sequence); #change, in the reference sequence, the vf } - + #need to find coverage information if different from reference my $indAdaptor = $self->adaptor->db->get_db_adaptor('variation')->get_IndividualAdaptor; my $ref_strain = $indAdaptor->get_reference_strain_name; @@ -293,7 +295,7 @@ sub seq { return 'N' x $self->length(); } -sub expanded_length() { +sub expanded_length { my $self = shift; my $length = $self->SUPER::length(); @@ -335,7 +337,8 @@ sub _add_coverage_information{ # and unmasks seq where there is read coverage # get all length-changing vars - my @indels_ordered = sort {$a->start() <=> $b->start()} @{$self->{'alignIndels'}} if (defined $self->{'alignIndels'}); + my @indels_ordered; + @indels_ordered = sort {$a->start() <=> $b->start()} @{$self->{'alignIndels'}} if (defined $self->{'alignIndels'}); my $masked_seq = '~' x length($$reference_sequence); @@ -724,7 +727,8 @@ sub mapper{ my $mapper = Bio::EnsEMBL::Mapper->new('Slice','StrainSlice'); #align with Slice #get all the VariationFeatures in the strain Slice, from start to end in the Slice - my @variation_features_ordered = sort {$a->start() <=> $b->start()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); + my @variation_features_ordered; + @variation_features_ordered = sort {$a->start() <=> $b->start()} @{$self->{'alleleFeatures'}} if (defined $self->{'alleleFeatures'}); my $start_slice = 1; my $end_slice; diff --git a/modules/Bio/EnsEMBL/SubSlicedFeature.pm b/modules/Bio/EnsEMBL/SubSlicedFeature.pm index 99059a829bd7319b1002dbea9af2787b5857a2b6..8cb07118dbd3f4b3b888f2fcf7f2a1edff01db10 100644 --- a/modules/Bio/EnsEMBL/SubSlicedFeature.pm +++ b/modules/Bio/EnsEMBL/SubSlicedFeature.pm @@ -43,6 +43,9 @@ my $transcripts = $truncated_gene->get_all_Transcripts(); package Bio::EnsEMBL::SubSlicedFeature; +use strict; +use warnings; + use Bio::EnsEMBL::Utils::Argument qw/rearrange/; use base qw/Bio::EnsEMBL::Utils::Proxy/; diff --git a/modules/Bio/EnsEMBL/Utils/Collector.pm b/modules/Bio/EnsEMBL/Utils/Collector.pm index c09e1d0c96952c00ebbcd20673f68a58d3e8b91e..50a7db826e0df11c11e4dd5b41c6f0de88ef223d 100644 --- a/modules/Bio/EnsEMBL/Utils/Collector.pm +++ b/modules/Bio/EnsEMBL/Utils/Collector.pm @@ -85,11 +85,12 @@ bypassing object creation via the related BaseFeatureAdaptor method. package Bio::EnsEMBL::Utils::Collector; -use Bio::EnsEMBL::Utils::Argument ('rearrange'); -use Bio::EnsEMBL::Utils::Exception ('throw'); use strict; use warnings; +use Bio::EnsEMBL::Utils::Argument ('rearrange'); +use Bio::EnsEMBL::Utils::Exception ('throw'); + ### Global package config vars # Defaults diff --git a/modules/Bio/EnsEMBL/Utils/ConfParser.pm b/modules/Bio/EnsEMBL/Utils/ConfParser.pm index 0553b3374a0bb297e6166283e7daf57b3f672cd7..7dc2aeaa6e0b6f425139afcb879c3c2a0e35705c 100644 --- a/modules/Bio/EnsEMBL/Utils/ConfParser.pm +++ b/modules/Bio/EnsEMBL/Utils/ConfParser.pm @@ -157,13 +157,13 @@ sub parse_options { $conffile = abs_path($conffile); if (-e $conffile) { - open(CONF, $conffile) or throw( + open(my $fh, '<', $conffile) or throw( "Unable to open configuration file $conffile for reading: $!"); my $serverroot = $self->serverroot; my $last; - while (my $line = <CONF>) { + while (my $line = <$fh>) { chomp $line; # remove leading and trailing whitespace @@ -200,6 +200,7 @@ sub parse_options { } $self->param($name, $val); } + close($fh); $self->param('conffile', $conffile); } @@ -556,14 +557,14 @@ sub list_or_file { # we didn't get a list of values, but a file to read values from @vals = (); - open(IN, $firstval) or throw("Cannot open $firstval for reading: $!"); + open(my $fh, '<', $firstval) or throw("Cannot open $firstval for reading: $!"); - while(<IN>){ + while(<$fh>){ chomp; push(@vals, $_); } - close(IN); + close($fh); $self->param($param, @vals); } diff --git a/modules/Bio/EnsEMBL/Utils/ConversionSupport.pm b/modules/Bio/EnsEMBL/Utils/ConversionSupport.pm index 488288c85de2378bb96af8d6e35786cb4ad02a6b..4c16938044fadfe9bdf7e086a2992cff1df48dfc 100644 --- a/modules/Bio/EnsEMBL/Utils/ConversionSupport.pm +++ b/modules/Bio/EnsEMBL/Utils/ConversionSupport.pm @@ -135,10 +135,10 @@ sub parse_common_options { my $conffile = $h{'conffile'} || $self->serverroot . "/sanger-plugins/vega/conf/ini-files/Conversion.ini"; $conffile = abs_path($conffile); if (-e $conffile) { - open(CONF, $conffile) or throw( + open(my $fh, '<', $conffile) or throw( "Unable to open configuration file $conffile for reading: $!"); my $serverroot = $self->serverroot; - while (<CONF>) { + while (<$fh>) { chomp; # remove comments @@ -155,6 +155,7 @@ sub parse_common_options { } $self->param($name, $val); } + close $fh; $self->param('conffile', $conffile); } elsif ($conffile) { @@ -616,12 +617,12 @@ sub list_or_file { if (scalar(@vals) == 1 && -e $firstval) { # we didn't get a list of values, but a file to read values from @vals = (); - open(IN, $firstval) or throw("Cannot open $firstval for reading: $!"); - while(<IN>){ + open(my $fh, '<', $firstval) or throw("Cannot open $firstval for reading: $!"); + while(<$fh>){ chomp; push(@vals, $_); } - close(IN); + close($fh); $self->param($param, @vals); } $self->comma_to_list($param); @@ -881,11 +882,11 @@ sub dynamic_use { my ($self, $classname) = @_; my ($parent_namespace, $module) = $classname =~/^(.*::)(.*)$/ ? ($1,$2) : ('::', $classname); - no strict 'refs'; + no strict 'refs'; ## no critic # return if module has already been imported return 1 if $parent_namespace->{$module.'::'} && %{ $parent_namespace->{$module.'::'}||{} }; - eval "require $classname"; + eval "require $classname"; ## no critic throw("Failed to require $classname: $@") if ($@); $classname->import(); @@ -1278,7 +1279,7 @@ sub log { sub lock_log { my ($self) = @_; - + ## no critic my $fh = $self->{'_log_filehandle'}; return if -t $fh or -p $fh; # Shouldn't lock such things flock($self->{'_log_filehandle'},LOCK_EX) || return 0; @@ -1294,7 +1295,7 @@ sub lock_log { sub unlock_log { my ($self) = @_; - + ## no critic my $fh = $self->{'_log_filehandle'}; return if -t $fh or -p $fh; # We don't lock such things # flush is implicit in flock diff --git a/modules/Bio/EnsEMBL/Utils/Converter.pm b/modules/Bio/EnsEMBL/Utils/Converter.pm index 652e01e84cc865215296bb5953cc459c5f8ce07e..a939e6041c05b04c1a6277403d17fcdc42bc233c 100644 --- a/modules/Bio/EnsEMBL/Utils/Converter.pm +++ b/modules/Bio/EnsEMBL/Utils/Converter.pm @@ -171,7 +171,7 @@ sub _convert_single{ foreach my $field (qw(in out)){ my $slot=__PACKAGE__ ."::$field"; - no strict 'refs'; + no strict 'refs'; ## no critic *$field=sub{ my $self=shift; $self->{$slot}=shift if @_; diff --git a/modules/Bio/EnsEMBL/Utils/Converter/ens_bio_featurePair.pm b/modules/Bio/EnsEMBL/Utils/Converter/ens_bio_featurePair.pm index 21ab6e6281673f58270c1d74b2c03f362eb158c2..2d0d8b16d70397d023c874e83d5dee15b7feebaf 100644 --- a/modules/Bio/EnsEMBL/Utils/Converter/ens_bio_featurePair.pm +++ b/modules/Bio/EnsEMBL/Utils/Converter/ens_bio_featurePair.pm @@ -83,7 +83,7 @@ sub _convert_single_repeatFeature { ); my $output_module = $self->out; - require "$output_module"; + eval "require $output_module"; ## no critic return new Bio::SeqFeature::FeaturePair( -feature1 => $feature1, -feature2 => $feature2 diff --git a/modules/Bio/EnsEMBL/Utils/Exception.pm b/modules/Bio/EnsEMBL/Utils/Exception.pm index 077dee25ff11fbdca6dc7c5996f2fa2d5a2ca09d..f8b06dc5166c26ee84768967982d8472da8fb76f 100644 --- a/modules/Bio/EnsEMBL/Utils/Exception.pm +++ b/modules/Bio/EnsEMBL/Utils/Exception.pm @@ -519,6 +519,7 @@ sub deprecate { =cut +## no critic sub try (&$) { my ($try, $catch) = @_; eval { &$try }; @@ -529,6 +530,7 @@ sub try (&$) { } } +## no critic sub catch (&) { shift; } diff --git a/modules/Bio/EnsEMBL/Utils/Proxy.pm b/modules/Bio/EnsEMBL/Utils/Proxy.pm index 559782893bc816f6eb533ac55537a8c80a5c5dd5..ef1cc49e19c8d4b5a2c3908391dfcd1eabc42916 100644 --- a/modules/Bio/EnsEMBL/Utils/Proxy.pm +++ b/modules/Bio/EnsEMBL/Utils/Proxy.pm @@ -58,6 +58,9 @@ most classes. package Bio::EnsEMBL::Utils::Proxy; +use strict; +use warnings; + use Bio::EnsEMBL::Utils::Exception qw/throw/; use vars '$AUTOLOAD'; @@ -179,7 +182,10 @@ sub AUTOLOAD { my $type = ref $self ? 'object' : 'class'; throw qq{Can't locate $type method "$method_name" via package "$package_name". No subroutine was generated}; } - *$AUTOLOAD = $sub; + { + no strict 'refs'; ## no critic ProhibitNoStrict + *{$AUTOLOAD} = $sub; + } goto &$sub; } diff --git a/modules/Bio/EnsEMBL/Utils/SchemaConversion.pm b/modules/Bio/EnsEMBL/Utils/SchemaConversion.pm index bce988747a366476cd36e7d87580c96263b12e1f..ba0224ee0efc342e43d3792e47d39fe4d5c232a7 100644 --- a/modules/Bio/EnsEMBL/Utils/SchemaConversion.pm +++ b/modules/Bio/EnsEMBL/Utils/SchemaConversion.pm @@ -48,10 +48,10 @@ Bio::EnsEMBL::Utils::ConversionSupport package Bio::EnsEMBL::Utils::SchemaConversion; -use Bio::EnsEMBL::Utils::ConversionSupport; use strict; use warnings; +use Bio::EnsEMBL::Utils::ConversionSupport; use Data::Dumper; =head2 new @@ -184,7 +184,7 @@ sub choose_conversion_type { $species = $self->species_alias($self->conv_support->param('source_db')); if ($self->conv_support->param('do_vega_sc')) { $species = "vega::".$species; - eval "require SeqStoreConverter::$species"; + eval "require SeqStoreConverter::$species"; ## no critic if($@) { warn("Could not require conversion module SeqStoreConverter::$species\ for vega conversion\n" . "Using SeqStoreConverter::BasicConverter instead:\n$@"); @@ -196,7 +196,7 @@ sub choose_conversion_type { } } else { - eval "require SeqStoreConverter::$species"; + eval "require SeqStoreConverter::$species"; ## no critic if($@) { warn("Could not require conversion module SeqStoreConverter::$species for Ensembl conversion\n" . "Using SeqStoreConverter::BasicConverter instead:\n$@"); diff --git a/modules/Bio/EnsEMBL/Utils/ScriptUtils.pm b/modules/Bio/EnsEMBL/Utils/ScriptUtils.pm index 8a63aa28c623594e9b00db3f9780b4cfb7483948..4fcd444380e2309cc99b8c219b89eae3b971f66e 100644 --- a/modules/Bio/EnsEMBL/Utils/ScriptUtils.pm +++ b/modules/Bio/EnsEMBL/Utils/ScriptUtils.pm @@ -243,12 +243,12 @@ sub inject { my $classname = shift; my ($parent_namespace, $module) = $classname =~/^(.*::)(.*)$/ ? ($1,$2) : ('::', $classname); - no strict 'refs'; + no strict 'refs'; ## no critic # return if module has already been imported return 1 if $parent_namespace->{$module.'::'}; - eval "require $classname"; + eval "require $classname"; ## no critic die("Failed to require $classname: $@") if ($@); $classname->import(); diff --git a/modules/t/externalFeatureAdaptor.t b/modules/t/externalFeatureAdaptor.t index 09dbe45d58a06399c7ce57a7183db54215d8d5ab..0cb5d4ea5ca649c3bb1ffdd3f6c754fe2843e2a1 100644 --- a/modules/t/externalFeatureAdaptor.t +++ b/modules/t/externalFeatureAdaptor.t @@ -1,4 +1,7 @@ +## no critic (RequireFilenameMatchesPackage) + use strict; +use warnings; our $verbose = 0; @@ -75,7 +78,7 @@ package ExternalFF2; -package Test; +package main; use Bio::EnsEMBL::Test::TestUtils; diff --git a/modules/t/fullIdCaching.t b/modules/t/fullIdCaching.t index 262a72def510a57283d3b6c9a6e1e3f9547f3f66..764b7819b0f68646032e46f151c792100b3cd61f 100644 --- a/modules/t/fullIdCaching.t +++ b/modules/t/fullIdCaching.t @@ -18,7 +18,7 @@ my $gene_ids = [18256, 18257, 18258]; my $genes = $gene_adaptor->fetch_all_by_dbID_list($gene_ids); sub BEGIN { - no strict 'refs'; + no strict 'refs'; ## no critic *Bio::EnsEMBL::DBSQL::GeneAdaptor::_build_id_cache = sub { my ($self) = @_; return Bio::EnsEMBL::DBSQL::Support::FullIdCache->new($self); diff --git a/modules/t/gffSerialiser.t b/modules/t/gffSerialiser.t index 22369d157b2014386f8468fe7bd1aec65dd2e270..ad888cfcc312567b7113830d8bdea6cf14343d99 100644 --- a/modules/t/gffSerialiser.t +++ b/modules/t/gffSerialiser.t @@ -1,4 +1,7 @@ +## no critic (RequireFilenameMatchesPackage) package Test::SO::Term; +use strict; +use warnings; sub new { my ($class) = @_; diff --git a/modules/t/housekeeping_perlCritic.t b/modules/t/housekeeping_perlCritic.t new file mode 100644 index 0000000000000000000000000000000000000000..715e3ec5f22eb81d5ebc0eafa8f97b14031d251d --- /dev/null +++ b/modules/t/housekeeping_perlCritic.t @@ -0,0 +1,43 @@ +use strict; +use warnings; + +use Cwd; +use File::Spec; +use File::Basename qw/dirname/; +use Test::More; + +if ( not $ENV{TEST_AUTHOR} ) { + my $msg = 'Author test. Set $ENV{TEST_AUTHOR} to a true value to run.'; + plan( skip_all => $msg ); +} + +eval { + require Test::Perl::Critic; + require Perl::Critic::Utils; +}; +if($@) { + plan( skip_all => 'Test::Perl::Critic required.' ); + note $@; +} + +#chdir into the file's target & request cwd() which should be fully resolved now. +#then go back +my $file_dir = dirname(__FILE__); +my $original_dir = cwd(); +chdir($file_dir); +my $cur_dir = cwd(); +chdir($original_dir); +my $root = File::Spec->catdir($cur_dir, File::Spec->updir(),File::Spec->updir()); + +# Configure critic +Test::Perl::Critic->import(-profile => File::Spec->catfile($root, 'perlcriticrc'), -severity => 5, -verbose => 8); + +#Find all files & run +my @perl_files = Perl::Critic::Utils::all_perl_files( + File::Spec->catdir($root, 'modules') +); +foreach my $perl (@perl_files) { + critic_ok($perl); +} + +done_testing(); \ No newline at end of file diff --git a/modules/t/lruIdCaching.t b/modules/t/lruIdCaching.t index 94242b9ae6a92daace2d98a6a694a23b7a39442d..152f46d27a132ff5cef0c5910bd32db2180f34f4 100644 --- a/modules/t/lruIdCaching.t +++ b/modules/t/lruIdCaching.t @@ -1,3 +1,5 @@ +## no critic (RequireFilenameMatchesPackage) + package main; use strict; @@ -18,7 +20,7 @@ my $gene_ids = [18256, 18257, 18258]; my $genes = $gene_adaptor->fetch_all_by_dbID_list($gene_ids); sub BEGIN { - no strict 'refs'; + no strict 'refs'; ##no critic *Bio::EnsEMBL::DBSQL::GeneAdaptor::_build_id_cache = sub { my ($self) = @_; return Bio::EnsEMBL::DBSQL::Support::LruIdCache->new($self, 3); diff --git a/modules/t/mapper.t b/modules/t/mapper.t index b9f6fda8197d98bf356de0220b63d3694c1789bc..a603db4d238cc78dc03d03970dc492dcd8d67d2a 100644 --- a/modules/t/mapper.t +++ b/modules/t/mapper.t @@ -1,8 +1,5 @@ ## Bioperl Test Harness Script for Modules ## -# CVS Version -# $Id$ - # Before `make install' is performed this script should be runnable with # `make test'. After `make install' it should work as `perl test.t' @@ -20,6 +17,8 @@ #----------------------------------------------------------------------- use Test::More; +use strict; +use warnings; # This test script heavily edited by ihh@fruitfly.org @@ -37,7 +36,7 @@ ok( 1 ); use Bio::EnsEMBL::Mapper; -$mapper = Bio::EnsEMBL::Mapper->new( "rawcontig", "virtualcontig" ); +my $mapper = Bio::EnsEMBL::Mapper->new( "rawcontig", "virtualcontig" ); load_sgp_dump( $mapper, undef ); # loading done successfully @@ -335,10 +334,10 @@ chr1 625359 1214016 1216330 1 2315 1 @sgp_dump = reverse (@sgp_dump) if defined $reverse; # test the auto-sorting feature my $first = 1; - for $line ( @sgp_dump ) { + for my $local_line ( @sgp_dump ) { if( $first ) { $first = 0; next; } my ( $chr_name, $contig_id, $chr_start, - $chr_end, $contig_start, $contig_end, $contig_ori ) = split ( /\t/, $line ); + $chr_end, $contig_start, $contig_end, $contig_ori ) = split ( /\t/, $local_line ); # new argument order: $map->add_map_coordinates( $contig_id, $contig_start, $contig_end, $contig_ori, diff --git a/modules/t/proteinAlignFeatureAdaptor.t b/modules/t/proteinAlignFeatureAdaptor.t index dbb254e0c3d1e6f12b0688f9d73449ee1813427f..965d2f64272a649594f5d668540e57215c0d6763 100644 --- a/modules/t/proteinAlignFeatureAdaptor.t +++ b/modules/t/proteinAlignFeatureAdaptor.t @@ -1,6 +1,8 @@ - use Test::More; +use strict; +use warnings; + use Bio::EnsEMBL::Test::MultiTestDB; use Bio::EnsEMBL::Test::TestUtils; diff --git a/modules/t/seqEdit.t b/modules/t/seqEdit.t index 471aa36f2cb1659411fcb9700a1dafb1a84639fb..85f2c478c9c28ae2d6845f197db47bf53a93c4db 100644 --- a/modules/t/seqEdit.t +++ b/modules/t/seqEdit.t @@ -1,5 +1,7 @@ use Test::More; +use strict; +use warnings; use Bio::EnsEMBL::SeqEdit; use Bio::EnsEMBL::Attribute; diff --git a/modules/t/utilsScalar.t b/modules/t/utilsScalar.t index f651be67b9ffd3626fccf56066221f14f584200f..18547fb0d13366a2ae3b90e17f01e40e122e0976 100644 --- a/modules/t/utilsScalar.t +++ b/modules/t/utilsScalar.t @@ -117,7 +117,7 @@ my $scalar; my $other_scalar; open my $scalar_fh, '>', \$scalar; open my $other_scalar_fh, '>', \$other_scalar; -bless($other_scalar_fh); +bless($other_scalar_fh, __PACKAGE__); my $io_handle = IO::Handle->new(); # no need to close as it isn't opened yet just created throws_ok { assert_file_handle(undef) } qr/undefined/, 'Passing in undefined scalar means death'; dies_ok { assert_file_handle(bless(1, 'Brian'), 'met')} 'Passing in a blessed scalar means death'; diff --git a/perlcriticrc b/perlcriticrc new file mode 100644 index 0000000000000000000000000000000000000000..eea328b4fc36ec007604598c0ced41e052d587f1 --- /dev/null +++ b/perlcriticrc @@ -0,0 +1,4 @@ +severity = 5 + +[Subroutines::ProhibitExplicitReturnUndef] +severity=4 \ No newline at end of file