Feature. Enable confiuring of ktlint reporter in gradle plugin #724

aktsay6 · 2021-01-25T08:55:13Z

What's done

Added configuring
Added tests

This pull request closes #714

### What's done: * Logic began

### What's done: * Fixed bugs

### What's done: * Added tests

codecov · 2021-01-25T09:12:24Z

Codecov Report

Merging #724 (cf1fb82) into master (c4c32b2) will decrease coverage by 0.01%.
The diff coverage is 78.26%.

@@             Coverage Diff              @@
##             master     #724      +/-   ##
============================================
- Coverage     80.17%   80.15%   -0.02%     
- Complexity     1913     1922       +9     
============================================
  Files            91       91              
  Lines          4937     4958      +21     
  Branches       1580     1584       +4     
============================================
+ Hits           3958     3974      +16     
- Misses          238      241       +3     
- Partials        741      743       +2

Flag	Coverage Δ	Complexity Δ
unittests	`80.15% <78.26%> (-0.02%)`	`0.00 <9.00> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
...rg/cqfn/diktat/plugin/gradle/DiktatGradlePlugin.kt	`100.00% <ø> (ø)`	`2.00 <0.00> (ø)`
...qfn/diktat/plugin/gradle/DiktatJavaExecTaskBase.kt	`88.46% <76.19%> (-4.53%)`	`25.00 <7.00> (+8.00)`	⬇️
...n/org/cqfn/diktat/plugin/gradle/DiktatExtension.kt	`100.00% <100.00%> (ø)`	`8.00 <2.00> (+1.00)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4c32b2...cf1fb82. Read the comment docs.

petertrr · 2021-01-25T10:04:24Z

diktat-gradle-plugin/src/main/kotlin/org/cqfn/diktat/plugin/gradle/DiktatJavaExecTaskBase.kt

@@ -50,6 +53,21 @@ open class DiktatJavaExecTaskBase @Inject constructor(
        } else {
            main = "com.pinterest.ktlint.Main"
        }
