From 32a50567902c073629768f21c8fc1e276fb0e187 Mon Sep 17 00:00:00 2001 From: Anmol202005 Date: Wed, 29 Oct 2025 04:13:38 +0530 Subject: [PATCH] Issue #122: Support multiple instances of the same Checkstyle check --- .../checkstyle/autofix/CheckstyleAutoFix.java | 4 +- .../checkstyle/autofix/CheckstyleCheck.java | 10 ++++- .../autofix/CheckstyleConfigModule.java | 38 ++++++++++++++++++ .../autofix/CheckstyleRecipeRegistry.java | 39 +++++++++++++++++-- .../autofix/parser/CheckstyleViolation.java | 14 +++---- .../autofix/parser/ConfigurationLoader.java | 17 +++++--- .../autofix/parser/SarifReportParser.java | 13 ++----- .../autofix/parser/XmlReportParser.java | 19 +++------ .../parser/CheckstyleReportsParserTest.java | 3 +- .../singlelocaltest/DiffSingleLocalTest.diff | 2 +- .../singlelocaltest/InputSingleLocalTest.java | 4 +- .../OutputSingleLocalTest.java | 4 +- 12 files changed, 120 insertions(+), 47 deletions(-) create mode 100644 src/main/java/org/checkstyle/autofix/CheckstyleConfigModule.java diff --git a/src/main/java/org/checkstyle/autofix/CheckstyleAutoFix.java b/src/main/java/org/checkstyle/autofix/CheckstyleAutoFix.java index 76b76f3..c909a65 100644 --- a/src/main/java/org/checkstyle/autofix/CheckstyleAutoFix.java +++ b/src/main/java/org/checkstyle/autofix/CheckstyleAutoFix.java @@ -88,7 +88,7 @@ public List getRecipeList() { final ReportParser reportParser = createReportParser(getViolationReportPath()); final List violations = reportParser .parse(Path.of(getViolationReportPath())); - final Map configuration = loadCheckstyleConfiguration(); return CheckstyleRecipeRegistry.getRecipes(violations, configuration); @@ -108,7 +108,7 @@ else if (path.endsWith(".sarif") || path.endsWith(".sarif.json")) { return result; } - private Map loadCheckstyleConfiguration() { + private Map loadCheckstyleConfiguration() { return ConfigurationLoader.loadConfiguration(getConfigurationPath(), getPropertiesPath()); } } diff --git a/src/main/java/org/checkstyle/autofix/CheckstyleCheck.java b/src/main/java/org/checkstyle/autofix/CheckstyleCheck.java index fef6b72..9b18706 100644 --- a/src/main/java/org/checkstyle/autofix/CheckstyleCheck.java +++ b/src/main/java/org/checkstyle/autofix/CheckstyleCheck.java @@ -37,9 +37,15 @@ public String getId() { return id; } - public static Optional fromSource(String source) { + public static Optional fromSource(String checkId) { return Arrays.stream(values()) - .filter(check -> check.getId().contains(source)) + .filter(check -> check.getId().contains(checkId)) + .findFirst(); + } + + public static Optional fromSourceExact(String checkId) { + return Arrays.stream(values()) + .filter(check -> check.getId().equals(checkId)) .findFirst(); } } diff --git a/src/main/java/org/checkstyle/autofix/CheckstyleConfigModule.java b/src/main/java/org/checkstyle/autofix/CheckstyleConfigModule.java new file mode 100644 index 0000000..7a6ebc7 --- /dev/null +++ b/src/main/java/org/checkstyle/autofix/CheckstyleConfigModule.java @@ -0,0 +1,38 @@ +/////////////////////////////////////////////////////////////////////////////////////////////// +// checkstyle-openrewrite-recipes: Automatically fix Checkstyle violations with OpenRewrite. +// Copyright (C) 2025 The Checkstyle OpenRewrite Recipes Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +/////////////////////////////////////////////////////////////////////////////////////////////// + +package org.checkstyle.autofix; + +public class CheckstyleConfigModule { + private final CheckstyleCheck check; + private final String id; + + public CheckstyleConfigModule(CheckstyleCheck check, String id) { + this.check = check; + this.id = id; + } + + public boolean matchesId(String input) { + return id != null && id.equals(input); + } + + public boolean matchesCheck(String input) { + return CheckstyleCheck.fromSourceExact(input) + .map(checkFromInput -> checkFromInput == check) + .orElse(false); + } +} diff --git a/src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java b/src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java index 997be2b..366a117 100644 --- a/src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java +++ b/src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java @@ -43,6 +43,8 @@ public final class CheckstyleRecipeRegistry { CheckConfiguration, Recipe>> RECIPE_MAP_WITH_CONFIG = new EnumMap<>(CheckstyleCheck.class); + private static final String HASH_SEPARATOR = "#"; + static { RECIPE_MAP.put(CheckstyleCheck.UPPER_ELL, UpperEll::new); RECIPE_MAP.put(CheckstyleCheck.HEX_LITERAL_CASE, HexLiteralCase::new); @@ -65,18 +67,49 @@ private CheckstyleRecipeRegistry() { * @return a list of generated Recipe objects */ public static List getRecipes(List violations, - Map config) { + Map config) { return violations.stream() - .collect(Collectors.groupingBy(CheckstyleViolation::getSource)) + .collect(Collectors.groupingBy(CheckstyleViolation::getCheckId)) .entrySet() .stream() .map(entry -> { - return createRecipe(entry.getValue(), config.get(entry.getKey())); + final CheckConfiguration configuration = + findMatchingConfiguration(entry.getKey(), config); + return createRecipe(entry.getValue(), configuration); }) .filter(Objects::nonNull) .collect(Collectors.toList()); } + private static CheckConfiguration findMatchingConfiguration(String source, + Map config) { + return config.entrySet().stream() + .filter(configEntry -> matchesSource(configEntry.getKey(), source)) + .map(Map.Entry::getValue) + .findFirst() + .orElse(null); + } + + private static boolean matchesSource(CheckstyleConfigModule module, String source) { + final boolean matches; + if (source.contains(HASH_SEPARATOR)) { + matches = matchesWithHashSeparator(module, source); + } + else { + matches = module.matchesId(source) || module.matchesCheck(source); + } + return matches; + } + + private static boolean matchesWithHashSeparator(CheckstyleConfigModule module, String source) { + final String[] parts = source.split(HASH_SEPARATOR, 2); + final String checkPart = parts[0]; + final String idPart = parts[1]; + final boolean exactMatch = module.matchesCheck(checkPart) && module.matchesId(idPart); + final boolean individualMatch = module.matchesId(source) || module.matchesCheck(source); + return exactMatch || individualMatch; + } + private static Recipe createRecipe(List violations, CheckConfiguration checkConfig) { Recipe result = null; diff --git a/src/main/java/org/checkstyle/autofix/parser/CheckstyleViolation.java b/src/main/java/org/checkstyle/autofix/parser/CheckstyleViolation.java index d03dadb..6344a5a 100644 --- a/src/main/java/org/checkstyle/autofix/parser/CheckstyleViolation.java +++ b/src/main/java/org/checkstyle/autofix/parser/CheckstyleViolation.java @@ -19,8 +19,6 @@ import java.nio.file.Path; -import org.checkstyle.autofix.CheckstyleCheck; - public final class CheckstyleViolation { private final int line; @@ -29,24 +27,24 @@ public final class CheckstyleViolation { private final String severity; - private final CheckstyleCheck source; + private final String checkId; private final String message; private final Path filePath; public CheckstyleViolation(int line, int column, String severity, - CheckstyleCheck source, String message, Path filePath) { + String source, String message, Path filePath) { this.line = line; this.column = column; this.severity = severity; - this.source = source; + this.checkId = source; this.message = message; this.filePath = filePath; } public CheckstyleViolation(int line, String severity, - CheckstyleCheck source, String message, Path filePath) { + String source, String message, Path filePath) { this(line, -1, severity, source, message, filePath); } @@ -58,8 +56,8 @@ public Integer getColumn() { return column; } - public CheckstyleCheck getSource() { - return source; + public String getCheckId() { + return checkId; } public String getMessage() { diff --git a/src/main/java/org/checkstyle/autofix/parser/ConfigurationLoader.java b/src/main/java/org/checkstyle/autofix/parser/ConfigurationLoader.java index 1f1247a..5f5214e 100644 --- a/src/main/java/org/checkstyle/autofix/parser/ConfigurationLoader.java +++ b/src/main/java/org/checkstyle/autofix/parser/ConfigurationLoader.java @@ -20,11 +20,13 @@ import java.io.FileInputStream; import java.io.IOException; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; import java.util.Properties; import org.checkstyle.autofix.CheckstyleCheck; +import org.checkstyle.autofix.CheckstyleConfigModule; import com.puppycrawl.tools.checkstyle.PropertiesExpander; import com.puppycrawl.tools.checkstyle.api.CheckstyleException; @@ -36,14 +38,19 @@ private ConfigurationLoader() { // utility class } - public static Map mapConfiguration(Configuration config) { - final Map result = new HashMap<>(); + public static Map mapConfiguration(Configuration config) { + + final Map result = new LinkedHashMap<>(); final Map inherited = getProperties(config); final Optional module = CheckstyleCheck.fromSource(config.getName()); module.ifPresent(checkstyleCheck -> { - result.put(checkstyleCheck, new CheckConfiguration(checkstyleCheck, new HashMap<>(), - getProperties(config))); + final Map properties = getProperties(config); + final CheckstyleConfigModule configModule = new CheckstyleConfigModule( + checkstyleCheck, properties.get("id")); + result.put(configModule, + new CheckConfiguration(checkstyleCheck, new HashMap<>(), properties)); }); for (Configuration child : config.getChildren()) { @@ -69,7 +76,7 @@ private static Map getProperties(Configuration config) { return props; } - public static Map loadConfiguration( + public static Map loadConfiguration( String checkstyleConfigurationPath, String propFile) { Properties props = new Properties(); if (propFile == null) { diff --git a/src/main/java/org/checkstyle/autofix/parser/SarifReportParser.java b/src/main/java/org/checkstyle/autofix/parser/SarifReportParser.java index 5035b32..94c4bb3 100644 --- a/src/main/java/org/checkstyle/autofix/parser/SarifReportParser.java +++ b/src/main/java/org/checkstyle/autofix/parser/SarifReportParser.java @@ -25,8 +25,6 @@ import java.util.List; import java.util.Optional; -import org.checkstyle.autofix.CheckstyleCheck; - import de.jcup.sarif_2_1_0.SarifSchema210ImportExportSupport; import de.jcup.sarif_2_1_0.model.PhysicalLocation; import de.jcup.sarif_2_1_0.model.Region; @@ -57,17 +55,14 @@ public List parse(Path reportPath) { for (final Run run: report.getRuns()) { if (run.getResults() != null) { run.getResults().forEach(resultEntry -> { - CheckstyleCheck.fromSource(resultEntry.getRuleId()).ifPresent(check -> { - final CheckstyleViolation violation = createViolation(check, resultEntry); - result.add(violation); - }); + result.add(createViolation(resultEntry.getRuleId(), resultEntry)); }); } } return result; } - private CheckstyleViolation createViolation(CheckstyleCheck check, Result result) { + private CheckstyleViolation createViolation(String checkId, Result result) { final String severity = result.getLevel().name(); final String message = result.getMessage().getText(); final PhysicalLocation location = result.getLocations().get(0).getPhysicalLocation(); @@ -76,8 +71,8 @@ private CheckstyleViolation createViolation(CheckstyleCheck check, Result result final int line = region.getStartLine(); final Optional columnMaybe = Optional.ofNullable(region.getStartColumn()); return columnMaybe.map(column -> { - return new CheckstyleViolation(line, column, severity, check, message, filePath); - }).orElse(new CheckstyleViolation(line, severity, check, message, filePath)); + return new CheckstyleViolation(line, column, severity, checkId, message, filePath); + }).orElse(new CheckstyleViolation(line, severity, checkId, message, filePath)); } private Path getFilePath(PhysicalLocation location) { diff --git a/src/main/java/org/checkstyle/autofix/parser/XmlReportParser.java b/src/main/java/org/checkstyle/autofix/parser/XmlReportParser.java index a2c1f4d..457ab0a 100644 --- a/src/main/java/org/checkstyle/autofix/parser/XmlReportParser.java +++ b/src/main/java/org/checkstyle/autofix/parser/XmlReportParser.java @@ -25,7 +25,6 @@ import java.util.Iterator; import java.util.List; import java.util.Objects; -import java.util.Optional; import javax.xml.stream.XMLEventReader; import javax.xml.stream.XMLInputFactory; @@ -34,8 +33,6 @@ import javax.xml.stream.events.StartElement; import javax.xml.stream.events.XMLEvent; -import org.checkstyle.autofix.CheckstyleCheck; - public class XmlReportParser implements ReportParser { private static final String FILE_TAG = "file"; @@ -78,7 +75,7 @@ public List parse(Path xmlPath) { } else if (ERROR_TAG.equals(startElementName)) { Objects.requireNonNull(filename, "File name can not be null"); - parseErrorTag(startElement, filename).ifPresent(result::add); + result.add(parseErrorTag(startElement, filename)); } } } @@ -111,14 +108,13 @@ private String parseFileTag(StartElement startElement) { return fileName; } - private Optional parseErrorTag(StartElement startElement, + private CheckstyleViolation parseErrorTag(StartElement startElement, String filename) { int line = -1; int column = -1; String message = null; String severity = null; - CheckstyleViolation violation = null; - Optional source = Optional.empty(); + String source = null; final Iterator attributes = startElement.getAttributes(); while (attributes.hasNext()) { @@ -138,17 +134,14 @@ private Optional parseErrorTag(StartElement startElement, message = attribute.getValue(); break; case SOURCE_ATTR: - source = CheckstyleCheck.fromSource(attribute.getValue()); + source = attribute.getValue(); break; default: break; } } - if (source.isPresent()) { - violation = new CheckstyleViolation(line, column, severity, - source.get(), message, Path.of(filename)); - } - return Optional.ofNullable(violation); + return new CheckstyleViolation(line, column, severity, + source, message, Path.of(filename)); } } diff --git a/src/test/java/org/checkstyle/autofix/parser/CheckstyleReportsParserTest.java b/src/test/java/org/checkstyle/autofix/parser/CheckstyleReportsParserTest.java index 16a0eb5..270e468 100644 --- a/src/test/java/org/checkstyle/autofix/parser/CheckstyleReportsParserTest.java +++ b/src/test/java/org/checkstyle/autofix/parser/CheckstyleReportsParserTest.java @@ -25,7 +25,6 @@ import java.util.Map; import java.util.stream.Collectors; -import org.checkstyle.autofix.CheckstyleCheck; import org.junit.jupiter.api.Test; public class CheckstyleReportsParserTest { @@ -49,7 +48,7 @@ public void testParseFromResource() throws Exception { assertEquals(13, record.getColumn()); assertEquals("error", record.getSeverity()); assertEquals("Example message", record.getMessage()); - assertEquals(CheckstyleCheck.UPPER_ELL, record.getSource()); + assertEquals("com.puppycrawl.tools.checkstyle.checks.UpperEllCheck", record.getCheckId()); assertEquals(Path.of("Example.java"), record.getFilePath()); } diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/DiffSingleLocalTest.diff b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/DiffSingleLocalTest.diff index daa0519..1d24bed 100644 --- a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/DiffSingleLocalTest.diff +++ b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/DiffSingleLocalTest.diff @@ -1,6 +1,6 @@ --- src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java +++ src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java -@@ -15,37 +15,37 @@ +@@ -17,37 +17,37 @@ import java.util.HashMap; import java.util.List; diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java index 62a141b..290a7f1 100644 --- a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java +++ b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java @@ -1,7 +1,9 @@ /*xml - + + + diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java index 891c53c..0da9cad 100644 --- a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java +++ b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java @@ -1,7 +1,9 @@ /*xml - + + +