Code clean-up

* Use finals as and when applicable
* delete commented out code
* convert inner classes into static

Author: krmahadevan

testng-ant/src/main/java/org/testng/TestNGAntTask.java

@@ -678,20 +678,12 @@ private void addStringIfNotBlank(List<String> argv, String name, String value) {
   }
 
   private void addXmlFiles(List<String> argv) {
-    for (String file : getSuiteFileNames()) {
-      argv.add(file);
-    }
+    argv.addAll(getSuiteFileNames());
   }
 
   /** @return the list of the XML file names. This method can be overridden by subclasses. */
   protected List<String> getSuiteFileNames() {
-    List<String> result = Lists.newArrayList();
-
-    for (String file : getFiles(m_xmlFilesets)) {
-      result.add(file);
-    }
-
-    return result;
+    return Lists.newArrayList(getFiles(m_xmlFilesets));
   }
 
   private void delegateCommandSystemProperties() {

testng-asserts/src/main/java/org/testng/Assert.java

@@ -1873,7 +1873,7 @@ public static void assertEqualsNoOrder(Iterator<?> actual, Iterator<?> expected,
     if (actualCollection.size() != expectedCollection.size()) {
       failAssertNoEqual(
           "Iterators do not have the same size: "
-              + +actualCollection.size()
+              + actualCollection.size()
               + " != "
               + expectedCollection.size(),
           message);

testng-asserts/src/main/java/org/testng/FileAssert.java

@@ -2,6 +2,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Optional;
 
 /**
  * Assertion tool for File centric assertions. Conceptually, this is an extension of {@link Assert}.
@@ -80,7 +81,10 @@ public static void assertFile(File tstvalue) {
   public static void assertLength(File tstvalue, long expected, String message) {
     long actual = -1L;
     try {
-      actual = tstvalue.isDirectory() ? tstvalue.list().length : tstvalue.length();
+      actual =
+          tstvalue.isDirectory()
+              ? Optional.ofNullable(tstvalue.list()).orElse(new String[0]).length
+              : tstvalue.length();
     } catch (SecurityException e) {
       failSecurity(e, tstvalue, String.valueOf(actual), String.valueOf(expected), message);
     }
@@ -284,12 +288,6 @@ private static void failFile(File path, String actual, String expected, String m
             + (expected != null ? "<" + expected + ">" : ""));
   }
 
-  /**
-   * @param tstvalue
-   * @param string
-   * @param string2
-   * @param message
-   */
   private static void failSecurity(
       Exception e, File path, String actual, String expected, String message) {
     String formatted = "";

testng-asserts/src/test/java/org/testng/AssertTest.java

@@ -565,7 +565,7 @@ public void testAssertNotEqualsWithNull() {
     Assert.assertNotEquals(obj, obj);
   }
 
-  class Contrived {
+  static class Contrived {
 
     int integer;
 

testng-asserts/src/test/java/test/assertion/AssertionListContainsTest.java

@@ -7,9 +7,9 @@
 
 public class AssertionListContainsTest {
 
-  private User userJack = new User("Jack", 22);
-  private User userJohn = new User("John", 32);
-  private List<User> users = List.of(userJack, userJohn);
+  private final User userJack = new User("Jack", 22);
+  private final User userJohn = new User("John", 32);
+  private final List<User> users = List.of(userJack, userJohn);
 
   @Test
   public void assertListContainsObject() {
@@ -31,10 +31,10 @@ public void testAssertListNotContainsByPredicate() {
     Assert.assertListNotContains(users, user -> user.age.equals(19), "user with age 19");
   }
 
-  private class User {
+  private static class User {
 
-    private String name;
-    private Integer age;
+    private final String name;
+    private final Integer age;
 
     public User(String name, Integer age) {
       this.name = name;
@@ -56,11 +56,7 @@ public int hashCode() {
 
     @Override
     public String toString() {
-      final StringBuilder sb = new StringBuilder("User{");
-      sb.append("name='").append(name).append('\'');
-      sb.append(", age=").append(age);
-      sb.append('}');
-      return sb.toString();
+      return "User{" + "name='" + name + '\'' + ", age=" + age + '}';
     }
   }
 }

testng-asserts/src/test/java/test/asserttests/ArrayEqualityAssertTest.java

@@ -6,7 +6,6 @@
 import static org.testng.Assert.assertNotEqualsDeep;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -76,15 +75,15 @@ public void mixedArraysAssertNotEquals() {
 
   @Test(expectedExceptions = AssertionError.class)
   public void arrayInsideListAssertEquals() {
-    List<int[]> list = Arrays.asList(new int[] {42});
-    List<int[]> listCopy = Arrays.asList(new int[] {42});
+    List<int[]> list = List.of(new int[] {42});
+    List<int[]> listCopy = List.of(new int[] {42});
     assertEquals(list, listCopy, "arrays inside lists are compared by reference in assertEquals");
   }
 
   @Test
   public void arrayInsideListAssertNotEquals() {
-    List<int[]> list = Arrays.asList(new int[] {42});
-    List<int[]> listCopy = Arrays.asList(new int[] {42});
+    List<int[]> list = List.of(new int[] {42});
+    List<int[]> listCopy = List.of(new int[] {42});
     assertNotEquals(
         list, listCopy, "arrays inside lists are compared by reference in assertNotEquals");
   }
@@ -225,8 +224,8 @@ public void arrayInsideSetAssertNotEqualsDeep() {
 
   @Test(expectedExceptions = AssertionError.class)
   public void arrayDeepInListsAssertEquals() {
-    List<List<int[]>> list = Collections.singletonList(Arrays.asList(new int[] {42}));
-    List<List<int[]>> listCopy = Collections.singletonList(Arrays.asList(new int[] {42}));
+    List<List<int[]>> list = Collections.singletonList(List.of(new int[] {42}));
+    List<List<int[]>> listCopy = Collections.singletonList(List.of(new int[] {42}));
 
     assertEquals(
         list,
@@ -271,9 +270,9 @@ public void arrayDeepInListAndMapAssertEquals() {
   @Test(expectedExceptions = AssertionError.class)
   public void arrayDeepInMapAndListAssertEquals() {
     Map<String, List<int[]>> map = new HashMap<>();
-    map.put("list", Arrays.asList(new int[] {42}));
+    map.put("list", List.of(new int[] {42}));
     Map<String, List<int[]>> mapCopy = new HashMap<>();
-    mapCopy.put("list", Arrays.asList(new int[] {42}));
+    mapCopy.put("list", List.of(new int[] {42}));
 
     assertEquals(
         map,
@@ -283,8 +282,8 @@ public void arrayDeepInMapAndListAssertEquals() {
 
   @Test(expectedExceptions = AssertionError.class)
   public void arrayInsideIterableAssertEquals() {
-    Iterable<int[]> iterable = Arrays.asList(new int[] {42});
-    Iterable<int[]> iterableCopy = Arrays.asList(new int[] {42});
+    Iterable<int[]> iterable = List.of(new int[] {42});
+    Iterable<int[]> iterableCopy = List.of(new int[] {42});
     assertEquals(
         iterable,
         iterableCopy,
@@ -293,8 +292,8 @@ public void arrayInsideIterableAssertEquals() {
 
   @Test(expectedExceptions = AssertionError.class)
   public void arrayDeepInIterablesAssertEquals() {
-    List<List<int[]>> iterable = Collections.singletonList(Arrays.asList(new int[] {42}));
-    List<List<int[]>> iterableCopy = Collections.singletonList(Arrays.asList(new int[] {42}));
+    List<List<int[]>> iterable = Collections.singletonList(List.of(new int[] {42}));
+    List<List<int[]>> iterableCopy = Collections.singletonList(List.of(new int[] {42}));
 
     assertEquals(
         iterable,
@@ -323,8 +322,8 @@ public void arrayDeepInArraysAssertEquals() {
   @SuppressWarnings("unchecked")
   @Test(expectedExceptions = AssertionError.class)
   public void arrayDeepInArrayAndListAssertEquals() {
-    List<int[]>[] array = new List[] {Arrays.asList(new int[] {42})};
-    List<int[]>[] arrayCopy = new List[] {Arrays.asList(new int[] {42})};
+    List<int[]>[] array = new List[] {List.of(new int[] {42})};
+    List<int[]>[] arrayCopy = new List[] {List.of(new int[] {42})};
 
     assertEquals(
         array,

testng-core-api/src/main/java/org/testng/annotations/Parameters.java

@@ -12,7 +12,7 @@
 @Retention(RUNTIME)
 @Target({METHOD, CONSTRUCTOR, TYPE})
 public @interface Parameters {
-  public static final String NULL_VALUE = "null";
+  String NULL_VALUE = "null";
 
   /**
    * The list of variables used to fill the parameters of this method. These variables must be

testng-core-api/src/main/java/org/testng/internal/Utils.java

@@ -72,11 +72,11 @@ public static void writeUtf8File(
       final File outDir =
           (outputDir != null) ? new File(outputDir) : new File("").getAbsoluteFile();
       if (!outDir.exists()) {
-        outDir.mkdirs();
+        boolean ignored = outDir.mkdirs();
       }
       final File file = new File(outDir, fileName);
       if (!file.exists()) {
-        file.createNewFile();
+        boolean ignored = file.createNewFile();
       }
       try (final OutputStreamWriter w =
           new OutputStreamWriter(new FileOutputStream(file), StandardCharsets.UTF_8)) {
@@ -138,12 +138,12 @@ private static void writeFile(
         outDir = new File("").getAbsoluteFile();
       }
       if (!outDir.exists()) {
-        outDir.mkdirs();
+        boolean ignored = outDir.mkdirs();
       }
 
       fileName = replaceSpecialCharacters(fileName);
       File outputFile = new File(outDir, fileName);
-      outputFile.delete();
+      boolean ignored = outputFile.delete();
       log(FORMAT, 3, "Attempting to create " + outputFile);
       log(FORMAT, 3, "  Directory " + outDir + " exists: " + outDir.exists());
       outputFile.createNewFile();
@@ -186,18 +186,18 @@ public static BufferedWriter openWriter(@Nullable String outputDir, String fileN
     String outDirPath = outputDir != null ? outputDir : "";
     File outDir = new File(outDirPath);
     if (!outDir.exists()) {
-      outDir.mkdirs();
+      boolean ignored = outDir.mkdirs();
     }
     fileName = replaceSpecialCharacters(fileName);
     File outputFile = new File(outDir, fileName);
-    outputFile.delete();
+    boolean ignored = outputFile.delete();
     return openWriter(outputFile, null);
   }
 
   private static BufferedWriter openWriter(File outputFile, @Nullable String encoding)
       throws IOException {
     if (!outputFile.exists()) {
-      outputFile.createNewFile();
+      boolean ignored = outputFile.createNewFile();
     }
     OutputStreamWriter osw;
     if (null != encoding) {
@@ -227,7 +227,7 @@ public static void log(String msg) {
   public static void log(String cls, int level, String msg) {
     // Why this coupling on a static member of getVerbose()?
     if (getVerbose() >= level) {
-      if (cls.length() > 0) {
+      if (!cls.isEmpty()) {
         LOG.info("[" + cls + "] " + msg);
       } else {
         LOG.info(msg);
@@ -245,7 +245,7 @@ public static void warn(String warnMsg) {
 
   /* Tokenize the string using the separator. */
   public static String[] split(String string, String sep) {
-    if ((string == null) || (string.length() == 0)) {
+    if (string == null || string.isEmpty()) {
       return new String[0];
     }
 
@@ -275,16 +275,14 @@ public static void writeResourceToFile(File file, String resourceName, Class<?>
       LOG.error("Couldn't find resource on the class path: " + resourceName);
       return;
     }
-    try {
+    try (inputStream) {
       try (FileOutputStream outputStream = new FileOutputStream(file)) {
         int nread;
         byte[] buffer = new byte[4096];
         while (0 < (nread = inputStream.read(buffer))) {
           outputStream.write(buffer, 0, nread);
         }
       }
-    } finally {
-      inputStream.close();
     }
   }
 
@@ -293,11 +291,11 @@ public static String defaultIfStringEmpty(String s, String defaultValue) {
   }
 
   public static boolean isStringBlank(String s) {
-    return s == null || "".equals(s.trim());
+    return s == null || s.trim().isEmpty();
   }
 
   public static boolean isStringEmpty(String s) {
-    return s == null || "".equals(s);
+    return s == null || s.isEmpty();
   }
 
   public static boolean isStringNotBlank(String s) {
@@ -526,7 +524,7 @@ public static String arrayToString(String[] strings) {
    */
   public static String replaceSpecialCharacters(String fileNameParameter) {
     String fileName = fileNameParameter;
-    if (fileName == null || fileName.length() == 0) {
+    if (fileName == null || fileName.isEmpty()) {
       return fileName;
     }
     for (char element : SPECIAL_CHARACTERS) {

testng-core-api/src/main/java/org/testng/internal/objects/InstanceCreator.java

@@ -2,6 +2,7 @@
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.util.Objects;
 import org.testng.TestNGException;
 import org.testng.internal.ClassHelper;
 
@@ -16,6 +17,7 @@ private InstanceCreator() {
 
   public static <T> T newInstance(String className, Object... parameters) {
     Class<?> clazz = ClassHelper.forName(className);
+    Objects.requireNonNull(clazz, "Could not find a valid class");
     return (T) newInstance(clazz, parameters);
   }
 

testng-core-api/src/main/java/org/testng/reporters/FileStringBuffer.java

@@ -24,7 +24,7 @@
  * @since Nov 9, 2012
  */
 public class FileStringBuffer implements IBuffer {
-  private static int MAX = 100000;
+  private static final int MAX = 100000;
   private static final boolean VERBOSE = RuntimeBehavior.verboseMode();
   private static final Logger LOGGER = Logger.getLogger(FileStringBuffer.class);