Skip to content

Review do PR upstream #1437 - #6

Open
caioradtke wants to merge 1 commit into
masterfrom
pr-1437
Open

caioradtke wants to merge 1 commit into
masterfrom
pr-1437

Conversation

@caioradtke

@caioradtke caioradtke commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

Cópia de google#1437

Summary by CodeRabbit

  • Bug Fixes
    • Fixed formatting that could leave an extra blank line after unused imports were removed, including before a class Javadoc. The formatter now preserves clean spacing in cases involving multiple imports, files without a package declaration, and imports that remain in use. Output is consistent whether formatting is performed through the library or the command-line tool.

When every import between the package blank and the type blank is
removed, collapse one of the stacked blank lines so a single CLI pass
stays style-compliant (format-then-fix-imports path).
@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Unused-import removal now coalesces adjacent deletion ranges and adjusts spacing around removed imports in specified cases. New tests check the resulting output for several import arrangements through RemoveUnusedImports, Formatter, and Main.

Changes: Unused import cleanup

Layer / File(s) Summary
Deletion range cleanup
core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java, core/src/test/java/com/google/googlejavaformat/java/RemoveUnusedImportsTest.java, core/src/test/java/com/google/googlejavaformat/java/MainTest.java
The removal logic coalesces adjacent deletion ranges and removes an extra separator when the surrounding blank-line conditions are met. Tests cover package and non-package cases, used imports, and output through Formatter and Main.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: cushon, sormuras

Merge Risk: Minimal · up to 1b2e6

Removing unused imports now avoids leaving a doubled blank line in the specified cases, and regression tests cover those cases. No merge-blocking risk was identified.

Pre-merge checks: Passed 3 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Title check Inconclusive The title "Review do PR upstream #1437" (Portuguese for "Review of upstream PR #1437") is not descriptive of the changeset. It names no change. The changes collapse blank lines left after removing unu… Rewrite the title to name the main change. For example: "Collapse double blank line after removing unused imports" or a similar phrase that names the behavior in RemoveUnusedImports. Optionally keep the upstream reference #1437 at the end o…
Passed Passed checks: 3 passed

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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.
Full details: Title check

Explanation

The title "Review do PR upstream #1437" (Portuguese for "Review of upstream PR #1437") is not descriptive of the changeset. It names no change. The changes collapse blank lines left after removing unused imports, between the package and type declarations, so Javadoc keeps a single blank line. The title is vague and generic and conveys no meaningful information about the change.

Resolution

Rewrite the title to name the main change. For example: "Collapse double blank line after removing unused imports" or a similar phrase that names the behavior in RemoveUnusedImports. Optionally keep the upstream reference #1437 at the end of the title.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a3e9ddab-87cc-4172-9362-e61b9d81667b
Commits

Reviewing files that changed from the base of the PR and between 35bf6ff and 1b2e657.

Files selected for processing: 3

  • core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedImports.java
  • core/src/test/java/com/google/googlejavaformat/java/MainTest.java
  • core/src/test/java/com/google/googlejavaformat/java/RemoveUnusedImportsTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

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