From 1f184eb36dd5a6ef46147722a2426382ca55db54 Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Thu, 9 Oct 2025 17:44:02 -0400 Subject: [PATCH 01/10] test: increase spotbugs rank let's see how high we can increase the rank without too much pain and suffering... --- build-coatjava.sh | 7 ++-- .../jlab/detector/geant4/DCGeant4Factory.java | 2 +- .../cnuphys/splot/plot/ColorScaleModel.java | 4 +-- .../java/cnuphys/splot/plot/Environment.java | 4 +-- .../java/cnuphys/splot/plot/ImageManager.java | 2 +- .../java/cnuphys/splot/plot/PlotCanvas.java | 1 + libexec/spotbugs.sh | 5 +++ spotbugs-exclude.xml | 35 ++++++++++++++++--- 8 files changed, 44 insertions(+), 16 deletions(-) create mode 100755 libexec/spotbugs.sh diff --git a/build-coatjava.sh b/build-coatjava.sh index 22559520f0..8486d16a89 100755 --- a/build-coatjava.sh +++ b/build-coatjava.sh @@ -180,11 +180,8 @@ else fi if [ $runSpotBugs == "yes" ]; then - # mvn com.github.spotbugs:spotbugs-maven-plugin:spotbugs # spotbugs goal produces a report target/spotbugsXml.xml for each module - $mvn com.github.spotbugs:spotbugs-maven-plugin:check # check goal produces a report and produces build failed if bugs - # the spotbugsXml.xml file is easiest read in a web browser - # see http://spotbugs.readthedocs.io/en/latest/maven.html and https://spotbugs.github.io/spotbugs-maven-plugin/index.html for more info - if [ $? != 0 ] ; then echo "spotbugs failure" ; exit 1 ; fi + libexec/spotbugs.sh ${mvnArgs[@]:-} || (echo "ERROR: spotbugs failure" >&2 && exit 1) + echo "spotbugs spotted no bugs!" fi # installation diff --git a/common-tools/clas-geometry/src/main/java/org/jlab/detector/geant4/DCGeant4Factory.java b/common-tools/clas-geometry/src/main/java/org/jlab/detector/geant4/DCGeant4Factory.java index 6fc50fbec0..01fe5065c6 100644 --- a/common-tools/clas-geometry/src/main/java/org/jlab/detector/geant4/DCGeant4Factory.java +++ b/common-tools/clas-geometry/src/main/java/org/jlab/detector/geant4/DCGeant4Factory.java @@ -50,7 +50,7 @@ final class DCdatabase { private DCdatabase() { } - public static DCdatabase getInstance() { + public static synchronized DCdatabase getInstance() { if (instance == null) { instance = new DCdatabase(); } diff --git a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java index aa280eb6fb..9ae741d98e 100644 --- a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java +++ b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java @@ -52,7 +52,7 @@ public ColorScaleModel(double minVal, double maxVal, Color[] colors) { * * @return a standard blue to red color scale */ - public static ColorScaleModel blueToRed() { + public static synchronized ColorScaleModel blueToRed() { if (_blueToRed == null) { Color colors[] = { // new Color(0, 0, 139), @@ -245,4 +245,4 @@ private Color getInterpretedColor(Color c1, Color c2, double fract) { return new Color(r3, g3, b3, a3); } -} \ No newline at end of file +} diff --git a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/Environment.java b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/Environment.java index 0780cfcabb..203cbccba4 100644 --- a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/Environment.java +++ b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/Environment.java @@ -95,7 +95,7 @@ public Font getCommonFont(int size) { * * @return the singleton object. */ - public static Environment getInstance() { + public static synchronized Environment getInstance() { if (instance == null) { instance = new Environment(); } @@ -287,4 +287,4 @@ public static void memoryReport(String message) { public static void main(String arg[]) { System.out.println(Environment.getInstance()); } -} \ No newline at end of file +} diff --git a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ImageManager.java b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ImageManager.java index 01f63c03b8..e9fd486ea2 100644 --- a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ImageManager.java +++ b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ImageManager.java @@ -42,7 +42,7 @@ private ImageManager() { * * @return the image manager singleton. */ - public static ImageManager getInstance() { + public static synchronized ImageManager getInstance() { if (imageManager == null) { try { imageManager = new ImageManager(); diff --git a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/PlotCanvas.java b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/PlotCanvas.java index 7fd2c9131a..873316bfac 100644 --- a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/PlotCanvas.java +++ b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/PlotCanvas.java @@ -151,6 +151,7 @@ public PlotCanvas(DataSet dataSet, String plotTitle, String xLabel, String yLabe } catch (DataSetException e) { e.printStackTrace(); + System.exit(1); } } diff --git a/libexec/spotbugs.sh b/libexec/spotbugs.sh new file mode 100755 index 0000000000..fd275e5faf --- /dev/null +++ b/libexec/spotbugs.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +# run spotbugs; additional arguments are passed to `mvn` (such as `-T`) +set -euo pipefail +mvn spotbugs:check "$@" +# mvn spotbugs:gui --no-transfer-progress # must be single-threaded diff --git a/spotbugs-exclude.xml b/spotbugs-exclude.xml index 3134dd3058..cc2f1fd16f 100644 --- a/spotbugs-exclude.xml +++ b/spotbugs-exclude.xml @@ -1,8 +1,33 @@ - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + From 3d17d894e3606429f2f5c893e03f4279ea772b56 Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Thu, 9 Oct 2025 17:53:13 -0400 Subject: [PATCH 02/10] fix: 5 --- spotbugs-exclude.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spotbugs-exclude.xml b/spotbugs-exclude.xml index cc2f1fd16f..8754c3ce57 100644 --- a/spotbugs-exclude.xml +++ b/spotbugs-exclude.xml @@ -8,7 +8,7 @@ 15-20 informational --> - + From d2073c16c92fb138908dacb32e4790d5acb736b2 Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Thu, 9 Oct 2025 18:04:27 -0400 Subject: [PATCH 03/10] fix: 6 --- spotbugs-exclude.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spotbugs-exclude.xml b/spotbugs-exclude.xml index 8754c3ce57..f008e051da 100644 --- a/spotbugs-exclude.xml +++ b/spotbugs-exclude.xml @@ -8,7 +8,7 @@ 15-20 informational --> - + From b3f491ba06eab4ee2046e0c2672317c5153f8d04 Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Mon, 13 Oct 2025 09:33:17 -0400 Subject: [PATCH 04/10] fix: thread-safe date formatter --- .../java/cnuphys/magfield/MagneticFields.java | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java b/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java index 0e21160ee8..070dc817b2 100644 --- a/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java +++ b/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java @@ -7,10 +7,11 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintStream; -import java.text.SimpleDateFormat; +import java.time.format.DateTimeFormatter; +import java.time.ZoneId; +import java.time.Instant; import java.util.ArrayList; import java.util.StringTokenizer; -import java.util.TimeZone; import java.util.logging.Level; import java.util.logging.Logger; @@ -40,14 +41,9 @@ public class MagneticFields { /** * A formatter to get the time in down to seconds (no day info). */ - private static SimpleDateFormat formatterlong; - - static { - TimeZone tz = TimeZone.getDefault(); - - formatterlong = new SimpleDateFormat("yyyy/MM/dd HH:mm:ss"); - formatterlong.setTimeZone(tz); - } + private static final DateTimeFormatter formatterlong = DateTimeFormatter + .ofPattern("yyyy/MM/dd HH:mm:ss") + .withZone(ZoneId.systemDefault()); // version of mag field package private static String VERSION = "1.20"; @@ -1660,7 +1656,7 @@ public String fileBaseNames() { * @return a string representation of the current time, down to seconds. */ public static String dateStringLong(long longtime) { - return formatterlong.format(longtime); + return formatterlong.format(Instant.ofEpochMilli(longtime)); } /** From e6c1d5583368d1878b2c2a67d5bf49d9269df563 Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Mon, 13 Oct 2025 09:41:39 -0400 Subject: [PATCH 05/10] fix: repeated conditional --- .../src/main/java/cnuphys/magfield/converter/Converter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/converter/Converter.java b/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/converter/Converter.java index 758334b9a2..763e936146 100644 --- a/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/converter/Converter.java +++ b/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/converter/Converter.java @@ -196,7 +196,7 @@ public void done() { zIndex++; } - if ((gdata[PHI].max > 31) && (gdata[PHI].max > 31)) { + if (gdata[PHI].max > 31) { System.err.println("Correcting PhiMax to 360 from " + gdata[PHI].max); } @@ -850,4 +850,4 @@ public static void memoryToGEMCTorusConverter() { } -} \ No newline at end of file +} From 3ed1a1f9828bda482b32759ed2ae5d9127baba5e Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Thu, 16 Oct 2025 11:17:08 -0400 Subject: [PATCH 06/10] fix: disable bug-prone `ClasUtilsFile.getFileList(String,String,String)`, since unused --- .../org/jlab/utils/system/ClasUtilsFile.java | 41 ++++++++++--------- spotbugs-exclude.xml | 2 +- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/common-tools/clas-utils/src/main/java/org/jlab/utils/system/ClasUtilsFile.java b/common-tools/clas-utils/src/main/java/org/jlab/utils/system/ClasUtilsFile.java index d47d2eb9c3..f4352500c6 100644 --- a/common-tools/clas-utils/src/main/java/org/jlab/utils/system/ClasUtilsFile.java +++ b/common-tools/clas-utils/src/main/java/org/jlab/utils/system/ClasUtilsFile.java @@ -132,25 +132,28 @@ public static List getFileList(String env, String rpath){ } return ClasUtilsFile.getFileList(directory); } - /** - * returns a file list that contains files with given extension - * @param env - * @param rpath - * @param ext - * @return - */ - public static List getFileList(String env, String rpath, String ext){ - String directory = ClasUtilsFile.getResourceDir(env, rpath); - if(directory!=null) return new ArrayList<>(); - - List files = ClasUtilsFile.getFileList(directory); - List selected = new ArrayList<>(); - for(String item : files){ - if(item.endsWith(ext)==true) selected.add(item); - } - return selected; - } - + + // FIXME: unused, and spotbugs detects rank-7 bug `NP_NULL_PARAM_DEREF_NONVIRTUAL`; + // should we just remove this function? + // /** + // * returns a file list that contains files with given extension + // * @param env + // * @param rpath + // * @param ext + // * @return + // */ + // public static List getFileList(String env, String rpath, String ext){ + // String directory = ClasUtilsFile.getResourceDir(env, rpath); + // if(directory!=null) return new ArrayList<>(); + // + // List files = ClasUtilsFile.getFileList(directory); + // List selected = new ArrayList<>(); + // for(String item : files){ + // if(item.endsWith(ext)==true) selected.add(item); + // } + // return selected; + // } + public static void writeFile(String filename, List lines){ System.out.println("writing file ---> " + filename); BufferedWriter writer = null; diff --git a/spotbugs-exclude.xml b/spotbugs-exclude.xml index f008e051da..8110aa1556 100644 --- a/spotbugs-exclude.xml +++ b/spotbugs-exclude.xml @@ -8,7 +8,7 @@ 15-20 informational --> - + From 44ea4df4c889c8f0ab3daed1b38d57e2235c81fe Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Thu, 16 Oct 2025 11:31:17 -0400 Subject: [PATCH 07/10] fix: Non-virtual method call in constructor passes null for... ...non-null parameter of another constructor (`NP_NULL_PARAM_DEREF_NONVIRTUAL`) --- .../ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/reconstruction/ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java b/reconstruction/ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java index e7c152bd77..eba8c7d60f 100644 --- a/reconstruction/ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java +++ b/reconstruction/ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java @@ -7,6 +7,7 @@ import java.util.LinkedList; import java.util.List; +import javax.annotation.Nullable; import org.jlab.detector.banks.RawDataBank; import org.jlab.geom.prim.Vector3D; @@ -85,9 +86,9 @@ public static List loadHits(DataEvent event) { LTCCHit(RawDataBank bank, int index, - IndexedTable spe, - IndexedTable timing_offset, - IndexedTable status_table) { + @Nullable IndexedTable spe, + @Nullable IndexedTable timing_offset, + @Nullable IndexedTable status_table) { this.sector = bank.getByte("sector", index); this.segment = bank.getShort("component", index); this.side = bank.trueOrder(index) + 1; From a6123402a46d3dc594be8dd7d64487368bf40674 Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Thu, 16 Oct 2025 11:49:40 -0400 Subject: [PATCH 08/10] fix: `setMass` `mass` parameter is dead upon entry but overwritten... spotbugs rank-7 bug `IP_PARAMETER_IS_DEAD_BUT_OVERWRITTEN` --- .../jlab/rec/dc/track/fit/StateVecsDoca.java | 44 ++++++++++--------- .../org/jlab/rec/fmt/track/fit/StateVecs.java | 44 ++++++++++--------- 2 files changed, 48 insertions(+), 40 deletions(-) diff --git a/reconstruction/dc/src/main/java/org/jlab/rec/dc/track/fit/StateVecsDoca.java b/reconstruction/dc/src/main/java/org/jlab/rec/dc/track/fit/StateVecsDoca.java index 70d56dd75c..6097c5f15a 100644 --- a/reconstruction/dc/src/main/java/org/jlab/rec/dc/track/fit/StateVecsDoca.java +++ b/reconstruction/dc/src/main/java/org/jlab/rec/dc/track/fit/StateVecsDoca.java @@ -318,26 +318,30 @@ private void delA_delt(double tx, double ty, double Bx, double By, double Bz, do private double beta = 1.0; - public void setMass(int hypo, double mass) { - - switch (hypo) { - case 0: - mass = 0.000510998; - break; - case 1: - mass = 0.13957018; - break; - case 2: - mass = 0.493677; - break; - case 3: - mass = 0.105658369; - break; - case 4: - mass = 0.938272029; - break; - } - } + + // FIXME: `mass` parameter is dead upon entry but overwritten + // (spotbugs bug `IP_PARAMETER_IS_DEAD_BUT_OVERWRITTEN`) + // Can we just remove this? + // public void setMass(int hypo, double mass) { + // + // switch (hypo) { + // case 0: + // mass = 0.000510998; + // break; + // case 1: + // mass = 0.13957018; + // break; + // case 2: + // mass = 0.493677; + // break; + // case 3: + // mass = 0.105658369; + // break; + // case 4: + // mass = 0.938272029; + // break; + // } + // } /** diff --git a/reconstruction/fmt/src/main/java/org/jlab/rec/fmt/track/fit/StateVecs.java b/reconstruction/fmt/src/main/java/org/jlab/rec/fmt/track/fit/StateVecs.java index 620faf7214..17c881eaff 100644 --- a/reconstruction/fmt/src/main/java/org/jlab/rec/fmt/track/fit/StateVecs.java +++ b/reconstruction/fmt/src/main/java/org/jlab/rec/fmt/track/fit/StateVecs.java @@ -293,26 +293,30 @@ private void delA_delt(double tx, double ty, double Bx, double By, double Bz, do private double beta = 1.0; - public void setMass(int hypo, double mass) { - - switch (hypo) { - case 0: - mass = 0.000510998; - break; - case 1: - mass = 0.13957018; - break; - case 2: - mass = 0.493677; - break; - case 3: - mass = 0.105658369; - break; - case 4: - mass = 0.938272029; - break; - } - } + + // FIXME: `mass` parameter is dead upon entry but overwritten + // (spotbugs bug `IP_PARAMETER_IS_DEAD_BUT_OVERWRITTEN`) + // Can we just remove this? + // public void setMass(int hypo, double mass) { + // + // switch (hypo) { + // case 0: + // mass = 0.000510998; + // break; + // case 1: + // mass = 0.13957018; + // break; + // case 2: + // mass = 0.493677; + // break; + // case 3: + // mass = 0.105658369; + // break; + // case 4: + // mass = 0.938272029; + // break; + // } + // } /** From 119963f1596347d484fc553db7b5c4bc30b1c221 Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Thu, 16 Oct 2025 17:00:43 -0400 Subject: [PATCH 09/10] fix: undeclared dependency --- pom.xml | 6 ++++++ reconstruction/ltcc/pom.xml | 4 ++++ .../ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 554ebb5fd3..3948d175a8 100644 --- a/pom.xml +++ b/pom.xml @@ -40,6 +40,12 @@ 4.9.6 + + jakarta.annotation + jakarta.annotation-api + 3.0.0 + + junit junit diff --git a/reconstruction/ltcc/pom.xml b/reconstruction/ltcc/pom.xml index 588d3fc3e2..a8f5be4491 100644 --- a/reconstruction/ltcc/pom.xml +++ b/reconstruction/ltcc/pom.xml @@ -19,6 +19,10 @@ javafx-base linux + + jakarta.annotation + jakarta.annotation-api + org.jlab groot diff --git a/reconstruction/ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java b/reconstruction/ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java index eba8c7d60f..af0556a406 100644 --- a/reconstruction/ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java +++ b/reconstruction/ltcc/src/main/java/org/jlab/service/ltcc/LTCCHit.java @@ -7,7 +7,7 @@ import java.util.LinkedList; import java.util.List; -import javax.annotation.Nullable; +import jakarta.annotation.Nullable; import org.jlab.detector.banks.RawDataBank; import org.jlab.geom.prim.Vector3D; From bbc8215add82759c7cc0d44ae05aae3e3806e3fb Mon Sep 17 00:00:00 2001 From: Christopher Dilks Date: Thu, 16 Oct 2025 18:05:15 -0400 Subject: [PATCH 10/10] fix: fix or suppress some rank-8 bugs --- .../org/jlab/utils/benchmark/Benchmark.java | 2 +- .../java/cnuphys/magfield/MagneticFields.java | 2 +- .../cnuphys/splot/plot/ColorScaleModel.java | 2 +- spotbugs-exclude.xml | 23 +++++++++++++++---- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/common-tools/clas-utils/src/main/java/org/jlab/utils/benchmark/Benchmark.java b/common-tools/clas-utils/src/main/java/org/jlab/utils/benchmark/Benchmark.java index dddbfaf69c..0bc4a2e00e 100644 --- a/common-tools/clas-utils/src/main/java/org/jlab/utils/benchmark/Benchmark.java +++ b/common-tools/clas-utils/src/main/java/org/jlab/utils/benchmark/Benchmark.java @@ -26,7 +26,7 @@ public class Benchmark { private Timer updateTimer = null; - public Benchmark(){ + private Benchmark(){ } diff --git a/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java b/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java index 070dc817b2..9d439bd794 100644 --- a/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java +++ b/common-tools/cnuphys/magfield/src/main/java/cnuphys/magfield/MagneticFields.java @@ -123,7 +123,7 @@ public String getVersion() { * * @return the MagneticFields singleton */ - public static MagneticFields getInstance() { + public static synchronized MagneticFields getInstance() { if (instance == null) { instance = new MagneticFields(); } diff --git a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java index 9ae741d98e..d2d89a4db0 100644 --- a/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java +++ b/common-tools/cnuphys/splot/src/main/java/cnuphys/splot/plot/ColorScaleModel.java @@ -41,7 +41,7 @@ public class ColorScaleModel { * @param minVal the minimum value; * @param maxVal the maximum value the array of colors. */ - public ColorScaleModel(double minVal, double maxVal, Color[] colors) { + private ColorScaleModel(double minVal, double maxVal, Color[] colors) { _colors = colors; _minVal = minVal; _maxVal = maxVal; diff --git a/spotbugs-exclude.xml b/spotbugs-exclude.xml index 8110aa1556..0af045eb64 100644 --- a/spotbugs-exclude.xml +++ b/spotbugs-exclude.xml @@ -8,26 +8,39 @@ 15-20 informational --> - + - + - + + + + + + + + + + + + + + - + + -