feat: `self_import` (old) by KSXGitHub · Pull Request #152 · KSXGitHub/perfectionist

KSXGitHub · 2026-05-26T00:02:49Z

Summary

Implements the self_import rule: a project-wide policy for how self appears in use statements. Inactive by default and direction-less — opt in via [perfectionist].enable and set a mandatory style:

forbid — rewrites every self-as-module form to the bare import:
- use foo::bar::{self}; → use foo::bar;
- use foo::bar::{self, Baz}; → splits into two statements
- the brace-only foo::bar::self form → bare module (with a note that ::self is rejected in some positions)
- nested groups (use a::{b::{self}, c};) rewritten in place
combined — folds adjacent module + item imports (either source order) into use foo::bar::{self, Baz};, requiring true adjacency.

Applicability: rewrites that change which namespaces enter scope are MaybeIncorrect; the {self}→combined fold (no namespace change) is MachineApplicable, downgraded if a comment sits in the deleted gap.

Notes

Directory module (forbid / combined / render) per the catalogue conventions; DEFAULT_STATE = Inactive, style validated only when enabled.
Adjacency detection is kept local rather than shared with import_granularity, which isn't implemented yet — a future PR for that rule can extract a shared helper.
Planning file planned-rules/self-import.md removed and its cross-references updated; rules/self_import.md regenerated.

Test plan

forbid and combined UI fixtures under ui-toml/self_import/
Config unit tests (style deserialization, missing/invalid style)
Full local gate: fmt, build, doc + check-rules-md, clippy, test, self-lint

https://claude.ai/code/session_01PQZFCig1h6RHuV5hDeQKiG

Add the `self_import` rule, which enforces a project-wide policy for how `self` appears in `use` statements. Inactive by default and direction- less: a project opts in via `[perfectionist].enable` and sets a mandatory `style`: - `forbid` rewrites every `self`-as-module form (`use foo::bar::{self};`, the brace `foo::bar::self`, and the `self` member of `use foo::bar::{self, Baz};`) to the bare module import, splitting a `{self, X}` group into separate statements. - `combined` folds adjacent module + item imports into a single `use foo::bar::{self, X};`. Rewrites that change which namespaces an import brings into scope are `MaybeIncorrect`; the `{self}`-to-combined fold with no namespace change is `MachineApplicable`. Implementation lives in a directory module (`forbid`, `combined`, `render`) per the catalogue conventions, with the planning file removed and its cross-references updated.

Copilot

Pull request overview

Implements the new opt-in lint perfectionist::self_import, enforcing a configurable project-wide policy for how self is used in use statements (either forbidding module-through-self imports or combining adjacent module + item imports via {self, ...}).

Changes:

Added the self_import rule with two styles (forbid and combined), including rewrite / fold logic and shared use-tree rendering helpers.
Added UI fixtures + a dedicated UI test harness for both styles under ui-toml/self_import/.
Removed the corresponding planned-rule spec and updated documentation/catalogue entries (rules/ and planned-rules/ cross-references).

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
ui-toml/self_import/forbid/fixture.rs	UI fixture covering `forbid`-style rewrites for multiple `self` forms.
ui-toml/self_import/forbid/fixture.stderr	Expected diagnostics/suggestions for `forbid` UI fixture.
ui-toml/self_import/combined/fixture.rs	UI fixture covering `combined`-style adjacency folding cases.
ui-toml/self_import/combined/fixture.stderr	Expected diagnostics/suggestions for `combined` UI fixture.
tests/self_import.rs	Adds UI test driver enabling the rule and selecting style per fixture directory.
src/rules/self_import.rs	Declares lint, configuration, pass wiring, and crate/module/block traversal to preserve source-order adjacency.
src/rules/self_import/render.rs	Shared helpers for recognizing `self` forms and rendering `use` trees / visibility for suggestions.
src/rules/self_import/forbid.rs	Implements `forbid` style: detects `self`-as-module patterns and rewrites to bare module imports (and splits when needed).
src/rules/self_import/combined.rs	Implements `combined` style: finds adjacent module + item imports and folds into `{self, ...}` with applicability handling.
src/rules.rs	Exposes the new `self_import` rule module.
src/lib.rs	Registers the new lint/pass with the lint store.
rules/self_import.md	Adds generated rule documentation for `perfectionist::self_import`.
rules/README.md	Adds `self_import` to the rendered rules index.
planned-rules/self-import.md	Removes the planning document now that the rule is implemented.
planned-rules/README.md	Removes `self_import` from the planned-rules index.
planned-rules/qualified-paths.md	Updates cross-reference to the implemented lint name instead of the removed planning file.
planned-rules/import-granularity.md	Updates cross-reference to point to `perfectionist::self_import` (implemented lint) instead of the removed planning file.

