Merge branch 'main' into pr/pinin4fjords/41

flying-sheep · flying-sheep · commit 4bb2803371eb · 2026-05-16T12:11:52.000+02:00
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -120,3 +120,20 @@ jobs:
       - uses: rustsec/audit-check@v2
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
+
+  # Check that all tests defined above pass. This makes it easy to set a single "required" test in branch
+  # protection instead of having to update it frequently. See https://github.com/re-actors/alls-green#why.
+  check:
+    name: Checks pass
+    if: always()
+    needs:
+      - test
+      - fmt 
+      - clippy
+      - msrv
+      - audit
+    runs-on: ubuntu-latest
+    steps:
+        - uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe # ratchet:re-actors/alls-green@v1.2.2
+          with:
+            jobs: ${{ toJSON(needs) }}
diff --git a/src/chimeric/output.rs b/src/chimeric/output.rs
@@ -22,8 +22,7 @@ impl ChimericJunctionWriter {
     ///
     /// Creates file: {prefix}Chimeric.out.junction
     pub fn new(prefix: &str) -> Result<Self, Error> {
-        let mut path = PathBuf::from(prefix);
-        path.push("Chimeric.out.junction");
+        let path = PathBuf::from(format!("{prefix}Chimeric.out.junction"));
 
         let file = File::create(&path).map_err(|e| Error::io(e, &path))?;
 
@@ -229,23 +228,39 @@ mod tests {
     #[test]
     fn test_chimeric_junction_writer_creation() {
         let dir = tempdir().unwrap();
-        let prefix = dir.path().to_str().unwrap();
+        let prefix = format!("{}/", dir.path().display());
 
-        let writer = ChimericJunctionWriter::new(prefix);
+        let writer = ChimericJunctionWriter::new(&prefix);
         assert!(writer.is_ok());
 
-        let mut path = PathBuf::from(prefix);
-        path.push("Chimeric.out.junction");
+        let path = PathBuf::from(format!("{prefix}Chimeric.out.junction"));
         assert!(path.exists());
     }
 
+    #[test]
+    fn test_chimeric_junction_writer_bare_dot_prefix() {
+        let dir = tempdir().unwrap();
+        let prefix = format!("{}/SAMPLE.", dir.path().display());
+
+        let writer = ChimericJunctionWriter::new(&prefix);
+        assert!(writer.is_ok());
+
+        let path = PathBuf::from(format!("{prefix}Chimeric.out.junction"));
+        assert!(path.exists(), "expected {} to exist", path.display());
+        assert!(
+            path.file_name().unwrap().to_str().unwrap() == "SAMPLE.Chimeric.out.junction",
+            "expected literal concatenation, got {}",
+            path.display()
+        );
+    }
+
     #[test]
     fn test_write_inter_chromosomal() {
         use cigar::op::{Kind, Op};
         let dir = tempdir().unwrap();
-        let prefix = dir.path().to_str().unwrap();
+        let prefix = format!("{}/", dir.path().display());
 
-        let mut writer = ChimericJunctionWriter::new(prefix).unwrap();
+        let mut writer = ChimericJunctionWriter::new(&prefix).unwrap();
 
         // Create mock chimeric alignment (chr9 -> chr22, BCR-ABL fusion)
         let donor = ChimericSegment {
@@ -290,8 +305,7 @@ mod tests {
         writer.flush().unwrap();
 
         // Read file and verify
-        let mut path = PathBuf::from(prefix);
-        path.push("Chimeric.out.junction");
+        let path = PathBuf::from(format!("{prefix}Chimeric.out.junction"));
 
         let mut content = String::new();
         File::open(&path)
@@ -323,9 +337,9 @@ mod tests {
     fn test_write_strand_break() {
         use cigar::op::{Kind, Op};
         let dir = tempdir().unwrap();
-        let prefix = dir.path().to_str().unwrap();
+        let prefix = format!("{}/", dir.path().display());
 
-        let mut writer = ChimericJunctionWriter::new(prefix).unwrap();
+        let mut writer = ChimericJunctionWriter::new(&prefix).unwrap();
 
         // Create mock chimeric alignment (same chr, opposite strands)
         let donor = ChimericSegment {
@@ -370,8 +384,7 @@ mod tests {
         writer.flush().unwrap();
 
         // Read file and verify
-        let mut path = PathBuf::from(prefix);
-        path.push("Chimeric.out.junction");
+        let path = PathBuf::from(format!("{prefix}Chimeric.out.junction"));
 
         let mut content = String::new();
         File::open(&path)
diff --git a/src/io/sam.rs b/src/io/sam.rs
@@ -1162,11 +1162,11 @@ pub(crate) fn apply_pe_transcriptome_mate_fields(
     let pos2 = (t2.genome_start + 1) as usize;
     *rec1.mate_alignment_start_mut() = Some(
         pos2.try_into()
-            .map_err(|e| Error::Alignment(format!("invalid mate position {}: {}", pos2, e)))?,
+            .map_err(|e| Error::Alignment(format!("invalid mate position {pos2}: {e}")))?,
     );
     *rec2.mate_alignment_start_mut() = Some(
         pos1.try_into()
-            .map_err(|e| Error::Alignment(format!("invalid mate position {}: {}", pos1, e)))?,
+            .map_err(|e| Error::Alignment(format!("invalid mate position {pos1}: {e}")))?,
     );
 
     let tlen = calculate_insert_size(t1, t2);
diff --git a/src/lib.rs b/src/lib.rs
@@ -273,7 +273,7 @@ fn align_reads(params: &Parameters) -> anyhow::Result<()> {
     let time_finish = chrono::Local::now();
 
     // Write Log.final.out
-    let log_path = params.out_file_name_prefix.join("Log.final.out");
+    let log_path = params.output_path("Log.final.out");
     if let Some(parent) = log_path.parent() {
         std::fs::create_dir_all(parent)?;
     }
@@ -282,7 +282,7 @@ fn align_reads(params: &Parameters) -> anyhow::Result<()> {
 
     // Write ReadsPerGene.out.tab if quantMode GeneCounts was requested.
     if let Some(ref ctx) = quant_ctx {
-        let quant_path = params.out_file_name_prefix.join("ReadsPerGene.out.tab");
+        let quant_path = params.output_path("ReadsPerGene.out.tab");
         ctx.counts.write_output(&quant_path, &ctx.gene_ann)?;
         info!("Wrote {}", quant_path.display());
     }
@@ -313,9 +313,7 @@ fn run_single_pass(
 
     // Open transcriptome BAM writer if requested.
     let mut tr_writer: Option<BamWriter> = if let Some(tidx) = tr.as_ref() {
-        let path = params
-            .out_file_name_prefix
-            .join("Aligned.toTranscriptome.out.bam");
+        let path = params.output_path("Aligned.toTranscriptome.out.bam");
         info!("Writing transcriptome BAM to {}", path.display());
         if let Some(parent) = path.parent() {
             std::fs::create_dir_all(parent)?;
@@ -332,15 +330,15 @@ fn run_single_pass(
     let is_paired = params.read_files_in.len() == 2;
     let mut unmapped_w1: Option<UnmappedFastqWriter> =
         if params.out_reads_unmapped == OutReadsUnmapped::Fastx {
-            let path = params.out_file_name_prefix.join("Unmapped.out.mate1");
+            let path = params.output_path("Unmapped.out.mate1");
             info!("Writing unmapped reads to {}", path.display());
             Some(UnmappedFastqWriter::create(&path)?)
         } else {
             None
         };
     let mut unmapped_w2: Option<UnmappedFastqWriter> =
         if params.out_reads_unmapped == OutReadsUnmapped::Fastx && is_paired {
-            let path = params.out_file_name_prefix.join("Unmapped.out.mate2");
+            let path = params.output_path("Unmapped.out.mate2");
             info!("Writing unmapped mate2 reads to {}", path.display());
             Some(UnmappedFastqWriter::create(&path)?)
         } else {
@@ -379,7 +377,7 @@ fn run_single_pass(
         }
         OutStd::None => match out_type.format {
             OutSamFormat::Sam => {
-                let output_path = params.out_file_name_prefix.join("Aligned.out.sam");
+                let output_path = params.output_path("Aligned.out.sam");
                 info!("Writing SAM to {}", output_path.display());
                 if let Some(parent) = output_path.parent() {
                     std::fs::create_dir_all(parent)?;
@@ -389,11 +387,9 @@ fn run_single_pass(
             OutSamFormat::Bam => {
                 let sorted = out_type.sort_order == Some(OutSamSortOrder::SortedByCoordinate);
                 let output_path = if sorted {
-                    params
-                        .out_file_name_prefix
-                        .join("Aligned.sortedByCoord.out.bam")
+                    params.output_path("Aligned.sortedByCoord.out.bam")
                 } else {
-                    params.out_file_name_prefix.join("Aligned.out.bam")
+                    params.output_path("Aligned.out.bam")
                 };
                 info!("Writing BAM to {}", output_path.display());
                 if let Some(parent) = output_path.parent() {
@@ -451,7 +447,7 @@ fn run_single_pass(
     }
 
     // 5. Write SJ.out.tab file
-    let sj_output_path = params.out_file_name_prefix.join("SJ.out.tab");
+    let sj_output_path = params.output_path("SJ.out.tab");
     if !sj_stats.is_empty() {
         info!(
             "Writing splice junction statistics to {}",
@@ -480,7 +476,7 @@ fn run_two_pass(
     let (sj_stats_pass1, novel_junctions) = run_pass1(index, params)?;
 
     // Write SJ.pass1.out.tab
-    let pass1_path = params.out_file_name_prefix.join("SJ.pass1.out.tab");
+    let pass1_path = params.output_path("SJ.pass1.out.tab");
 
     // Create output directory if it doesn't exist
     if let Some(parent) = pass1_path.parent() {
@@ -887,12 +883,11 @@ fn align_reads_single_end<W: AlignmentWriter + ?Sized>(
     // Create chimeric output writer if enabled
     let mut chimeric_writer = if params.chim_segment_min > 0 && params.chim_out_junctions() {
         use crate::chimeric::ChimericJunctionWriter;
-        let prefix = params.out_file_name_prefix.to_str().unwrap_or(".");
         info!(
             "Chimeric detection enabled (chimSegmentMin={})",
             params.chim_segment_min
         );
-        Some(ChimericJunctionWriter::new(prefix)?)
+        Some(ChimericJunctionWriter::new(&params.out_file_name_prefix)?)
     } else {
         None
     };
@@ -1318,12 +1313,11 @@ fn align_reads_paired_end<W: AlignmentWriter + ?Sized>(
     // Create chimeric output writer if enabled
     let mut chimeric_writer = if params.chim_segment_min > 0 && params.chim_out_junctions() {
         use crate::chimeric::ChimericJunctionWriter;
-        let prefix = params.out_file_name_prefix.to_str().unwrap_or(".");
         info!(
             "Chimeric detection enabled (chimSegmentMin={})",
             params.chim_segment_min
         );
-        Some(ChimericJunctionWriter::new(prefix)?)
+        Some(ChimericJunctionWriter::new(&params.out_file_name_prefix)?)
     } else {
         None
     };
diff --git a/src/params.rs b/src/params.rs
@@ -344,7 +344,7 @@ pub struct Parameters {
     // ── Output ──────────────────────────────────────────────────────────
     /// Output file name prefix (including path)
     #[arg(long = "outFileNamePrefix", default_value = "./")]
-    pub out_file_name_prefix: PathBuf,
+    pub out_file_name_prefix: String,
 
     /// Output type: SAM, BAM Unsorted, BAM SortedByCoordinate, None.
     /// Provide as space-separated tokens, e.g. "BAM SortedByCoordinate".
@@ -704,6 +704,11 @@ pub struct Parameters {
 }
 
 impl Parameters {
+    /// Build an output path by concatenating `suffix` onto `out_file_name_prefix`.
+    pub fn output_path(&self, suffix: &str) -> PathBuf {
+        PathBuf::from(format!("{}{suffix}", self.out_file_name_prefix))
+    }
+
     /// Parse the raw `--outSAMtype` tokens into a structured `OutSamType`.
     pub fn out_sam_type(&self) -> Result<OutSamType, String> {
         match self
@@ -1008,7 +1013,7 @@ mod tests {
         assert_eq!(p.read_map_number, -1);
         assert_eq!(p.clip5p_nbases, 0);
         assert_eq!(p.clip3p_nbases, 0);
-        assert_eq!(p.out_file_name_prefix, PathBuf::from("./"));
+        assert_eq!(p.out_file_name_prefix, "./");
         assert_eq!(p.out_sam_type_raw, vec!["SAM".to_string()]);
         assert_eq!(p.out_sam_strand_field, "None");
         assert_eq!(p.out_sam_attributes, vec!["Standard".to_string()]);
@@ -1145,7 +1150,7 @@ mod tests {
             sam_type.sort_order,
             Some(OutSamSortOrder::SortedByCoordinate)
         );
-        assert_eq!(p.out_file_name_prefix, PathBuf::from("/out/sample1_"));
+        assert_eq!(p.out_file_name_prefix, "/out/sample1_");
         assert_eq!(p.out_filter_multimap_nmax, 20);
         assert_eq!(p.align_intron_max, 1_000_000);
         assert_eq!(p.sjdb_gtf_file, Some(PathBuf::from("gencode.gtf")));
@@ -1487,4 +1492,50 @@ mod tests {
         ]);
         assert_eq!(p.align_sj_stitch_mismatch_nmax, vec![1, -1, 2, 3]);
     }
+
+    #[test]
+    fn output_path_bare_dot_prefix() {
+        let p = parse(&["--readFilesIn", "r.fq", "--outFileNamePrefix", "SAMPLE."]);
+        assert_eq!(p.out_file_name_prefix, "SAMPLE.");
+        assert_eq!(
+            p.output_path("Aligned.out.bam"),
+            PathBuf::from("SAMPLE.Aligned.out.bam")
+        );
+        assert_eq!(
+            p.output_path("Log.final.out"),
+            PathBuf::from("SAMPLE.Log.final.out")
+        );
+    }
+
+    #[test]
+    fn output_path_trailing_slash_prefix() {
+        let p = parse(&["--readFilesIn", "r.fq", "--outFileNamePrefix", "out/"]);
+        assert_eq!(
+            p.output_path("Aligned.out.bam"),
+            PathBuf::from("out/Aligned.out.bam")
+        );
+    }
+
+    #[test]
+    fn output_path_default_prefix() {
+        let p = parse(&["--readFilesIn", "r.fq"]);
+        assert_eq!(
+            p.output_path("Aligned.out.bam"),
+            PathBuf::from("./Aligned.out.bam")
+        );
+    }
+
+    #[test]
+    fn output_path_path_with_underscore() {
+        let p = parse(&[
+            "--readFilesIn",
+            "r.fq",
+            "--outFileNamePrefix",
+            "/out/sample1_",
+        ]);
+        assert_eq!(
+            p.output_path("Aligned.out.bam"),
+            PathBuf::from("/out/sample1_Aligned.out.bam")
+        );
+    }
 }
diff --git a/tests/alignment_features.rs b/tests/alignment_features.rs
