Skip to content

Commit d420aa0

Browse files
blacelleblacelle
committed
Enable rules exclusion
1 parent 9b59173 commit d420aa0

22 files changed

+381
-110
lines changed

‎config/src/main/java/eu/solven/cleanthat/github/CleanthatJavaProcessorProperties.java

-61
This file was deleted.

‎config/src/main/java/eu/solven/cleanthat/github/CleanthatRefFilterProperties.java

+19-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package eu.solven.cleanthat.github;
22

3-
import java.util.Arrays;
43
import java.util.List;
54
import java.util.Objects;
5+
import java.util.stream.Collectors;
6+
import java.util.stream.Stream;
7+
8+
import org.slf4j.Logger;
9+
import org.slf4j.LoggerFactory;
610

711
import com.fasterxml.jackson.databind.PropertyNamingStrategy;
812
import com.fasterxml.jackson.databind.annotation.JsonNaming;
@@ -17,14 +21,27 @@
1721
*/
1822
@JsonNaming(PropertyNamingStrategy.SnakeCaseStrategy.class)
1923
public class CleanthatRefFilterProperties {
24+
private static final Logger LOGGER = LoggerFactory.getLogger(CleanthatRefFilterProperties.class);
25+
26+
// https://git-scm.com/book/en/v2/Git-Internals-Git-References
27+
private static final String REFS_PREFIX = "refs/heads/";
28+
2029
// By default, we clean a set of standard default branch names
21-
private List<String> branches = Arrays.asList("refs/heads/master", "refs/heads/develop");
30+
// https://docs.github.com/en/github/administering-a-repository/managing-branches-in-your-repository/changing-the-default-branch
31+
// 'main' is the new default branch as Github
32+
private List<String> branches =
33+
Stream.of("master", "develop", "main").map(s -> REFS_PREFIX + s).collect(Collectors.toList());
2234

2335
public List<String> getBranches() {
2436
return branches;
2537
}
2638

39+
// TODO If a ref does not starts with 'refs/heads', add automatically 'refs/heads/' as prefix?
2740
public void setBranches(List<String> labels) {
41+
labels.stream()
42+
.filter(s -> !s.startsWith(REFS_PREFIX))
43+
.forEach(weirdRef -> LOGGER.warn("Weird ref: {}", weirdRef));
44+
2845
this.branches = List.copyOf(labels);
2946
}
3047

‎config/src/main/java/eu/solven/cleanthat/language/CleanthatLanguageProperties.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ public class CleanthatLanguageProperties implements ILanguageProperties {
2222

2323
private ISourceCodeProperties sourceCodeProperties;
2424

25-
private String language;
25+
private String language = "none";
2626

2727
// https://stackoverflow.com/questions/2591083/getting-java-version-at-runtime
28-
private String languageVersion;
28+
private String languageVersion = "0";
2929

3030
// The (ordered) processors to apply
3131
// @JsonDeserialize(using = ProcessorsDeseralizer.class)

‎config/src/main/resources/default_safe.yaml

+14-5
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,33 @@
1-
syntax_version: "2"
1+
syntax_version: "2021-08-02"
22
meta:
33
labels:
44
- "cleanthat"
55
refs:
66
branches:
7-
- "ref/heads/master"
8-
- "ref/heads/develop"
7+
- "refs/heads/develop"
8+
- "refs/heads/main"
9+
- "refs/heads/master"
910
languages:
1011
- language: "java"
1112
language_version: "11"
13+
source_code:
14+
includes:
15+
- "regex:.*\\.java"
1216
processors:
17+
- engine: "rules"
18+
parameters:
19+
excluded:
20+
-
1321
- engine: "revelc_imports"
1422
parameters:
23+
# Organize imports like in Eclipse
1524
remove_unused: true
1625
groups: "java.,javax.,org.,com."
1726
static_groups: "java,*"
27+
# Use Spring formatting convention
28+
- engine: "spring_formatter"
1829
source_code:
1930
excludes:
2031
- "regex:.*/generated/.*"
21-
includes:
22-
- "regex:.*\\.java"
2332
encoding: "UTF-8"
2433
line_ending: "LF"

‎github/src/main/java/eu/solven/cleanthat/github/event/GithubWebhookHandler.java

+22-17
Original file line numberDiff line numberDiff line change
@@ -343,23 +343,7 @@ public WebhookRelevancyResult filterWebhookEventTargetRelevantBranch(ICodeCleane
343343
throw new IllegalStateException("Should not happen");
344344
}
345345
if (optSha1.isPresent()) {
346-
if (GHPermissionType.WRITE == githubAuthAsInst.getPermissions().get("checks")) {
347-
// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#check_run
348-
// https://docs.github.com/en/rest/reference/checks#runs
349-
// https://docs.github.com/en/rest/reference/permissions-required-for-github-apps#permission-on-checks
350-
GHCheckRunBuilder checkRunBuilder = baseRepo.createCheckRun("CleanThat", optSha1.get());
351-
try {
352-
GHCheckRun checkRun = checkRunBuilder.create();
353-
checkRun.update().withStatus(Status.COMPLETED);
354-
} catch (IOException e) {
355-
// TODO Should we check we have the proper permission anyway?
356-
LOGGER.warn("Issue creating the CheckRun", e);
357-
}
358-
} else {
359-
// Invite users to go into:
360-
// https://github.com/organizations/solven-eu/settings/installations/9086720
361-
LOGGER.warn("We are not allowed to write checks (permissions=checks:write)");
362-
}
346+
createCheckRun(githubAuthAsInst, baseRepo, optSha1.get());
363347
}
364348

365349
// //
@@ -378,6 +362,27 @@ public WebhookRelevancyResult filterWebhookEventTargetRelevantBranch(ICodeCleane
378362
return WebhookRelevancyResult.relevant(refToClean.get(), offlineResult.optBaseRef());
379363
}
380364

365+
public void createCheckRun(GithubAndToken githubAuthAsInst, GHRepository baseRepo, String sha1) {
366+
if (GHPermissionType.WRITE == githubAuthAsInst.getPermissions().get("checks")) {
367+
// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#check_run
368+
// https://docs.github.com/en/rest/reference/checks#runs
369+
// https://docs.github.com/en/rest/reference/permissions-required-for-github-apps#permission-on-checks
370+
GHCheckRunBuilder checkRunBuilder = baseRepo.createCheckRun("CleanThat", sha1);
371+
try {
372+
// baseRepo.getCheckRuns("master").asList();
373+
GHCheckRun checkRun = checkRunBuilder.create();
374+
checkRun.update().withStatus(Status.COMPLETED);
375+
} catch (IOException e) {
376+
// TODO Should we check we have the proper permission anyway?
377+
LOGGER.warn("Issue creating the CheckRun", e);
378+
}
379+
} else {
380+
// Invite users to go into:
381+
// https://github.com/organizations/solven-eu/settings/installations/9086720
382+
LOGGER.warn("We are not allowed to write checks (permissions=checks:write)");
383+
}
384+
}
385+
381386
public ResultOrError<GitRepoBranchSha1, WebhookRelevancyResult> prepareTheRef(long baseRepoId,
382387
GHRepository baseRepo,
383388
GitRepoBranchSha1 gitRepoBranchSha1,

‎github/src/main/java/eu/solven/cleanthat/github/event/GithubWebhookHandlerFactory.java

+5
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,12 @@
66
import java.util.List;
77
import java.util.concurrent.TimeUnit;
88

9+
import org.kohsuke.github.GHCheckRun;
10+
import org.kohsuke.github.GHCheckRunBuilder;
11+
import org.kohsuke.github.GHPermissionType;
912
import org.kohsuke.github.GitHub;
1013
import org.kohsuke.github.GitHubBuilder;
14+
import org.kohsuke.github.GHCheckRun.Status;
1115
import org.springframework.core.env.Environment;
1216

1317
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -50,6 +54,7 @@ public IGithubWebhookHandler makeWithFreshJwt() throws IOException, JOSEExceptio
5054
// This leads to 401. Why?
5155
// .withRateLimitChecker(new NoWaitRateLimitChecker())
5256
.build();
57+
5358
return new GithubWebhookHandler(github.getApp(), objectMappers);
5459
}
5560

‎java/src/main/java/eu/solven/cleanthat/java/mutators/JavaRulesMutatorProperties.java

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package eu.solven.cleanthat.java.mutators;
2+
3+
import java.util.List;
4+
import java.util.Objects;
5+
6+
import com.fasterxml.jackson.databind.PropertyNamingStrategy;
7+
import com.fasterxml.jackson.databind.annotation.JsonNaming;
8+
9+
/**
10+
* The configuration of what is not related to a language.
11+
*
12+
* @author Benoit Lacelle
13+
*
14+
*/
15+
@JsonNaming(PropertyNamingStrategy.SnakeCaseStrategy.class)
16+
@SuppressWarnings("PMD.ImmutableField")
17+
public class JavaRulesMutatorProperties {
18+
19+
private List<String> excluded = List.of();
20+
private boolean productionReadyOnly = true;
21+
22+
@Override
23+
public int hashCode() {
24+
return Objects.hash(excluded, productionReadyOnly);
25+
}
26+
27+
@Override
28+
public boolean equals(Object obj) {
29+
if (this == obj) {
30+
return true;
31+
}
32+
if (obj == null) {
33+
return false;
34+
}
35+
if (getClass() != obj.getClass()) {
36+
return false;
37+
}
38+
JavaRulesMutatorProperties other = (JavaRulesMutatorProperties) obj;
39+
return Objects.equals(excluded, other.excluded)
40+
&& Objects.equals(productionReadyOnly, other.productionReadyOnly);
41+
}
42+
43+
public void setExcluded(List<String> excluded) {
44+
this.excluded = excluded;
45+
}
46+
47+
public List<String> getExcluded() {
48+
return excluded;
49+
}
50+
51+
public boolean isProductionReadyOnly() {
52+
return productionReadyOnly;
53+
}
54+
55+
public void setProductionReadyOnly(boolean productionReadyOnly) {
56+
this.productionReadyOnly = productionReadyOnly;
57+
}
58+
59+
}

‎java/src/main/java/eu/solven/cleanthat/java/mutators/RulesJavaMutator.java

+27-5
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,15 @@
3232

3333
import eu.solven.cleanthat.formatter.ISourceCodeFormatter;
3434
import eu.solven.cleanthat.formatter.LineEnding;
35-
import eu.solven.cleanthat.github.CleanthatJavaProcessorProperties;
3635
import eu.solven.cleanthat.language.ILanguageProperties;
3736
import eu.solven.cleanthat.rules.CreateTempFilesUsingNio;
3837
import eu.solven.cleanthat.rules.EnumsWithoutEquals;
3938
import eu.solven.cleanthat.rules.IJdkVersionConstants;
4039
import eu.solven.cleanthat.rules.ModifierOrder;
4140
import eu.solven.cleanthat.rules.OptionalNotEmpty;
4241
import eu.solven.cleanthat.rules.PrimitiveBoxedForString;
42+
import eu.solven.cleanthat.rules.UseDiamondOperator;
43+
import eu.solven.cleanthat.rules.UseDiamondOperatorJdk8;
4344
import eu.solven.cleanthat.rules.UseIsEmptyOnCollections;
4445
import eu.solven.cleanthat.rules.VariableEqualsConstant;
4546
import eu.solven.cleanthat.rules.meta.IClassTransformer;
@@ -56,28 +57,49 @@ public class RulesJavaMutator implements ISourceCodeFormatter {
5657
private static final Logger LOGGER = LoggerFactory.getLogger(RulesJavaMutator.class);
5758

5859
private final ILanguageProperties languageProperties;
59-
private final CleanthatJavaProcessorProperties properties;
60+
private final JavaRulesMutatorProperties properties;
6061

6162
private static final List<IClassTransformer> ALL_TRANSFORMERS = Arrays.asList(new CreateTempFilesUsingNio(),
6263
new EnumsWithoutEquals(),
6364
new PrimitiveBoxedForString(),
6465
new OptionalNotEmpty(),
6566
new ModifierOrder(),
66-
// new UseDiamondOperator(),
67-
// new UseDiamondOperatorJdk8(),
67+
new UseDiamondOperator(),
68+
new UseDiamondOperatorJdk8(),
6869
new UseIsEmptyOnCollections(),
6970
new VariableEqualsConstant());
7071

7172
private final List<IClassTransformer> transformers;
7273

73-
public RulesJavaMutator(ILanguageProperties languageProperties, CleanthatJavaProcessorProperties properties) {
74+
public RulesJavaMutator(ILanguageProperties languageProperties, JavaRulesMutatorProperties properties) {
7475
this.languageProperties = languageProperties;
7576
this.properties = properties;
7677

7778
VersionWrapper languageVersion = new VersionWrapper(languageProperties.getLanguageVersion());
79+
80+
List<String> excludedRules = properties.getExcluded();
81+
boolean productionReadyOnly = properties.isProductionReadyOnly();
82+
7883
this.transformers = ALL_TRANSFORMERS.stream().filter(ct -> {
7984
VersionWrapper transformerVersion = new VersionWrapper(ct.minimalJavaVersion());
8085
return languageVersion.compareTo(transformerVersion) >= 0;
86+
}).filter(ct -> {
87+
boolean isExclusion = excludedRules.stream()
88+
.filter(excludedRule -> ct.getIds().contains(excludedRule))
89+
.findAny()
90+
.isPresent();
91+
92+
if (isExclusion) {
93+
LOGGER.info("We exclude '{}'", ct.getIds());
94+
}
95+
96+
return !isExclusion;
97+
}).filter(ct -> {
98+
if (!productionReadyOnly) {
99+
return true;
100+
} else {
101+
return ct.isProductionReady();
102+
}
81103
}).collect(Collectors.toList());
82104

83105
this.transformers.forEach(ct -> {

‎java/src/main/java/eu/solven/cleanthat/language/java/JavaFormatter.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
import cormoran.pepper.collection.PepperMapHelper;
1515
import eu.solven.cleanthat.formatter.ALanguageFormatter;
1616
import eu.solven.cleanthat.formatter.ISourceCodeFormatter;
17-
import eu.solven.cleanthat.github.CleanthatJavaProcessorProperties;
1817
import eu.solven.cleanthat.github.EclipseJavaFormatterProcessorProperties;
18+
import eu.solven.cleanthat.java.mutators.JavaRulesMutatorProperties;
1919
import eu.solven.cleanthat.java.mutators.RulesJavaMutator;
2020
import eu.solven.cleanthat.language.ILanguageProperties;
2121
import eu.solven.cleanthat.language.IStringFormatter;
@@ -94,8 +94,8 @@ protected ISourceCodeFormatter makeFormatter(Map<String, ?> rawProcessor, ILangu
9494
}
9595
break;
9696
case "rules": {
97-
CleanthatJavaProcessorProperties processorConfig =
98-
objectMapper.convertValue(parameters, CleanthatJavaProcessorProperties.class);
97+
JavaRulesMutatorProperties processorConfig =
98+
objectMapper.convertValue(parameters, JavaRulesMutatorProperties.class);
9999
processor = new RulesJavaMutator(languageProperties, processorConfig);
100100
}
101101
break;

‎java/src/main/java/eu/solven/cleanthat/rules/ATodoJavaParserRule.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,13 @@
1313
* @author Benoit Lacelle
1414
*/
1515
public abstract class ATodoJavaParserRule extends AJavaParserRule implements IClassTransformer {
16-
1716
private static final Logger LOGGER = LoggerFactory.getLogger(ATodoJavaParserRule.class);
1817

18+
@Override
19+
public boolean isProductionReady() {
20+
return false;
21+
}
22+
1923
@Override
2024
public String minimalJavaVersion() {
2125
return IJdkVersionConstants.JDK_LATEST;

0 commit comments

Comments
 (0)