Skip to content

Conversation

@Brett-Best
Copy link
Contributor

Attempts to resolve #6374.

@SwiftLintBot
Copy link

SwiftLintBot commented Dec 13, 2025

1 Warning
⚠️ This PR may need tests.
19 Messages
📖 Building this branch resulted in a binary size of 26873.88 KiB vs 26873.84 KiB when built on main (0% larger).
📖 Linting Aerial with this PR took 0.73 s vs 0.78 s on main (6% faster).
📖 Linting Alamofire with this PR took 1.06 s vs 1.04 s on main (1% slower).
📖 Linting Brave with this PR took 6.86 s vs 6.87 s on main (0% faster).
📖 Linting DuckDuckGo with this PR took 22.59 s vs 22.65 s on main (0% faster).
📖 Linting Firefox with this PR took 10.95 s vs 10.96 s on main (0% faster).
📖 Linting Kickstarter with this PR took 7.71 s vs 7.67 s on main (0% slower).
📖 Linting Moya with this PR took 0.46 s vs 0.42 s on main (9% slower).
📖 Linting NetNewsWire with this PR took 2.29 s vs 2.28 s on main (0% slower).
📖 Linting Nimble with this PR took 0.67 s vs 0.64 s on main (4% slower).
📖 Linting PocketCasts with this PR took 7.22 s vs 6.99 s on main (3% slower).
📖 Linting Quick with this PR took 0.44 s vs 0.41 s on main (7% slower).
📖 Linting Realm with this PR took 3.23 s vs 3.27 s on main (1% faster).
📖 Linting Sourcery with this PR took 1.78 s vs 1.77 s on main (0% slower).
📖 Linting Swift with this PR took 4.31 s vs 4.31 s on main (0% slower).
📖 Linting SwiftLintPerformanceTests with this PR took 0.33 s vs 0.3 s on main (10% slower).
📖 Linting VLC with this PR took 1.1 s vs 1.1 s on main (0% slower).
📖 Linting Wire with this PR took 17.24 s vs 17.24 s on main (0% slower).
📖 Linting WordPress with this PR took 11.4 s vs 11.35 s on main (0% slower).

Generated by 🚫 Danger

Copy link
Collaborator

@SimplyDanny SimplyDanny left a comment

Choose a reason for hiding this comment

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

This makes sense to me, especially if the rule behaved like that before the rewrite. Apparently, there didn't exist enough tests to cover that. Thanks for the fix!

We'd need violating examples/tests that proof the right ordering after correction (in addition to the non-triggering ones).

@Brett-Best Brett-Best force-pushed the feature/sort-imports-access-level-handling branch 2 times, most recently from 881a1c8 to d998649 Compare December 17, 2025 21:11
@Brett-Best Brett-Best marked this pull request as ready for review December 17, 2025 21:11
Copilot AI review requested due to automatic review settings December 17, 2025 21:11
@Brett-Best
Copy link
Contributor Author

Thanks, I thought I had an edge case I couldn't solve for when the access level and attributes were combined but I couldn't remember it.

This should be a good improvement to merge in nonetheless 🙏🏻

I think I have included sufficient coverage in the examples and corrections now.

Let me know!

PS: As a side note, we'd appreciate even a pre-release of SwiftLint since we use the artifactbundle it is a pain for us to use commits that are untagged. Thanks for the consideration!

Copy link

Copilot AI left a comment

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 reinstates handling of access level modifiers in the sorted_imports rule, addressing issue #6374. When the grouping: attributes configuration is enabled, imports are now sorted by attributes first, then by access level modifiers (public, package, internal, fileprivate, private), and finally by module name.

Key Changes

  • Added access level modifier detection and sorting logic for Swift imports when using the attributes grouping mode
  • Imports are sorted with more restrictive access levels (public) appearing before less restrictive ones (private), with plain imports appearing last
  • Added comprehensive test examples covering various combinations of attributes and access level modifiers

Reviewed changes

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

File Description
Source/SwiftLintBuiltInRules/Rules/Style/SortedImportsRule.swift Adds modifier field to Import struct and implements modifier extraction and comparison logic for access-level sorting
Source/SwiftLintBuiltInRules/Rules/Style/SortedImportsRuleExamples.swift Adds non-triggering, triggering, and correction examples demonstrating the new access level modifier sorting behavior
CHANGELOG.md Documents the bug fix in the Main section with proper credit and issue reference

@Brett-Best Brett-Best force-pushed the feature/sort-imports-access-level-handling branch 2 times, most recently from 07559e5 to b4b9a13 Compare December 17, 2025 21:27
@Brett-Best
Copy link
Contributor Author

Providing CI passes and copilot is happy this is good for review 🙏🏻

@Brett-Best Brett-Best force-pushed the feature/sort-imports-access-level-handling branch from b4b9a13 to 3c2d1e0 Compare December 18, 2025 20:36
@SimplyDanny SimplyDanny enabled auto-merge (squash) December 19, 2025 08:45
@SimplyDanny SimplyDanny merged commit fb04679 into realm:main Dec 19, 2025
25 checks passed
@Brett-Best Brett-Best deleted the feature/sort-imports-access-level-handling branch December 19, 2025 09:30
@SimplyDanny
Copy link
Collaborator

PS: As a side note, we'd appreciate even a pre-release of SwiftLint since we use the artifactbundle it is a pain for us to use commits that are untagged. Thanks for the consideration!

There you go! 🎁

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.

sorted_imports rule rewrite doesn't handle access level well

3 participants