Add spotbugs and correct found problems

> Task :spotbugsTest M D NP: Possible null pointer dereference in network.brightspots.rcv.TabulatorTests.runTabulationTest(String) due to return value of called method Dereferenced at TabulatorTests.java:[line 152] M B OS: network.brightspots.rcv.TabulatorTests.fileCompare(String, String) may fail to close stream At TabulatorTests.java:[line 58] H I Dm: Found reliance on default encoding in network.brightspots.rcv.TabulatorTests.fileCompare(String, String): new java.io.FileReader(String) At TabulatorTests.java:[line 57] > Task :spotbugsTest FAILED > Task :spotbugsMain M X OBL: network.brightspots.rcv.JsonParser.readFromFile(String, Class, boolean) may fail to clean up java.io.Reader Obligation to clean up resource created at JsonParser.java:[line 42] is not discharged H I Dm: Found reliance on default encoding in network.brightspots.rcv.JsonParser.readFromFile(String, Class, boolean): new java.io.FileReader(String) At JsonParser.java:[line 42] M P WMI: network.brightspots.rcv.ResultsWriter.getCandidatesWithRanksList(Map) makes inefficient use of keySet iterator instead of entrySet iterator At ResultsWriter.java:[line 168] M P WMI: network.brightspots.rcv.ResultsWriter.setCandidatesToRoundEliminated(Map) makes inefficient use of keySet iterator instead of entrySet iterator At ResultsWriter.java:[line 245] M P WMI: network.brightspots.rcv.ResultsWriter.setWinnerToRound(Map) makes inefficient use of keySet iterator instead of entrySet iterator At ResultsWriter.java:[line 256] M P WMI: network.brightspots.rcv.ResultsWriter.generatePrecinctSummaryFiles(Map, Map, Map) makes inefficient use of keySet iterator instead of entrySet iterator At ResultsWriter.java:[line 289] M P WMI: network.brightspots.rcv.ResultsWriter.updateCandidateNamesInTally(Map) makes inefficient use of keySet iterator instead of entrySet iterator At ResultsWriter.java:[line 880] M P WMI: network.brightspots.rcv.ResultsWriter.addActionObjects(String, List, int, ArrayList, TallyTransfers) makes inefficient use of keySet iterator instead of entrySet iterator At ResultsWriter.java:[line 921] M P WMI: network.brightspots.rcv.ClearBallotCvrReader.readCastVoteRecords(List, String) makes inefficient use of keySet iterator instead of entrySet iterator At ClearBallotCvrReader.java:[line 109] H I Dm: Found reliance on default encoding in network.brightspots.rcv.ClearBallotCvrReader.readCastVoteRecords(List, String): new java.io.FileReader(String) At ClearBallotCvrReader.java:[line 51] M D REC: Exception is caught when Exception is not thrown in network.brightspots.rcv.HartCvrReader.readCastVoteRecord(List, Path) At HartCvrReader.java:[line 128] M X OBL: network.brightspots.rcv.HartCvrReader.readCastVoteRecord(List, Path) may fail to clean up java.io.InputStream Obligation to clean up resource created at HartCvrReader.java:[line 80] is not discharged M X OBL: network.brightspots.rcv.CommonDataFormatReader.parseXml(List) may fail to clean up java.io.InputStream Obligation to clean up resource created at CommonDataFormatReader.java:[line 132] is not discharged M P WMI: network.brightspots.rcv.Tabulator.updatePastWinnerTallies() makes inefficient use of keySet iterator instead of entrySet iterator At Tabulator.java:[line 319] M P WMI: network.brightspots.rcv.Tabulator.updatePastWinnerTallies() makes inefficient use of keySet iterator instead of entrySet iterator At Tabulator.java:[line 273] M P WMI: network.brightspots.rcv.Tabulator.updatePastWinnerTallies() makes inefficient use of keySet iterator instead of entrySet iterator At Tabulator.java:[line 297] M P WMI: network.brightspots.rcv.Tabulator.identifyWinners(Map, SortedMap) makes inefficient use of keySet iterator instead of entrySet iterator At Tabulator.java:[line 481] M P WMI: network.brightspots.rcv.Tabulator.dropCandidatesBelowThreshold(SortedMap) makes inefficient use of keySet iterator instead of entrySet iterator At Tabulator.java:[line 567] M P WMI: network.brightspots.rcv.Tabulator.runBatchElimination(SortedMap) makes inefficient use of keySet iterator instead of entrySet iterator At Tabulator.java:[line 740] M P WMI: network.brightspots.rcv.Tabulator.computeTalliesForRound(int) makes inefficient use of keySet iterator instead of entrySet iterator At Tabulator.java:[line 1006] M C RCN: Nullcheck of config at line 143 of value previously dereferenced in network.brightspots.rcv.TabulatorSession.tabulate() At TabulatorSession.java:[line 143] H I Dm: Found reliance on default encoding in network.brightspots.rcv.TabulatorSession.tabulate(): new java.io.FileReader(String) At TabulatorSession.java:[line 152] M B OS: network.brightspots.rcv.GuiConfigController.loadTxtFileIntoString(String) may fail to close stream At GuiConfigController.java:[line 272] H I Dm: Found reliance on default encoding in network.brightspots.rcv.GuiConfigController.loadTxtFileIntoString(String): new java.io.InputStreamReader(InputStream) At GuiConfigController.java:[line 275] H I Dm: Found reliance on default encoding in network.brightspots.rcv.Tiebreak.doInteractiveCli(List): new java.util.Scanner(InputStream) At Tiebreak.java:[line 169] > Task :spotbugsMain FAILED
BrightSpots · May 2, 2022 · 43a7357 · 43a7357
1 parent ff8bd5e
commit 43a7357
Show file tree

Hide file tree

Showing 12 changed files with 272 additions and 271 deletions.
diff --git a/build.gradle b/build.gradle
@@ -1,6 +1,7 @@
 plugins {
     id "application"
     id "checkstyle"
+    id "com.github.spotbugs" version "5.0.6"
     id "idea"
     id "java-library"
     id "org.beryx.jlink" version "2.20.0"
@@ -43,9 +44,14 @@ checkstyle {
     maxWarnings = 0
     ignoreFailures = false
 }
-System.setProperty( "org.checkstyle.google.suppressionfilter.config",
+System.setProperty("org.checkstyle.google.suppressionfilter.config",
         "$projectDir/config/checkstyle/checkstyle-suppressions.xml")
 
+spotbugs {
+    toolVersion = '4.6.0'
+    excludeFilter = file("config/spotbugs/exclude.xml")
+}
+
 // ### Idea plugin settings
 idea {
     module {

diff --git a/config/spotbugs/exclude.xml b/config/spotbugs/exclude.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<FindBugsFilter
+    xmlns="https://github.com/spotbugs/filter/4.6.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+  xsi:schemaLocation="https://raw.githubusercontent.com/spotbugs/spotbugs/4.6.0/spotbugs/etc/findbugsfilter.xsd">
+  <Match>
+    <Class name="~network\.brightspots\.rcv\.(HartCvrReader\$(Contest|Option|PrecinctSplit)|RawContestConfig\$ContestRules)"/>
+    <Bug code="UwF"/>
+  </Match>
+  <Match>
+    <Class name="~network\.brightspots\.rcv\.HartCvrReader\$(Party|Contest|WriteInData|Option)"/>
+    <Bug code="UuF"/>
+  </Match>
+  <Match>
+    <Class name="network.brightspots.rcv.HartCvrReader"/>
+    <Method name="readCastVoteRecord"/>
+    <Bug code="NP"/>
+  </Match>
+</FindBugsFilter>
diff --git a/src/main/java/network/brightspots/rcv/ClearBallotCvrReader.java b/src/main/java/network/brightspots/rcv/ClearBallotCvrReader.java
@@ -20,6 +20,7 @@
 import java.io.FileNotFoundException;
 import java.io.FileReader;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -48,7 +49,7 @@ void readCastVoteRecords(List<CastVoteRecord> castVoteRecords, String contestId)
     // map for tracking unrecognized candidates during parsing
     Map<String, Integer> unrecognizedCandidateCounts = new HashMap<>();
     try {
-      csvReader = new BufferedReader(new FileReader(this.cvrPath));
+      csvReader = new BufferedReader(new FileReader(this.cvrPath, StandardCharsets.UTF_8));
       // each "choice column" in the input Csv corresponds to a unique ranking: candidate+rank pair
       // we parse these rankings from the header row into a map for lookup during CVR parsing
       String firstRow = csvReader.readLine();
@@ -103,11 +104,10 @@ void readCastVoteRecords(List<CastVoteRecord> castVoteRecords, String contestId)
         // parse rankings
         String[] cvrData = row.split(",");
         ArrayList<Pair<Integer, String>> rankings = new ArrayList<>();
-        for (int columnIndex : columnIndexToRanking.keySet()) {
-          if (Integer.parseInt(cvrData[columnIndex]) == 1) {
+        for (Map.Entry<Integer, Pair<Integer, String>> entry : columnIndexToRanking.entrySet()) {
+          if (Integer.parseInt(cvrData[entry.getKey()]) == 1) {
             // user marked this column
-            Pair<Integer, String> ranking = columnIndexToRanking.get(columnIndex);
-            rankings.add(ranking);
+            rankings.add(entry.getValue());
           }
         }
         // create the cast vote record

diff --git a/src/main/java/network/brightspots/rcv/CommonDataFormatReader.java b/src/main/java/network/brightspots/rcv/CommonDataFormatReader.java
@@ -129,165 +129,168 @@ void parseXml(List<CastVoteRecord> castVoteRecords)
     // load XML
     XmlMapper xmlMapper = new XmlMapper();
     xmlMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
-    FileInputStream inputStream = new FileInputStream(new File(filePath));
-    CastVoteRecordReport cvrReport = xmlMapper.readValue(inputStream, CastVoteRecordReport.class);
-
-    // Parse static election data:
-
-    // Find the Contest we are tabulating:
-    // Note: Contest is different from CVRContest objects which appear in CVRSnapshots
-    Contest contestToTabulate = null;
-    for (Election election : cvrReport.Election) {
-      for (Contest contest : election.Contest) {
-        if (contest.Name.equals(this.contestId)) {
-          contestToTabulate = contest;
-          break;
+    try (FileInputStream inputStream = new FileInputStream(filePath)) {
+      CastVoteRecordReport cvrReport = xmlMapper.readValue(inputStream, CastVoteRecordReport.class);
+      inputStream.close();
+
+      // Parse static election data:
+
+      // Find the Contest we are tabulating:
+      // Note: Contest is different from CVRContest objects which appear in CVRSnapshots
+      Contest contestToTabulate = null;
+      for (Election election : cvrReport.Election) {
+        for (Contest contest : election.Contest) {
+          if (contest.Name.equals(this.contestId)) {
+            contestToTabulate = contest;
+            break;
+          }
         }
       }
-    }
-
-    if (contestToTabulate == null) {
-      Logger.severe("Contest \"%s\" from config file not found!", this.contestId);
-      throw new CvrParseException();
-    }
 
-    // build a map of Candidates
-    HashMap<String, Candidate> candidateById = new HashMap<>();
-    for (Election election : cvrReport.Election) {
-      for (Candidate candidate : election.Candidate) {
-        candidateById.put(candidate.ObjectId, candidate);
+      if (contestToTabulate == null) {
+        Logger.severe("Contest \"%s\" from config file not found!", this.contestId);
+        throw new CvrParseException();
       }
-    }
 
-    // ContestSelections
-    HashMap<String, ContestSelection> contestSelectionById = new HashMap<>();
-    for (ContestSelection contestSelection : contestToTabulate.ContestSelection) {
-      contestSelectionById.put(contestSelection.ObjectId, contestSelection);
-    }
+      // build a map of Candidates
+      HashMap<String, Candidate> candidateById = new HashMap<>();
+      for (Election election : cvrReport.Election) {
+        for (Candidate candidate : election.Candidate) {
+          candidateById.put(candidate.ObjectId, candidate);
+        }
+      }
 
-    // build a map of GpUnits (aka precinct or district)
-    HashMap<String, GpUnit> gpUnitById = new HashMap<>();
-    for (GpUnit gpUnit : cvrReport.GpUnit) {
-      gpUnitById.put(gpUnit.ObjectId, gpUnit);
-    }
+      // ContestSelections
+      HashMap<String, ContestSelection> contestSelectionById = new HashMap<>();
+      for (ContestSelection contestSelection : contestToTabulate.ContestSelection) {
+        contestSelectionById.put(contestSelection.ObjectId, contestSelection);
+      }
 
-    // process the Cvrs
-    int cvrIndex = 0;
-    String fileName = new File(filePath).getName();
-    for (CVR cvr : cvrReport.CVR) {
-      CVRContest contest = getCvrContestXml(cvr, contestToTabulate);
-      if (contest == null) {
-        // the CVR does not contain any votes for this contest
-        continue;
+      // build a map of GpUnits (aka precinct or district)
+      HashMap<String, GpUnit> gpUnitById = new HashMap<>();
+      for (GpUnit gpUnit : cvrReport.GpUnit) {
+        gpUnitById.put(gpUnit.ObjectId, gpUnit);
       }
-      List<Pair<Integer, String>> rankings = new ArrayList<>();
-      // parse CVRContestSelections into rankings
-      // they will be null for an undervote
-      if (contest.CVRContestSelection != null) {
-        for (CVRContestSelection cvrContestSelection : contest.CVRContestSelection) {
-          if (cvrContestSelection.Status != null
-              && cvrContestSelection.Status.equals("needs-adjudication")) {
-            Logger.info("Contest Selection needs adjudication. Skipping.");
-            continue;
-          }
-          String contestSelectionId = cvrContestSelection.ContestSelectionId;
-          ContestSelection contestSelection = contestSelectionById.get(contestSelectionId);
-          if (contestSelection == null) {
-            Logger.severe("ContestSelection \"%s\" from CVR not found!", contestSelectionId);
-            throw new CvrParseException();
-          }
-          String candidateId;
-          // check for declared write-in:
-          if (contestSelection.IsWriteIn != null
-              && contestSelection.IsWriteIn.equals(BOOLEAN_TRUE)) {
-            candidateId = Tabulator.UNDECLARED_WRITE_IN_OUTPUT_LABEL;
-          } else {
-            // validate candidate Ids:
-            // CDF allows multiple candidate Ids to support party ticket voting options
-            // but in practice this is always a single candidate id
-            if (contestSelection.CandidateIds == null
-                || contestSelection.CandidateIds.length == 0) {
-              Logger.severe(
-                  "CandidateSelection \"%s\" has no CandidateIds!", contestSelection.ObjectId);
-              throw new CvrParseException();
-            }
-            if (contestSelection.CandidateIds.length > 1) {
-              Logger.warning(
-                  "CandidateSelection \"%s\" has multiple CandidateIds. "
-                      + "Only the first one will be processed.",
-                  contestSelection.ObjectId);
-            }
 
-            Candidate candidate = candidateById.get(contestSelection.CandidateIds[0]);
-            if (candidate == null) {
-              Logger.severe(
-                  "CandidateId \"%s\" from ContestSelectionId \"%s\" not found!",
-                  contestSelection.CandidateIds[0], contestSelection.ObjectId);
-              throw new CvrParseException();
+      // process the Cvrs
+      int cvrIndex = 0;
+      String fileName = new File(filePath).getName();
+      for (CVR cvr : cvrReport.CVR) {
+        CVRContest contest = getCvrContestXml(cvr, contestToTabulate);
+        if (contest == null) {
+          // the CVR does not contain any votes for this contest
+          continue;
+        }
+        List<Pair<Integer, String>> rankings = new ArrayList<>();
+        // parse CVRContestSelections into rankings
+        // they will be null for an undervote
+        if (contest.CVRContestSelection != null) {
+          for (CVRContestSelection cvrContestSelection : contest.CVRContestSelection) {
+            if (cvrContestSelection.Status != null
+                && cvrContestSelection.Status.equals("needs-adjudication")) {
+              Logger.info("Contest Selection needs adjudication. Skipping.");
+              continue;
             }
-            candidateId = candidate.Name;
-            if (candidateId.equals(overvoteLabel)) {
-              candidateId = Tabulator.EXPLICIT_OVERVOTE_LABEL;
-            } else if (!config.getCandidateCodeList().contains(candidateId)) {
-              Logger.severe("Unrecognized candidate found in CVR: %s", candidateId);
-              unrecognizedCandidateCounts.merge(candidateId, 1, Integer::sum);
+            String contestSelectionId = cvrContestSelection.ContestSelectionId;
+            ContestSelection contestSelection = contestSelectionById.get(contestSelectionId);
+            if (contestSelection == null) {
+              Logger.severe("ContestSelection \"%s\" from CVR not found!", contestSelectionId);
+              throw new CvrParseException();
             }
-          }
-
-          if (cvrContestSelection.Rank == null) {
-            for (SelectionPosition selectionPosition : cvrContestSelection.SelectionPosition) {
-              if (selectionPosition.CVRWriteIn != null) {
-                candidateId = Tabulator.UNDECLARED_WRITE_IN_OUTPUT_LABEL;
-              }
-              // ignore if no indication is present (NIST 1500-103 section 3.4.2)
-              if (selectionPosition.HasIndication != null
-                  && selectionPosition.HasIndication.equals(STATUS_NO)) {
-                continue;
+            String candidateId;
+            // check for declared write-in:
+            if (contestSelection.IsWriteIn != null
+                && contestSelection.IsWriteIn.equals(BOOLEAN_TRUE)) {
+              candidateId = Tabulator.UNDECLARED_WRITE_IN_OUTPUT_LABEL;
+            } else {
+              // validate candidate Ids:
+              // CDF allows multiple candidate Ids to support party ticket voting options
+              // but in practice this is always a single candidate id
+              if (contestSelection.CandidateIds == null
+                  || contestSelection.CandidateIds.length == 0) {
+                Logger.severe(
+                    "CandidateSelection \"%s\" has no CandidateIds!", contestSelection.ObjectId);
+                throw new CvrParseException();
               }
-              // skip if not allocable
-              if (selectionPosition.IsAllocable.equals(STATUS_NO)) {
-                continue;
+              if (contestSelection.CandidateIds.length > 1) {
+                Logger.warning(
+                    "CandidateSelection \"%s\" has multiple CandidateIds. "
+                        + "Only the first one will be processed.",
+                    contestSelection.ObjectId);
               }
-              if (selectionPosition.Rank == null) {
+
+              Candidate candidate = candidateById.get(contestSelection.CandidateIds[0]);
+              if (candidate == null) {
                 Logger.severe(
-                    "No Rank found on CVR \"%s\" Contest \"%s\"!", cvr.UniqueId, contest.ContestId);
+                    "CandidateId \"%s\" from ContestSelectionId \"%s\" not found!",
+                    contestSelection.CandidateIds[0], contestSelection.ObjectId);
                 throw new CvrParseException();
               }
-              Integer rank = Integer.parseInt(selectionPosition.Rank);
+              candidateId = candidate.Name;
+              if (candidateId.equals(overvoteLabel)) {
+                candidateId = Tabulator.EXPLICIT_OVERVOTE_LABEL;
+              } else if (!config.getCandidateCodeList().contains(candidateId)) {
+                Logger.severe("Unrecognized candidate found in CVR: %s", candidateId);
+                unrecognizedCandidateCounts.merge(candidateId, 1, Integer::sum);
+              }
+            }
+
+            if (cvrContestSelection.Rank == null) {
+              for (SelectionPosition selectionPosition : cvrContestSelection.SelectionPosition) {
+                if (selectionPosition.CVRWriteIn != null) {
+                  candidateId = Tabulator.UNDECLARED_WRITE_IN_OUTPUT_LABEL;
+                }
+                // ignore if no indication is present (NIST 1500-103 section 3.4.2)
+                if (selectionPosition.HasIndication != null
+                    && selectionPosition.HasIndication.equals(STATUS_NO)) {
+                  continue;
+                }
+                // skip if not allocable
+                if (selectionPosition.IsAllocable.equals(STATUS_NO)) {
+                  continue;
+                }
+                if (selectionPosition.Rank == null) {
+                  Logger.severe(
+                      "No Rank found on CVR \"%s\" Contest \"%s\"!", cvr.UniqueId,
+                      contest.ContestId);
+                  throw new CvrParseException();
+                }
+                Integer rank = Integer.parseInt(selectionPosition.Rank);
+                rankings.add(new Pair<>(rank, candidateId));
+              }
+            } else {
+              Integer rank = Integer.parseInt(cvrContestSelection.Rank);
               rankings.add(new Pair<>(rank, candidateId));
             }
-          } else {
-            Integer rank = Integer.parseInt(cvrContestSelection.Rank);
-            rankings.add(new Pair<>(rank, candidateId));
           }
         }
-      }
 
-      // Extract GPUnit if provided
-      String precinctId = null;
-      if (cvr.BallotStyleUnitId != null) {
-        GpUnit unit = gpUnitById.get(cvr.BallotStyleUnitId);
-        if (unit == null) {
-          Logger.severe(
-              "GpUnit \"%s\" for CVR \"%s\" not found!", cvr.BallotStyleUnitId, cvr.UniqueId);
-          throw new CvrParseException();
+        // Extract GPUnit if provided
+        String precinctId = null;
+        if (cvr.BallotStyleUnitId != null) {
+          GpUnit unit = gpUnitById.get(cvr.BallotStyleUnitId);
+          if (unit == null) {
+            Logger.severe(
+                "GpUnit \"%s\" for CVR \"%s\" not found!", cvr.BallotStyleUnitId, cvr.UniqueId);
+            throw new CvrParseException();
+          }
+          precinctId = unit.Name;
         }
-        precinctId = unit.Name;
-      }
 
-      String computedCastVoteRecordId = String.format("%s(%d)", fileName, ++cvrIndex);
-      // create the new CastVoteRecord
-      CastVoteRecord newRecord =
-          new CastVoteRecord(computedCastVoteRecordId, cvr.UniqueId, precinctId, null, rankings);
-      castVoteRecords.add(newRecord);
+        String computedCastVoteRecordId = String.format("%s(%d)", fileName, ++cvrIndex);
+        // create the new CastVoteRecord
+        CastVoteRecord newRecord =
+            new CastVoteRecord(computedCastVoteRecordId, cvr.UniqueId, precinctId, null, rankings);
+        castVoteRecords.add(newRecord);
 
-      // provide some user feedback on the CVR count
-      if (castVoteRecords.size() % 50000 == 0) {
-        Logger.info("Parsed %d cast vote records.", castVoteRecords.size());
+        // provide some user feedback on the CVR count
+        if (castVoteRecords.size() % 50000 == 0) {
+          Logger.info("Parsed %d cast vote records.", castVoteRecords.size());
+        }
+      }
+      if (unrecognizedCandidateCounts.size() > 0) {
+        throw new UnrecognizedCandidatesException(unrecognizedCandidateCounts);
       }
-    }
-    if (unrecognizedCandidateCounts.size() > 0) {
-      throw new UnrecognizedCandidatesException(unrecognizedCandidateCounts);
     }
   }
 
@@ -461,7 +464,7 @@ void parseJson(List<CastVoteRecord> castVoteRecords)
   // The following classes are based on the NIST 1500-103 UML structure.
   // Many of the elements represented here will not be present on any particular implementation of
   // a Cdf Cvr report. Many of these elements are also irrelevant for tabulation purposes.
-  // However they are included here for completeness and to aid in interpreting the UML.
+  // However, they are included here for completeness and to aid in interpreting the UML.
   // Note that fields identified as "boolean-like" can be (yes, no, 1, 0, or null)
   static class ContestSelection {