Skip to content

Add detekt - #84

Merged
mmathieum merged 6 commits into
masterfrom
mm/add_detekt
Jul 31, 2026
Merged

Add detekt#84
mmathieum merged 6 commits into
masterfrom
mm/add_detekt

Conversation

@mmathieum

Copy link
Copy Markdown
Member

@mmathieum mmathieum self-assigned this Jul 31, 2026
@mmathieum
mmathieum marked this pull request as ready for review July 31, 2026 19:16
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add Detekt static analysis with baseline and code cleanups

⚙️ Configuration changes ✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Introduce Detekt with a baseline to adopt linting without failing existing violations.
• Refactor Kotlin sources to satisfy Detekt rules (naming, line length, suppressions).
• Improve enum/test documentation and readability while keeping behavior unchanged.
Diagram

graph TD
  Dev["Developer / CI"] --> Gradle["Gradle build"] --> Detekt["Detekt task"] --> Report["Reports / gating"]
  Detekt --> Baseline["detekt-baseline.xml"]
  Detekt --> Code["Kotlin sources"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fix all existing Detekt findings upfront (no baseline)
  • ➕ No long-lived suppression list to maintain
  • ➕ Immediate code-quality uplift and consistent standards
  • ➖ Very large PR with high merge-conflict risk
  • ➖ Hard to review and may block shipping for non-functional reasons
2. Disable/selectively relax rules instead of baselining
  • ➕ Less baseline churn
  • ➕ Ruleset can be tuned to the project’s conventions
  • ➖ May under-enforce valuable rules long term
  • ➖ Tuning can become subjective and drift over time
3. Phase-in by module/package (partial Detekt application)
  • ➕ Clear migration path with bounded scope per phase
  • ➕ Reduces baseline size and concentrates improvements
  • ➖ More complex Gradle configuration
  • ➖ Inconsistent standards across modules during rollout

Recommendation: Keeping Detekt enabled with a baseline is the best adoption path for this repo right now: it prevents regressions (new findings) without requiring a massive cleanup PR. As follow-up work, periodically regenerate/trim the baseline (or migrate to module-by-module enforcement) to ensure the baseline trends toward zero.

Files changed (10) +304 / -57

Refactor (7) +55 / -51
CloseableUtils.ktSilence unused exception variable in closeQuietly +1/-1

Silence unused exception variable in closeQuietly

• Renames the caught IOException variable to '_' to avoid unused-variable warnings. Behavior remains unchanged (still ignores close failures).

src/main/java/org/mtransit/commons/CloseableUtils.kt

GFrequency.ktExpose start/end time fields and remove redundant backing vals +8/-12

Expose start/end time fields and remove redundant backing vals

• Replaces private constructor fields (_startTime/_endTime) with public startTime/endTime properties and updates derived getters and mapping logic accordingly. This reduces duplication and aligns the model with static-analysis expectations.

src/main/java/org/mtransit/parser/gtfs/data/GFrequency.kt

GStopTime.ktExpose arrival/departure time fields and update usages +20/-24

Expose arrival/departure time fields and update usages

• Replaces private backing fields (_arrivalTime/_departureTime) with public arrivalTime/departureTime properties and updates helpers, comparison, UID/debug string generation, and DTO conversion. This is primarily a refactor but changes the data class constructor property visibility.

src/main/java/org/mtransit/parser/gtfs/data/GStopTime.kt

MDataChangedManager.ktWrap long expressions and log messages for lint compliance +11/-3

Wrap long expressions and log messages for lint compliance

• Reformats long lines (multi-arg calls and log strings) to satisfy max-line-length/static-analysis rules. No behavioral changes expected.

src/main/java/org/mtransit/parser/mt/MDataChangedManager.kt

MDirectionHeadSignFinder.ktAdd Detekt suppression and align stop-time copy calls +3/-2

Add Detekt suppression and align stop-time copy calls

• Adds suppression for a large destructuring declaration and updates copy() calls to use renamed GStopTime properties (arrivalTime/departureTime). Keeps existing headsign merge behavior while remaining compatible with the refactored stop-time model.

src/main/java/org/mtransit/parser/mt/MDirectionHeadSignFinder.kt

MReader.ktReformat regex builder helpers for readability/lint +9/-6

Reformat regex builder helpers for readability/lint

• Wraps Regex constructor calls across multiple lines to improve readability and satisfy linting rules. No change to the regex patterns themselves.

src/main/java/org/mtransit/parser/mt/MReader.kt

MSchedule.ktRename local variable for clarity in trip-id export logic +3/-3

Rename local variable for clarity in trip-id export logic

• Renames a local variable from arrivalDiff to departureArrivalDiff to better reflect its meaning. Output behavior remains the same.

src/main/java/org/mtransit/parser/mt/data/MSchedule.kt

Tests (1) +4 / -1
MDirectionHeadSignFinderTest.ktClarify test intent with KDoc +4/-1

Clarify test intent with KDoc

• Moves an inline explanatory comment into a KDoc block above the test for readability and maintainability. Test logic is unchanged.

src/test/java/org/mtransit/parser/mt/MDirectionHeadSignFinderTest.kt

Documentation (1) +31 / -5
GLocationType.ktAdd KDoc for GTFS location type enum values +31/-5

Add KDoc for GTFS location type enum values

• Converts inline comments into structured KDoc blocks per enum entry for readability and documentation quality. No functional changes to enum values/IDs.

src/main/java/org/mtransit/parser/gtfs/data/GLocationType.kt

Other (1) +214 / -0
detekt-baseline.xmlAdd Detekt baseline to suppress existing findings +214/-0

Add Detekt baseline to suppress existing findings

• Introduces a Detekt baseline file capturing current rule violations (complexity, magic numbers, newline, etc.). Enables incremental adoption by allowing only newly introduced issues to fail the build.

detekt-baseline.xml

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@mmathieum, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3a94eb40-d053-4797-8d17-8bbe028464d4

📥 Commits

Reviewing files that changed from the base of the PR and between 28de886 and 5f66194.

📒 Files selected for processing (4)
  • detekt-baseline.xml
  • src/main/java/org/mtransit/parser/gtfs/data/GCalendarDate.kt
  • src/main/java/org/mtransit/parser/gtfs/data/GLocationType.kt
  • src/main/java/org/mtransit/parser/mt/MDataChangedManager.kt
📝 Walkthrough

Summary by CodeRabbit

  • Documentation

    • Expanded documentation for GTFS location types.
  • Refactor

    • Improved access to stop and frequency time values while preserving existing behavior.
    • Simplified internal parsing and resource-handling code.
  • Chores

    • Added a static-analysis baseline for existing issues.
    • Reformatted logging and clarified internal naming.

Walkthrough

The change adds a Detekt baseline, exposes GTFS time values as public constructor properties, updates related consumers, and applies formatting, naming, exception, and documentation cleanup.

Changes

Detekt and Kotlin cleanup

Layer / File(s) Summary
Detekt baseline recording
detekt-baseline.xml
Adds a baseline containing 209 current Detekt issues and no manual suppressions.
GTFS time property exposure
src/main/java/org/mtransit/parser/gtfs/data/GFrequency.kt, src/main/java/org/mtransit/parser/gtfs/data/GStopTime.kt, src/main/java/org/mtransit/parser/mt/MDirectionHeadSignFinder.kt
Makes frequency and stop-time values public constructor properties. Related conversions, comparisons, formatting, serialization, and stop-time merging use the public properties.
Kotlin formatting and documentation cleanup
src/main/java/org/mtransit/commons/CloseableUtils.kt, src/main/java/org/mtransit/parser/gtfs/data/GLocationType.kt, src/main/java/org/mtransit/parser/mt/MDataChangedManager.kt, src/main/java/org/mtransit/parser/mt/MReader.kt, src/main/java/org/mtransit/parser/mt/data/MSchedule.kt, src/test/java/org/mtransit/parser/mt/MDirectionHeadSignFinderTest.kt
Updates ignored exception syntax, enum KDoc, log formatting, regex factory syntax, a local variable name, and test documentation without changing behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: adding Detekt static analysis configuration.
Description check ✅ Passed The description references issue #226, which is related to the Detekt configuration changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mm/add_detekt

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR appears intended to support adopting Detekt (per linked issue #226) by adding a Detekt baseline and making a set of Kotlin refactors/formatting changes that look aligned with static-analysis cleanup (line wrapping, unused catch parameter, loop style, naming, and a suppression).

Changes:

  • Add detekt-baseline.xml to baseline existing Detekt findings.
  • Apply formatting/naming/style adjustments across parser + GTFS model code (regex builders, loops, variable names, catch parameter).
  • Adjust GTFS GStopTime/GFrequency data classes to expose time fields directly and update call sites accordingly.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/test/java/org/mtransit/parser/mt/MDirectionHeadSignFinderTest.kt Moves an inline comment into KDoc above a test method.
src/main/java/org/mtransit/parser/mt/MReader.kt Reformats regex factory helpers to multi-line Regex(...) calls.
src/main/java/org/mtransit/parser/mt/MDirectionHeadSignFinder.kt Adds a Detekt suppression and updates GStopTime.copy(...) named args to match renamed fields.
src/main/java/org/mtransit/parser/mt/MDataChangedManager.kt Wraps long calls/log strings onto multiple lines.
src/main/java/org/mtransit/parser/mt/data/MSchedule.kt Renames a local variable to better reflect meaning.
src/main/java/org/mtransit/parser/gtfs/data/GStopTime.kt Renames primary-constructor time fields to arrivalTime/departureTime and updates related methods.
src/main/java/org/mtransit/parser/gtfs/data/GLocationType.kt Replaces inline enum comments with KDoc blocks.
src/main/java/org/mtransit/parser/gtfs/data/GFrequency.kt Renames primary-constructor time fields to startTime/endTime and updates related methods.
src/main/java/org/mtransit/parser/gtfs/data/GCalendarDate.kt Replaces forEach with a for loop.
src/main/java/org/mtransit/commons/CloseableUtils.kt Avoids an unused exception variable by renaming it to _.
detekt-baseline.xml Adds a Detekt baseline file for existing issues.
Suppressed comments (1)

src/main/java/org/mtransit/parser/gtfs/data/GLocationType.kt:28

  • KDoc typo: "pathways define" should be "pathways defined".
     * A location within a station, not matching any other location_type, that may be used to link together pathways define in pathways.txt.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main/java/org/mtransit/parser/gtfs/data/GLocationType.kt Outdated
Comment thread detekt-baseline.xml
@mmathieum
mmathieum merged commit 3d05f8a into master Jul 31, 2026
5 checks passed
@mmathieum
mmathieum deleted the mm/add_detekt branch July 31, 2026 22:56
montransit added a commit to mtransitapps/mtransit-for-android that referenced this pull request Jul 31, 2026
…parser':

- commons: Add `detekt` mtransitapps/commons#836
- commons-android: Add `detekt` mtransitapps/commons-android#193
- commons-java: Add `detekt` mtransitapps/commons-java#46
- parser: Add `detekt` mtransitapps/parser#84
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