From f2b3d0dd4698cc2f9a6e441c4f11deeb4ad6e7fb Mon Sep 17 00:00:00 2001 From: Awakened-Redstone <40528665+Awakened-Redstone@users.noreply.github.com> Date: Sat, 3 Aug 2024 00:08:48 -0300 Subject: [PATCH] chore: Improve checkstyle checks --- build.gradle.kts | 16 +- .../src/main/kotlin/ValidateCheckstyleTask.kt | 28 +++ checkstyle-rules/build.gradle.kts | 20 ++ .../checkstyle/SameLineEmptyBlockCheck.java | 178 ++++++++++++++++++ .../checkstyle/TranslatableStringCheck.java | 62 ++++++ checkstyle.xml | 27 ++- settings.gradle.kts | 2 + 7 files changed, 314 insertions(+), 19 deletions(-) create mode 100644 buildSrc/src/main/kotlin/ValidateCheckstyleTask.kt create mode 100644 checkstyle-rules/build.gradle.kts create mode 100644 checkstyle-rules/src/main/java/com/awakenedredstone/checkstyle/SameLineEmptyBlockCheck.java create mode 100644 checkstyle-rules/src/main/java/com/awakenedredstone/checkstyle/TranslatableStringCheck.java diff --git a/build.gradle.kts b/build.gradle.kts index 09576e3..27d9360 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -158,14 +158,10 @@ dependencies { // region Tests "testmodImplementation"(sourceSets.main.get().output) + checkstyle(project(":checkstyle-rules")) //endregion } -checkstyle { - configFile = rootProject.file("checkstyle.xml") - toolVersion = "10.12.4" -} - //region Misc tasks.processResources { val map = mapOf( @@ -196,6 +192,16 @@ tasks.jar { rename { "${it}_${archivesBaseName}" } } } + +checkstyle { + configFile = rootProject.file("checkstyle.xml") + toolVersion = "10.17.0" +} + +tasks.register("validateCheckstyle") { + group = "verification" // Optional: Add the task to a group + description = "Validates the Checkstyle configuration file." +} //endregion //region Publishing diff --git a/buildSrc/src/main/kotlin/ValidateCheckstyleTask.kt b/buildSrc/src/main/kotlin/ValidateCheckstyleTask.kt new file mode 100644 index 0000000..2468b96 --- /dev/null +++ b/buildSrc/src/main/kotlin/ValidateCheckstyleTask.kt @@ -0,0 +1,28 @@ +import com.puppycrawl.tools.checkstyle.Checker +import com.puppycrawl.tools.checkstyle.ConfigurationLoader +import com.puppycrawl.tools.checkstyle.ConfigurationLoader.IgnoredModulesOptions +import com.puppycrawl.tools.checkstyle.PropertiesExpander +import com.puppycrawl.tools.checkstyle.api.CheckstyleException +import org.gradle.api.DefaultTask +import org.gradle.api.tasks.TaskAction + +open class ValidateCheckstyleTask : DefaultTask() { + @TaskAction + fun validateCheckstyle() { + try { + val configuration = ConfigurationLoader.loadConfiguration( + project.file("checkstyle.xml").absolutePath, // Path to your checkstyle.xml + PropertiesExpander(System.getProperties()), + IgnoredModulesOptions.OMIT + ) + Checker( + // ... (configuration and other parameters required by Checker) + ) + + println("Checkstyle configuration is valid.") + } catch (e: CheckstyleException) { + println("Error in Checkstyle configuration: ${e.message}") + throw e // Fail the build on error + } + } +} diff --git a/checkstyle-rules/build.gradle.kts b/checkstyle-rules/build.gradle.kts new file mode 100644 index 0000000..28c88ef --- /dev/null +++ b/checkstyle-rules/build.gradle.kts @@ -0,0 +1,20 @@ +plugins { + id("java") + `maven-publish` +} + +group = "com.awakenedredstone.checkstyle" +version = "1.0.0-dev.1" + +repositories { + mavenCentral() +} + +dependencies { + implementation("com.puppycrawl.tools:checkstyle:10.17.0") +} + +java { + sourceCompatibility = JavaVersion.VERSION_11 + targetCompatibility = JavaVersion.VERSION_11 +} diff --git a/checkstyle-rules/src/main/java/com/awakenedredstone/checkstyle/SameLineEmptyBlockCheck.java b/checkstyle-rules/src/main/java/com/awakenedredstone/checkstyle/SameLineEmptyBlockCheck.java new file mode 100644 index 0000000..c3cc22b --- /dev/null +++ b/checkstyle-rules/src/main/java/com/awakenedredstone/checkstyle/SameLineEmptyBlockCheck.java @@ -0,0 +1,178 @@ +package com.awakenedredstone.checkstyle; + +import com.puppycrawl.tools.checkstyle.api.AbstractCheck; +import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.utils.CodePointUtil; +import com.puppycrawl.tools.checkstyle.utils.CommonUtil; + +import java.util.Arrays; +import java.util.Optional; + +public class SameLineEmptyBlockCheck extends AbstractCheck { + @Override + public int[] getDefaultTokens() { + return new int[] { + TokenTypes.LITERAL_WHILE, + TokenTypes.LITERAL_TRY, // try blocks + TokenTypes.LITERAL_CATCH, // catch blocks + TokenTypes.LITERAL_FINALLY, // finally blocks + TokenTypes.LITERAL_DO, + TokenTypes.LITERAL_IF, + TokenTypes.LITERAL_ELSE, + TokenTypes.LITERAL_FOR, + TokenTypes.INSTANCE_INIT, + TokenTypes.STATIC_INIT, + TokenTypes.LITERAL_SWITCH, + TokenTypes.LITERAL_SYNCHRONIZED, + TokenTypes.LITERAL_CASE, + TokenTypes.LITERAL_DEFAULT, + TokenTypes.ARRAY_INIT, + + TokenTypes.CTOR_DEF, // Constructors + TokenTypes.METHOD_DEF, // Methods + TokenTypes.CLASS_DEF, // Classes + TokenTypes.INTERFACE_DEF, // Interfaces + TokenTypes.ENUM_DEF, // Enums + TokenTypes.RECORD_DEF, // Records + TokenTypes.SLIST, // Standalone blocks + }; + } + + @Override + public int[] getAcceptableTokens() { + return getDefaultTokens(); + } + + @Override + public int[] getRequiredTokens() { + return CommonUtil.EMPTY_INT_ARRAY; + } + + @Override + public void visitToken(DetailAST ast) { + final Optional leftCurly = getLeftCurly(ast); + if (leftCurly.isPresent()) { + final DetailAST leftCurlyAST = leftCurly.orElseThrow(); + + final boolean emptyBlock; + if (leftCurlyAST.getType() == TokenTypes.LCURLY) { + final DetailAST nextSibling = leftCurlyAST.getNextSibling(); + emptyBlock = nextSibling.getType() != TokenTypes.CASE_GROUP && nextSibling.getType() != TokenTypes.SWITCH_RULE; + } else { + emptyBlock = leftCurlyAST.getChildCount() <= 1; + } + + if (emptyBlock) { + final DetailAST rightCurlyAST = getRightCurly(leftCurlyAST); + if (rightCurlyAST == null) { + return; + } + + if (hasText(leftCurlyAST)) { + return; + } + + if (leftCurlyAST.getLineNo() != rightCurlyAST.getLineNo()) { + log(leftCurlyAST, "Empty block should be on a single line."); + } else { + final int[] codeBetweenBraces = Arrays.copyOfRange(getLineCodePoints(leftCurlyAST.getLineNo() - 1), leftCurlyAST.getColumnNo() + 1, rightCurlyAST.getColumnNo()); + if (codeBetweenBraces.length < 1 || (codeBetweenBraces.length > 1 && codeBetweenBraces[0] != 32)) { // Check if only one whitespace exists + log(leftCurlyAST, "Empty block should be on a single line with no spaces between braces."); + } + } + } + } + } + + /** + * Checks if SLIST token contains any text. + * + * @param slistAST a {@code DetailAST} value + * @return whether the SLIST token contains any text. + */ + private boolean hasText(final DetailAST slistAST) { + final DetailAST rightCurly = slistAST.findFirstToken(TokenTypes.RCURLY); + final DetailAST rightCurlyAST; + + if (rightCurly == null) { + rightCurlyAST = slistAST.getParent().findFirstToken(TokenTypes.RCURLY); + } else { + rightCurlyAST = rightCurly; + } + final int slistLineNo = slistAST.getLineNo(); + final int slistColNo = slistAST.getColumnNo(); + final int rightCurlyLineNo = rightCurlyAST.getLineNo(); + final int rightCurlyColNo = rightCurlyAST.getColumnNo(); + boolean returnValue = false; + if (slistLineNo == rightCurlyLineNo) { + // Handle braces on the same line + final int[] txt = Arrays.copyOfRange(getLineCodePoints(slistLineNo - 1), slistColNo + 1, rightCurlyColNo); + + if (!CodePointUtil.isBlank(txt)) { + returnValue = true; + } + } else { + final int[] codePointsFirstLine = getLineCodePoints(slistLineNo - 1); + final int[] firstLine = Arrays.copyOfRange(codePointsFirstLine, slistColNo + 1, codePointsFirstLine.length); + final int[] codePointsLastLine = getLineCodePoints(rightCurlyLineNo - 1); + final int[] lastLine = Arrays.copyOfRange(codePointsLastLine, 0, rightCurlyColNo); + // check if all lines are also only whitespace + returnValue = !(CodePointUtil.isBlank(firstLine) && CodePointUtil.isBlank(lastLine)) || !checkIsAllLinesAreWhitespace(slistLineNo, rightCurlyLineNo); + } + return returnValue; + } + + /** + * Checks is all lines from 'lineFrom' to 'lineTo' (exclusive) + * contain whitespaces only. + * + * @param lineFrom + * check from this line number + * @param lineTo + * check to this line numbers + * @return true if lines contain only whitespaces + */ + private boolean checkIsAllLinesAreWhitespace(int lineFrom, int lineTo) { + boolean result = true; + for (int i = lineFrom; i < lineTo - 1; i++) { + if (!CodePointUtil.isBlank(getLineCodePoints(i))) { + result = false; + break; + } + } + return result; + } + + private static Optional getLeftCurly(DetailAST ast) { + final DetailAST parent = ast.getParent(); + final int parentType = parent.getType(); + final Optional leftCurly; + + if (parentType == TokenTypes.SWITCH_RULE) { + // get left curly of a case or default that is in switch rule + leftCurly = Optional.ofNullable(parent.findFirstToken(TokenTypes.SLIST)); + } else if (parentType == TokenTypes.CASE_GROUP) { + // get left curly of a case or default that is in switch statement + leftCurly = Optional.ofNullable(ast.getNextSibling()) + .map(DetailAST::getFirstChild) + .filter(node -> node.getType() == TokenTypes.SLIST); + } else if (ast.findFirstToken(TokenTypes.SLIST) != null) { + // we have a left curly that is part of a statement list, but not in a case or default + leftCurly = Optional.of(ast.findFirstToken(TokenTypes.SLIST)); + } else { + // get the first left curly that we can find, if it is present + leftCurly = Optional.ofNullable(ast.findFirstToken(TokenTypes.LCURLY)); + } + return leftCurly; + } + + private static DetailAST getRightCurly(DetailAST leftCurly) { + DetailAST rightCurly = leftCurly.findFirstToken(TokenTypes.RCURLY); + if (rightCurly == null) { + return leftCurly.getParent().findFirstToken(TokenTypes.RCURLY); + } else { + return rightCurly; + } + } +} diff --git a/checkstyle-rules/src/main/java/com/awakenedredstone/checkstyle/TranslatableStringCheck.java b/checkstyle-rules/src/main/java/com/awakenedredstone/checkstyle/TranslatableStringCheck.java new file mode 100644 index 0000000..a5e4664 --- /dev/null +++ b/checkstyle-rules/src/main/java/com/awakenedredstone/checkstyle/TranslatableStringCheck.java @@ -0,0 +1,62 @@ +package com.awakenedredstone.checkstyle; + +import com.puppycrawl.tools.checkstyle.api.AbstractCheck; +import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; + +public class TranslatableStringCheck extends AbstractCheck { + private static final String MSG_KEY = "Not a valid translation key: \"{0}\""; + private static final String DEFAULT_PATTERN = "^[a-z0-9_]+(\\.[a-z0-9_]+)*$"; + private String pattern = DEFAULT_PATTERN; + + public void setPattern(String pattern) { + this.pattern = pattern; + } + + @Override + public int[] getDefaultTokens() { + return new int[]{TokenTypes.METHOD_CALL}; + } + + @Override + public int[] getAcceptableTokens() { + return new int[]{TokenTypes.METHOD_CALL}; + } + + @Override + public int[] getRequiredTokens() { + return new int[]{TokenTypes.METHOD_CALL}; + } + + @Override + public void visitToken(DetailAST ast) { + // Check if it's a call to Texts.translatable + if (isTextsTranslatableCall(ast)) { + // Get the argument of the Texts.translatable call + DetailAST firstToken = ast.getFirstChild().getNextSibling().findFirstToken(TokenTypes.EXPR); + if (firstToken == null) { + return; + } + + DetailAST keyArg = firstToken.getFirstChild(); + + // Check if the argument is a string literal + if (keyArg.getType() == TokenTypes.STRING_LITERAL) { + String key = keyArg.getText().replaceAll("^\"|\"$", ""); // Remove quotes + + // Check if the key matches the pattern + if (!key.matches(pattern)) { + log(ast.getLineNo(), ast.getColumnNo(), MSG_KEY, key, pattern); + } + } + } + } + + private boolean isTextsTranslatableCall(DetailAST ast) { + return ast.getChildCount() >= 2 // At least 2 children: identifier and arguments + && ast.getFirstChild().getType() == TokenTypes.DOT // First child is a dot + && ast.getFirstChild().getFirstChild().getType() == TokenTypes.IDENT // Identifier before dot + && ("Texts".equals(ast.getFirstChild().getFirstChild().getText()) || "Text".equals(ast.getFirstChild().getFirstChild().getText())) // Identifier is "Texts" or "Text" + && "translatable".equals(ast.getFirstChild().getLastChild().getText()); // Method name is "translatable" + } +} diff --git a/checkstyle.xml b/checkstyle.xml index 556d32e..b8557a9 100644 --- a/checkstyle.xml +++ b/checkstyle.xml @@ -1,6 +1,7 @@ + @@ -15,20 +16,6 @@ - - - - - - - @@ -49,6 +36,12 @@ + + + + + + @@ -88,6 +81,12 @@ + + + + + + diff --git a/settings.gradle.kts b/settings.gradle.kts index 198d1c4..2330920 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -5,3 +5,5 @@ pluginManagement { maven("https://maven.fabricmc.net") } } + +include("checkstyle-rules")