Skip to content

Issue #16378: Validate that all Inputs mentioned all default properties #16760

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.

Sign up for GitHub

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

Merged
romani merged 1 commit into from
May 19, 2025
Merged

Issue #16378: Validate that all Inputs mentioned all default properties #16760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
153 changes: 149 additions & 4 deletions src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
import com.puppycrawl.tools.checkstyle.PropertiesExpander;
import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
import com.puppycrawl.tools.checkstyle.api.Configuration;
import com.puppycrawl.tools.checkstyle.meta.ModuleDetails;
import com.puppycrawl.tools.checkstyle.meta.ModulePropertyDetails;
import com.puppycrawl.tools.checkstyle.meta.XmlMetaReader;
import com.puppycrawl.tools.checkstyle.utils.JavadocUtil;
import com.puppycrawl.tools.checkstyle.utils.TokenUtil;

Expand Down Expand Up @@ -313,9 +316,77 @@ public final class InlineConfigParser {
"com.puppycrawl.tools.checkstyle.CheckerTest$VerifyPositionAfterTabFileSet"
);

/**
* Modules missing default property mentions in input files.
* Until <a href="https://github.com/checkstyle/checkstyle/issues/16807">#16807</a>.
*/
private static final Set<String> SUPPRESSED_MODULES = Set.of(
"com.puppycrawl.tools.checkstyle.checks.DescendantTokenCheck",
"com.puppycrawl.tools.checkstyle.checks.TodoCommentCheck",
"com.puppycrawl.tools.checkstyle.checks.blocks.LeftCurlyCheck",
"com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.EqualsAvoidNullCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.HiddenFieldCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.IllegalTypeCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.MagicNumberCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.MatchXpathCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.ModifiedControlVariableCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.NestedIfDepthCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.OneStatementPerLineCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.RequireThisCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck",
"com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck",
"com.puppycrawl.tools.checkstyle.checks.design.HideUtilityClassConstructorCheck",
"com.puppycrawl.tools.checkstyle.checks.imports.CustomImportOrderCheck",
"com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheck",
"com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocContentLocationCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocParagraphCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocStyleCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTypeCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocVariableCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocMethodCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocPackageCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.SummaryJavadocCheck",
"com.puppycrawl.tools.checkstyle.checks.javadoc.WriteTagCheck",
"com.puppycrawl.tools.checkstyle.checks.metrics.BooleanExpressionComplexityCheck",
"com.puppycrawl.tools.checkstyle.checks.metrics.ClassFanOutComplexityCheck",
"com.puppycrawl.tools.checkstyle.checks.metrics.CyclomaticComplexityCheck",
"com.puppycrawl.tools.checkstyle.checks.modifier.RedundantModifierCheck",
"com.puppycrawl.tools.checkstyle.checks.naming.AbbreviationAsWordInNameCheck",
"com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck",
"com.puppycrawl.tools.checkstyle.checks.naming.LocalFinalVariableNameCheck",
"com.puppycrawl.tools.checkstyle.checks.naming.LocalVariableNameCheck",
"com.puppycrawl.tools.checkstyle.checks.naming.MemberNameCheck",
"com.puppycrawl.tools.checkstyle.checks.naming.MethodNameCheck",
"com.puppycrawl.tools.checkstyle.checks.naming.ParameterNameCheck",
"com.puppycrawl.tools.checkstyle.checks.regexp.RegexpCheck",
"com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineCheck",
"com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineJavaCheck",
"com.puppycrawl.tools.checkstyle.checks.sizes.FileLengthCheck",
"com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck",
"com.puppycrawl.tools.checkstyle.checks.sizes.ParameterNumberCheck",
"com.puppycrawl.tools.checkstyle.checks.whitespace.MethodParamPadCheck",
"com.puppycrawl.tools.checkstyle.checks.whitespace.NoWhitespaceAfterCheck",
"com.puppycrawl.tools.checkstyle.checks.whitespace.ParenPadCheck",
"com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAfterCheck",
"com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAroundCheck",
"com.puppycrawl.tools.checkstyle.checks.SuppressWarningsHolder",
"com.puppycrawl.tools.checkstyle.filters.SuppressWithPlainTextCommentFilter",
"com.puppycrawl.tools.checkstyle.filters.SuppressionCommentFilter",
"com.puppycrawl.tools.checkstyle.filters.SuppressionXpathFilter",
"com.puppycrawl.tools.checkstyle.filters.SuppressionXpathSingleFilter"
);

// This is a hack until https://github.com/checkstyle/checkstyle/issues/13845
private static final Map<String, String> MODULE_MAPPINGS = new HashMap<>();

private static final Map<String, ModuleDetails> PUBLIC_MODULE_DETAILS_MAP = new HashMap<>();

// -@cs[ExecutableStatementCount] Suppressing due to large module mappings
static {
MODULE_MAPPINGS.put("IllegalCatch",
Expand Down Expand Up @@ -568,6 +639,33 @@ private static void handleKeyValueConfig(TestInputConfiguration.Builder testInpu
}
}

private static Map<String, String> getDefaultProperties(String fullyQualifiedClassName) {

final Map<String, String> defaultProperties = new HashMap<>();
final boolean isSuppressedModule = SUPPRESSED_MODULES.contains(fullyQualifiedClassName);

if (PUBLIC_MODULE_DETAILS_MAP.isEmpty()) {
XmlMetaReader.readAllModulesIncludingThirdPartyIfAny().forEach(module -> {
PUBLIC_MODULE_DETAILS_MAP.put(module.getFullQualifiedName(), module);
});
}

final ModuleDetails moduleDetails = PUBLIC_MODULE_DETAILS_MAP.get(fullyQualifiedClassName);

if (!isSuppressedModule && moduleDetails != null) {
defaultProperties.putAll(moduleDetails.getProperties().stream()
.filter(prop -> {
return prop.getName() != null && prop.getDefaultValue() != null;
})
.collect(Collectors.toUnmodifiableMap(
ModulePropertyDetails::getName,
ModulePropertyDetails::getDefaultValue
)));
}

return defaultProperties;
Comment thread
romani marked this conversation as resolved.
}

private static String getFullyQualifiedClassName(String filePath, String moduleName)
throws CheckstyleException {
String fullyQualifiedClassName;
Comment thread
romani marked this conversation as resolved.
Expand Down Expand Up @@ -831,6 +929,51 @@ private static String readPropertiesContent(int beginLineNo, List<String> lines)
return stringBuilder.toString();
}

private static void validateProperties(Map<String, String> propertiesWithMissingDefaultTag,
List<String> unusedProperties) throws CheckstyleException {

if (!propertiesWithMissingDefaultTag.isEmpty()) {
Comment thread
romani marked this conversation as resolved.

final String propertiesList = propertiesWithMissingDefaultTag.entrySet().stream()
.map(entry -> {
return String.format(Locale.ROOT, "%s = (default)%s",
entry.getKey(), entry.getValue());
})
.collect(Collectors.joining(", "));

final String message = String.format(Locale.ROOT,
"Default properties must use the '(default)' tag."
+ " Properties missing the '(default)' tag: %s", propertiesList);
throw new CheckstyleException(message);
Comment thread
romani marked this conversation as resolved.
}
if (!unusedProperties.isEmpty()) {
final String message = String.format(Locale.ROOT,
"All properties must be explicitly specified."
+ " Found unused properties: %s", unusedProperties);
throw new CheckstyleException(message);
Comment thread
romani marked this conversation as resolved.
}
}

private static void validateDefaultProperties(
Map<Object, Object> actualProperties,
Map<String, String> defaultProperties) throws CheckstyleException {

final Map<String, String> matchedProperties = actualProperties.entrySet().stream()
Comment thread
romani marked this conversation as resolved.
.filter(entry -> {
return entry.getValue()
.equals(defaultProperties.get(entry.getKey().toString()));
})
.collect(HashMap::new,
(map, entry) -> {
map.put(entry.getKey().toString(), entry.getValue().toString());
}, HashMap::putAll);
final List<String> missingProperties = defaultProperties.keySet().stream()
.filter(propertyName -> !actualProperties.containsKey(propertyName))
.collect(Collectors.toUnmodifiableList());

validateProperties(matchedProperties, missingProperties);
Comment thread
romani marked this conversation as resolved.
Comment thread
romani marked this conversation as resolved.
}

private static void setProperties(ModuleInputConfiguration.Builder inputConfigBuilder,
String inputFilePath,
List<String> lines,
Expand All @@ -839,6 +982,10 @@ private static void setProperties(ModuleInputConfiguration.Builder inputConfigBu

final String propertyContent = readPropertiesContent(beginLineNo, lines);
final Map<Object, Object> properties = loadProperties(propertyContent);
final String fullyQualifiedClassName =
getFullyQualifiedClassName(inputFilePath, moduleName);

validateDefaultProperties(properties, getDefaultProperties(fullyQualifiedClassName));

for (final Map.Entry<Object, Object> entry : properties.entrySet()) {
final String key = entry.getKey().toString();
Expand All @@ -857,10 +1004,7 @@ else if (value.startsWith("(file)")) {
}
else if (value.startsWith("(default)")) {
final String defaultValue = value.substring(value.indexOf(')') + 1);
final String fullyQualifiedModuleName =
getFullyQualifiedClassName(inputFilePath, moduleName);

validateDefault(key, defaultValue, fullyQualifiedModuleName);
validateDefault(key, defaultValue, fullyQualifiedClassName);

if (NULL_STRING.equals(defaultValue)) {
inputConfigBuilder.addDefaultProperty(key, null);
Expand Down Expand Up @@ -1207,4 +1351,5 @@ private static boolean isNull(String propertyDefaultValue) {
|| "null".equals(propertyDefaultValue)
|| "\"\"".equals(propertyDefaultValue);
}

}
97 changes: 48 additions & 49 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/FinalParametersCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,17 @@ protected String getPackageLocation() {
@Test
public void testDefaultTokens() throws Exception {
final String[] expected = {
"27:26: " + getCheckMessage(MSG_KEY, "s"),
"42:26: " + getCheckMessage(MSG_KEY, "i"),
"47:26: " + getCheckMessage(MSG_KEY, "s"),
"57:17: " + getCheckMessage(MSG_KEY, "s"),
"73:17: " + getCheckMessage(MSG_KEY, "s"),
"79:17: " + getCheckMessage(MSG_KEY, "s"),
"94:45: " + getCheckMessage(MSG_KEY, "e"),
"97:36: " + getCheckMessage(MSG_KEY, "e"),
"114:18: " + getCheckMessage(MSG_KEY, "aParam"),
"117:18: " + getCheckMessage(MSG_KEY, "args"),
"120:18: " + getCheckMessage(MSG_KEY, "args"),
"28:26: " + getCheckMessage(MSG_KEY, "s"),
"43:26: " + getCheckMessage(MSG_KEY, "i"),
"48:26: " + getCheckMessage(MSG_KEY, "s"),
"58:17: " + getCheckMessage(MSG_KEY, "s"),
"74:17: " + getCheckMessage(MSG_KEY, "s"),
"80:17: " + getCheckMessage(MSG_KEY, "s"),
"95:45: " + getCheckMessage(MSG_KEY, "e"),
"98:36: " + getCheckMessage(MSG_KEY, "e"),
"115:18: " + getCheckMessage(MSG_KEY, "aParam"),
"118:18: " + getCheckMessage(MSG_KEY, "args"),
"121:18: " + getCheckMessage(MSG_KEY, "args"),
};
verifyWithInlineConfigParser(
getPath("InputFinalParameters.java"), expected);
Expand All @@ -55,9 +55,9 @@ public void testDefaultTokens() throws Exception {
@Test
public void testCtorToken() throws Exception {
final String[] expected = {
"28:27: " + getCheckMessage(MSG_KEY, "s"),
"43:27: " + getCheckMessage(MSG_KEY, "i"),
"48:27: " + getCheckMessage(MSG_KEY, "s"),
"29:27: " + getCheckMessage(MSG_KEY, "s"),
"44:27: " + getCheckMessage(MSG_KEY, "i"),
"49:27: " + getCheckMessage(MSG_KEY, "s"),
};
verifyWithInlineConfigParser(
getPath("InputFinalParameters2.java"), expected);
Expand All @@ -66,14 +66,14 @@ public void testCtorToken() throws Exception {
@Test
public void testMethodToken() throws Exception {
final String[] expected = {
"58:17: " + getCheckMessage(MSG_KEY, "s"),
"74:17: " + getCheckMessage(MSG_KEY, "s"),
"80:17: " + getCheckMessage(MSG_KEY, "s"),
"95:45: " + getCheckMessage(MSG_KEY, "e"),
"98:36: " + getCheckMessage(MSG_KEY, "e"),
"115:18: " + getCheckMessage(MSG_KEY, "aParam"),
"118:18: " + getCheckMessage(MSG_KEY, "args"),
"121:18: " + getCheckMessage(MSG_KEY, "args"),
"59:17: " + getCheckMessage(MSG_KEY, "s"),
"75:17: " + getCheckMessage(MSG_KEY, "s"),
"81:17: " + getCheckMessage(MSG_KEY, "s"),
"96:45: " + getCheckMessage(MSG_KEY, "e"),
"99:36: " + getCheckMessage(MSG_KEY, "e"),
"116:18: " + getCheckMessage(MSG_KEY, "aParam"),
"119:18: " + getCheckMessage(MSG_KEY, "args"),
"122:18: " + getCheckMessage(MSG_KEY, "args"),
};
verifyWithInlineConfigParser(
getPath("InputFinalParameters3.java"), expected);
Expand All @@ -82,9 +82,9 @@ public void testMethodToken() throws Exception {
@Test
public void testCatchToken() throws Exception {
final String[] expected = {
"130:16: " + getCheckMessage(MSG_KEY, "npe"),
"136:16: " + getCheckMessage(MSG_KEY, "e"),
"139:16: " + getCheckMessage(MSG_KEY, "e"),
"131:16: " + getCheckMessage(MSG_KEY, "npe"),
"137:16: " + getCheckMessage(MSG_KEY, "e"),
"140:16: " + getCheckMessage(MSG_KEY, "e"),
};
verifyWithInlineConfigParser(
getPath("InputFinalParameters4.java"), expected);
Expand All @@ -93,8 +93,8 @@ public void testCatchToken() throws Exception {
@Test
public void testForEachClauseToken() throws Exception {
final String[] expected = {
"157:13: " + getCheckMessage(MSG_KEY, "s"),
"165:13: " + getCheckMessage(MSG_KEY, "s"),
"158:13: " + getCheckMessage(MSG_KEY, "s"),
"166:13: " + getCheckMessage(MSG_KEY, "s"),
};
verifyWithInlineConfigParser(
getPath("InputFinalParameters5.java"), expected);
Expand All @@ -103,13 +103,13 @@ public void testForEachClauseToken() throws Exception {
@Test
public void testIgnorePrimitiveTypesParameters() throws Exception {
final String[] expected = {
"14:22: " + getCheckMessage(MSG_KEY, "k"),
"15:15: " + getCheckMessage(MSG_KEY, "s"),
"15:25: " + getCheckMessage(MSG_KEY, "o"),
"19:15: " + getCheckMessage(MSG_KEY, "array"),
"20:31: " + getCheckMessage(MSG_KEY, "s"),
"21:22: " + getCheckMessage(MSG_KEY, "l"),
"21:32: " + getCheckMessage(MSG_KEY, "s"),
"15:22: " + getCheckMessage(MSG_KEY, "k"),
"16:15: " + getCheckMessage(MSG_KEY, "s"),
"16:25: " + getCheckMessage(MSG_KEY, "o"),
"20:15: " + getCheckMessage(MSG_KEY, "array"),
"21:31: " + getCheckMessage(MSG_KEY, "s"),
"22:22: " + getCheckMessage(MSG_KEY, "l"),
"22:32: " + getCheckMessage(MSG_KEY, "s"),
};
verifyWithInlineConfigParser(
getPath("InputFinalParametersPrimitiveTypes.java"), expected);
Expand All @@ -118,20 +118,20 @@ public void testIgnorePrimitiveTypesParameters() throws Exception {
@Test
public void testPrimitiveTypesParameters() throws Exception {
final String[] expected = {
"13:14: " + getCheckMessage(MSG_KEY, "i"),
"14:15: " + getCheckMessage(MSG_KEY, "i"),
"14:22: " + getCheckMessage(MSG_KEY, "k"),
"14:32: " + getCheckMessage(MSG_KEY, "s"),
"19:15: " + getCheckMessage(MSG_KEY, "s"),
"19:25: " + getCheckMessage(MSG_KEY, "o"),
"19:35: " + getCheckMessage(MSG_KEY, "l"),
"24:15: " + getCheckMessage(MSG_KEY, "array"),
"25:15: " + getCheckMessage(MSG_KEY, "i"),
"25:22: " + getCheckMessage(MSG_KEY, "x"),
"25:31: " + getCheckMessage(MSG_KEY, "s"),
"30:15: " + getCheckMessage(MSG_KEY, "x"),
"30:22: " + getCheckMessage(MSG_KEY, "l"),
"30:32: " + getCheckMessage(MSG_KEY, "s"),
"14:14: " + getCheckMessage(MSG_KEY, "i"),
"15:15: " + getCheckMessage(MSG_KEY, "i"),
"15:22: " + getCheckMessage(MSG_KEY, "k"),
"15:32: " + getCheckMessage(MSG_KEY, "s"),
"20:15: " + getCheckMessage(MSG_KEY, "s"),
"20:25: " + getCheckMessage(MSG_KEY, "o"),
"20:35: " + getCheckMessage(MSG_KEY, "l"),
"25:15: " + getCheckMessage(MSG_KEY, "array"),
"26:15: " + getCheckMessage(MSG_KEY, "i"),
"26:22: " + getCheckMessage(MSG_KEY, "x"),
"26:31: " + getCheckMessage(MSG_KEY, "s"),
"31:15: " + getCheckMessage(MSG_KEY, "x"),
"31:22: " + getCheckMessage(MSG_KEY, "l"),
"31:32: " + getCheckMessage(MSG_KEY, "s"),
};
verifyWithInlineConfigParser(
getPath("InputFinalParametersPrimitiveTypes2.java"), expected);
Expand Down Expand Up @@ -173,5 +173,4 @@ public void testUnnamedParametersPropertyFalse() throws Exception {
verifyWithInlineConfigParser(
getNonCompilablePath("InputFinalParametersUnnamedPropertyFalse.java"), expected);
}

}
1 change: 1 addition & 0 deletions ...esources/com/puppycrawl/tools/checkstyle/checks/finalparameters/InputFinalParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
FinalParameters
ignorePrimitiveTypes = (default)false
ignoreUnnamedParameters = (default)true
tokens = (default)METHOD_DEF, CTOR_DEF


Expand Down
1 change: 1 addition & 0 deletions ...sources/com/puppycrawl/tools/checkstyle/checks/finalparameters/InputFinalParameters2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
FinalParameters
ignorePrimitiveTypes = (default)false
ignoreUnnamedParameters = (default)true
tokens = CTOR_DEF


Expand Down
1 change: 1 addition & 0 deletions ...sources/com/puppycrawl/tools/checkstyle/checks/finalparameters/InputFinalParameters3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
FinalParameters
ignorePrimitiveTypes = (default)false
ignoreUnnamedParameters = (default)true
tokens = METHOD_DEF


Expand Down
1 change: 1 addition & 0 deletions ...sources/com/puppycrawl/tools/checkstyle/checks/finalparameters/InputFinalParameters4.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
FinalParameters
ignorePrimitiveTypes = (default)false
ignoreUnnamedParameters = (default)true
tokens = LITERAL_CATCH


Expand Down
1 change: 1 addition & 0 deletions ...sources/com/puppycrawl/tools/checkstyle/checks/finalparameters/InputFinalParameters5.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
FinalParameters
ignorePrimitiveTypes = (default)false
ignoreUnnamedParameters = (default)true
tokens = FOR_EACH_CLAUSE


Expand Down
Loading