Skip to content

support ktlint-plugins.properties for ktlint version#1034

Merged
wakingrufus merged 1 commit intomainfrom
ktlint-version-coordination
Mar 12, 2026
Merged

support ktlint-plugins.properties for ktlint version#1034
wakingrufus merged 1 commit intomainfrom
ktlint-version-coordination

Conversation

@wakingrufus
Copy link
Copy Markdown
Collaborator

@wakingrufus wakingrufus commented Feb 16, 2026

support ktlint-plugins.properties for version coordination with intellij plugin

@wakingrufus wakingrufus changed the title support ktlint-plugins.properties for version coordination with intel… support ktlint-plugins.properties for ktlint version Feb 16, 2026
@wakingrufus wakingrufus force-pushed the ktlint-version-coordination branch from 8e59337 to a40c5d6 Compare February 16, 2026 21:28
@wakingrufus wakingrufus marked this pull request as ready for review February 16, 2026 22:05
filterTargetApplier,
kotlinScriptAdditionalPathApplier
)
val kotlinPluginsPropertiesFile = target.layout.projectDirectory.file(KTLINT_PLUGINS_PROPERTIES_FILE_NAME)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This won't track the file as an input. If the ktlint-plugins.properties file changes, the task won't be rerun. This can result in confusing/unintuitive behavior for the user

I know the copliot PR is sloppy, but there are test cases in there that we should also verify we pass here as well

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think it does do this, since it is piped through target.providers.fileContents but ill include test cases to prove it

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't think it will if it's not marked explicitly as an @Input, but I could be wrong

Copy link
Copy Markdown
Collaborator Author

@wakingrufus wakingrufus Feb 19, 2026

Choose a reason for hiding this comment

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

Input is for task-level caching (build cache). in my PR, we are setting the existing property on the extension, which is bound to the existing task Input field.
this is happening at configuration time, so when reading the file at configuration time (not within a task) it needs to be configuration cache compatible. @Input does not help at all here. for this, we need gradle.providers which allows configuration cache safe access to files, gradle properties, environment variables, and system properties

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

My understanding is that @Input or @InputFile is also used for task up-to-date checking as well

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is. but the configuration file is not an input to the task. it is an input to the plugin code that configures the dependencies.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I updated the test to show that up to date checks work properly

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I also added some assertions that show that configuration cache is not violated

@wakingrufus wakingrufus force-pushed the ktlint-version-coordination branch 2 times, most recently from 7e8ffbb to e53016b Compare March 12, 2026 14:02
@wakingrufus wakingrufus force-pushed the ktlint-version-coordination branch from e53016b to 3c9887b Compare March 12, 2026 14:27
@wakingrufus wakingrufus requested a review from JLLeitschuh March 12, 2026 14:28
@wakingrufus wakingrufus merged commit 4a672a7 into main Mar 12, 2026
24 checks passed
@wakingrufus wakingrufus deleted the ktlint-version-coordination branch March 12, 2026 16:18
implementation(libs.jgit)
implementation(libs.commons.io)

testImplementation(libs.assertj.core)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can we drop the dependency on assertj and replace it all with nebula-test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants