Skip to content

Deterministic sort - #5

Merged
mstdokumaci merged 3 commits into
mainfrom
deterministic-sort
Aug 4, 2026
Merged

mstdokumaci merged 3 commits into
mainfrom
deterministic-sort

Conversation

@mstdokumaci

@mstdokumaci mstdokumaci commented Aug 4, 2026 •

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Improved import sorting across standard, third-party, local, alias, and member imports.
    • Added deterministic ordering independent of source order.
    • Improved handling of aliases and chained member imports.
    • Updated the tool version to 0.2.0.
  • Bug Fixes

    • Preserved import comments while excluding blank lines.
    • Improved grouping and separator placement.
    • Corrected handling of member imports and unresolved aliases.
  • Documentation

    • Clarified import ordering rules and comment behavior.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ce0073cf-f19b-42b7-bfa2-e95034fbe942

📥 Commits

Reviewing files that changed from the base of the PR and between f175e16 and 1762757.

📒 Files selected for processing (5)
  • AGENTS.md
  • README.md
  • src/ast_scan.zig
  • src/zsort.zig
  • src/zsort_test.zig
🚧 Files skipped from review as they are similar to previous changes (4)
  • AGENTS.md
  • src/zsort.zig
  • src/ast_scan.zig
  • README.md

📝 Walkthrough

Walkthrough

The import scanner records declaration metadata, classifies member imports, resolves aliases, and sorts imports deterministically. The sorter groups import bands, preserves comment lines, updates the version, and adds tests and guidance.

Changes

Import ordering

Layer / File(s) Summary
Import metadata and alias analysis
src/ast_scan.zig
Import records now include declaration names, member status, and declaration text. Dotted imports and aliases receive expanded analysis and sorting behavior.
Band sorting and output formatting
src/zsort.zig
Import groups now separate alias, class, and member bands. Attached output keeps comment lines and removes blank lines. The version is 0.2.0.
Documentation and validation coverage
src/zsort_test.zig, README.md, AGENTS.md
Tests cover ordering, alias resolution, member imports, source-order independence, and hoisting. Documentation describes the ordering and validation rules.

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

Sequence Diagram(s)

sequenceDiagram
  participant Source
  participant ast_scan
  participant zsort
  participant Output
  Source->>ast_scan: Analyze import declarations
  ast_scan->>ast_scan: Resolve aliases and classify import bands
  ast_scan-->>zsort: Return sorted import records
  zsort->>Output: Emit imports, separators, and comment lines
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: deterministic import sorting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch deterministic-sort

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
AGENTS.md (1)

3-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the dual-version validation procedure explicit.

Each listed zig command runs only the executable on PATH. After Line 1 maps binary names to versions, invoke every gate and both test/consumer gates with each resolved Zig 0.15.2 and Zig 0.16 binary. Otherwise the procedure can validate only one version.

Proposed guidance update
-Lint gates: for both 0.15.2 and 0.16
-- zig build test
-- zig build check-imports
-- zig fmt --check src
-- zlint --deny-warnings
-- zwanzig src build.zig
-- also verify build and check-imports still work for the consumer in test/consumer
+For each binary mapped above to Zig 0.15.2 and Zig 0.16, run:
+- `$zig_bin build test`
+- `$zig_bin build check-imports`
+- `$zig_bin fmt --check src`
+- `zlint --deny-warnings`
+- `zwanzig src build.zig`
+- `(cd test/consumer && $zig_bin build && $zig_bin build check-imports)`

As per coding guidelines, “For both Zig 0.15.2 and Zig 0.16, all lint and validation gates must pass.” Based on learnings, verify which available local Zig binaries exist and determine the version of each.

🤖 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 `@AGENTS.md` around lines 3 - 9, Update the validation procedure in AGENTS.md
to explicitly resolve and identify the available Zig 0.15.2 and Zig 0.16
binaries, then run every listed gate—including all test/consumer build and
check-imports checks—once with each version-specific binary rather than relying
on the PATH default.

Sources: Coding guidelines, Learnings

🤖 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 `@README.md`:
- Around line 121-127: Update the import-group description near the ordering
list to document that blank lines separate every change in import class or
import kind: plain and member groups are emitted separately for std/builtin,
third-party, and local imports, followed by the alias group ordered std/builtin
→ third-party → local without blank lines. Replace the “four bands” framing with
wording that accurately reflects these emitted groups.

In `@src/ast_scan.zig`:
- Around line 339-343: Update the alias classification around isAliasChain to
accept only identifier-only field chains, rejecting postfix expressions such as
calls and index access before they reach lineBlockEnd and import rebuilding. Add
regression coverage for both factory().Config and items[0].Config to ensure
neither is treated as an alias.
- Around line 284-296: Replace the source-text parsing in declName with
AST-based name extraction: use tree.tokenSlice(vd.ast.mut_token + 1) so modifier
whitespace, including tabs, is handled correctly. Update the function or its
callers to receive the relevant tree and variable declaration context, and
remove the manual modifier and identifier scanning.

---

Nitpick comments:
In `@AGENTS.md`:
- Around line 3-9: Update the validation procedure in AGENTS.md to explicitly
resolve and identify the available Zig 0.15.2 and Zig 0.16 binaries, then run
every listed gate—including all test/consumer build and check-imports
checks—once with each version-specific binary rather than relying on the PATH
default.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7017e12b-5478-4968-99a6-27a038813e8b

📥 Commits

Reviewing files that changed from the base of the PR and between 20e3cba and f175e16.

📒 Files selected for processing (5)
  • AGENTS.md
  • README.md
  • src/ast_scan.zig
  • src/zsort.zig
  • src/zsort_test.zig

Comment thread README.md Outdated
Comment thread src/ast_scan.zig Outdated
Comment thread src/ast_scan.zig
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.

1 participant