+        diktatExtension.reporter = when(diktatExtension.reporterType) {


Actually, I think it should be passed to ktlint via CLI argument like --reporter=json. Adding classpath dependency might even be not needed (depending on whether they already include all reporters in ktlint.jar

petertrr · 2021-01-25T10:05:14Z

diktat-gradle-plugin/src/main/kotlin/org/cqfn/diktat/plugin/gradle/DiktatJavaExecTaskBase.kt

+            "html" -> {
+                diktatConfiguration.dependencies.add(project.dependencies.create("com.pinterest.ktlint:ktlint-reporter-html:$KTLINT_VERSION"))
+                diktatConfiguration.dependencies.remove(diktatConfiguration.dependencies.find { it.name == "ktlint-reporter-plain" })
+                HtmlReporter(System.out)


We also should support custom output files for reporters. Please check out ktlint's docs regarding supported options for different reporters.

.../src/functionalTest/kotlin/org/cqfn/diktat/plugin/gradle/DiktatGradlePluginFunctionalTest.kt

### What's done: * Fixed bugs

### What's done: * Logic remade * Added tests

.../src/functionalTest/kotlin/org/cqfn/diktat/plugin/gradle/DiktatGradlePluginFunctionalTest.kt

petertrr · 2021-01-26T12:20:29Z

diktat-gradle-plugin/src/main/kotlin/org/cqfn/diktat/plugin/gradle/DiktatJavaExecTaskBase.kt

@@ -50,6 +54,9 @@ open class DiktatJavaExecTaskBase @Inject constructor(
        } else {
            main = "com.pinterest.ktlint.Main"
        }
+        if (diktatExtension.reporterType == "html") {


Is it needed? If yes, why onlu for html?

Others are already included in ktlint (plain, json, checkstyle)

Oh, ok, that's strange. Leave a comment in code to explain this, pls.

petertrr · 2021-01-26T12:23:27Z

diktat-gradle-plugin/src/main/kotlin/org/cqfn/diktat/plugin/gradle/DiktatJavaExecTaskBase.kt

+            "json" -> flag.append("--reporter=json")
+            "html" -> flag.append("--reporter=html")
+            "checkstyle" -> flag.append("--reporter=checkstyle")
+            else -> flag.append("--reporter=plain")


I think they already have baseline reporter (or was it added in 0.40.0?) Still, let's log a warning if unknown reporter is specified and then fallback to plain.

Also, one could imagine a use case, when end user adds a custom reporter as a dependency in our plugin's configuration and then specifies it's name (need to see, how it's loaded in ktlint). So it might be more versatile to allow arbitrary names here and then let ktlint validate them.

We need to specify it(--reporter=plain) here if a user will provide us an output here

diktat-gradle-plugin/src/main/kotlin/org/cqfn/diktat/plugin/gradle/DiktatJavaExecTaskBase.kt

### What's done: * Fixed bugs

petertrr · 2021-01-26T14:23:34Z

.../src/functionalTest/kotlin/org/cqfn/diktat/plugin/gradle/DiktatGradlePluginFunctionalTest.kt

        Assertions.assertEquals(TaskOutcome.FAILED, diktatCheckBuildResult.outcome)
-        val file = testProjectDir.root.walkTopDown().filter { it.name == "test.txt" }.first()
+        val file = testProjectDir.root.walkTopDown().filter { it.name == "test.txt" }.firstOrNull()
+        Assertions.assertTrue(file != null)


Does kotlin.test.assertNotNull work in this case? It should have contract to enable smart cast of file to non-nullable type.

diktat-gradle-plugin/build.gradle.kts

### What's done: * Fixed bugs

petertrr · 2021-01-26T15:07:36Z

.../src/functionalTest/kotlin/org/cqfn/diktat/plugin/gradle/DiktatGradlePluginFunctionalTest.kt

@@ -55,10 +56,10 @@ class DiktatGradlePluginFunctionalTest {
        val diktatCheckBuildResult = result.task(":$DIKTAT_CHECK_TASK")
        requireNotNull(diktatCheckBuildResult)
        Assertions.assertEquals(TaskOutcome.FAILED, diktatCheckBuildResult.outcome)
-        val file = testProjectDir.root.walkTopDown().filter { it.name == "test.txt" }.firstOrNull()
-        Assertions.assertTrue(file != null)
+        assertNotNull(testProjectDir.root.walkTopDown().filter { it.name == "test.txt" }.first())


Is this line needed?

No, it doesn't. Fixed

### What's done: * Fixed bugs

….com/cqfn/diKTat into feature/gradle-ktlint-reporter(#714)

aktsay6 added 3 commits January 21, 2021 17:25

feature/gradle-ktlint-reporter(#714)

0900ebb

### What's done: * Logic began

feature/gradle-ktlint-reporter(#714)

1d374c0

### What's done: * Fixed bugs

feature/gradle-ktlint-reporter(#714)

68a5b0c

### What's done: * Added tests

aktsay6 added the plugins Plugins for build systems to allow them to run diktat label Jan 25, 2021

aktsay6 requested review from petertrr and kentr0w January 25, 2021 08:55

aktsay6 marked this pull request as draft January 25, 2021 08:55

petertrr reviewed Jan 25, 2021

View reviewed changes

kentr0w reviewed Jan 25, 2021

View reviewed changes

.../src/functionalTest/kotlin/org/cqfn/diktat/plugin/gradle/DiktatGradlePluginFunctionalTest.kt Outdated Show resolved Hide resolved

aktsay6 added 2 commits January 25, 2021 19:19

feature/gradle-ktlint-reporter(#714)

56c7621

### What's done: * Fixed bugs

feature/gradle-ktlint-reporter(#714)

7965ff1

### What's done: * Logic remade * Added tests

aktsay6 marked this pull request as ready for review January 26, 2021 08:55

aktsay6 requested review from petertrr and kentr0w January 26, 2021 08:55

petertrr requested changes Jan 26, 2021

View reviewed changes

feature/gradle-ktlint-reporter(#714)

918ebfd

### What's done: * Fixed bugs

petertrr reviewed Jan 26, 2021

View reviewed changes

diktat-gradle-plugin/build.gradle.kts Outdated Show resolved Hide resolved

aktsay6 added 3 commits January 26, 2021 17:41

feature/gradle-ktlint-reporter(#714)

5843f9a

### What's done: * Fixed bugs

feature/gradle-ktlint-reporter(#714)

d67ae58

### What's done: * Fixed bugs

feature/gradle-ktlint-reporter(#714)

caaaba7

### What's done: * Fixed bugs

aktsay6 requested a review from petertrr January 26, 2021 15:02

petertrr approved these changes Jan 26, 2021

View reviewed changes

aktsay6 and others added 6 commits January 26, 2021 18:10

feature/gradle-ktlint-reporter(#714)

0b42e86

### What's done: * Fixed bugs

feature/gradle-ktlint-reporter(#714)

4af0153

### What's done: * Fixed bugs

feature/gradle-ktlint-reporter(#714)

6eaa275

### What's done: * Fixed bugs

Merge branch 'master' into feature/gradle-ktlint-reporter(#714)

02cc214

feature/gradle-ktlint-reporter(#714)

4b8b601

### What's done: * Fixed bugs

Merge branch 'feature/gradle-ktlint-reporter(#714)' of https://github…

cf1fb82

….com/cqfn/diKTat into feature/gradle-ktlint-reporter(#714)

aktsay6 merged commit 00a00ae into master Jan 27, 2021

aktsay6 deleted the feature/gradle-ktlint-reporter(#714) branch January 27, 2021 08:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature. Enable confiuring of ktlint reporter in gradle plugin #724

Feature. Enable confiuring of ktlint reporter in gradle plugin #724

aktsay6 commented Jan 25, 2021

codecov bot commented Jan 25, 2021 •

edited

Loading

petertrr Jan 25, 2021

petertrr Jan 25, 2021

petertrr Jan 26, 2021

aktsay6 Jan 26, 2021

petertrr Jan 26, 2021

petertrr Jan 26, 2021

petertrr Jan 26, 2021

aktsay6 Jan 26, 2021 •

edited

Loading

petertrr Jan 26, 2021

petertrr Jan 26, 2021

aktsay6 Jan 26, 2021

Feature. Enable confiuring of ktlint reporter in gradle plugin #724

Feature. Enable confiuring of ktlint reporter in gradle plugin #724

Conversation

aktsay6 commented Jan 25, 2021

What's done

codecov bot commented Jan 25, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aktsay6 Jan 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 25, 2021 •

edited

Loading

aktsay6 Jan 26, 2021 •

edited

Loading