Addresses self-review findings: - Render `use`-tree paths and renames through `Ident` (raw-identifier aware) rather than `Symbol`, so a module named `r#match` is rewritten as `r#match`, not the bare keyword `match` — which produced an autofix that failed to parse. - `forbid`'s `{self, X}` split now replays the item's attributes onto the injected second statement instead of bailing to a help-only diagnostic, matching the spec's "preserving attributes and visibility". - `combined` now folds adjacent imports whose attributes *match* (not only attribute-free pairs), per the spec's "matching attrs" precondition. - Drop the rustdoc `### Configuration` heading that collided with the generated `## Configuration` section in the rule catalogue. Adds fixture coverage for raw-identifier paths and attribute handling.

Round-2 self-review follow-ups: - `combined` measures the inter-statement gap up to `second`'s attributes (`span_with_attributes`), so a matching attribute on the second import is no longer mistaken for gap content and no longer needlessly downgrades an otherwise-clean fold to `MaybeIncorrect`. - Doc: the bare `use foo::bar::self;` form is a hard error in current Rust, so describe the brace-list forms the rule actually encounters instead of presenting an example that never compiles. - Add fixture coverage for visibility-preserving splits/folds (`pub use`), a `{self, *}` glob group, and a mismatched-visibility pair that must not fold.

Round-3 self-review follow-ups: - Preserve a path's leading `::` (PathRoot) in every rewrite. `::foo` names an external crate while `foo` is resolved crate-relative / via the extern prelude, and the two differ when a local item shadows a crate name — so rendering `::foo::bar::{self}` as `use foo::bar;` could change which item is imported. Rendering now restores the `::`, and `combined` refuses to fold across a leading-`::` mismatch. - Revert the combined gap probe to span up to `second.span` (its `use` keyword) rather than `second.span_with_attributes()`. The narrower probe missed a comment sitting between `second`'s attribute and its `use` keyword, which the deletion still removed — under a `MachineApplicable` fix. The wider probe catches any such comment; attributed folds are now conservatively `MaybeIncorrect`. Adds fixtures for a rooted (`::std`) forbid rewrite, a rooted combined fold, and a rooted/relative pair that must not fold.

The note claimed the `a::b::self` form is "rejected in some positions", but it fires on the brace-nested `use foo::{bar::self};` form, which compiles fine — only the brace-free `use foo::bar::self;` is a hard error, and that never reaches this arm. Reword to describe the actual (redundant-but-valid) form the note accompanies.

