Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert github-milestone-report.groovy to main.kts #2777

Merged
merged 3 commits into from
Jun 7, 2020

Conversation

cortinico
Copy link
Member

I'm converting the github-milestone-report.groovy script to use kotlin-main-kts. That's the officially supported scripting support introduced in 1.3.70.
https://github.com/Kotlin/KEEP/blob/master/proposals/scripting-support.md#kotlin-main-kts

For the cli parsing I've used kotlinx.cli, that this is still experimental but given the nature of this script shouldn't be a problem.

The script should just work out of the box, as long as you have kotlin installed locally.

$ ./github-milestone-report.main.kts

#### 1.10.0

##### Notable Changes

##### Migration

##### Changelog

- Remove deprecated HierarchicalConfig which could lead to OOM when reusing Config objects - [#2768](https://github.com/detekt/detekt/pull/2768)
- Support layout property for ImportOrdering rule - [#2763](https://github.com/detekt/detekt/pull/2763)
- Wrap three new experimental KtLint rules - [#2762](https://github.com/detekt/detekt/pull/2762)
- Upgrade to ktlint 0.37.0 - [#2760](https://github.com/detekt/detekt/pull/2760)
- Introduce reporting extensions - [#2755](https://github.com/detekt/detekt/pull/2755)
- Add default print methods to ForbiddenMethodCall - [#2753](https://github.com/detekt/detekt/pull/2753)
- Add the `ignoreAnnotated` array parameter to the FunctionNaming rule - [#2734](https://github.com/detekt/detekt/pull/2734)
- FunctionNaming: Needs "ignoreAnnotated" - [#2733](https://github.com/detekt/detekt/issues/2733)
- State that speeding the detekt task just applies to version < 1.7.0 - [#2730](https://github.com/detekt/detekt/pull/2730)
- Add cognitive complexity in complexity report - [#2727](https://github.com/detekt/detekt/pull/2727)
- add better documentation for the LongParameterList ignoreAnnotated - [#2714](https://github.com/detekt/detekt/pull/2714)
- IgnoreReturnValue: config options - [#2712](https://github.com/detekt/detekt/pull/2712)
- Use experimental indentation rule set instead of the unused from the standard rule set - [#2709](https://github.com/detekt/detekt/pull/2709)
- Remove idea integration - [#2706](https://github.com/detekt/detekt/pull/2706)
- Improve issue reporting/report at identifiers and package declarations - #2699 - [#2702](https://github.com/detekt/detekt/pull/2702)
- Feature request - limit number of lines for an issue to 1 - [#2699](https://github.com/detekt/detekt/issues/2699)
- New Rule: IgnoredReturnValue - [#2698](https://github.com/detekt/detekt/pull/2698)
- New rule: NoPrintStatement - [#2678](https://github.com/detekt/detekt/issues/2678)
- Add default values to SwallowedException rule - [#2661](https://github.com/detekt/detekt/pull/2661)
- [V1.6.0 -> V1.7.4] Error reading configuration file, java.util.zip.ZipException: invalid code lengths set. - [#2582](https://github.com/detekt/detekt/issues/2582)
- New rule: Warn on ignored return value - [#2239](https://github.com/detekt/detekt/issues/2239)
- File 'C\...\.idea' specified for property 'ideaExtension.path' is not a file. - [#2172](https://github.com/detekt/detekt/issues/2172)
- ktlint integration does not report most errors - [#2161](https://github.com/detekt/detekt/issues/2161)
- Non deterministic output. False positives on Indentation rule - [#1633](https://github.com/detekt/detekt/issues/1633)

##### Housekeeping & Refactorings

- Prepare 1.10.0-RC1 release - [#2776](https://github.com/detekt/detekt/pull/2776)
- Fix memory leak with not closing processing settings - [#2775](https://github.com/detekt/detekt/pull/2775)
- Do not print passing tests on the console - [#2774](https://github.com/detekt/detekt/pull/2774)
- Run in parallel by default - [#2773](https://github.com/detekt/detekt/pull/2773)
- Remove core module dependency for detekt-test - [#2771](https://github.com/detekt/detekt/pull/2771)
- Unify extension debug printing - [#2770](https://github.com/detekt/detekt/pull/2770)
- Package editorconfig dependency into the jar for formatting module - [#2769](https://github.com/detekt/detekt/pull/2769)
- Update spek to 2.0.11 disabling timeouts - [#2767](https://github.com/detekt/detekt/pull/2767)
- Introduce additional changelog section filtering developing/refactoring noise for the users - [#2766](https://github.com/detekt/detekt/pull/2766)
- Move config validation from cli to core - [#2764](https://github.com/detekt/detekt/pull/2764)
- Improve the performance of tests which use type resolution - [#2756](https://github.com/detekt/detekt/pull/2756)
- Move reporting logic to core module - [#2754](https://github.com/detekt/detekt/pull/2754)
- Cleanup tests in ProtectedMemberInFinalClass - [#2752](https://github.com/detekt/detekt/pull/2752)
- Add referential equality test case in EqualsAlwaysReturnsTrueOrFalse - [#2751](https://github.com/detekt/detekt/pull/2751)
- Extract xml and html reports to own modules - [#2750](https://github.com/detekt/detekt/pull/2750)
- Separate console and output report loading - [#2749](https://github.com/detekt/detekt/pull/2749)
- Bump actions/cache to v2 - [#2746](https://github.com/detekt/detekt/pull/2746)
- Fix EqualsAlwaysReturnsTrueOrFalse doc - [#2744](https://github.com/detekt/detekt/pull/2744)
- Simplify core facade class - [#2743](https://github.com/detekt/detekt/pull/2743)
- Mark some well known cli functions as implicit unsupported api - [#2742](https://github.com/detekt/detekt/pull/2742)
- Move baseline feature to core module - [#2741](https://github.com/detekt/detekt/pull/2741)
- Make baseline entities internal - [#2740](https://github.com/detekt/detekt/pull/2740)
- Simplify baseline data structures - [#2739](https://github.com/detekt/detekt/pull/2739)
- Move baseline utils to the baseline package - [#2738](https://github.com/detekt/detekt/pull/2738)
- Bump github-pages from 204 to 206 in /docs - [#2737](https://github.com/detekt/detekt/pull/2737)
- Update gradle scan plugin - [#2736](https://github.com/detekt/detekt/pull/2736)
- Update test dependencies - [#2735](https://github.com/detekt/detekt/pull/2735)
- Move three core-related tests to core module - [#2731](https://github.com/detekt/detekt/pull/2731)
- Update to Gradle 6.4.1 - [#2729](https://github.com/detekt/detekt/pull/2729)
- Migrate to resource function of test-utils - [#2728](https://github.com/detekt/detekt/pull/2728)
- Remove own collectByType function as Kotlin's does not crash anymore - [#2726](https://github.com/detekt/detekt/pull/2726)
- Move processors to metrics module - [#2725](https://github.com/detekt/detekt/pull/2725)
- Create publish tasks lazily - [#2723](https://github.com/detekt/detekt/pull/2723)
- Faster documentation generation - [#2722](https://github.com/detekt/detekt/pull/2722)
- Modularize test module - [#2720](https://github.com/detekt/detekt/pull/2720)
- Introduce parser and psi module - [#2716](https://github.com/detekt/detekt/pull/2716)
- Clean up code by using builtin associateBy function - [#2715](https://github.com/detekt/detekt/pull/2715)
- [Security] Bump activesupport from 6.0.2.1 to 6.0.3.1 in /docs - [#2708](https://github.com/detekt/detekt/pull/2708)
- Correct formatting issues - [#2707](https://github.com/detekt/detekt/pull/2707)
- [Gradle plugin/rule authors]: Invalidate jars on modified date changes - [#2703](https://github.com/detekt/detekt/pull/2703)

See all issues at: [1.10.0](https://github.com/detekt/detekt/milestone/67)

@cortinico cortinico changed the title Convert milestore-report to main.kts Convert github-milestone-report.groovy to main.kts Jun 6, 2020
@codecov
Copy link

codecov bot commented Jun 6, 2020

Codecov Report

Merging #2777 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2777   +/-   ##
=========================================
  Coverage     80.53%   80.53%           
  Complexity     2321     2321           
=========================================
  Files           386      386           
  Lines          6952     6952           
  Branches       1260     1260           
=========================================
  Hits           5599     5599           
  Misses          725      725           
  Partials        628      628           

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 98b89f4...15db19b. Read the comment docs.

@arturbosch
Copy link
Member

Looks good!

When testing the script locally I get following :/

Screenshot from 2020-06-06 17-03-28
Screenshot from 2020-06-06 17-11-03

I've installed kotlinc via sdkman

@cortinico
Copy link
Member Author

cortinico commented Jun 6, 2020

You're on Linux right? I got no problem running it on macos.

The problem is related to parsing of cli arguments when running a script. It's described in details here: Kotlin/KEEP#75 (comment)

So ideally we should replace the first line with:

#!/bin/sh
//bin/true; exec kotlinc -script "$0" "$@"

(can you try?)

That's not really elegant, but will make the script usable on every platform.

@arturbosch
Copy link
Member

You're on Linux right? I got no problem running it on macos.

Yes!

The problem is related to parsing of cli arguments when running a script. It's described in details here: Kotlin/KEEP#75 (comment)

So ideally we should replace the first line with:

#!/bin/sh
//bin/true; exec kotlinc -script "$0" "$@"

(can you try?)

That's not really elegant, but will make the script usable on every platform.

This works for me so the kotlin compiler is called.

I still get this errors:

:: problems summary ::
:::: WARNINGS
		module not found: org.jetbrains.kotlinx#kotlinx-cli-jvm;0.2.1

	==== central: tried

	  https://repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-cli-jvm/0.2.1/kotlinx-cli-jvm-0.2.1.pom

	  -- artifact org.jetbrains.kotlinx#kotlinx-cli-jvm;0.2.1!kotlinx-cli-jvm.jar:

	  https://repo1.maven.org/maven2/org/jetbrains/kotlinx/kotlinx-cli-jvm/0.2.1/kotlinx-cli-jvm-0.2.1.jar

error: cannot access script base class 'org.jetbrains.kotlin.mainKts.MainKtsScript'. Check your module classpath for missing or conflicting dependencies (github-milestone-report.main.kts:10:1)

The last time I tried kotlin scripts (some months ago) they were still kinda slow to execute ~ 2 seconds. I prototyped a kts compile daemon which still had a 200/300ms overhead :(

I hope it gets better with the 1.4 release. For bigger scripts I still use Groovy with groovyserv which let scripts start instantly.

@cortinico
Copy link
Member Author

The last time I tried kotlin scripts (some months ago) they were still kinda slow to execute ~ 2 seconds. I prototyped a kts compile daemon which still had a 200/300ms overhead :(
I hope it gets better with the 1.4 release. For bigger scripts I still use Groovy with groovyserv which let scripts start instantly.

Agree, there is space for improvement on the scripting side.
On the other had we get the nice side effect that detekt runs over those as well.

Regarding your issue, seems like is this one
https://youtrack.jetbrains.com/issue/KT-37293
I've tried using the Kotlin compiler for 1.4-M2 on a Linux machine and the issue is still there.

@arturbosch
Copy link
Member

The last time I tried kotlin scripts (some months ago) they were still kinda slow to execute ~ 2 seconds. I prototyped a kts compile daemon which still had a 200/300ms overhead :(
I hope it gets better with the 1.4 release. For bigger scripts I still use Groovy with groovyserv which let scripts start instantly.

Agree, there is space for improvement on the scripting side.
On the other had we get the nice side effect that detekt runs over those as well.

Regarding your issue, seems like is this one
https://youtrack.jetbrains.com/issue/KT-37293
I've tried using the Kotlin compiler for 1.4-M2 on a Linux machine and the issue is still there.

Thanks for investigating this. Lets leave this PR open then.

@cortinico
Copy link
Member Author

Thanks for investigating this. Lets leave this PR open then.

Can you give it another try @arturbosch ?
I've changed the CLI parsing lib to clikt that is available on Central so the @file:Repository annotation is not needed. We can move back to kotlinx.cli as soon as it eventually lands on Central.

It works fine on my end, both on Linux and MacOS.

Copy link
Member

@arturbosch arturbosch left a comment

Choose a reason for hiding this comment

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

Works! clikt seems to be a cool lib.

@arturbosch arturbosch merged commit 56adb86 into detekt:master Jun 7, 2020
@arturbosch arturbosch added housekeeping Marker for housekeeping tasks and refactorings and removed blocked labels Jun 7, 2020
@arturbosch arturbosch added this to the 1.10.0 milestone Jun 7, 2020
@cortinico cortinico deleted the milestone-report-kts branch June 7, 2020 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Marker for housekeeping tasks and refactorings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants