Skip to content

Commit

Permalink
Bump Kotlin-related dependencies (#225)
Browse files Browse the repository at this point in the history 
With this change, we bump all Kotlin-related dependencies to recent
versions. This includes a Kotlin bump from 1.3 to 1.4, the migration
from com.github.shyiko.ktlint to com.pinterest.ktlint and a significant
upgrade of Detekt from 1.0.0-RC14 to 1.14.0.

For running Detekt, a new tooling API was introduced. This means that
the code that invoke Detekt can change not to use internal classes
anymore. Calling a Kotlin DSL from Java looks a little verbose, but it
still works nicely.

For tests, the Detekt YAML file was reset from the latest default, then
adapted slightly so the expected rules still get triggered. A few more
minor changes were made to match expectations with the latest version.
I also add a specific test for a new Detekt rule that has been added since
the previous version we were using here...

In terms of migration/changelong:

- Changelog of Detekt RC versions:
  https://arturbosch.github.io/detekt/changelog-rc.html

- Detekt include/exclude migration:
  https://arturbosch.github.io/detekt/howto-migratetestpattern.html

- Release notes of Detekt, since:
  https://arturbosch.github.io/detekt/changelog.html

- KtLint changelog:
  https://github.com/pinterest/ktlint/releases

Co-authored-by: Julien Lehuen <julien.lehuen@imc.com>
  • Loading branch information
@snooze92
snooze92 and Julien Lehuen committed Nov 9, 2020
1 parent b991f9d commit 6f1869b
Show file tree
Hide file tree
Showing 13 changed files with 694 additions and 238 deletions.
13 changes: 6 additions & 7 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,16 @@ dependencies {
// external processes
compile 'org.zeroturnaround:zt-exec:1.8'

compile 'org.jetbrains.kotlin:kotlin-stdlib:1.3.31'
compile 'org.jetbrains.kotlin:kotlin-stdlib:1.4.10'

// ktlint https://github.com/shyiko/ktlint
compile 'com.github.shyiko.ktlint:ktlint-core:0.31.0'
compile 'com.github.shyiko.ktlint:ktlint-ruleset-standard:0.31.0'
compile 'com.pinterest.ktlint:ktlint-core:0.39.0'
compile 'com.pinterest.ktlint:ktlint-ruleset-standard:0.39.0'

// detekt
compile 'io.gitlab.arturbosch.detekt:detekt-core:1.0.0-RC14'
compile 'io.gitlab.arturbosch.detekt:detekt-rules:1.0.0-RC14'
// dependency only for obtaining default configuration
compile 'io.gitlab.arturbosch.detekt:detekt-cli:1.0.0-RC14'
compile 'io.gitlab.arturbosch.detekt:detekt-tooling:1.14.0'
runtimeOnly 'io.gitlab.arturbosch.detekt:detekt-core:1.14.0'
runtimeOnly 'io.gitlab.arturbosch.detekt:detekt-rules:1.14.0'

// transitive dependency that used non-SSL version of Maven Central
// and version 1.74 that was not found
Expand Down
89 changes: 45 additions & 44 deletions src/main/java/pl/touk/sputnik/processor/detekt/DetektProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
package pl.touk.sputnik.processor.detekt;

import io.gitlab.arturbosch.detekt.api.Config;
import io.gitlab.arturbosch.detekt.api.Detektion;
import io.gitlab.arturbosch.detekt.api.YamlConfig;
import io.gitlab.arturbosch.detekt.cli.ClasspathResourceConverter;
import io.gitlab.arturbosch.detekt.core.DetektFacade;
import io.gitlab.arturbosch.detekt.core.ProcessingSettings;
import io.gitlab.arturbosch.detekt.core.RuleSetLocator;
import io.github.detekt.tooling.api.AnalysisResult;
import io.github.detekt.tooling.api.Detekt;
import io.github.detekt.tooling.api.DetektProvider;
import io.github.detekt.tooling.api.MaxIssuesReached;
import io.github.detekt.tooling.api.spec.ProcessingSpec;
import kotlin.Unit;
import lombok.AllArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.jetbrains.annotations.NotNull;
Expand All @@ -22,11 +21,9 @@
import java.io.File;
import java.io.IOException;
import java.io.PrintStream;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.util.ArrayList;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ForkJoinPool;
Expand All @@ -51,11 +48,17 @@ public ReviewResult process(@NotNull Review review) {
if (files.isEmpty()) {
return new ReviewResult();
}
DetektFacade detektFacade = buildDetectFacade(files);

Detektion detektion = detektFacade.run();

return new ResultBuilder(detektion).build(files);
Detekt detekt = buildDetectFacade(files);

AnalysisResult result = detekt.run();
if (result.getError() != null) {
if (result.getError() instanceof MaxIssuesReached) {
log.info("Detekt reached issues threshold: {}", result.getError().getMessage());
} else {
log.error("Detekt run resulted in an error", result.getError());
}
}
return new ResultBuilder(result).build(files);
}

@NotNull
Expand All @@ -64,34 +67,32 @@ private List<String> getFilesForReview(@NotNull Review review) {
}

@NotNull
private DetektFacade buildDetectFacade(List<String> files) {
String configFilename = configuration.getProperty(GeneralOption.DETEKT_CONFIG_FILE);
Config config;
FileSystem fileSystem = FileSystems.getDefault();
if (configFilename != null) {
Path configPath = fileSystem.getPath(configFilename);
config = YamlConfig.Companion.load(configPath);
} else {
config = loadDefaultConfig();
}
ProcessingSettings processingSettings = new ProcessingSettings(
files.stream().map(f -> fileSystem.getPath(f)).collect(Collectors.toList()),
config,
new ArrayList<>(),
false,
false,
new ArrayList<>(),
executor,
printStream,
printStream
);

return DetektFacade.Companion.create(processingSettings, new RuleSetLocator(processingSettings).load(), Arrays.asList(new LoggingFileProcessor()));
}

@NotNull
private Config loadDefaultConfig() {
return YamlConfig.Companion.loadResource(new ClasspathResourceConverter().convert("default-detekt-config.yml"));
private Detekt buildDetectFacade(List<String> files) {
return DetektProvider.Companion.load(DetektProvider.class.getClassLoader()).get(
ProcessingSpec.Companion.invoke(processingSpecBuilder -> {
processingSpecBuilder.logging(loggingSpecBuilder -> {
loggingSpecBuilder.setOutputChannel(printStream);
loggingSpecBuilder.setErrorChannel(printStream);
return Unit.INSTANCE;
});
processingSpecBuilder.project(projectSpecBuilder -> {
projectSpecBuilder.setInputPaths(files.stream().map(Paths::get).collect(Collectors.toList()));
return Unit.INSTANCE;
});
processingSpecBuilder.config(configSpecBuilder -> {
configSpecBuilder.setUseDefaultConfig(true);
String configFilename = configuration.getProperty(GeneralOption.DETEKT_CONFIG_FILE);
configSpecBuilder.setConfigPaths(configFilename == null
? Collections.emptyList()
: Collections.singleton(Paths.get(configFilename)));
return Unit.INSTANCE;
});
processingSpecBuilder.execution(executionSpecBuilder -> {
executionSpecBuilder.setExecutorService(executor);
return Unit.INSTANCE;
});
return Unit.INSTANCE;
}));
}

@NotNull
Expand Down
37 changes: 32 additions & 5 deletions src/main/java/pl/touk/sputnik/processor/detekt/LoggingFileProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,57 @@
import io.gitlab.arturbosch.detekt.api.Detektion;
import io.gitlab.arturbosch.detekt.api.FileProcessListener;
import io.gitlab.arturbosch.detekt.api.Finding;
import io.gitlab.arturbosch.detekt.api.SetupContext;
import lombok.extern.slf4j.Slf4j;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.kotlin.psi.KtFile;
import org.jetbrains.kotlin.resolve.BindingContext;

import java.util.List;
import java.util.Map;

@Slf4j
class LoggingFileProcessor implements FileProcessListener {
@Override
public void onStart(List<? extends KtFile> list) {
public void onStart( @NotNull List<? extends KtFile> list) {
log.debug("Found {} files for review", list.size());
}

@Override
public void onProcess(KtFile ktFile) {
public void onStart(@NotNull List<? extends KtFile> list, @NotNull BindingContext bindingContext) {
onStart(list);
}

@Override
public void onProcess( @NotNull KtFile ktFile) {
log.debug("Processing {}", ktFile.getName());
}

@Override
public void onProcessComplete(KtFile ktFile, Map<String, ? extends List<? extends Finding>> map) {
public void onProcess(@NotNull KtFile ktFile, @NotNull BindingContext bindingContext) {
onProcess(ktFile);
}

@Override
public void onProcessComplete( @NotNull KtFile ktFile, @NotNull Map<String, ? extends List<? extends Finding>> map) {
log.debug("Processed {} and found {} problems", ktFile.getName(), countProblems(map));
}

@Override
public void onFinish(List<? extends KtFile> list, Detektion detektion) {
public void onProcessComplete(@NotNull KtFile ktFile, @NotNull Map<String, ? extends List<? extends Finding>> map, @NotNull BindingContext bindingContext) {
onProcessComplete(ktFile, map);
}

@Override
public void onFinish( @NotNull List<? extends KtFile> list, @NotNull Detektion detektion) {
log.debug("Processed {} files and found {} problems", list.size(), countProblems(detektion));
}

@Override
public void onFinish(@NotNull List<? extends KtFile> list, @NotNull Detektion detektion, @NotNull BindingContext bindingContext) {
onFinish(list, detektion);
}

private int countProblems(Detektion detektion) {
return countProblems(detektion.getFindings());
}
Expand All @@ -53,7 +75,12 @@ public int getPriority() {
}

@Override
public void init(Config config) {
public void init(@NotNull Config config) {

}

@Override
public void init(@NotNull SetupContext setupContext) {

}
}
30 changes: 17 additions & 13 deletions src/main/java/pl/touk/sputnik/processor/detekt/ResultBuilder.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package pl.touk.sputnik.processor.detekt;

import io.github.detekt.tooling.api.AnalysisResult;
import io.gitlab.arturbosch.detekt.api.Detektion;
import io.gitlab.arturbosch.detekt.api.Severity;
import lombok.AllArgsConstructor;
Expand All @@ -13,23 +14,26 @@
@AllArgsConstructor
class ResultBuilder {

private final Detektion detektion;
private final AnalysisResult result;

@NotNull
ReviewResult build(List<String> filesForReview) {
ReviewResult reviewResult = new ReviewResult();
detektion.getFindings().forEach((ruleSet, value) -> value.forEach(finding -> {
String filePath = finding.getLocation().getFile();
Optional<String> file = filesForReview.stream().filter(filePath::endsWith).findFirst();
file.ifPresent(f ->
{
reviewResult.add(new Violation(f,
finding.getLocation().getSource().getLine(),
buildMessage(ruleSet, finding.getIssue().getId(), finding.getIssue().getDescription()),
mapSeverity(finding.getIssue().getSeverity())));
}
);
}));
Detektion detektion = result.getContainer();
if (detektion != null) {
detektion.getFindings().forEach((ruleSet, value) -> value.forEach(finding -> {
String filePath = finding.getLocation().getFile();
Optional<String> file = filesForReview.stream().filter(filePath::endsWith).findFirst();
file.ifPresent(f ->
{
reviewResult.add(new Violation(f,
finding.getLocation().getSource().getLine(),
buildMessage(ruleSet, finding.getIssue().getId(), finding.getIssue().getDescription()),
mapSeverity(finding.getIssue().getSeverity())));
}
);
}));
}

return reviewResult;
}
Expand Down
17 changes: 13 additions & 4 deletions src/main/java/pl/touk/sputnik/processor/ktlint/KtlintProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package pl.touk.sputnik.processor.ktlint;

import com.github.shyiko.ktlint.core.KtLint;
import com.github.shyiko.ktlint.core.RuleSet;
import com.github.shyiko.ktlint.core.RuleSetProvider;
import com.pinterest.ktlint.core.KtLint;
import com.pinterest.ktlint.core.RuleSet;
import com.pinterest.ktlint.core.RuleSetProvider;
import org.apache.commons.io.IOUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -19,6 +19,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.ServiceLoader;

Expand Down Expand Up @@ -61,7 +62,15 @@ private ReviewResult processFiles(List<String> filePaths) {
ReviewResult result = new ReviewResult();
for (String filePath : filePaths) {
String text = readFile(filePath);
KtLint.INSTANCE.lint(text, ruleSets, new LintErrorConverter(result, filePath, excludedRules));
KtLint.INSTANCE.lint(new KtLint.Params(
null,
text,
ruleSets,
Collections.emptyMap(),
new LintErrorConverter(result, filePath, excludedRules),
false,
null,
false));
}
return result;
}
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/pl/touk/sputnik/processor/ktlint/LintErrorConverter.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package pl.touk.sputnik.processor.ktlint;

import com.github.shyiko.ktlint.core.LintError;
import com.pinterest.ktlint.core.LintError;
import kotlin.Unit;
import kotlin.jvm.functions.Function1;
import kotlin.jvm.functions.Function2;
import lombok.RequiredArgsConstructor;
import org.jetbrains.annotations.NotNull;
import pl.touk.sputnik.review.ReviewResult;
Expand All @@ -12,13 +12,14 @@
import java.util.List;

@RequiredArgsConstructor
class LintErrorConverter implements Function1<LintError, Unit> {
class LintErrorConverter implements Function2<LintError, Boolean, Unit> {

private final ReviewResult result;
private final String filePath;
private final List<String> excludedRules;

public Unit invoke(LintError e) {
@Override
public Unit invoke(LintError e, Boolean corrected) {
if (!excludedRules.contains(e.getRuleId())) {
result.add(fromLintError(e, filePath));
}
Expand Down
21 changes: 17 additions & 4 deletions src/test/java/pl/touk/sputnik/processor/detekt/DetektProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class DetektProcessorTest {
private static final String VIOLATIONS_1 = "src/test/resources/detekt/testFiles/Violations1.kt";
private static final String VIOLATIONS_2 = "src/test/resources/detekt/testFiles/sub/Violations2.kt";
private static final String VIOLATIONS_3 = "src/test/resources/detekt/testFiles/Violations3.kt";
private static final String VIOLATIONS_4 = "src/test/resources/detekt/testFiles/Violations4.kt";
private static final String REVIEW_GROOVY_FILE = "src/test/resources/codeNarc/testFiles/FileWithOneViolationLevel2.groovy";

private DetektProcessor sut;
Expand All @@ -43,7 +44,7 @@ void shouldReturnViolationsOnlyForOneRequestedFile() {
assertThat(result).isNotNull();
assertThat(result.getViolations())
.hasSize(3)
.contains(new Violation(VIOLATIONS_1, 14, "[style/NewLineAtEndOfFile] Checks whether files end with a line separator.", Severity.INFO))
.contains(new Violation(VIOLATIONS_1, 1, "[style/NewLineAtEndOfFile] Checks whether files end with a line separator.", Severity.INFO))
.contains(new Violation(VIOLATIONS_1, 3, "[style/WildcardImport] Wildcard imports should be replaced with imports using fully qualified class names. Wildcard imports can lead to naming conflicts. A library update can introduce naming clashes with your classes which results in compilation errors.", Severity.INFO))
.contains(new Violation(VIOLATIONS_1, 7, "[style/MagicNumber] Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.", Severity.INFO));
}
Expand All @@ -58,8 +59,20 @@ void shouldReturnViolationsOnlyForRequestedFiles() {
assertThat(result.getViolations())
.hasSize(3)
.contains(new Violation(VIOLATIONS_3, 3, "[empty-blocks/EmptyClassBlock] Empty block of code detected. As they serve no purpose they should be removed.", Severity.INFO))
.contains(new Violation(VIOLATIONS_2, 3, "[style/NewLineAtEndOfFile] Checks whether files end with a line separator.", Severity.INFO))
.contains(new Violation(VIOLATIONS_3, 4, "[style/NewLineAtEndOfFile] Checks whether files end with a line separator.", Severity.INFO));
.contains(new Violation(VIOLATIONS_2, 1, "[style/NewLineAtEndOfFile] Checks whether files end with a line separator.", Severity.INFO))
.contains(new Violation(VIOLATIONS_3, 1, "[style/NewLineAtEndOfFile] Checks whether files end with a line separator.", Severity.INFO));
}

@Test
void shouldReturnGlobalScopeViolation() {
Review review = getReview(VIOLATIONS_4);

ReviewResult result = sut.process(review);

assertThat(result).isNotNull();
assertThat(result.getViolations())
.hasSize(1)
.contains(new Violation(VIOLATIONS_4, 7, "[coroutines/GlobalCoroutineUsage] Usage of GlobalScope instance is highly discouraged", Severity.ERROR));
}

@Test
Expand Down Expand Up @@ -101,4 +114,4 @@ private Review getReview(String... filePaths) {
}
return new Review(files, ReviewFormatterFactory.get(config));
}
}
}
Loading

0 comments on commit 6f1869b

Please sign in to comment.