- Vary the `forbid` suggestion label per rewrite shape: "by its bare path" for collapses, "split into its own `use` statement" for the root split, "alongside the group's other items" for the in-place nested expansion — so the help text no longer reads "import the module by its bare path: `inner, inner::Baz`". - Reword `module_import`'s doc: it's folding a bare `use module;` source that narrows to the module namespace (hence `MaybeIncorrect`), not the word "bare" itself. - Fix the rustdoc `forbid` example, which showed `use foo::bar;` twice (reading as a duplicate import); use distinct module paths and note each statement is fixed independently. (The fourth comment — a duplicate `## Configuration` heading — was already resolved earlier in the branch.)

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Brings in the rule additions/refactors merged to master since this branch diverged (#150 unicode-ellipsis config split, #151 bare-email doc trim, #154 print-macro-split). Conflicts, all "two rules registered in parallel", resolved by keeping both entries in sorted order: - src/lib.rs — `print_macro_split` + `self_import` in the register list - src/rules.rs — both `pub mod` declarations - rules/README.md — regenerated via `gen-docs write-md` (25 rules, in sync) No shared convention changed on master, so `self_import` needs no paradigm update. Full gate (fmt, clippy, doc + check-md, tests, self-lint) passes on the merged tree.

Brings in #153 (`import_granularity`), the sibling rule `self_import` references. Conflicts (both modify/delete) resolved by deleting the planning files, since both rules are now fully implemented: - planned-rules/import-granularity.md — deleted by master (#153 implemented it); our branch had only reworded a cross-reference. - planned-rules/self-import.md — deleted by our branch (self_import implemented); master had only reworded its cross-references. The rule-registration files (src/lib.rs, src/rules.rs) and the generated indexes auto-merged; `import_granularity` and `self_import` land in different sorted positions. `import_granularity` is active by default and applies `module` granularity to this crate's own source — self-lint stays green, so `self_import`'s imports already comply. It also fires on the `self_import` UI fixtures (which deliberately use crate-style nested `use`s to test nested-`self` handling), so those tests now disable `import_granularity` to keep their snapshots focused on `self_import`. Full gate (fmt, clippy, doc + check-md, tests, self-lint) passes.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

`combined`'s applicability probe only inspected the gap up to `second`'s `use` keyword, so a comment *inside* the second statement (e.g. `use foo::/* c */ Baz;`) was invisible to it — yet the deletion removed the whole statement, and with the `{self}` module form the fix was `MachineApplicable`, silently discarding the comment under `cargo fix`. Also scan the entire deleted region for a comment and downgrade to `MaybeIncorrect` when one is present, so no auto-applied fold ever loses a comment. The common attribute/comment-free fold stays `MachineApplicable`. (Applicability isn't rendered in the UI snapshots, so they are unchanged.)

The previous fix scanned only the deleted region for comments, but the fold's other edit replaces `first`'s tree wholesale — so a comment inside it (e.g. `use crate::defs::inner::{/* c */ self};`) was still dropped under a `MachineApplicable` fix. Extend the comment check to `first_tree`'s span as well, so a comment in either edited region downgrades the fold to `MaybeIncorrect`.

Brings in #156 (remove `thiserror_paths` config from `prefer_derive_more_over_thiserror`). It touches only that rule's files — no overlap with `self_import` or shared registration/convention files — so the merge is conflict-free and no `self_import` documentation or paradigm is affected. Full gate (fmt, clippy, doc + check-md, tests, self-lint) passes on the merged tree.

Drop the justification for having no `preserve`/no-op variant from the `Style` doc comment; the enum doc just states what it is. The rationale already lives in `IMPLEMENTATION_CONVENTIONS.md`'s "Mandatory configuration on opt-in rules".

KSXGitHub

I have some questions, Claude Code.

Per review feedback (#152): there *is* a syntactic signal for a mandatory config field — it just requires typing the field as the bare type instead of `Option<T>`. - `self_import`'s `style` is now `style: Style` (no `Option`, no `serde(default)`), so an enabled rule with no `style` fails to deserialize rather than silently defaulting to a direction. Read it with `dylint_linting::config` (not `config_or_default`, which needs `Config: Default` and would thus force a default direction): `Ok(None)` for an absent table and `Err` for a present-but-missing/invalid one are both turned into the "set `style`" diagnostic. The config is read only for an enabled rule, so a disabled rule never errors on the absent field. - gen-docs now detects a mandatory field syntactically — a field that is neither `Option<…>` nor covered by a serde `default` (container or field) — instead of the `**Mandatory**` doc sentinel, which is removed along with its strip logic. The field doc reads naturally; the badge conveys mandatory-ness. Empirically verified via the rule's unit tests (an empty config table is an error; `style = "forbid"/"combined"` deserialize) and the gen-docs extraction tests (only a no-container-default, non-`Option` field is flagged required). Full gate green.

A config field that is neither `Option<…>` nor covered by a serde `default` (container or field) has no default and so must be set. Detect that syntactically and render a `mandatory` badge — in both the HTML catalogue and the per-rule markdown — in place of the default `optional` one, and drop the blanket "every field is optional" intro for a config that has such a field.

`style` has no neutral default, so type it as a bare `Style` (no `Option`, no `serde(default)`) and read it with `dylint_linting::config` rather than `config_or_default`: an enabled rule with no `style` now fails to deserialize (`Ok(None)` for an absent table, `Err` for a present-but-missing one) instead of silently defaulting to a direction. The bare type is also the syntactic signal gen-docs reads to badge the field `mandatory`, so the field doc no longer needs a marker.

Render a `mandatory` badge for a config field that has no default and so must be set, instead of the unconditional `optional` badge. A field is mandatory when it is neither `Option<…>` nor covered by a serde `default` (container-level or per-field) — a purely syntactic signal, via new `serde_has_default` / `is_option` helpers. This adopts the approach settled in #152 (commit 82c5fe2), which supersedes the earlier `**Mandatory…**` doc-sentinel detection: a mandatory field is expressed as a bare type rather than an `Option`, so the type itself is the signal and the field's doc prose stays clean. The mechanism is inert on `master` until a rule declares such a field; the first is `self_import`'s `style`, which lands with #152. Refs #157.

KSXGitHub deployed to cloudflare-preview May 26, 2026 00:02 — with GitHub Actions View deployment

KSXGitHub requested a review from Copilot May 26, 2026 00:07

Copilot started reviewing on behalf of KSXGitHub May 26, 2026 00:07 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

Comment thread src/rules/self_import/combined.rs Outdated

Comment thread src/rules/self_import.rs Outdated

Comment thread src/rules/self_import.rs Outdated

Comment thread src/rules/self_import/forbid.rs Outdated

KSXGitHub deployed to cloudflare-preview May 26, 2026 00:26 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 00:34 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 00:45 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 01:08 — with GitHub Actions View deployment

KSXGitHub requested a review from Copilot May 26, 2026 01:48

Copilot started reviewing on behalf of KSXGitHub May 26, 2026 01:49 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

Comment thread src/rules/self_import/forbid.rs

Comment thread src/rules/self_import/render.rs

Comment thread src/rules/self_import/render.rs

Comment thread src/rules/self_import/combined.rs

Comment thread src/rules/self_import/combined.rs

KSXGitHub deployed to cloudflare-preview May 26, 2026 02:03 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 02:29 — with GitHub Actions View deployment

KSXGitHub requested a review from Copilot May 26, 2026 02:33

Copilot started reviewing on behalf of KSXGitHub May 26, 2026 02:33 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

Comment thread src/rules/self_import/combined.rs Outdated

KSXGitHub deployed to cloudflare-preview May 26, 2026 02:59 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 03:06 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 03:14 — with GitHub Actions View deployment

KSXGitHub marked this pull request as ready for review May 26, 2026 03:40

KSXGitHub marked this pull request as draft May 26, 2026 03:42

docs(self_import): trim the Style enum doc

939c3fa

Drop the justification for having no `preserve`/no-op variant from the `Style` doc comment; the enum doc just states what it is. The rationale already lives in `IMPLEMENTATION_CONVENTIONS.md`'s "Mandatory configuration on opt-in rules".

KSXGitHub deployed to cloudflare-preview May 26, 2026 03:54 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 04:07 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 05:04 — with GitHub Actions View deployment

KSXGitHub deployed to cloudflare-preview May 26, 2026 05:07 — with GitHub Actions View deployment

This was referenced May 26, 2026

gen-docs: mandatory config fields render an optional badge #157

Closed

fix(pages): actually render mandatory badge #158

Merged

KSXGitHub commented May 26, 2026

View reviewed changes

Comment thread tools/gen-docs/src/extract/config.rs Outdated

Comment thread src/rules/self_import.rs Outdated

KSXGitHub deployed to cloudflare-preview May 26, 2026 06:16 — with GitHub Actions View deployment

claude added 3 commits May 26, 2026 09:05

chore(git): merge master

9398a04

KSXGitHub mentioned this pull request May 26, 2026

draft: verify #158 integrates with #152 (self_import + gen-docs mandatory badge) #159

Draft

KSXGitHub force-pushed the claude/beautiful-pascal-PA5G4 branch from 82c5fe2 to bfe80ea Compare May 26, 2026 09:13

KSXGitHub deployed to cloudflare-preview May 26, 2026 09:13 — with GitHub Actions View deployment

KSXGitHub changed the title ~~feat: self_import~~ feat: self_import (old) May 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `self_import` (old)#152

feat: `self_import` (old)#152
KSXGitHub wants to merge 15 commits into
masterfrom
claude/beautiful-pascal-PA5G4

KSXGitHub commented May 26, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

KSXGitHub left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

KSXGitHub commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

KSXGitHub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KSXGitHub commented May 26, 2026 •

edited

Loading