Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Automate more static code checks #2028

Merged
merged 1 commit into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions config/checkstyle/appium-style.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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."/>
Expand Down Expand Up @@ -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"/>
Expand All @@ -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>
6 changes: 3 additions & 3 deletions src/main/java/io/appium/java_client/AppiumDriver.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}

Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/appium/java_client/LogsEvents.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
3 changes: 2 additions & 1 deletion src/main/java/io/appium/java_client/MobileCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 : "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/appium/java_client/internal/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down Expand Up @@ -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));
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/io/appium/java_client/proxy/Interceptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading