chore: Automate more static code checks (#2028)

appium · Oct 3, 2023 · f875c3b · f875c3b
1 parent a5a25b5
commit f875c3b
Show file tree

Hide file tree

Showing 31 changed files with 77 additions and 60 deletions.
diff --git a/config/checkstyle/appium-style.xml b/config/checkstyle/appium-style.xml
@@ -128,6 +128,7 @@
             <message key="name.invalidPattern"
              value="Interface type name ''{0}'' must match pattern ''{1}''."/>
         </module>
+        <module name="ConstantName"/>
         <module name="GenericWhitespace">
             <message key="ws.followed"
              value="GenericWhitespace ''{0}'' is followed by whitespace."/>
@@ -198,6 +199,12 @@
 
         </module>
         <module name="CommentsIndentation"/>
+        <module name="MissingOverride"/>
+        <module name="UnnecessaryParentheses"/>
+        <module name="HideUtilityClassConstructor"/>
+
+        <!-- Make the @SuppressWarnings annotations available to Checkstyle -->
+        <module name="SuppressWarningsHolder"/>
     </module>
     <module name="LineLength">
         <property name="max" value="120"/>
@@ -208,4 +215,7 @@
         <property name="file" value="${config_loc}/suppressions.xml"/>
         <property name="optional" value="false"/>
     </module>
+
+    <!-- Filter out Checkstyle warnings that have been suppressed with the @SuppressWarnings annotation -->
+    <module name="SuppressWarningsFilter"/>
 </module>
diff --git a/src/main/java/io/appium/java_client/AppiumDriver.java b/src/main/java/io/appium/java_client/AppiumDriver.java
@@ -70,7 +70,7 @@ public class AppiumDriver extends RemoteWebDriver implements
         CanRememberExtensionPresence,
         HasSettings {
 
-    private static final ErrorHandler errorHandler = new ErrorHandler(new ErrorCodesMobile(), true);
+    private static final ErrorHandler ERROR_HANDLER = new ErrorHandler(new ErrorCodesMobile(), true);
     // frequently used command parameters
     private final URL remoteAddress;
     protected final RemoteLocationContext locationContext;
@@ -89,7 +89,7 @@ public AppiumDriver(HttpCommandExecutor executor, Capabilities capabilities) {
         super(executor, capabilities);
         this.executeMethod = new AppiumExecutionMethod(this);
         locationContext = new RemoteLocationContext(executeMethod);
-        super.setErrorHandler(errorHandler);
+        super.setErrorHandler(ERROR_HANDLER);
         this.remoteAddress = executor.getAddressOfRemoteServer();
     }
 
@@ -167,7 +167,7 @@ public AppiumDriver(URL remoteSessionAddress, String platformName, String automa
         setCommandExecutor(executor);
         this.executeMethod = new AppiumExecutionMethod(this);
         locationContext = new RemoteLocationContext(executeMethod);
-        super.setErrorHandler(errorHandler);
+        super.setErrorHandler(ERROR_HANDLER);
         this.remoteAddress = executor.getAddressOfRemoteServer();
 
         setSessionId(sessionAddress.getId());

diff --git a/src/main/java/io/appium/java_client/CommandExecutionHelper.java b/src/main/java/io/appium/java_client/CommandExecutionHelper.java
@@ -17,8 +17,6 @@
 package io.appium.java_client;
 
 import com.google.common.collect.ImmutableMap;
-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
 import org.openqa.selenium.remote.Response;
 
 import javax.annotation.Nullable;
@@ -28,9 +26,11 @@
 
 import static org.openqa.selenium.remote.DriverCommand.EXECUTE_SCRIPT;
 
-@NoArgsConstructor(access = AccessLevel.PRIVATE)
 public final class CommandExecutionHelper {
 
+    private CommandExecutionHelper() {
+    }
+
     @Nullable
     public static <T> T execute(
             ExecutesMethod executesMethod, Map.Entry<String, Map<String, ?>> keyValuePair

diff --git a/src/main/java/io/appium/java_client/LogsEvents.java b/src/main/java/io/appium/java_client/LogsEvents.java
@@ -63,8 +63,8 @@ default ServerEvents getEvents() {
             .stream()
             .map((Map<String, Object> cmd) -> new CommandEvent(
                 (String) cmd.get("cmd"),
-                ((Long) cmd.get("startTime")),
-                ((Long) cmd.get("endTime"))
+                (Long) cmd.get("startTime"),
+                (Long) cmd.get("endTime")
             ))
             .collect(Collectors.toList());
 

diff --git a/src/main/java/io/appium/java_client/MobileCommand.java b/src/main/java/io/appium/java_client/MobileCommand.java
@@ -38,6 +38,7 @@
  * Most of these commands are platform-specific obsolete things and should eventually be replaced with
  * calls to corresponding `mobile:` extensions, so we don't abuse non-w3c APIs
  */
+@SuppressWarnings({"checkstyle:HideUtilityClassConstructor", "checkstyle:ConstantName"})
 public class MobileCommand {
     //General
     @Deprecated
@@ -434,7 +435,7 @@ public static ImmutableMap<String, Object> prepareArguments(String[] params,
                                                                 Object[] values) {
         ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
         for (int i = 0; i < params.length; i++) {
-            if (!StringUtils.isBlank(params[i]) && (values[i] != null)) {
+            if (!StringUtils.isBlank(params[i]) && values[i] != null) {
                 builder.put(params[i], values[i]);
             }
         }

diff --git a/src/main/java/io/appium/java_client/android/AndroidMobileCommandHelper.java b/src/main/java/io/appium/java_client/android/AndroidMobileCommandHelper.java
@@ -214,8 +214,7 @@ public class AndroidMobileCommandHelper extends MobileCommand {
         String intentAction, String intentCategory, String intentFlags,
         String optionalIntentArguments, boolean stopApp) throws IllegalArgumentException {
 
-        checkArgument((!StringUtils.isBlank(appPackage)
-                && !StringUtils.isBlank(appActivity)),
+        checkArgument(!StringUtils.isBlank(appPackage) && !StringUtils.isBlank(appActivity),
             String.format("'%s' and '%s' are required.", "appPackage", "appActivity"));
 
         String targetWaitPackage = !StringUtils.isBlank(appWaitPackage) ? appWaitPackage : "";

diff --git a/src/main/java/io/appium/java_client/internal/CapabilityHelpers.java b/src/main/java/io/appium/java_client/internal/CapabilityHelpers.java
@@ -16,8 +16,6 @@
 
 package io.appium.java_client.internal;
 
-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
 import org.openqa.selenium.Capabilities;
 
 import javax.annotation.Nullable;
@@ -28,10 +26,12 @@
 import java.util.List;
 import java.util.function.Function;
 
-@NoArgsConstructor(access = AccessLevel.PRIVATE)
 public class CapabilityHelpers {
     public static final String APPIUM_PREFIX = "appium:";
 
+    private CapabilityHelpers() {
+    }
+
     /**
      * Helper that is used for capability values retrieval.
      * Supports both prefixed W3C and "classic" capability names.

diff --git a/src/main/java/io/appium/java_client/internal/Config.java b/src/main/java/io/appium/java_client/internal/Config.java
@@ -11,7 +11,7 @@
 public class Config {
     private static Config mainInstance = null;
     private static final String MAIN_CONFIG = "main.properties";
-    private static final Map<String, Properties> cache = new ConcurrentHashMap<>();
+    private static final Map<String, Properties> CACHE = new ConcurrentHashMap<>();
     private final String configName;
 
     /**
@@ -58,7 +58,7 @@ public <T> T getValue(String key, Class<T> valueType) {
      * @throws ClassCastException if the retrieved value cannot be cast to `valueType` type
      */
     public <T> Optional<T> getOptionalValue(String key, Class<T> valueType) {
-        final Properties cachedProps = cache.computeIfAbsent(configName, k -> {
+        final Properties cachedProps = CACHE.computeIfAbsent(configName, k -> {
             try (InputStream configFileStream = getClass().getClassLoader().getResourceAsStream(configName)) {
                 final Properties p = new Properties();
                 p.load(configFileStream);

diff --git a/src/main/java/io/appium/java_client/internal/ReflectionHelpers.java b/src/main/java/io/appium/java_client/internal/ReflectionHelpers.java
@@ -16,15 +16,15 @@
 
 package io.appium.java_client.internal;
 
-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
 import org.openqa.selenium.WebDriverException;
 
 import java.lang.reflect.Field;
 
-@NoArgsConstructor(access = AccessLevel.PRIVATE)
 public class ReflectionHelpers {
 
+    private ReflectionHelpers() {
+    }
+
     /**
      * Sets the given value to a private instance field.
      *

diff --git a/src/main/java/io/appium/java_client/internal/SessionHelpers.java b/src/main/java/io/appium/java_client/internal/SessionHelpers.java
@@ -16,9 +16,7 @@
 
 package io.appium.java_client.internal;
 
-import lombok.AccessLevel;
 import lombok.Data;
-import lombok.NoArgsConstructor;
 import org.openqa.selenium.InvalidArgumentException;
 import org.openqa.selenium.WebDriverException;
 
@@ -27,10 +25,12 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-@NoArgsConstructor(access = AccessLevel.PRIVATE)
 public class SessionHelpers {
     private static final Pattern SESSION = Pattern.compile("/session/([^/]+)");
 
+    private SessionHelpers() {
+    }
+
     @Data public static class SessionAddress {
         private final URL serverUrl;
         private final String id;

diff --git a/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java b/src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java
@@ -62,7 +62,7 @@
  */
 public class AppiumFieldDecorator implements FieldDecorator {
 
-    private static final List<Class<? extends WebElement>> availableElementClasses = ImmutableList.of(
+    private static final List<Class<? extends WebElement>> AVAILABLE_ELEMENT_CLASSES = ImmutableList.of(
             WebElement.class,
             RemoteWebElement.class
     );
@@ -169,7 +169,7 @@ protected boolean isDecoratableList(Field field) {
                 List<Type> bounds = (listType instanceof TypeVariable)
                         ? Arrays.asList(((TypeVariable<?>) listType).getBounds())
                         : Collections.emptyList();
-                return availableElementClasses.stream()
+                return AVAILABLE_ELEMENT_CLASSES.stream()
                         .anyMatch(webElClass -> webElClass.equals(listType) || bounds.contains(webElClass));
             }
         };

diff --git a/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java b/src/main/java/io/appium/java_client/pagefactory/DefaultElementByBuilder.java
@@ -186,7 +186,7 @@ protected By buildMobileNativeBy() {
     @Override
     public boolean isLookupCached() {
         AnnotatedElement annotatedElement = annotatedElementContainer.getAnnotated();
-        return (annotatedElement.getAnnotation(CacheLookup.class) != null);
+        return annotatedElement.getAnnotation(CacheLookup.class) != null;
     }
 
     private By returnMappedBy(By byDefault, By nativeAppBy) {

diff --git a/src/main/java/io/appium/java_client/pagefactory/OverrideWidgetReader.java b/src/main/java/io/appium/java_client/pagefactory/OverrideWidgetReader.java
@@ -17,8 +17,6 @@
 package io.appium.java_client.pagefactory;
 
 import io.appium.java_client.pagefactory.bys.ContentType;
-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
 
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Constructor;
@@ -31,14 +29,16 @@
 import static io.appium.java_client.remote.MobilePlatform.IOS;
 import static io.appium.java_client.remote.MobilePlatform.WINDOWS;
 
-@NoArgsConstructor(access = AccessLevel.PRIVATE)
 class OverrideWidgetReader {
     private static final Class<? extends Widget> EMPTY = Widget.class;
     private static final String HTML = "html";
     private static final String ANDROID_UI_AUTOMATOR = "androidUIAutomator";
     private static final String IOS_XCUIT_AUTOMATION = "iOSXCUITAutomation";
     private static final String WINDOWS_AUTOMATION = "windowsAutomation";
 
+    private OverrideWidgetReader() {
+    }
+
     @SuppressWarnings("unchecked")
     private static Class<? extends Widget> getConvenientClass(Class<? extends Widget> declaredClass,
                                                               AnnotatedElement annotatedElement, String method) {

diff --git a/src/main/java/io/appium/java_client/pagefactory/ThrowableUtil.java b/src/main/java/io/appium/java_client/pagefactory/ThrowableUtil.java
@@ -16,17 +16,17 @@
 
 package io.appium.java_client.pagefactory;
 
-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
 import org.openqa.selenium.InvalidSelectorException;
 import org.openqa.selenium.StaleElementReferenceException;
 
 import java.lang.reflect.InvocationTargetException;
 
-@NoArgsConstructor(access = AccessLevel.PRIVATE)
 class ThrowableUtil {
     private static final String INVALID_SELECTOR_PATTERN = "Invalid locator strategy:";
 
+    private ThrowableUtil() {
+    }
+
     protected static boolean isInvalidSelectorRootCause(Throwable e) {
         if (e == null) {
             return false;

diff --git a/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java b/src/main/java/io/appium/java_client/pagefactory/WidgetInterceptor.java
@@ -67,7 +67,7 @@ protected Object getObject(WebElement element, Method method, Object[] args) thr
         ContentType type = getCurrentContentType(element);
         WebElement cachedElement = cachedElementReference == null ? null : cachedElementReference.get();
         if (cachedElement == null || !cachedInstances.containsKey(type)
-            || (locator != null && !((CacheableLocator) locator).isLookUpCached())
+            || locator != null && !((CacheableLocator) locator).isLookUpCached()
         ) {
             cachedElementReference = new WeakReference<>(element);
 

diff --git a/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java b/src/main/java/io/appium/java_client/pagefactory/WidgetListInterceptor.java
@@ -71,7 +71,7 @@ protected Object getObject(List<WebElement> elements, Method method, Object[] ar
                     .filter(Objects::nonNull)
                     .collect(Collectors.toList());
         if (cachedElements.size() != cachedWidgets.size()
-                || (locator != null && !((CacheableLocator) locator).isLookUpCached())) {
+                || locator != null && !((CacheableLocator) locator).isLookUpCached()) {
             cachedWidgets.clear();
             cachedElementReferences.clear();
 

diff --git a/src/main/java/io/appium/java_client/pagefactory/WithTimeout.java b/src/main/java/io/appium/java_client/pagefactory/WithTimeout.java
@@ -16,9 +16,6 @@
 
 package io.appium.java_client.pagefactory;
 
-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
-
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -46,8 +43,10 @@
      */
     ChronoUnit chronoUnit();
 
-    @NoArgsConstructor(access = AccessLevel.PRIVATE)
     class DurationBuilder {
+        private DurationBuilder() {
+        }
+
         static Duration build(WithTimeout withTimeout) {
             return Duration.of(withTimeout.time(), withTimeout.chronoUnit());
         }

diff --git a/src/main/java/io/appium/java_client/pagefactory/utils/WebDriverUnpackUtility.java b/src/main/java/io/appium/java_client/pagefactory/utils/WebDriverUnpackUtility.java
@@ -18,8 +18,6 @@
 
 import io.appium.java_client.HasBrowserCheck;
 import io.appium.java_client.pagefactory.bys.ContentType;
-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
 import org.openqa.selenium.ContextAware;
 import org.openqa.selenium.SearchContext;
 import org.openqa.selenium.WebDriver;
@@ -33,10 +31,12 @@
 import static java.util.Optional.ofNullable;
 import static org.apache.commons.lang3.StringUtils.containsIgnoreCase;
 
-@NoArgsConstructor(access = AccessLevel.PRIVATE)
 public final class WebDriverUnpackUtility {
     private static final String NATIVE_APP_PATTERN = "NATIVE_APP";
 
+    private WebDriverUnpackUtility() {
+    }
+
     /**
      * This method extract an instance of {@link WebDriver} from the given {@link SearchContext}.
      * @param searchContext is an instance of {@link SearchContext}. It may be the instance of

diff --git a/src/main/java/io/appium/java_client/proxy/Interceptor.java b/src/main/java/io/appium/java_client/proxy/Interceptor.java
@@ -16,8 +16,6 @@
 
 package io.appium.java_client.proxy;
 
-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
 import net.bytebuddy.implementation.bind.annotation.AllArguments;
 import net.bytebuddy.implementation.bind.annotation.Origin;
 import net.bytebuddy.implementation.bind.annotation.RuntimeType;
@@ -31,9 +29,11 @@
 import java.util.UUID;
 import java.util.concurrent.Callable;
 
-@NoArgsConstructor(access = AccessLevel.PRIVATE)
 public class Interceptor {
-    private static final Logger logger = LoggerFactory.getLogger(Interceptor.class);
+    private static final Logger LOGGER = LoggerFactory.getLogger(Interceptor.class);
+
+    private Interceptor() {
+    }
 
     /**
      * A magic method used to wrap public method calls in classes
@@ -64,7 +64,7 @@ public static Object intercept(
             } catch (NotImplementedException e) {
                 // ignore
             } catch (Exception e) {
-                logger.atError()
+                LOGGER.atError()
                         .addArgument(() -> self.getClass().getName())
                         .addArgument(method::getName)
                         .log("Got an unexpected error in beforeCall listener of {}.{} method", e);
@@ -110,7 +110,7 @@ public static Object intercept(
             } catch (NotImplementedException e) {
                 // ignore
             } catch (Exception e) {
-                logger.atError()
+                LOGGER.atError()
                         .addArgument(() -> self.getClass().getName())
                         .addArgument(method::getName)
                         .log("Got an unexpected error in afterCall listener of {}.{} method", e);