Skip to content

Commit

Permalink
Cleanup code (openhab#458)
Browse files Browse the repository at this point in the history
This cleanup includes:

* Fix deprecations
* Fix format argument mismatches
* Fix JavaDocs
* Fix typos
* Remove redundant boxing
* Remove redundant imports
* Remove redundant modifiers
* Remove redundant toString calls
* Remove redundant variable initialization
* Use diamond operator
* Use enhanced for loops
* Use secure URLs with XML
* Use try-with-resources with Files.list
* Replace Collectors.joining with String.join
* Replace lambdas with method references

Signed-off-by: Wouter Born <[email protected]>
  • Loading branch information
wborn authored Jan 10, 2024
1 parent 13982a8 commit 8d02268
Show file tree
Hide file tree
Showing 53 changed files with 143 additions and 169 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void setCheckInnerUnits(boolean checkInnerUnits) {
}

public void setRequiredContributionDescriptions(String[] contributionDescriptions) {
this.requiredContributionDescriptions = new ArrayList<String>(Arrays.asList(contributionDescriptions));
this.requiredContributionDescriptions = new ArrayList<>(Arrays.asList(contributionDescriptions));
setWarningMessageFirstAuthorDescription(requiredContributionDescriptions);
}

Expand All @@ -97,8 +97,7 @@ public void setRequiredContributionDescriptions(String[] contributionDescription
*/
private void setWarningMessageFirstAuthorDescription(List<String> contributionDescriptions) {
warningMessageFirstAuthorDescription = contributionDescriptions.stream().map(Object::toString)
.collect(Collectors.joining(WARNING_MESSAGE_DELIMITER, WARNING_MESSAGE_PREFIX, WARNING_MESSAGE_SUFFIX))
.toString();
.collect(Collectors.joining(WARNING_MESSAGE_DELIMITER, WARNING_MESSAGE_PREFIX, WARNING_MESSAGE_SUFFIX));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void beginTree(DetailAST rootAST) {
public void finishTree(DetailAST ast) {
importsToLineNumbers.entrySet().stream()
.filter(entry -> forbiddenPackages.stream().anyMatch(entry.getKey()::contains))
.filter(entry -> !exceptions.stream().anyMatch(entry.getKey()::contains))
.filter(entry -> exceptions.stream().noneMatch(entry.getKey()::contains))
.forEach(entry -> log(entry.getValue(), String.format(MESSAGE, entry.getKey())));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ public int[] getDefaultTokens() {

@Override
public int[] getAcceptableTokens() {
/**
* The check will be executed for method and constructor declarations.
*/
// The check will be executed for method and constructor declarations.
return new int[] { TokenTypes.METHOD_DEF, TokenTypes.CTOR_DEF, };
}

Expand All @@ -139,7 +137,7 @@ private void visit(DetailAST ast) {

if (textBlock != null) {
String[] text = textBlock.getText();
lineNumberToLineText = new HashMap<Integer, String>();
lineNumberToLineText = new HashMap<>();
tagLines = new ArrayList<>();

for (int javadocLineIndex = 0; javadocLineIndex < text.length; javadocLineIndex++) {
Expand Down Expand Up @@ -206,7 +204,6 @@ private void logInformation(String group, String message) {
lineNumberToLineText.forEach((lineNumber, lineText) -> {
if (lineText.contains(group)) {
log(lineNumber, message);
return;
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ protected void processFiltered(File file, FileText fileText) throws CheckstyleEx

boolean isFound = false;

for (int i = 0; i < individualPaths.length; i++) {
String singlePath = individualPaths[i];

for (String singlePath : individualPaths) {
Path featurePath = resolveRecursively(file.toPath(), Paths.get(singlePath));

if (featurePath == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
*/
public class NullAnnotationsCheck extends AbstractCheck {

private List<String> imports = new ArrayList<String>();
private List<String> imports = new ArrayList<>();

private static final String NONNULL_ANNOTATION = NonNull.class.getSimpleName();
private static final String NULLABLE_ANNOTATION = Nullable.class.getSimpleName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public void setMaxLabelLengthError(final String maxLabelLength) {
}

public void setCheckWordCasing(final String check) {
this.doCheckWordCasing = Boolean.valueOf(check);
this.doCheckWordCasing = Boolean.parseBoolean(check);
}

@Override
Expand All @@ -134,7 +134,7 @@ protected void checkThingTypeFile(final FileText xmlFileText) throws CheckstyleE

private void evaluateExpressionOnFile(final FileText xmlFileText, final String xPathExpression, final String key)
throws CheckstyleException {
final String type = fiterType(xPathExpression);
final String type = filterType(xPathExpression);
final Map<String, Integer> lineNumberMap = new HashMap<>();
final NodeList nodes = getNodes(xmlFileText, xPathExpression);
final File file = xmlFileText.getFile();
Expand Down Expand Up @@ -170,8 +170,8 @@ private void checkWordCasing(final FileText xmlFileText, final Map<String, Integ
for (int i = 0; i < words.length; i++) {
final String word = words[i];

final int firtCharType = Character.getType(word.charAt(0));
final boolean lowerCase = firtCharType == Character.LOWERCASE_LETTER;
final int firstCharType = Character.getType(word.charAt(0));
final boolean lowerCase = firstCharType == Character.LOWERCASE_LETTER;
final boolean firstOrLastWord = i == 0 || i == words.length - 1;
boolean log = false;

Expand All @@ -180,7 +180,7 @@ private void checkWordCasing(final FileText xmlFileText, final Map<String, Integ
log = true;
}
} else if (!firstOrLastWord && LOWER_CASE_WORDS.contains(word)
&& firtCharType == Character.UPPERCASE_LETTER) {
&& firstCharType == Character.UPPERCASE_LETTER) {
log = true;
}
if (log) {
Expand Down Expand Up @@ -228,7 +228,7 @@ private String getReferenceId(final String key, final Node node) {
return node.getParentNode().getParentNode().getAttributes().getNamedItem(key).getNodeValue();
}

private String fiterType(final String expression) {
private String filterType(final String expression) {
final Matcher matcher = TYPE_PATTERN.matcher(expression);
return matcher.find() ? matcher.group(1) : "";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.stream.Stream;

import org.openhab.tools.analysis.checkstyle.api.AbstractOhInfXmlCheck;
import org.openhab.tools.analysis.checkstyle.api.CheckConstants;
Expand Down Expand Up @@ -137,9 +138,8 @@ private Map<String, File> getConfigurableServiceRefs(Path basePath) {
if (!Files.exists(osgiInfPath)) {
return uriFileMap;
}
try {
Files.list(osgiInfPath)
.forEach(xmlPath -> uriFileMap.putAll(getConfigurableServiceRefsFromXml(xmlPath)));
try (Stream<Path> pathStream = Files.list(osgiInfPath)) {
pathStream.forEach(xmlPath -> uriFileMap.putAll(getConfigurableServiceRefsFromXml(xmlPath)));
} catch (IOException e) {
}
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

import org.openhab.tools.analysis.checkstyle.api.AbstractOhInfXmlCheck;
import org.openhab.tools.analysis.utils.CachingHttpClient;
import org.openhab.tools.analysis.utils.ContentReceviedCallback;
import org.openhab.tools.analysis.utils.ContentReceivedCallback;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xml.sax.SAXException;
Expand Down Expand Up @@ -94,7 +94,7 @@ public OhInfXmlValidationCheck() {

@Override
public void beginProcessing(String charset) {
ContentReceviedCallback<Schema> callback = new ContentReceviedCallback<Schema>() {
ContentReceivedCallback<Schema> callback = new ContentReceivedCallback<>() {
SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ public void setFileTypes(String[] value) {

@Override
protected void processFiltered(File file, FileText fileText) {
processTabIdentationCheck(fileText);
processTabIndentationCheck(fileText);
}

private void processTabIdentationCheck(FileText fileText) {
private void processTabIndentationCheck(FileText fileText) {
for (int lineNumber = 0; lineNumber < fileText.size(); lineNumber++) {
String line = fileText.get(lineNumber);
// if line is empty and does not contain only tabs for indentation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ protected void processFiltered(File file, FileText fileText) throws CheckstyleEx
try {
nodes = (NodeList) xpathExpression.evaluate(document, XPathConstants.NODESET);
} catch (XPathExpressionException e) {
logger.error("An error has occured while parsing the pom.xml. Check if the file is valid.", e);
logger.error("An error has occurred while parsing the pom.xml. Check if the file is valid.", e);
}

if (nodes != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.List;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
import java.util.stream.Collectors;

import org.apache.commons.io.FilenameUtils;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -74,7 +73,7 @@ public void setValues(String[] values) {
@Override
protected void postProcessHeaderLines() {
final List<String> headerLines = getHeaderLines();
String header = headerLines.stream().collect(Collectors.joining(SEPARATOR));
String header = String.join(SEPARATOR, headerLines);
String formattedHeader = MessageFormat.format(header, values);

for (String line : formattedHeader.split(SEPARATOR)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private void processPomXmlFile(FileText fileText) throws CheckstyleException {
// parent folder)
if (parentPom.exists()) {
Optional<Document> maybeDocument = getParsedPom(parentPom);
if (!maybeDocument.isPresent()) {
if (maybeDocument.isEmpty()) {
return;
}

Expand Down Expand Up @@ -163,7 +163,7 @@ public void finishProcessing() {

private Optional<String> getPomVersion(Document pomXmlDocument, String filePath) throws CheckstyleException {
Optional<String> maybeVersionNodeValue = getNodeValue(pomXmlDocument, POM_VERSION_XPATH_EXPRESSION, filePath);
if (!maybeVersionNodeValue.isPresent()) {
if (maybeVersionNodeValue.isEmpty()) {
return getNodeValue(pomXmlDocument, POM_PARENT_VERSION_XPATH_EXPRESSION, filePath);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ protected Document parseDomDocumentFromFile(FileText fileText) throws Checkstyle
Document document = builder.parse(getInputStream(fileText));
return document;
} catch (ParserConfigurationException e) {
throw new CheckstyleException("Serious configuration error occured while creating a DocumentBuilder.", e);
throw new CheckstyleException("Serious configuration error occurred while creating a DocumentBuilder.", e);
} catch (SAXException e) {
throw new CheckstyleException("Unable to read from file: " + fileText.getFile().getAbsolutePath(), e);
} catch (IOException e) {
Expand Down Expand Up @@ -160,17 +160,17 @@ protected org.jsoup.nodes.Document parseHTMLDocumentFromFile(FileText fileText)
/**
* Compiles an XPathExpression
*
* @param expresion the XPath expression
* @param expression the XPath expression
* @return compiled XPath expression
* @throws CheckstyleException if an error occurred during the compilation
*/
protected XPathExpression compileXPathExpression(String expresion) throws CheckstyleException {
protected XPathExpression compileXPathExpression(String expression) throws CheckstyleException {
XPathFactory factory = XPathFactory.newInstance();
XPath xpath = factory.newXPath();
try {
return xpath.compile(expresion);
return xpath.compile(expression);
} catch (XPathExpressionException e) {
throw new CheckstyleException("Unable to compile the expression" + expresion, e);
throw new CheckstyleException("Unable to compile the expression" + expression, e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@ public MarkdownCheck() {

@Override
protected void processFiltered(File file, FileText fileText) throws CheckstyleException {
switch (file.getName()) {
case README_MD_FILE_NAME:
checkReadMe(fileText);
break;
if (file.getName().equals(README_MD_FILE_NAME)) {
checkReadMe(fileText);
}
}

Expand All @@ -59,12 +57,7 @@ private void checkReadMe(FileText fileText) {

Node readmeMarkdownNode = parseMarkdown(fileText, options);
// CallBack is used in order to use the protected log method of the AbstractStaticCheck in the Visitor
MarkdownVisitorCallback callBack = new MarkdownVisitorCallback() {
@Override
public void log(int line, String message) {
MarkdownCheck.this.log(line + 1, message);
}
};
MarkdownVisitorCallback callBack = (line, message) -> MarkdownCheck.this.log(line + 1, message);
MarkdownVisitor visitor = new MarkdownVisitor(callBack, fileText);
visitor.visit(readmeMarkdownNode);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,9 @@ private void checkEmptyLineBefore(ListBlock listBlock) {
// Not checking if the line above is empty if it's another list item
return;
} else {
boolean isListfirstLineInFile = firstLineOfList == 0;
boolean isListFirstLineInFile = firstLineOfList == 0;
// The first line of the file can NOT be list
if (isListfirstLineInFile || !StringUtils.isBlank(fileText.get(firstLineOfList - 1))) {
if (isListFirstLineInFile || !StringUtils.isBlank(fileText.get(firstLineOfList - 1))) {
// Log the one-based first line of the list
callback.log(firstLineOfList, EMPTY_LINE_BEFORE_LIST_MSG);
}
Expand All @@ -145,10 +145,10 @@ private void checkEmptyLineAfterList(ListBlock listBlock) {

boolean isListEnd = lastListItemContent instanceof Paragraph;
if (isListEnd) {
String[] lastListItemlines = lastListItemContent.getChars().toString().split(REGEX_NEW_LINES);
if (lastListItemlines.length > 1) {
for (int i = 1; i < lastListItemlines.length; i++) {
if (!lastListItemlines[i].startsWith(" ")) {
String[] lastListItemLines = lastListItemContent.getChars().toString().split(REGEX_NEW_LINES);
if (lastListItemLines.length > 1) {
for (int i = 1; i < lastListItemLines.length; i++) {
if (!lastListItemLines[i].startsWith(" ")) {
// Log the one-based line where there is an empty line
callback.log(lastListItemContent.getLineNumber(), EMPTY_LINE_AFTER_LIST_MSG);
break;
Expand All @@ -159,7 +159,7 @@ private void checkEmptyLineAfterList(ListBlock listBlock) {
}

public void visit(ListBlock list) {
list.getChildIterator().forEachRemaining(listItem -> visit(listItem));
list.getChildIterator().forEachRemaining(this::visit);
processListBlock(list);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ public interface MarkdownVisitorCallback {
* @param line line number of the logged message
* @param message the message that is logged for the error
*/
public void log(int line, String message);
void log(int line, String message);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
/**
* A simple caching HttpClient
*
* A {@link ContentReceviedCallback} is used to convert the downloaded data.
* A {@link ContentReceivedCallback} is used to convert the downloaded data.
* The cached entry expires after {@link #RETRY_TIME}
*
* @author Svilen Valkanov - Initial contribution
Expand All @@ -43,9 +43,9 @@ public class CachingHttpClient<T> {
private static Cache<URL, Optional<byte[]>> cache = CacheBuilder.newBuilder()
.expireAfterWrite(RETRY_TIME, TimeUnit.MINUTES).build();

private ContentReceviedCallback<T> callback;
private ContentReceivedCallback<T> callback;

public CachingHttpClient(ContentReceviedCallback<T> callback) {
public CachingHttpClient(ContentReceivedCallback<T> callback) {
this.callback = callback;
}

Expand All @@ -64,14 +64,14 @@ public synchronized T get(URL url) throws IOException {
throw new IllegalArgumentException("URL must not be null");
}

Optional<byte[]> content = Optional.empty();
Optional<byte[]> content;
try {
content = cache.get(url, () -> Optional.of(getContent(url)));
} catch (ExecutionException e) {
cache.put(url, Optional.empty());
throw new IOException("Unable to get " + url.toString(), e.getCause());
throw new IOException("Unable to get " + url, e.getCause());
}
return content.isPresent() ? callback.transform(content.get()) : null;
return content.map(bytes -> callback.transform(bytes)).orElse(null);
}

private byte[] getContent(URL url) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
*
* @param <T> - the type of the object
*/
public interface ContentReceviedCallback<T> {
public interface ContentReceivedCallback<T> {
/**
* Called after a successful download attempt is made by the {@link CachingHttpClient}
* and should transform the data into a object of type T
*
* @param content HTTP request content, can`t be null
* @return the transformed data
*/
public T transform(byte[] content);
T transform(byte[] content);
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ public void testOuterAndInnerClassesWithNoOtherAuthorContributionDescription() t
* warning messages are expected at the lines where other authors of outer and inner
* classes are located, because other author contribution descriptions are missing there
*/
int firstWarningLineOthertAuthor = 3;
int firstWarningLineOtherAuthor = 3;
int secondWarningLineOtherAuthor = 10;
lineNumberToWarningMessageExpected.put(firstWarningLineOthertAuthor,
lineNumberToWarningMessageExpected.put(firstWarningLineOtherAuthor,
EXPECTED_WARNING_MESSAGE_OTHER_AUTHOR_DESCRIPTION);
lineNumberToWarningMessageExpected.put(secondWarningLineOtherAuthor,
EXPECTED_WARNING_MESSAGE_OTHER_AUTHOR_DESCRIPTION);
Expand Down Expand Up @@ -280,7 +280,7 @@ public void testOuterAndInnerClassesWithAuthorContributionDescriptions() throws

private void checkFileForAuthorContributionDescription(boolean checkInnerUnits, String fileName) throws Exception {
String filePath = getPath(fileName);
String[] expected = null;
String[] expected;

if (lineNumberToWarningMessageExpected.isEmpty()) {
expected = CommonUtil.EMPTY_STRING_ARRAY;
Expand Down
Loading

0 comments on commit 8d02268

Please sign in to comment.