Skip to content
Open
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
4 changes: 2 additions & 2 deletions src/main/java/org/checkstyle/autofix/CheckstyleAutoFix.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public List<Recipe> getRecipeList() {
final ReportParser reportParser = createReportParser(getViolationReportPath());
final List<CheckstyleViolation> violations = reportParser
.parse(Path.of(getViolationReportPath()));
final Map<CheckstyleCheck,
final Map<CheckstyleConfigModule,
CheckConfiguration> configuration = loadCheckstyleConfiguration();

return CheckstyleRecipeRegistry.getRecipes(violations, configuration);
Expand All @@ -108,7 +108,7 @@ else if (path.endsWith(".sarif") || path.endsWith(".sarif.json")) {
return result;
}

private Map<CheckstyleCheck, CheckConfiguration> loadCheckstyleConfiguration() {
private Map<CheckstyleConfigModule, CheckConfiguration> loadCheckstyleConfiguration() {
return ConfigurationLoader.loadConfiguration(getConfigurationPath(), getPropertiesPath());
}
}
6 changes: 6 additions & 0 deletions src/main/java/org/checkstyle/autofix/CheckstyleCheck.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,10 @@ public static Optional<CheckstyleCheck> fromSource(String source) {
.filter(check -> check.getId().contains(source))
.findFirst();
}

public static Optional<CheckstyleCheck> fromSourceExact(String source) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename parameter please.

return Arrays.stream(values())
.filter(check -> check.getId().equals(source))
.findFirst();
}
}
38 changes: 38 additions & 0 deletions src/main/java/org/checkstyle/autofix/CheckstyleConfigModule.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
37 changes: 35 additions & 2 deletions src/main/java/org/checkstyle/autofix/CheckstyleRecipeRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -65,18 +67,49 @@ private CheckstyleRecipeRegistry() {
* @return a list of generated Recipe objects
*/
public static List<Recipe> getRecipes(List<CheckstyleViolation> violations,
Map<CheckstyleCheck, CheckConfiguration> config) {
Map<CheckstyleConfigModule, CheckConfiguration> config) {
return violations.stream()
.collect(Collectors.groupingBy(CheckstyleViolation::getSource))
.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<CheckstyleConfigModule, CheckConfiguration> 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<CheckstyleViolation> violations,
CheckConfiguration checkConfig) {
Recipe result = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

import java.nio.file.Path;

import org.checkstyle.autofix.CheckstyleCheck;

public final class CheckstyleViolation {

private final int line;
Expand All @@ -29,14 +27,14 @@ public final class CheckstyleViolation {

private final String severity;

private final CheckstyleCheck source;
private final String source;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename "source" to "checkId" ?

I know xml attribute is "source", very bad name, very confusing, and now we copied it new project. Better to have better name that is logical and explain what is this.


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;
Expand All @@ -46,7 +44,7 @@ public CheckstyleViolation(int line, int column, String severity,
}

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);
}

Expand All @@ -58,7 +56,7 @@ public Integer getColumn() {
return column;
}

public CheckstyleCheck getSource() {
public String getSource() {
return source;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,14 +38,19 @@ private ConfigurationLoader() {
// utility class
}

public static Map<CheckstyleCheck, CheckConfiguration> mapConfiguration(Configuration config) {
final Map<CheckstyleCheck, CheckConfiguration> result = new HashMap<>();
public static Map<CheckstyleConfigModule,
CheckConfiguration> mapConfiguration(Configuration config) {

final Map<CheckstyleConfigModule, CheckConfiguration> result = new LinkedHashMap<>();
final Map<String, String> inherited = getProperties(config);

final Optional<CheckstyleCheck> module = CheckstyleCheck.fromSource(config.getName());
module.ifPresent(checkstyleCheck -> {
result.put(checkstyleCheck, new CheckConfiguration(checkstyleCheck, new HashMap<>(),
getProperties(config)));
final Map<String, String> 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()) {
Expand All @@ -69,7 +76,7 @@ private static Map<String, String> getProperties(Configuration config) {
return props;
}

public static Map<CheckstyleCheck, CheckConfiguration> loadConfiguration(
public static Map<CheckstyleConfigModule, CheckConfiguration> loadConfiguration(
String checkstyleConfigurationPath, String propFile) {
Properties props = new Properties();
if (propFile == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -57,17 +55,14 @@ public List<CheckstyleViolation> 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 check, Result result) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is first parameter checkId ?

final String severity = result.getLevel().name();
final String message = result.getMessage().getText();
final PhysicalLocation location = result.getLocations().get(0).getPhysicalLocation();
Expand Down
19 changes: 6 additions & 13 deletions src/main/java/org/checkstyle/autofix/parser/XmlReportParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";
Expand Down Expand Up @@ -78,7 +75,7 @@ public List<CheckstyleViolation> 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));
}
}
}
Expand Down Expand Up @@ -111,14 +108,13 @@ private String parseFileTag(StartElement startElement) {
return fileName;
}

private Optional<CheckstyleViolation> 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<CheckstyleCheck> source = Optional.empty();
String source = null;

final Iterator<Attribute> attributes = startElement.getAttributes();
while (attributes.hasNext()) {
Expand All @@ -138,17 +134,14 @@ private Optional<CheckstyleViolation> 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));

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.getSource());
assertEquals(Path.of("Example.java"), record.getFilePath());
}

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/*xml
<module name="Checker">
<module name="TreeWalker">
<module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck"/>
<module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck">
<property name="id" value="UpperEll"/>
</module>
</module>
</module>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/*xml
<module name="Checker">
<module name="TreeWalker">
<module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck"/>
<module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck">
<property name="id" value="UpperEll"/>
</module>
</module>
</module>

Expand Down