Skip to content

ADFA-3119 | Fix crash when generating setters and getters#1041

Merged
jatezzz merged 2 commits into
stagefrom
fix/ADFA-3119-setters-getters-generator-crash
Mar 5, 2026
Merged

ADFA-3119 | Fix crash when generating setters and getters#1041
jatezzz merged 2 commits into
stagefrom
fix/ADFA-3119-setters-getters-generator-crash

Conversation

@jatezzz

@jatezzz jatezzz commented Mar 4, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixed a StringIndexOutOfBoundsException that caused the application to crash when generating setters and getters on files with specific formatting. Added bounds checking in EditHelper.java to ensure calculated line and column indices are never negative. Additionally, wrapped the editor text insertion in GenerateSettersAndGettersAction.kt with a try-catch block to gracefully handle any remaining insertion errors and display a user-friendly UI error message.

Details

  • Implemented defensive bounds using Math.max(0, ...) in insertAtEndOfClass to prevent negative column or line values.
  • Extracted a reusable showErrorMessage function to handle UI error flashing consistently.
  • Wrapped the asynchronous editor.text.insert operation in a try-catch block to prevent runtime crashes.

Before fix

Before.fix.mov
Before fix

After fix

After.fix.mov

Ticket

ADFA-3119

Observation

The root cause of the crash was that the AST end position could yield 0 or 1 for the column number depending on the file's formatting or missing closing braces. This resulted in a negative index when the EditHelper blindly subtracted values to find the insertion point. The UI fallback now ensures the user is informed if the code structure prevents generation.

@coderabbitai

coderabbitai Bot commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 59b7f0d1-e119-458b-b8b4-d64f35c9ac9b

📥 Commits

Reviewing files that changed from the base of the PR and between b3f374f and 4b37265.

📒 Files selected for processing (2)
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/generators/GenerateSettersAndGettersAction.kt
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/utils/EditHelper.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/utils/EditHelper.java

📝 Walkthrough
  • Fixed crash when generating setters and getters caused by negative text indices (StringIndexOutOfBoundsException). The app no longer crashes for files with certain formatting/missing braces; users now receive a UI error message instead.

  • Key changes

    • EditHelper.insertAtEndOfClass: added defensive validation for AST end position and computed line/column values. Throws a clear IllegalStateException when end/line/column cannot be determined and clamps computed column with Math.max(0, ...) to avoid negative indices.
    • GenerateSettersAndGettersAction: added a centralized showErrorMessage(error, context?) helper to log errors and surface a user-friendly UI message; wrapped asynchronous editor.text.insert (and related code path) in try/catch to catch StringIndexOutOfBoundsException and call the UI error helper.
  • Impact

    • Prevents runtime crashes when the AST reports unexpected end positions (e.g., 0 or 1 for column) due to formatting or missing braces.
    • Provides graceful user feedback instead of an unrecoverable editor crash.
  • Risks & best-practice considerations

    • The code now throws IllegalStateException for input-dependent conditions (malformed AST/end position). This may be considered a heavy-handed choice for recoverable/user-input errors; a custom exception or IllegalArgumentException could be more appropriate.
    • The user-facing message is generic; exposing more context or guidance could help users fix code structure issues.
    • The try-catch focuses on StringIndexOutOfBoundsException during insertion; other exceptions (e.g., during formatting) may still need broader handling to fully prevent crashes.

Walkthrough

Centralized error handling in GenerateSettersAndGettersAction via a new private helper and added try/catch around editor insert/format operations; EditHelper.insertAtEndOfClass now validates end and clamps/guards computed line/column, throwing on invalid positions.

Changes

Cohort / File(s) Summary
Generateters error handling
lsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/generators/GenerateSettersAndGettersAction.kt
Added private showErrorMessage(Throwable, Context?); replaced inline error handling with calls to this helper; wrapped editor insertion and formatting in try/catch that routes exceptions to the helper.
Edit position safety
lsp/java/src/main/java/com/itsaky/androidide/lsp/java/utils/EditHelper.java
Hardened insertAtEndOfClass: validate end >= 0, throw IllegalStateException when position cannot be determined, and clamp computed line/column using Math.max to avoid negative coordinates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • hal-eisen-adfa
  • jomen-adfa

Poem

🐰 I hopped through lines and stitched a seam,
Errors gathered into one calm stream.
Columns clipped so none fall through,
Inserts safe, and logs in view.
A tiny nibble—code feels gleam! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'ADFA-3119 | Fix crash when generating setters and getters' clearly and specifically summarizes the main change: fixing a crash in the setter/getter generation feature.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the StringIndexOutOfBoundsException fix, bounds checking, error handling, and root cause analysis.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/ADFA-3119-setters-getters-generator-crash

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/generators/GenerateSettersAndGettersAction.kt`:
- Around line 71-76: Always call log.error with the Throwable regardless of
whether a Context is available, and only execute the UI toast when Context is
non-null; update showErrorMessage so it unconditionally logs the error via
log.error("Unable to generate setters and getters", error) and wraps the
ThreadUtils.runOnUiThread { flashError(...) } block inside a conditional that
checks context != null (or change callers to call showErrorMessage with a
nullable Context and have it handle the null case). Apply the same change to the
other usages mentioned (the other call sites around
GenerateSettersAndGettersAction where showErrorMessage is skipped) so
diagnostics are always logged while toasts remain conditional.
- Line 141: The catch block in GenerateSettersAndGettersAction (inside the
method that builds/patches getters/setters) is too broad; change the handler
from catch (e: Exception) to catch (e: StringIndexOutOfBoundsException) so only
the reported string index error is caught, and preserve the existing
null-check/logging behavior (i.e., keep the same body that checks e?.message or
logs the exception). Ensure other exceptions continue to propagate (remove or
avoid a broad fallback catch) so unrelated failures are not masked.

In `@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/utils/EditHelper.java`:
- Around line 179-187: The current code in EditHelper returns new Position(0,0)
when end < 0 which causes silent insertion at file start; change the early
return to signal failure instead (e.g., return null or Optional.empty) from the
method in EditHelper (the method that computes Position from the class-end 'end'
value) and update all call sites listed (CreateMissingMethod.java,
ImplementAbstractMethods.java, OverrideSuperclassMethodsAction.kt,
GenerateRecordConstructor.java) to check for that failure and abort the edit
generation (do not create an edit if the Position is absent). Ensure method
signature and all callers are updated consistently so malformed class-end
metadata results in no edit rather than insertion at (0,0).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: de08502b-e425-4611-909d-89bc1301bb38

📥 Commits

Reviewing files that changed from the base of the PR and between 26528b5 and 0f7a513.

📒 Files selected for processing (2)
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/generators/GenerateSettersAndGettersAction.kt
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/utils/EditHelper.java

Comment thread lsp/java/src/main/java/com/itsaky/androidide/lsp/java/utils/EditHelper.java Outdated
@jatezzz jatezzz requested a review from a team March 4, 2026 17:55

@hal-eisen-adfa hal-eisen-adfa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@jatezzz jatezzz force-pushed the fix/ADFA-3119-setters-getters-generator-crash branch from b3f374f to 4b37265 Compare March 5, 2026 15:27
@jatezzz jatezzz force-pushed the fix/ADFA-3119-setters-getters-generator-crash branch from 4b37265 to 81bf22b Compare March 5, 2026 15:45
@jatezzz jatezzz merged commit 89ee046 into stage Mar 5, 2026
2 checks passed
@jatezzz jatezzz deleted the fix/ADFA-3119-setters-getters-generator-crash branch March 5, 2026 15:58
jatezzz added a commit that referenced this pull request Jun 22, 2026
* fix: prevent StringIndexOutOfBoundsException in setter/getter generator
Add safe coordinate bounds in EditHelper and try-catch fallback with UI error message.

* refactor: refine error handling for setter/getter generator
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