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.

Sign up for GitHub

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

Jump to bottom

Allow detekt to run on kts too. #628

Merged
merged 3 commits into from
Dec 20, 2017
Merged

Allow detekt to run on kts too. #628

merged 3 commits into from
Dec 20, 2017

Conversation

vanniktech
Copy link
Contributor

Fixes #612

Let's hope it'll be that easy

arturbosch
arturbosch requested changes Dec 18, 2017
View reviewed changes
detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/KtTreeCompiler.kt Outdated
@@ -39,7 +39,7 @@ class KtTreeCompiler(private val compiler: KtCompiler = KtCompiler(),
private fun Path.isKotlinFile(): Boolean {
val fullPath = this.toAbsolutePath().toString()
val kotlinEnding = fullPath.substring(fullPath.lastIndexOf('.') + 1)
return kotlinEnding.length == 2 && kotlinEnding.endsWith("kt")
return kotlinEnding == "kt" || kotlinEnding == "kts"
Copy link
Member

Choose a reason for hiding this comment

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

Please test for .kt and .kts or files like config.detekt will get analyzed too ^^

Copy link
Member

Choose a reason for hiding this comment

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

oh sorry, my fault the kotlinEnding line does the split

@arturbosch
Copy link
Member

Please add a test case where a kotlin script is successfully compiled.

@vanniktech
Copy link
Contributor Author

Sure, where's the place for that?

@arturbosch
Copy link
Member

arturbosch commented Dec 18, 2017 •
edited
Loading

Ok, the KtTreeCompiler does the isKotlinFile check so the test case should be in detekt-core/KtTreeCompilerSpec. In resources/cases you can put a kt script file.

Furthermore please change this line project.isFile() -> listOf(compiler.compile(project, project)) to
project.isFile() && project.isKotlinFile() -> listOf(compiler.compile(project, project)). Now you can use the compile(scriptPath) function to test it ^^

@vanniktech
Copy link
Contributor Author

it("should work with two or more filters") { fails now. Any hints?

@arturbosch
Copy link
Member

Well the script name is KotlinScript.kts and does not match one of the filters. I must admit the resource structure is not optimal for independent tests ....

@arturbosch arturbosch merged commit 2826385 into detekt:master Dec 20, 2017
@arturbosch arturbosch added this to the RC6 milestone Dec 20, 2017
@vanniktech vanniktech deleted the kts branch December 20, 2017 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arturbosch arturbosch arturbosch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
RC6
Development

Successfully merging this pull request may close these issues.
2 participants
@vanniktech @arturbosch