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

ktfmt does not use custom source directories #292

Closed
christophsturm opened this issue May 21, 2024 · 10 comments
Closed

ktfmt does not use custom source directories #292

christophsturm opened this issue May 21, 2024 · 10 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@christophsturm
Copy link

🐛 Describe the bug

in my project I configure a flat directory layout by stetting srcDirs for the source sets:

sourceSets.main {
    java.srcDirs("src")
    resources.srcDirs("resources")
}
sourceSets.test {
    java.srcDirs("test")
    resources.srcDirs("testResources")
}

⚠️ Current behavior

ktfmt does not format my sources,
gradle says NO SOURCE

2024-05-21T22:04:42.174+0200 [LIFECYCLE] [class org.gradle.internal.buildevents.TaskExecutionLogger] > Task :failgood:ktfmtFormatMain NO-SOURCE

✅ Expected behavior

Before I converted my project to the flat source directory structure I'm sure it worked.

💣 Steps to reproduce

clone https://github.com/failgood/failgood
run ./gradlew ktfmtFormat
notice that sources are not reformatted, for example failgood/src/failgood/junit/ContextFinder.kt contains 2 unused imports.

@cortinico cortinico added the help wanted Extra attention is needed label Jun 3, 2024
@cortinico cortinico added bug Something isn't working good first issue Good for newcomers labels Jun 21, 2024
@cortinico
Copy link
Owner

Yup this is a bug. I'm looking for someone to help me here, happy to review a PR.
The problem is that ktfmt-gradle listens for kotlin.sourceSet and registers for changes there, while here you're manipulating the top level sourceSets (so ktfmt-gradle never receives the information on your source sets).

@christophsturm
Copy link
Author

so the fix would be to listen to both? and the work around would be to set kotlin.sourceSet? why are there always so many different ways to do things in gradle....

@christophsturm
Copy link
Author

I changed it to

kotlin {
    sourceSets.main {
        kotlin.srcDirs("src")
        resources.srcDirs("resources")
    }
    sourceSets.test {
        kotlin.srcDirs("test")
        resources.srcDirs("testResources")
    }

}

and it still does not format.

@cortinico
Copy link
Owner

why are there always so many different ways to do things in gradle....

Yeah man don't tell me about it :|

@christophsturm
Copy link
Author

@cortinico I would be happy to help fix this, but it would be cool if we could come up with a workaround first. like i wrote above, it did not work for me when i set kotlin.sourceSets instead.

@cortinico
Copy link
Owner

Yeah I started to look into it but hit a road blocker. @simonhauck do you have a spare cycle to look at this one?

@simonhauck
Copy link
Collaborator

Yes I will finish my current PR an then have a look :)

@simonhauck
Copy link
Collaborator

@christophsturm For a fast workaround you could add the tasks manually. This is not so great, but at least you can format/check the code ;)

tasks.register<KtfmtFormatTask>("customFormat") {
    source = fileTree(layout.projectDirectory) { include("**/*.kt") }
}

simonhauck added a commit that referenced this issue Jul 17, 2024
…e project is using a flattened project structure
simonhauck added a commit that referenced this issue Jul 17, 2024
@simonhauck
Copy link
Collaborator

simonhauck commented Jul 17, 2024

I have an idea what could be the problem.

From the docs on the method SourceDirectorySet::getSourceDirectories() the docs say

    /**
     * Returns the source directories that make up this set, represented as a {@link FileCollection}. Does not filter source directories that do not exist.
     * Generally, it is preferable to use this method instead of {@link #getSrcDirs()}, as this method does not require the source directories to be calculated when it is called. Instead, the source directories are calculated when queried. The return value of this method also maintains dependency information.
     *
     * <p>The returned collection is live and reflects changes to this source directory set.
     */
    FileCollection getSourceDirectories();

My assumption is that the sourceSets are changed after the plugin is evaluated. That's why we do not detect the changes when we resolve it eagerly.

The solution is straightforward... expect one or two things... ;)
We have to evaluate the sourceSet lazily. I was able to make the test green with the project.provider {} logic. But I am not sure how to handle the android sources with AGP < 7. I am not really an Android developer, so maybe @cortinico you could give me some feedback there.

Additionally, when selecting in IntelliJ settings > Build, Execution, Deployment > Build Tools > Gradle > Run tests using = IntelliJ IDEA, some tests are failing with really weird issues. This setting is super useful for debugging, and I am not sure if that behavior is new and happened due to my changes or was always there. I have to check this in the coming days.

And also, the build is still red :D

@simonhauck simonhauck linked a pull request Jul 17, 2024 that will close this issue
6 tasks
simonhauck added a commit that referenced this issue Jul 18, 2024
…ix it by evaluating it lazy with providers

- Add dependency between sqldelight tasks and formatTasks. This is necessary, else gradle will complain that task use outputs from task not specified

- update changelog
simonhauck added a commit that referenced this issue Jul 18, 2024
simonhauck added a commit that referenced this issue Aug 5, 2024
… ignored. Set this value by default to the build directory
simonhauck added a commit that referenced this issue Aug 5, 2024
simonhauck added a commit that referenced this issue Aug 25, 2024
* #292 add test to reproduce issues with changed sources location and fix it by evaluating it lazy with providers
* #292 add a new regex extension property to specify what files should be ignored. Set this value by default to the build directory
* #292 update the api specification with the new property
* #292 update documentation

---------

Co-authored-by: Nicola Corti <corti.nico@gmail.com>
@simonhauck
Copy link
Collaborator

Done in 0.20.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants