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

Support Kotlin 1.4 #2981

Merged
merged 9 commits into from Aug 20, 2020
Merged

Support Kotlin 1.4 #2981

merged 9 commits into from Aug 20, 2020

Conversation

arturbosch
Copy link
Member

@arturbosch arturbosch commented Aug 18, 2020

Closes #2974

@arturbosch arturbosch added this to the 1.12.0-RC1 milestone Aug 18, 2020
bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, it] !in referenceDescriptors
val referenceDescriptors = (references + referencesViaOperator)
.mapNotNull { it.getResolvedCall(bindingContext)?.resultingDescriptor }
.map { it.original }
Copy link
Member Author

Choose a reason for hiding this comment

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

The resulting descriptors are not the same reference-wise. So I had to unwrap them in Kotlin 1.4.

@arturbosch arturbosch force-pushed the kotlin-1.4 branch 2 times, most recently from bcd8a02 to e8aa819 Compare August 18, 2020 12:27
@arturbosch arturbosch mentioned this pull request Aug 18, 2020
@arturbosch arturbosch added the migration Marker to add a migration step in the changelog label Aug 18, 2020
@arturbosch
Copy link
Member Author

I'm not sure how to resolve this Gradle warning:

> Task :detekt-gradle-plugin:compileKotlin FAILED
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-common-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-jdk7-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-jdk8-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-reflect-1.3.72.jar (version 1.3)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.4.0/e3765b66f0610afc92053ff1a93a87a544fca2b/kotlin-stdlib-jdk8-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk7/1.4.0/9cc187c3dfaf6e4001bdf962e3cdadff7690261b/kotlin-stdlib-jdk7-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.4.0/63e75298e93d4ae0b299bb869cf0c627196f8843/kotlin-stdlib-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.4.0/1c752cce0ead8d504ccc88a4fed6471fd83ab0dd/kotlin-stdlib-common-1.4.0.jar (version 1.4)
w: Consider providing an explicit dependency on kotlin-reflect 1.4 to prevent strange errors
w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
e: warnings found and -Werror specified

Including kotlin-reflect as runtime or implementation dependency does not fix this.

@codecov
Copy link

codecov bot commented Aug 18, 2020

Codecov Report

Merging #2981 into master will decrease coverage by 0.54%.
The diff coverage is 84.37%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2981      +/-   ##
============================================
- Coverage     80.31%   79.77%   -0.55%     
- Complexity     2477     2493      +16     
============================================
  Files           424      424              
  Lines          7478     7490      +12     
  Branches       1369     1409      +40     
============================================
- Hits           6006     5975      -31     
- Misses          760      768       +8     
- Partials        712      747      +35     
Impacted Files Coverage Δ Complexity Δ
...io/gitlab/arturbosch/detekt/core/BindingContext.kt 6.25% <0.00%> (+0.36%) 0.00 <0.00> (ø)
...ab/arturbosch/detekt/core/settings/LoggingAware.kt 42.85% <50.00%> (ø) 0.00 <0.00> (ø)
...turbosch/detekt/rules/style/UnusedPrivateMember.kt 91.42% <86.66%> (+0.16%) 5.00 <0.00> (ø)
...ab/arturbosch/detekt/cli/DetektProgressListener.kt 83.33% <100.00%> (ø) 4.00 <0.00> (ø)
...tlin/io/gitlab/arturbosch/detekt/cli/JCommander.kt 91.30% <100.00%> (ø) 0.00 <0.00> (ø)
...gitlab/arturbosch/detekt/cli/runners/AstPrinter.kt 100.00% <100.00%> (ø) 4.00 <0.00> (ø)
...ab/arturbosch/detekt/cli/runners/ConfigExporter.kt 75.00% <100.00%> (ø) 2.00 <0.00> (ø)
...ab/arturbosch/detekt/cli/runners/ElementPrinter.kt 96.42% <100.00%> (ø) 11.00 <0.00> (ø)
...ab/arturbosch/detekt/cli/runners/VersionPrinter.kt 100.00% <100.00%> (ø) 2.00 <1.00> (ø)
...b/arturbosch/detekt/core/reporting/OutputFacade.kt 94.11% <100.00%> (ø) 10.00 <0.00> (ø)
... and 33 more

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 f3877b4...b575a17. Read the comment docs.

config/detekt/detekt.yml Show resolved Hide resolved
config/detekt/format.yml Show resolved Hide resolved
gradle/wrapper/gradle-wrapper.properties Show resolved Hide resolved
@BraisGabin
Copy link
Member

I'm not sure how to resolve this Gradle warning:

> Task :detekt-gradle-plugin:compileKotlin FAILED
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-common-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-jdk7-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-jdk8-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-reflect-1.3.72.jar (version 1.3)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.4.0/e3765b66f0610afc92053ff1a93a87a544fca2b/kotlin-stdlib-jdk8-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk7/1.4.0/9cc187c3dfaf6e4001bdf962e3cdadff7690261b/kotlin-stdlib-jdk7-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.4.0/63e75298e93d4ae0b299bb869cf0c627196f8843/kotlin-stdlib-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.4.0/1c752cce0ead8d504ccc88a4fed6471fd83ab0dd/kotlin-stdlib-common-1.4.0.jar (version 1.4)
w: Consider providing an explicit dependency on kotlin-reflect 1.4 to prevent strange errors
w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
e: warnings found and -Werror specified

Including kotlin-reflect as runtime or implementation dependency does not fix this.

Really strange... The 1.3.72 version doesn't appear at ./gradlew :detekt-gradle-plugin:dependencies

@arturbosch
Copy link
Member Author

I'm not sure how to resolve this Gradle warning:

> Task :detekt-gradle-plugin:compileKotlin FAILED
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-common-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-jdk7-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-stdlib-jdk8-1.3.72.jar (version 1.3)
    /home/runner/.gradle/wrapper/dists/gradle-6.6-all/dm6whvs5m6hlkbnc6ae2jubui/gradle-6.6/lib/kotlin-reflect-1.3.72.jar (version 1.3)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.4.0/e3765b66f0610afc92053ff1a93a87a544fca2b/kotlin-stdlib-jdk8-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk7/1.4.0/9cc187c3dfaf6e4001bdf962e3cdadff7690261b/kotlin-stdlib-jdk7-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.4.0/63e75298e93d4ae0b299bb869cf0c627196f8843/kotlin-stdlib-1.4.0.jar (version 1.4)
    /home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.4.0/1c752cce0ead8d504ccc88a4fed6471fd83ab0dd/kotlin-stdlib-common-1.4.0.jar (version 1.4)
w: Consider providing an explicit dependency on kotlin-reflect 1.4 to prevent strange errors
w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
e: warnings found and -Werror specified

Including kotlin-reflect as runtime or implementation dependency does not fix this.

Really strange... The 1.3.72 version doesn't appear at ./gradlew :detekt-gradle-plugin:dependencies

It's the Kotlin version gradle-api ships with.

@arturbosch
Copy link
Member Author

arturbosch commented Aug 19, 2020

I would say lets merge this with the warning so our users have a working detekt version for Kotlin 1.4 if they do not use formatting to much.
Edit: code coverage should just be buggy here.

@realdadfish
Copy link
Contributor

Kotlin 1.4 is smoke-tested currently in gradle/gradle#12660 and - with some luck - available with Gradle 6.7 - if Gradle does not decide to skip this upgrade until Gradle 7.

Copy link
Contributor

@realdadfish realdadfish left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@@ -16,7 +16,7 @@ import org.jetbrains.kotlin.resolve.lazy.declarations.FileBasedDeclarationProvid
internal fun generateBindingContext(
environment: KotlinCoreEnvironment,
classpath: List<String>,
files: List<KtFile>
files: List<KtFile>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Out of interest - we can have trailing commas now, but why should we have them here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like IntelliJ autoformatted them for me here :/. Wasn't aware of.

README.md Outdated
@@ -63,7 +63,7 @@ You can find [other ways to install detekt here](https://detekt.github.io/detekt

#### with Gradle

Gradle 5.0+ is required:
Gradle 5.3+ is required:
Copy link
Member

Choose a reason for hiding this comment

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

Is this changed to reflect the new minimum Gradle version required by Kotlin? If so....

Suggested change
Gradle 5.3+ is required:
Gradle 5.4+ is required:

https://kotlinlang.org/docs/reference/using-gradle.html#plugin-and-versions:

The Kotlin Gradle plugin 1.4.0 works with Gradle 5.4 and later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting! It works with 5.3 and running it with 5.0 revealed an error message saying it supports Gradle 5.3+.
But lets follow the website's info ^^

@@ -29,9 +29,9 @@ object MultiVersionTest : Spek({

private fun getGradleVersionsUnderTest() =
if (getJdkVersion() < 13) {
listOf("5.0", "6.4.1")
listOf("5.3", "6.6")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
listOf("5.3", "6.6")
listOf("5.4", "6.6")

@arturbosch arturbosch merged commit ee49144 into master Aug 20, 2020
@arturbosch arturbosch deleted the kotlin-1.4 branch August 20, 2020 18:00
BraisGabin added a commit to BraisGabin/detekt that referenced this pull request Apr 2, 2021
This was removed in detekt#2981 for an issue with kotlin 1.4 but that issue
is fixed now so it's safe to enable it again
@BraisGabin BraisGabin mentioned this pull request Apr 2, 2021
BraisGabin added a commit to BraisGabin/detekt that referenced this pull request Apr 5, 2021
This was removed in detekt#2981 for an issue with kotlin 1.4 but that issue
is fixed now so it's safe to enable it again
BraisGabin added a commit to BraisGabin/detekt that referenced this pull request Apr 5, 2021
This was removed in detekt#2981 for an issue with kotlin 1.4 but that issue
is fixed now so it's safe to enable it again
BraisGabin added a commit that referenced this pull request Apr 17, 2021
* Deprecate Location.file in favor of Location.filePath

* Fix warnings

* Enable warningsAsErrors again

This was removed in #2981 for an issue with kotlin 1.4 but that issue
is fixed now so it's safe to enable it again
chao2zhang pushed a commit to chao2zhang/detekt that referenced this pull request May 13, 2021
* Deprecate Location.file in favor of Location.filePath

* Fix warnings

* Enable warningsAsErrors again

This was removed in detekt#2981 for an issue with kotlin 1.4 but that issue
is fixed now so it's safe to enable it again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration Marker to add a migration step in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make a release built against Kotlin 1.4
6 participants