build(spotless): add Spotless and Ktlint for formatting and linting - #44
build(spotless): add Spotless and Ktlint for formatting and linting#44LMLiam wants to merge 1 commit into
Conversation
LMLiam
added
type:maintenance
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR establishes repository-wide code formatting standards by introducing an ChangesCode Formatting Standards and Enforcement
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.editorconfig:
- Line 108: The .editorconfig setting ij_kotlin_packages_to_use_import_on_demand
currently enables wildcard imports for java.util.*,
kotlinx.android.synthetic.**, and io.ktor.**; update this setting to either
remove the wildcard entries or replace them with explicit package names you
intend to allow as on-demand imports so wildcard imports are not globally
enabled—modify the ij_kotlin_packages_to_use_import_on_demand entry accordingly
(e.g., remove java.util.*, kotlinx.android.synthetic.**, io.ktor.**) or
document/justify and restrict which specific packages should remain wildcarded
to match your project convention.
- Line 6: Change the .editorconfig setting insert_final_newline from false to
true to ensure files end with a newline and match the Gradle Spotless rule
endWithNewline(); update insert_final_newline to true so editor behavior is
consistent with the spotless/endWithNewline() configuration and POSIX text file
expectations.
In `@build.gradle`:
- Line 87: The Gradle configuration uses ratchetFrom 'origin/main' which limits
Spotless checks to changed files; to fix this, run a one-time full formatting
pass by executing ./gradlew spotlessApply across the repo to create a clean
baseline, then keep ratchetFrom 'origin/main' for incremental checks; ensure you
commit the resulting formatting changes so build.gradle's ratchetFrom behavior
only filters already-formatted files going forward.
- Around line 6-7: Update the pinned Gradle plugin versions for the ktlint and
Spotless plugins by changing the version strings for the plugin identifiers
org.jlleitschuh.gradle.ktlint and com.diffplug.spotless to the current latest
releases (replace "13.1.0" with "14.2.0" for org.jlleitschuh.gradle.ktlint and
"7.2.1" with "8.6.0" for com.diffplug.spotless) in the build.gradle plugin
declarations; run a Gradle build to confirm compatibility and adjust any
plugin-specific configuration if the newer plugin versions introduce breaking
changes.
- Around line 78-84: The build currently makes tasks.named("check") and
tasks.named("compileKotlin") depend on spotlessApply; change the dependency so
tasks.named("check") depends on "spotlessCheck" (not "spotlessApply") and remove
the dependency on spotless for "compileKotlin" entirely (i.e., drop
tasks.named("compileKotlin") { dependsOn(...) }), ensuring formatting is
enforced via spotlessCheck during the check lifecycle while developers run
spotlessApply manually.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b4d985e4-dbfd-4cd6-a8da-596c670ef231
📒 Files selected for processing (2)
.editorconfigbuild.gradle
| end_of_line = lf | ||
| indent_size = 4 | ||
| indent_style = space | ||
| insert_final_newline = false |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Reconsider disabling final newlines.
Setting insert_final_newline = false deviates from POSIX text file standards and common best practices. Most tools expect text files to end with a newline, and this setting conflicts with the endWithNewline() rule configured for Gradle files in build.gradle (line 94).
Consider setting this to true for consistency with spotless configuration and standard practices.
♻️ Proposed fix
-insert_final_newline = false
+insert_final_newline = true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| insert_final_newline = false | |
| insert_final_newline = true |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.editorconfig at line 6, Change the .editorconfig setting
insert_final_newline from false to true to ensure files end with a newline and
match the Gradle Spotless rule endWithNewline(); update insert_final_newline to
true so editor behavior is consistent with the spotless/endWithNewline()
configuration and POSIX text file expectations.
| ij_kotlin_method_parameters_wrap = on_every_item | ||
| ij_kotlin_name_count_to_use_star_import = 5 | ||
| ij_kotlin_name_count_to_use_star_import_for_members = 3 | ||
| ij_kotlin_packages_to_use_import_on_demand = java.util.*,kotlinx.android.synthetic.**,io.ktor.** |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Verify wildcard imports are intentional.
The configuration allows wildcard imports for java.util.*, kotlinx.android.synthetic.**, and io.ktor.**. Wildcard imports can reduce code clarity by hiding import sources and may cause namespace pollution.
Confirm this is intentional for your project's conventions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.editorconfig at line 108, The .editorconfig setting
ij_kotlin_packages_to_use_import_on_demand currently enables wildcard imports
for java.util.*, kotlinx.android.synthetic.**, and io.ktor.**; update this
setting to either remove the wildcard entries or replace them with explicit
package names you intend to allow as on-demand imports so wildcard imports are
not globally enabled—modify the ij_kotlin_packages_to_use_import_on_demand entry
accordingly (e.g., remove java.util.*, kotlinx.android.synthetic.**, io.ktor.**)
or document/justify and restrict which specific packages should remain
wildcarded to match your project convention.
| id "org.jlleitschuh.gradle.ktlint" version "13.1.0" apply false | ||
| id 'com.diffplug.spotless' version "7.2.1" apply false |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for latest versions and security advisories for Gradle plugins
echo "=== Checking ktlint-gradle plugin latest version ==="
curl -s "https://plugins.gradle.org/plugin/org.jlleitschuh.gradle.ktlint" | grep -o 'Version [0-9.]*' | head -1
echo -e "\n=== Checking spotless plugin latest version ==="
curl -s "https://plugins.gradle.org/plugin/com.diffplug.spotless" | grep -o 'Version [0-9.]*' | head -1
echo -e "\n=== Checking for GitHub security advisories ==="
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "org.jlleitschuh.gradle:ktlint-gradle") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "com.diffplug.spotless:spotless-plugin-gradle") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'Repository: LMLiam/SPI-Tooling
Length of output: 532
Update ktlint/spotless Gradle plugin versions (security impact currently unclear, but pinned versions are behind latest).
- ktlint-gradle: pinned 13.1.0; Plugin Portal shows latest 14.2.0; GitHub advisory query returned no vulnerabilities for this package.
- spotless: pinned 7.2.1; Plugin Portal shows latest 8.6.0; GitHub advisory found a HIGH issue but it applies to versions
< 3.20.0(so 7.2.1 is outside that range). - Consider bumping both plugins to the latest versions for improved compatibility/maintenance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.gradle` around lines 6 - 7, Update the pinned Gradle plugin versions
for the ktlint and Spotless plugins by changing the version strings for the
plugin identifiers org.jlleitschuh.gradle.ktlint and com.diffplug.spotless to
the current latest releases (replace "13.1.0" with "14.2.0" for
org.jlleitschuh.gradle.ktlint and "7.2.1" with "8.6.0" for
com.diffplug.spotless) in the build.gradle plugin declarations; run a Gradle
build to confirm compatibility and adjust any plugin-specific configuration if
the newer plugin versions introduce breaking changes.
| tasks.named("check") { | ||
| dependsOn("spotlessApply") | ||
| } | ||
|
|
||
| tasks.named("compileKotlin") { | ||
| dependsOn(tasks.named("spotlessApply")) | ||
| } |
There was a problem hiding this comment.
Critical: Replace spotlessApply with spotlessCheck in task dependencies.
Making check and compileKotlin depend on spotlessApply causes automatic code reformatting during builds, which is an anti-pattern:
- Fails silently: Formatting violations are auto-fixed instead of failing the build, hiding issues in CI
- Unexpected changes: Developers compiling code will have their files reformatted without explicit consent
- CI/CD risk: Code can be silently modified during build pipelines
The standard pattern is:
checkdepends onspotlessCheck(fails if formatting is wrong)- Developers run
./gradlew spotlessApplymanually or via pre-commit hooks to fix formatting
🐛 Proposed fix
tasks.named("check") {
- dependsOn("spotlessApply")
+ dependsOn("spotlessCheck")
}
-tasks.named("compileKotlin") {
- dependsOn(tasks.named("spotlessApply"))
-}Note: Remove the compileKotlin dependency entirely. Formatting checks should happen during check, not compilation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.gradle` around lines 78 - 84, The build currently makes
tasks.named("check") and tasks.named("compileKotlin") depend on spotlessApply;
change the dependency so tasks.named("check") depends on "spotlessCheck" (not
"spotlessApply") and remove the dependency on spotless for "compileKotlin"
entirely (i.e., drop tasks.named("compileKotlin") { dependsOn(...) }), ensuring
formatting is enforced via spotlessCheck during the check lifecycle while
developers run spotlessApply manually.
| } | ||
|
|
||
| spotless { | ||
| ratchetFrom 'origin/main' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Note: ratchetFrom only checks changed files.
The ratchetFrom 'origin/main' configuration means spotless will only check files modified since the origin/main branch. This is useful for incremental adoption but won't catch formatting issues in unchanged files.
Consider running a full ./gradlew spotlessApply across the entire codebase initially to establish a clean baseline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.gradle` at line 87, The Gradle configuration uses ratchetFrom
'origin/main' which limits Spotless checks to changed files; to fix this, run a
one-time full formatting pass by executing ./gradlew spotlessApply across the
repo to create a clean baseline, then keep ratchetFrom 'origin/main' for
incremental checks; ensure you commit the resulting formatting changes so
build.gradle's ratchetFrom behavior only filters already-formatted files going
forward.
1 participant
📌 Summary
Adds Spotless and Ktlint to enforce code syntax
🔍 Related Issues / Tickets
🛠 Changes in Detail
🧩 Modules Affected
📦 Versioning
🧪 Testing
Not necessary
✅ Checklist
./gradlew kotest)📜 Notes for Reviewers
Recreated from #10 — the original PR was auto-closed when
main’s history was rewritten to theverb(scope):convention and GitHub blocks reopening a PR whose branch was force-pushed. Same branch, rebased onto the rewrittenmain.Summary by CodeRabbit
Release Notes