Fix errorprone (#3151)

* move comments over line

* add missing test annotation

* fix complexBooleanConstant error
http://errorprone.info/bugpattern/ComplexBooleanConstant

* fix forgotten boolean constants

* Fix not running test
Fix comment

* fix more comment issues, f*ck named parameter assumptions

* Remove suppress warnings and replace unnecessary boolean conditions

src/test/java/org/jabref/logic/bibtexkeypattern/BibtexKeyPatternUtilTest.java

@@ -595,16 +595,17 @@ public void testFirstPageNull() {
         BibtexKeyPatternUtil.firstPage(null);
     }
 
+    @Test
     public void testPagePrefix() {
         assertEquals("L", BibtexKeyPatternUtil.pagePrefix("L7--27"));
-        assertEquals("L", BibtexKeyPatternUtil.pagePrefix("L--27"));
+        assertEquals("L--", BibtexKeyPatternUtil.pagePrefix("L--27"));
         assertEquals("L", BibtexKeyPatternUtil.pagePrefix("L"));
         assertEquals("L", BibtexKeyPatternUtil.pagePrefix("L42--111"));
         assertEquals("L", BibtexKeyPatternUtil.pagePrefix("L7,L41,L73--97"));
         assertEquals("L", BibtexKeyPatternUtil.pagePrefix("L41,L7,L73--97"));
         assertEquals("L", BibtexKeyPatternUtil.pagePrefix("L43+"));
         assertEquals("", BibtexKeyPatternUtil.pagePrefix("7--27"));
-        assertEquals("", BibtexKeyPatternUtil.pagePrefix("--27"));
+        assertEquals("--", BibtexKeyPatternUtil.pagePrefix("--27"));
         assertEquals("", BibtexKeyPatternUtil.pagePrefix(""));
         assertEquals("", BibtexKeyPatternUtil.pagePrefix("42--111"));
         assertEquals("", BibtexKeyPatternUtil.pagePrefix("7,41,73--97"));

src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java

@@ -58,11 +58,13 @@ public void setUp() throws IOException {
         context.setDatabaseFile(bibFolder.newFile("test.bib"));
 
         FileDirectoryPreferences fileDirPrefs = mock(FileDirectoryPreferences.class);
-        when(fileDirPrefs.isBibLocationAsPrimary()).thenReturn(true); //Biblocation as Primary overwrites all other dirs
+        //Biblocation as Primary overwrites all other dirs
+        when(fileDirPrefs.isBibLocationAsPrimary()).thenReturn(true);
 
         worker = new CleanupWorker(context,
+                //empty fileDirPattern for backwards compatibility
                 new CleanupPreferences("\\bibtexkey",
-                        "", //empty fileDirPattern for backwards compatibility
+                        "",
                         mock(LayoutFormatterPreferences.class),
                         fileDirPrefs));
 

src/test/java/org/jabref/logic/xmp/XMPUtilTest.java

@@ -232,7 +232,8 @@ public void setUp() throws IOException, COSVisitorException {
         pdfFile = tempFolder.newFile("JabRef.pdf");
 
         try (PDDocument pdf = new PDDocument()) {
-            pdf.addPage(new PDPage()); // Need page to open in Acrobat
+            //Need page to open in Acrobat
+            pdf.addPage(new PDPage());
             pdf.save(pdfFile.getAbsolutePath());
         }
 

src/test/java/org/jabref/model/database/BibDatabaseTest.java

@@ -305,6 +305,7 @@ public void getUsedStringsNoString() {
         assertEquals(Collections.emptyList(), usedStrings);
     }
 
+    @Test
     public void preambleIsEmptyIfNotSet() {
         assertEquals(Optional.empty(), database.getPreamble());
     }

src/test/java/org/jabref/model/entry/AuthorListTest.java

@@ -7,8 +7,7 @@
 
 public class AuthorListTest {
 
-    @SuppressWarnings("unused")
-	@Test
+    @Test
     public void testFixAuthorNatbib() {
         Assert.assertEquals("", AuthorList.fixAuthorNatbib(""));
         Assert.assertEquals("Smith", AuthorList.fixAuthorNatbib("John Smith"));
@@ -20,8 +19,8 @@ public void testFixAuthorNatbib() {
         // Is not cached!
         Assert.assertTrue(AuthorList
                 .fixAuthorNatbib("John von Neumann and John Smith and Black Brown, Peter").equals(AuthorList
-                        .fixAuthorNatbib("John von Neumann" + (0 == 1 ? "" : " and ")
-                                + "John Smith and Black Brown, Peter")));
+                        .fixAuthorNatbib("John von Neumann and John Smith and Black Brown, Peter")));
+
     }
 
     @Test
@@ -32,8 +31,7 @@ public void testGetAuthorList() {
         Assert.assertFalse(al.equals(AuthorList.parse("Smith")));
     }
 
-    @SuppressWarnings("unused")
-	@Test
+    @Test
     public void testFixAuthorFirstNameFirstCommas() {
 
         // No Commas
@@ -47,9 +45,9 @@ public void testFixAuthorFirstNameFirstCommas() {
 
         // Check caching
         Assert.assertTrue(AuthorList.fixAuthorFirstNameFirstCommas(
-                "John von Neumann and John Smith and Black Brown, Peter", true, false).equals(AuthorList
-                .fixAuthorFirstNameFirstCommas("John von Neumann" + (0 == 1 ? "" : " and ")
-                        + "John Smith and Black Brown, Peter", true, false)));
+                "John von Neumann and John Smith and Black Brown, Peter", true, false).equals(
+                        AuthorList
+                                .fixAuthorFirstNameFirstCommas("John von Neumann and John Smith and Black Brown, Peter", true, false)));
 
         Assert.assertEquals("John Smith and Peter Black Brown", AuthorList
                 .fixAuthorFirstNameFirstCommas("John Smith and Black Brown, Peter", false, false));
@@ -78,9 +76,9 @@ public void testFixAuthorFirstNameFirstCommas() {
 
         // Check caching
         Assert.assertTrue(AuthorList.fixAuthorFirstNameFirstCommas(
-                "John von Neumann and John Smith and Black Brown, Peter", true, true).equals(AuthorList
-                .fixAuthorFirstNameFirstCommas("John von Neumann" + (0 == 1 ? "" : " and ")
-                        + "John Smith and Black Brown, Peter", true, true)));
+                "John von Neumann and John Smith and Black Brown, Peter", true, true).equals(
+                        AuthorList
+                                .fixAuthorFirstNameFirstCommas("John von Neumann and John Smith and Black Brown, Peter", true, true)));
 
         Assert.assertEquals("John Smith and Peter Black Brown", AuthorList
                 .fixAuthorFirstNameFirstCommas("John Smith and Black Brown, Peter", false, true));
@@ -101,8 +99,7 @@ public void testFixAuthorFirstNameFirstCommas() {
 
     }
 
-    @SuppressWarnings("unused")
-	@Test
+    @Test
     public void testFixAuthorFirstNameFirst() {
         Assert.assertEquals("John Smith", AuthorList.fixAuthorFirstNameFirst("John Smith"));
 
@@ -118,13 +115,11 @@ public void testFixAuthorFirstNameFirst() {
         // Check caching
         Assert.assertTrue(AuthorList
                 .fixAuthorFirstNameFirst("John von Neumann and John Smith and Black Brown, Peter").equals(AuthorList
-                        .fixAuthorFirstNameFirst("John von Neumann" + (0 == 1 ? "" : " and ")
-                                + "John Smith and Black Brown, Peter")));
+                        .fixAuthorFirstNameFirst("John von Neumann and John Smith and Black Brown, Peter")));
 
     }
 
-    @SuppressWarnings("unused")
-	@Test
+    @Test
     public void testFixAuthorLastNameFirstCommasNoComma() {
         // No commas before and
         Assert.assertEquals("", AuthorList.fixAuthorLastNameFirstCommas("", true, false));
@@ -136,7 +131,7 @@ public void testFixAuthorLastNameFirstCommasNoComma() {
         String a = AuthorList.fixAuthorLastNameFirstCommas("John von Neumann and John Smith and Black Brown, Peter",
                 true, false);
         String b = AuthorList.fixAuthorLastNameFirstCommas(
-                "John von Neumann" + (0 == 1 ? "" : " and ") + "John Smith and Black Brown, Peter", true, false);
+                "John von Neumann and John Smith and Black Brown, Peter", true, false);
 
         // Check caching
         Assert.assertEquals(a, b);
@@ -156,7 +151,6 @@ public void testFixAuthorLastNameFirstCommasNoComma() {
                 AuthorList.fixAuthorLastNameFirstCommas("John Peter von Neumann", true, false));
     }
 
-    @SuppressWarnings("unused")
     @Test
     public void testFixAuthorLastNameFirstCommasOxfordComma() {
         // Oxford Commas
@@ -170,8 +164,7 @@ public void testFixAuthorLastNameFirstCommasOxfordComma() {
 
         String a = AuthorList.fixAuthorLastNameFirstCommas(
                 "John von Neumann and John Smith and Black Brown, Peter", true, true);
-        String b = AuthorList.fixAuthorLastNameFirstCommas("John von Neumann"
-                + (0 == 1 ? "" : " and ") + "John Smith and Black Brown, Peter", true, true);
+        String b = AuthorList.fixAuthorLastNameFirstCommas("John von Neumann and John Smith and Black Brown, Peter", true, true);
 
         // Check caching
         Assert.assertEquals(a, b);
@@ -193,8 +186,7 @@ public void testFixAuthorLastNameFirstCommasOxfordComma() {
                 "John Peter von Neumann", true, true));
     }
 
-    @SuppressWarnings("unused")
-	@Test
+    @Test
     public void testFixAuthorLastNameFirst() {
 
         // Test helper method
@@ -212,8 +204,7 @@ public void testFixAuthorLastNameFirst() {
 
         Assert.assertTrue(AuthorList
                 .fixAuthorLastNameFirst("John von Neumann and John Smith and Black Brown, Peter").equals(AuthorList
-                        .fixAuthorLastNameFirst("John von Neumann" + (0 == 1 ? "" : " and ")
-                                + "John Smith and Black Brown, Peter")));
+                        .fixAuthorLastNameFirst("John von Neumann and John Smith and Black Brown, Peter")));
 
         // Test Abbreviation == false
         Assert.assertEquals("Smith, John", AuthorList.fixAuthorLastNameFirst("John Smith", false));
@@ -229,9 +220,9 @@ public void testFixAuthorLastNameFirst() {
                 "von Last, Jr ,First", false));
 
         Assert.assertTrue(AuthorList.fixAuthorLastNameFirst(
-                "John von Neumann and John Smith and Black Brown, Peter", false).equals(AuthorList
-                .fixAuthorLastNameFirst("John von Neumann" + (0 == 1 ? "" : " and ")
-                        + "John Smith and Black Brown, Peter", false)));
+                "John von Neumann and John Smith and Black Brown, Peter", false).equals(
+                        AuthorList
+                                .fixAuthorLastNameFirst("John von Neumann and John Smith and Black Brown, Peter", false)));
 
         // Test Abbreviate == true
         Assert.assertEquals("Smith, J.", AuthorList.fixAuthorLastNameFirst("John Smith", true));
@@ -247,14 +238,13 @@ public void testFixAuthorLastNameFirst() {
                 true));
 
         Assert.assertTrue(AuthorList.fixAuthorLastNameFirst(
-                "John von Neumann and John Smith and Black Brown, Peter", true).equals(AuthorList
-                .fixAuthorLastNameFirst("John von Neumann" + (0 == 1 ? "" : " and ")
-                        + "John Smith and Black Brown, Peter", true)));
+                "John von Neumann and John Smith and Black Brown, Peter", true).equals(
+                        AuthorList
+                                .fixAuthorLastNameFirst("John von Neumann and John Smith and Black Brown, Peter", true)));
 
     }
 
-    @SuppressWarnings("unused")
-	@Test
+    @Test
     public void testFixAuthorLastNameOnlyCommas() {
 
         // No comma before and
@@ -263,9 +253,9 @@ public void testFixAuthorLastNameOnlyCommas() {
         Assert.assertEquals("Smith", AuthorList.fixAuthorLastNameOnlyCommas("Smith, Jr, John", false));
 
         Assert.assertTrue(AuthorList.fixAuthorLastNameOnlyCommas(
-                "John von Neumann and John Smith and Black Brown, Peter", false).equals(AuthorList
-                .fixAuthorLastNameOnlyCommas("John von Neumann" + (0 == 1 ? "" : " and ")
-                        + "John Smith and Black Brown, Peter", false)));
+                "John von Neumann and John Smith and Black Brown, Peter", false).equals(
+                        AuthorList
+                                .fixAuthorLastNameOnlyCommas("John von Neumann and John Smith and Black Brown, Peter", false)));
 
         Assert.assertEquals("von Neumann, Smith and Black Brown", AuthorList
                 .fixAuthorLastNameOnlyCommas(
@@ -276,9 +266,9 @@ public void testFixAuthorLastNameOnlyCommas() {
         Assert.assertEquals("Smith", AuthorList.fixAuthorLastNameOnlyCommas("Smith, Jr, John", true));
 
         Assert.assertTrue(AuthorList.fixAuthorLastNameOnlyCommas(
-                "John von Neumann and John Smith and Black Brown, Peter", true).equals(AuthorList
-                .fixAuthorLastNameOnlyCommas("John von Neumann" + (0 == 1 ? "" : " and ")
-                        + "John Smith and Black Brown, Peter", true)));
+                "John von Neumann and John Smith and Black Brown, Peter", true).equals(
+                        AuthorList
+                                .fixAuthorLastNameOnlyCommas("John von Neumann and John Smith and Black Brown, Peter", true)));
 
         Assert.assertEquals("von Neumann, Smith, and Black Brown", AuthorList
                 .fixAuthorLastNameOnlyCommas(

src/test/java/org/jabref/shared/DBMSProcessorTest.java

@@ -121,7 +121,8 @@ public void testUpdateNewerEntry() throws OfflineLockException, SQLException {
 
         dbmsProcessor.insertEntry(bibEntry);
 
-        bibEntry.getSharedBibEntryData().setVersion(0); // simulate older version
+        //simulate older version
+        bibEntry.getSharedBibEntryData().setVersion(0);
         bibEntry.setField("year", "1993");
 
         dbmsProcessor.updateEntry(bibEntry);
@@ -132,9 +133,8 @@ public void testUpdateEqualEntry() throws OfflineLockException, SQLException {
         BibEntry expectedBibEntry = getBibEntryExample();
 
         dbmsProcessor.insertEntry(expectedBibEntry);
-
-        expectedBibEntry.getSharedBibEntryData().setVersion(0); // simulate older version
-
+        //simulate older version
+        expectedBibEntry.getSharedBibEntryData().setVersion(0);
         dbmsProcessor.updateEntry(expectedBibEntry);
 
         Optional<BibEntry> actualBibEntryOptional = dbmsProcessor

src/test/java/org/jabref/shared/DBMSSynchronizerTest.java

@@ -181,8 +181,8 @@ public void testSynchronizeLocalDatabaseWithEntryUpdate() throws OfflineLockExce
         modifiedBibEntry.setType("article");
 
         dbmsProcessor.updateEntry(modifiedBibEntry);
-
-        dbmsSynchronizer.synchronizeLocalDatabase(); // testing point
+        //testing point
+        dbmsSynchronizer.synchronizeLocalDatabase();
 
         Assert.assertEquals(bibDatabase.getEntries(), dbmsProcessor.getSharedEntries());
     }

src/test/java/org/jabref/shared/SynchronizationTestSimulator.java

@@ -61,37 +61,47 @@ public static Collection<DBMSType> getTestingDatabaseSystems() {
 
     @Test
     public void simulateEntryInsertionAndManualPull() {
-        clientContextA.getDatabase().insertEntry(getBibEntryExample(1)); // client A inserts an entry
-        clientContextA.getDatabase().insertEntry(getBibEntryExample(2)); // client A inserts another entry
-        clientContextB.getDBMSSynchronizer().pullChanges(); // client B pulls the changes
+        //client A inserts an entry
+        clientContextA.getDatabase().insertEntry(getBibEntryExample(1));
+        //client A inserts another entry
+        clientContextA.getDatabase().insertEntry(getBibEntryExample(2));
+        //client B pulls the changes
+        clientContextB.getDBMSSynchronizer().pullChanges();
 
         Assert.assertEquals(clientContextA.getDatabase().getEntries(), clientContextB.getDatabase().getEntries());
     }
 
     @Test
     public void simulateEntryUpdateAndManualPull() {
         BibEntry bibEntry = getBibEntryExample(1);
-        clientContextA.getDatabase().insertEntry(bibEntry); // client A inserts an entry
-        bibEntry.setField("custom", "custom value"); // client A changes the entry
+        //client A inserts an entry
+        clientContextA.getDatabase().insertEntry(bibEntry);
+        //client A changes the entry
+        bibEntry.setField("custom", "custom value");
+        //client B pulls the changes
         bibEntry.clearField("author");
 
-        clientContextB.getDBMSSynchronizer().pullChanges(); // client B pulls the changes
+        clientContextB.getDBMSSynchronizer().pullChanges();
 
         Assert.assertEquals(clientContextA.getDatabase().getEntries(), clientContextB.getDatabase().getEntries());
     }
 
     @Test
     public void simulateEntryDelitionAndManualPull() {
         BibEntry bibEntry = getBibEntryExample(1);
-        clientContextA.getDatabase().insertEntry(bibEntry); // client A inserts an entry
-        clientContextB.getDBMSSynchronizer().pullChanges(); // client B pulls the entry
+        //client A inserts an entry
+        clientContextA.getDatabase().insertEntry(bibEntry);
+        //client B pulls the entry
+        clientContextB.getDBMSSynchronizer().pullChanges();
 
         Assert.assertFalse(clientContextA.getDatabase().getEntries().isEmpty());
         Assert.assertFalse(clientContextB.getDatabase().getEntries().isEmpty());
         Assert.assertEquals(clientContextA.getDatabase().getEntries(), clientContextB.getDatabase().getEntries());
 
-        clientContextA.getDatabase().removeEntry(bibEntry); // client A removes the entry
-        clientContextB.getDBMSSynchronizer().pullChanges(); // client B pulls the change
+        //client A removes the entry
+        clientContextA.getDatabase().removeEntry(bibEntry);
+        //client B pulls the change
+        clientContextB.getDBMSSynchronizer().pullChanges();
 
         Assert.assertTrue(clientContextA.getDatabase().getEntries().isEmpty());
         Assert.assertTrue(clientContextB.getDatabase().getEntries().isEmpty());
@@ -100,19 +110,22 @@ public void simulateEntryDelitionAndManualPull() {
     @Test
     public void simulateUpdateOnNoLongerExistingEntry() {
         BibEntry bibEntryOfClientA = getBibEntryExample(1);
-        clientContextA.getDatabase().insertEntry(bibEntryOfClientA); // client A inserts an entry
-        clientContextB.getDBMSSynchronizer().pullChanges(); // client B pulls the entry
+        //client A inserts an entry
+        clientContextA.getDatabase().insertEntry(bibEntryOfClientA);
+        //client B pulls the entry
+        clientContextB.getDBMSSynchronizer().pullChanges();
 
         Assert.assertFalse(clientContextA.getDatabase().getEntries().isEmpty());
         Assert.assertFalse(clientContextB.getDatabase().getEntries().isEmpty());
         Assert.assertEquals(clientContextA.getDatabase().getEntries(), clientContextB.getDatabase().getEntries());
 
-        clientContextA.getDatabase().removeEntry(bibEntryOfClientA); // client A removes the entry
+        //client A removes the entry
+        clientContextA.getDatabase().removeEntry(bibEntryOfClientA);
 
         Assert.assertFalse(clientContextB.getDatabase().getEntries().isEmpty());
         Assert.assertNull(eventListenerB.getSharedEntryNotPresentEvent());
-
-        BibEntry bibEntryOfClientB = clientContextB.getDatabase().getEntries().get(0); // client B tries to update the entry
+        //client B tries to update the entry
+        BibEntry bibEntryOfClientB = clientContextB.getDatabase().getEntries().get(0);
         bibEntryOfClientB.setField("year", "2009");
 
         // here a new SharedEntryNotPresentEvent has been thrown. In this case the user B would get an pop-up window.
@@ -123,20 +136,23 @@ public void simulateUpdateOnNoLongerExistingEntry() {
     @Test
     public void simulateEntryChangeConflicts() {
         BibEntry bibEntryOfClientA = getBibEntryExample(1);
-        clientContextA.getDatabase().insertEntry(bibEntryOfClientA); // client A inserts an entry
-        clientContextB.getDBMSSynchronizer().pullChanges(); // client B pulls the entry
+        //client A inserts an entry
+        clientContextA.getDatabase().insertEntry(bibEntryOfClientA);
+        //client B pulls the entry
+        clientContextB.getDBMSSynchronizer().pullChanges();
 
-        bibEntryOfClientA.setField("year", "2001"); // A now increases the version number
+        //A now increases the version number
+        bibEntryOfClientA.setField("year", "2001");
 
         // B does nothing here, so there is no event occurrence
-
         // B now tries to update the entry
         Assert.assertFalse(clientContextB.getDatabase().getEntries().isEmpty());
 
         Assert.assertNull(eventListenerB.getUpdateRefusedEvent());
 
         BibEntry bibEntryOfClientB = clientContextB.getDatabase().getEntries().get(0);
-        bibEntryOfClientB.setField("year", "2016"); // B also tries to change something
+        //B also tries to change something
+        bibEntryOfClientB.setField("year", "2016");
 
         // B now cannot update the shared entry, due to optimistic offline lock.
         // In this case an BibEntry merge dialog pops up.