-
Notifications
You must be signed in to change notification settings - Fork 34
feat(dylint): add core domain and security lints (DE0301, DE0308, DE0706) #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(dylint): add core domain and security lints (DE0301, DE0308, DE0706) #253
Conversation
📝 WalkthroughWalkthroughThree new lint crates added (DE0301, DE0308, DE0706) with implementations, docs, configs, UI tests, and workspace updates; lint utilities extended with path-exclusion and UseTree-to-string helpers. Changes
Sequence Diagram(s)(Skipped — changes are lint implementations and test artifacts; no multi-actor runtime control flow diagram required.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
✏️ Tip: You can disable this entire section by setting Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@dylint_lints/de07_security/de0706_no_direct_sqlx/README.md`:
- Around line 79-80: The "See Also" section contains broken references to lint
IDs "[DE0705]" and "[DE0407]"; either remove those two link lines or replace
them with correct existing lint IDs/links. Locate the "See Also" block in the
README (the lines starting with "- [DE0705]" and "- [DE0407]") and update each
entry to point to an actual lint in the codebase or delete the lines if no
appropriate replacements exist; ensure link text and relative path match the
target lint's real ID.
🧹 Nitpick comments (6)
dylint_lints/de07_security/de0706_no_direct_sqlx/README.md (1)
3-3: Fix heading hierarchy for markdown consistency.The heading jumps from h1 (
#) to h3 (###). Per markdown best practices, headings should increment by one level at a time.📝 Suggested fix
-### What it does +## What it doesApply the same change to other h3 headings (
### Why is this bad?,### Example,### Configuration,### See Also) to use h2 (##) instead.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (1)
83-106: Consider extractinguse_tree_to_stringtolint_utils.This function is duplicated in
de0308_no_http_in_domain/src/lib.rs(per relevant snippets). Extracting it tolint_utilswould reduce code duplication and ensure consistent behavior across lints.♻️ Potential refactor
Move to
lint_utils/src/lib.rs:/// Converts a UseTree to a string path representation. /// Handles Simple, Glob, and Nested use tree kinds. pub fn use_tree_to_string(tree: &rustc_ast::UseTree) -> String { // ... implementation }Then import in both lint crates:
use lint_utils::use_tree_to_string;dylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rs (1)
53-76: Consider extractinguse_tree_to_stringtolint_utils.This function is duplicated verbatim in
de0308_no_http_in_domain/src/lib.rs(lines 66-89). Extracting it tolint_utilswould reduce duplication and ensure consistent behavior across lints that need to stringify use trees.♻️ Suggested refactor
In
lint_utils/src/lib.rs, add:use rustc_ast::{UseTree, UseTreeKind}; pub fn use_tree_to_string(tree: &UseTree) -> String { match &tree.kind { UseTreeKind::Simple(..) | UseTreeKind::Glob => { tree.prefix.segments.iter() .map(|seg| seg.ident.name.as_str()) .collect::<Vec<_>>() .join("::") } UseTreeKind::Nested { items, .. } => { let prefix = tree.prefix.segments.iter() .map(|seg| seg.ident.name.as_str()) .collect::<Vec<_>>() .join("::"); for (nested_tree, _) in items { let nested_str = use_tree_to_string(nested_tree); if !nested_str.is_empty() { return format!("{}::{}", prefix, nested_str); } } prefix } } }Then both DE0308 and DE0706 can import from
lint_utils.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (2)
75-88: Nested use trees only check the first item.For imports like
use http::{StatusCode, Method};, onlyStatusCodeis checked because the function returns early on the first non-empty nested string. If a user writesuse http::{SomeAllowedType, StatusCode};, the violation onStatusCodewould be missed.Consider iterating through all nested items:
♻️ Proposed fix to check all nested items
UseTreeKind::Nested { items, .. } => { let prefix = tree.prefix.segments.iter() .map(|seg| seg.ident.name.as_str()) .collect::<Vec<_>>() .join("::"); - for (nested_tree, _) in items { - let nested_str = use_tree_to_string(nested_tree); - if !nested_str.is_empty() { - return format!("{}::{}", prefix, nested_str); - } + if items.is_empty() { + return prefix; } - prefix + // Return all nested paths for checking + items.iter() + .map(|(nested_tree, _)| { + let nested_str = use_tree_to_string(nested_tree); + if nested_str.is_empty() { + prefix.clone() + } else { + format!("{}::{}", prefix, nested_str) + } + }) + .collect::<Vec<_>>() + .join("|") }Alternatively, consider a different approach that collects all paths into a
Vec<String>to check each one individually.
52-65: Minor: Redundant patterns in HTTP_PATTERNS.
"http::"(line 54) already matches any path starting withhttp::, so specific entries like"http::StatusCode","http::Method", etc. (lines 55-59) are redundant sincecontains("http::")will catch them.This doesn't affect correctness, just a minor observation for cleanup.
dylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rs (1)
1-11: LGTM — test file correctly demonstrates the allowed pattern.The test file properly shows that
sea_ormimports should not trigger DE0706. This aligns with the lint's purpose of enforcing Sea-ORM/SecORM over direct sqlx usage.Based on learnings, consider using the standardized annotation format for consistency with
test_comment_annotations_match_stderrvalidation:-// Using sea_orm is allowed +// Should not trigger DE0706 - sea_orm usage is allowed use sea_orm::EntityTrait;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dylint_lints/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
dylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/README.mddylint_lints/de03_domain_layer/de0301_no_infra_in_domain/rust-toolchaindylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/README.mddylint_lints/de03_domain_layer/de0308_no_http_in_domain/rust-toolchaindylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/README.mddylint_lints/de07_security/de0706_no_direct_sqlx/rust-toolchaindylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.stderrdylint_lints/lint_utils/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (2)
dylint_lints/**/Cargo.toml
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/Cargo.toml: InCargo.tomlfor lint crates, reference common workspace dependencies (clippy_utils,dylint_linting,lint_utils) using.workspace = trueto avoid duplication
Define example targets in lint crateCargo.tomlwith[[example]]sections for each UI test case
Register new lint crates by adding them to thememberslist in the workspace rootCargo.toml
Files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.tomldylint_lints/Cargo.toml
dylint_lints/**/src/lib.rs
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/src/lib.rs: Usedeclare_pre_expansion_lint!for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
UseEarlyLintPasswithrustc_astfor pre-expansion lints that need to see#[derive(...)]attributes directly before macro expansion
Usedeclare_early_lint!for checking syntax and structure after macro expansion but before type resolution without type information
Usedeclare_late_lint!withLateLintPassandrustc_hirwhen type information or semantic analysis is required
Useui_test_exampleswhen tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Useui_testwhen all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Every lint crate MUST include atest_comment_annotations_match_stderrtest usinglint_utils::test_comment_annotations_match_stderrto validate comment/stderr alignment
Useis_in_contract_module_ast()helper fromlint_utilscrate to check if an AST item is in a contract/ directory
Document lint behavior in doc comments within the lint implementation
Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rs
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_early_lint!` for checking syntax and structure after macro expansion but before type resolution without type information
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Include at least one test case with no violations (empty or near-empty `.stderr` file) in UI tests
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Organize Rust lint crates with structure: `src/lib.rs` for implementation, `ui/` directory for UI test files with corresponding `.stderr` files
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test` when all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/main.rs : If generated `main.rs` and `main.stderr` are empty, remove them from the UI test directory
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : For struct/enum/function multiline spans in UI tests, place error comments on the first line of the item declaration (e.g., `pub struct Name`), not on the `#[derive(...)]` attribute line
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.stderr : Generate `.stderr` files by running tests (`cargo test --lib ui_examples`), copying normalized stderr output, and using `$DIR/` placeholder for file paths with exact line number matching
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Place test comment annotations (`// Should trigger DEXXX - description` or `// Should not trigger DEXXX - description`) on the line immediately before where the error is reported, not on attribute lines
Applied to files:
dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Applied to files:
dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de07_security/de0706_no_direct_sqlx/README.mddylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Applied to files:
dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de07_security/de0706_no_direct_sqlx/README.mddylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/README.mddylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/README.mddylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Applied to files:
dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Define example targets in lint crate `Cargo.toml` with `[[example]]` sections for each UI test case
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : In `Cargo.toml` for lint crates, reference common workspace dependencies (`clippy_utils`, `dylint_linting`, `lint_utils`) using `.workspace = true` to avoid duplication
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/rust-toolchaindylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/rust-toolchaindylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/rust-toolchaindylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_late_lint!` with `LateLintPass` and `rustc_hir` when type information or semantic analysis is required
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `EarlyLintPass` with `rustc_ast` for pre-expansion lints that need to see `#[derive(...)]` attributes directly before macro expansion
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_early_lint!` for checking syntax and structure after macro expansion but before type resolution without type information
Applied to files:
dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `is_in_contract_module_ast()` helper from `lint_utils` crate to check if an AST item is in a contract/ directory
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Verify tests pass with explicit `-p` flag to ensure correct package tests are running: `cargo test --lib -p <lint_name>`
Applied to files:
dylint_lints/Cargo.toml
🧬 Code graph analysis (8)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rs (1)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rs (1)
main(12-12)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rs (6)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs (1)
main(16-16)dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rs (1)
main(21-21)dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rs (1)
main(15-15)dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rs (1)
main(19-19)dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rs (1)
main(12-14)dylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rs (1)
main(9-11)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rs (6)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs (1)
main(16-16)dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rs (1)
main(15-15)dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rs (1)
main(12-12)dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rs (1)
main(19-19)dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rs (1)
main(12-14)dylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rs (1)
main(9-11)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (2)
dylint_lints/lint_utils/src/lib.rs (2)
is_in_domain_path(12-14)test_comment_annotations_match_stderr(201-304)dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (4)
use_tree_to_string(67-90)tree(70-72)tree(76-78)test_comment_annotations_match_stderr(129-132)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (2)
dylint_lints/lint_utils/src/lib.rs (1)
is_in_domain_path(12-14)dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (3)
use_tree_to_string(83-106)tree(86-88)tree(92-94)
dylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rs (1)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (5)
use_tree_to_string(67-90)tree(70-72)tree(76-78)check_item(112-118)test_comment_annotations_match_stderr(129-132)
dylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rs (1)
dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rs (1)
main(12-14)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs (3)
libs/modkit-db/src/advisory_locks.rs (1)
sqlx(151-151)dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rs (1)
main(12-14)dylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rs (1)
main(9-11)
🪛 markdownlint-cli2 (0.18.1)
dylint_lints/de07_security/de0706_no_direct_sqlx/README.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Run Dylint Lints
- GitHub Check: Test Suite (ubuntu-latest, rust=stable)
- GitHub Check: users_info integration (Postgres)
- GitHub Check: Test Suite (macos-latest, rust=stable)
- GitHub Check: Dylint Lints Tests
- GitHub Check: Test Suite (windows-latest, rust=stable)
- GitHub Check: UI macro tests (trybuild)
- GitHub Check: Code Coverage (cargo-llvm-cov)
🔇 Additional comments (33)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/rust-toolchain (1)
1-3: LGTM!The rust-toolchain configuration is correctly structured. The
nightly-2025-09-18channel and required components (rustc-devfor internal compiler APIs,llvm-tools-previewfor tooling) are appropriate for dylint lint development and consistent with other lint crates in the workspace.dylint_lints/de07_security/de0706_no_direct_sqlx/rust-toolchain (1)
1-1: Inconsistent toolchain format and missing components.This file uses plain text format specifying only the channel, while sibling lint crates (de0301, de0308) use TOML format with
[toolchain]section and include required components (rustc-dev,llvm-tools-preview). Dylint lints typically needrustc-devto compile.Additionally, this pins an older nightly (
2025-05-01) compared to2025-09-18used elsewhere in this PR.Is the older toolchain intentional for de0706? If not, consider aligning the format and version:
Suggested change for consistency
-nightly-2025-05-01 +[toolchain] +channel = "nightly-2025-09-18" +components = ["llvm-tools-preview", "rustc-dev"]dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/rust-toolchain (1)
1-3: LGTM!Proper TOML format with the required
rustc-devcomponent for dylint lint development.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.toml (1)
2-6: LGTM!Standard dylint linker configuration with helpful documentation for the newer
linkerdirective option.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.toml (1)
2-6: LGTM!Consistent with the linker configuration in sibling lint crates.
dylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.toml (1)
2-6: LGTM!Consistent with the linker configuration in sibling lint crates.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.stderr (1)
1-18: LGTM!The stderr artifact correctly captures the expected lint diagnostics. The error messages align with the test file violations (lines 7 and 10), use proper
$DIR/placeholders for paths, and include helpful guidance directing HTTP handling to the API layer.dylint_lints/Cargo.toml (2)
11-14: LGTM!New lint crates are properly registered as workspace members, following the coding guidelines requirement to add them to the
memberslist in the workspace rootCargo.toml.
47-54: LGTM!Workspace-level dev dependencies are properly defined for lint tests. Individual lint crates can now reference these using
.workspace = trueas per coding guidelines, avoiding version duplication across crates.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rs (1)
1-19: LGTM!Good UI test file that correctly demonstrates valid domain code without HTTP imports. This satisfies the requirement to include at least one test case with no violations. Based on learnings.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rs (1)
1-12: LGTM!Well-structured UI test file that correctly demonstrates HTTP imports in domain code. The test cases align with the expected stderr output (lines 7 and 10), and the file follows the established lint crate organization pattern. Based on learnings.
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderr (1)
1-11: LGTM - stderr format is correct for UI test expectations.The stderr correctly uses
$DIR/placeholder andLLmarkers. The single error forsea_ormaligns with the "mixed imports" test scenario where presumably only one infra import exists alongside valid domain imports.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.toml (1)
12-36: LGTM - Cargo.toml follows workspace conventions.Example targets are correctly defined for each UI test case, and workspace dependencies are properly referenced using
.workspace = true. Dev-dependencies include the necessary crates (sea-orm, sqlx, axum) for testing infrastructure import detection. As per coding guidelines.dylint_lints/lint_utils/src/lib.rs (1)
32-47: LGTM - Path exclusion helpers are well-documented and robust.The new
is_in_modkit_db_pathandis_in_hyperspot_server_pathfunctions properly check multiple path variants for cross-platform compatibility. The doc comments clearly explain the exclusion rationale for DE0706 sqlx restrictions.Minor note: The
modkit-db/src/andhyperspot-server/src/checks may be redundant since they're subpaths of the full variants, but they provide defense-in-depth for edge cases where paths might be truncated.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (4)
11-52: LGTM - Well-documented lint declaration.The
declare_early_lint!macro is correctly used for this syntax-level check that doesn't require type information. The doc comments thoroughly explain the purpose, rationale, and provide clear good/bad examples. This aligns with the coding guidelines for documenting lint behavior.
72-72: Verifyhttp::pattern matches intended imports.The pattern
"http::"includes trailing colons, which means it won't match a bareuse http;import (though such imports are rare). If the intent is to catch allhttpcrate usage, consider using"http"without the colons. The current pattern will matchuse http::StatusCode;but notuse http;.
114-124: First-match-only behavior may hide additional violations.The
breakon line 122 exits after reporting the first matching pattern. This means if a singleusestatement matches multiple patterns (e.g., bothsea_ormandcrate::infra), only the first is reported. This is likely acceptable for user experience (one error per import statement), but worth noting.
137-148: LGTM - Required tests are present.Both
ui_examplesandtest_comment_annotations_match_stderrtests are implemented as required by the coding guidelines. The comment pattern "infra in domain" matches the expected format.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.stderr (1)
1-27: LGTM - Expected test output is correctly formatted.The stderr file properly captures three DE0301 violations for
sea_orm,sqlx, andaxumimports. The format follows UI test conventions with$DIR/path placeholders andLLline markers. The help messages consistently guide toward moving infrastructure code to theinfra/layer.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs (1)
1-16: LGTM!The test file is well-structured:
- Simulated domain path comment for lint path detection
- Proper
#![allow(...)]attributes to suppress unrelated warnings- Comment annotations correctly placed on the line immediately before each
usestatement- Representative infrastructure imports (
sea_orm,sqlx,axum) that should all trigger DE0301dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rs (1)
1-15: LGTM!This mixed-scenario test file effectively validates that the lint distinguishes between:
- Allowed imports (
std::collections::HashMap,thiserror::Error)- Prohibited infrastructure imports (
sea_orm::DatabaseConnection)Comment annotations are correctly placed and follow the expected pattern. This complements
good_domain_0301.rs(no violations) andbad_infra_import.rs(all violations) to provide comprehensive test coverage.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rs (1)
1-21: LGTM!This test file serves as the required "no violations" case per the coding guidelines. It demonstrates valid domain-layer code patterns:
- Standard library imports (
std::sync::Arc)- Domain-appropriate external crates (
uuid,anyhow)- A representative domain trait (
UsersRepository)The corresponding
.stderrfile should be empty or near-empty, validating that these patterns don't trigger DE0301.dylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rs (3)
11-48: Well-documented lint declaration.The doc comments comprehensively cover:
- What the lint does
- Why direct sqlx usage is problematic (security, type safety, audit logging)
- Known exclusions (
libs/modkit-db/)- Bad and good usage examples
This follows the coding guideline to document lint behavior in doc comments. Based on learnings,
declare_early_lint!is the correct choice since this lint checks syntax/structure without requiring type information.
103-135: LGTM on lint pass implementation.The
EarlyLintPassimplementation correctly:
- Excludes
libs/modkit-db/andapps/hyperspot-server/usinglint_utilshelpers- Checks both
usestatements andextern cratedeclarations for sqlxOne minor note: The
ItemKind::ExternCrate(name, _ident)pattern is tied to the rustc AST version. Since tests pass, this is correct for the current toolchain.
137-152: Tests follow required patterns.The test module includes both required tests per coding guidelines:
ui_examples()for UI test executiontest_comment_annotations_match_stderr()to validate comment/stderr alignmentdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderr (1)
1-21: LGTM!The stderr artifact correctly captures:
- Two DE0706 violations at lines 7 and 10 of the test file
- Proper use of
$DIR/placeholder andLL |format for portability- Help and note messages matching the lint implementation
- Default deny annotation note on the first error only
This follows the coding guideline for generating
.stderrfiles with normalized output and exact line number matching.dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rs (1)
1-14: LGTM!The UI test file correctly demonstrates BAD patterns for DE0706. Comment annotations are properly placed on the lines immediately before the imports that should trigger the lint, following the coding guidelines.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml (2)
20-29: LGTM!Workspace dependencies (
clippy_utils,dylint_linting,lint_utils,dylint_testing,axum,anyhow) correctly use.workspace = trueper coding guidelines. Example targets are properly defined for UI test cases.
6-6: No issues. Rust 2024 edition is stable (released February 20, 2025) and appropriate for use.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (1)
111-133: LGTM!The
EarlyLintPassimplementation correctly gates onItemKind::Useand domain path checks. Both required tests (ui_examplesandtest_comment_annotations_match_stderr) are present per coding guidelines. Usingui_test_examplesis appropriate here since the tests require external dependencies (http, axum).dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/README.md (1)
1-56: LGTM!The README provides comprehensive documentation with clear explanations, well-structured examples demonstrating good vs. bad patterns, and appropriate cross-references to related lint DE0308. The configuration section clearly states the default behavior and scope.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/README.md (1)
1-92: LGTM!Excellent documentation that not only explains what's forbidden but also demonstrates the recommended transformation path from domain errors to API-layer responses. The examples are practical and align with the lint's implementation in
src/lib.rs.dylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.toml (1)
1-31: LGTM — Cargo.toml follows workspace conventions correctly.The manifest properly:
- Uses
.workspace = truefor common dependencies (clippy_utils,dylint_linting,lint_utils)- Defines
[[example]]sections for both UI test cases- Configures
cdylibcrate type required for dylint lints- Includes necessary dev-dependencies for testing (
sqlx,sea-orm)As per coding guidelines, this follows the workspace dependency pattern correctly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
Outdated
Show resolved
Hide resolved
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs
Outdated
Show resolved
Hide resolved
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs
Outdated
Show resolved
Hide resolved
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs
Outdated
Show resolved
Hide resolved
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
Outdated
Show resolved
Hide resolved
- DE0301: remove unused DB/HTTP frameworks, use starts_with - DE0308: remove actix_web, use starts_with - DE0706: update rust-toolchain to TOML format with components - lint_utils: add explanatory comments for path checking logic Note: modkit_db provides abstractions (traits) that domain layer SHOULD use. Direct database access (sqlx, sea_orm) is forbidden, but modkit_db abstractions are allowed. Resolves all 9 review comments from @Bechma in PR hypernetix#253
- DE0301: remove unused DB/HTTP frameworks, use starts_with - DE0308: remove actix_web, use starts_with - DE0706: update rust-toolchain to TOML format with components - lint_utils: add explanatory comments for path checking logic Note: modkit_db provides abstractions (traits) that domain layer SHOULD use. Direct database access (sqlx, sea_orm) is forbidden, but modkit_db abstractions are allowed. Resolves all 9 review comments from @Bechma in PR hypernetix#253 Signed-off-by: KvizadSaderah <kvizad@gmail.com>
65d0abd to
322f1c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs`:
- Around line 67-109: use_tree_to_string currently returns only the first nested
path causing missed HTTP imports; change it to collect and return all possible
import path strings (e.g., Vec<String>) by recursively traversing
UseTreeKind::Nested and concatenating prefix with every nested result, then
update check_use_in_domain to iterate over all returned paths and check each
against HTTP_PATTERNS and emit DE0308_NO_HTTP_IN_DOMAIN when any match is found;
reference functions use_tree_to_string and check_use_in_domain and constants
HTTP_PATTERNS and DE0308_NO_HTTP_IN_DOMAIN to locate where to change the return
type and the scan logic.
🧹 Nitpick comments (1)
dylint_lints/de07_security/de0706_no_direct_sqlx/README.md (1)
3-3: Consider using h2 (##) instead of h3 (###) for consistency.The heading jumps from h1 to h3, skipping h2. While functional, using h2 would follow markdown conventions more closely.
📝 Suggested heading structure
-### What it does +## What it doesApply the same pattern to other h3 headings in the file (lines 7, 16, 68, 77) if you prefer a consistent h2 structure.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/README.mddylint_lints/de07_security/de0706_no_direct_sqlx/rust-toolchaindylint_lints/lint_utils/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs
- dylint_lints/de07_security/de0706_no_direct_sqlx/rust-toolchain
- dylint_lints/lint_utils/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
dylint_lints/**/src/lib.rs
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/src/lib.rs: Usedeclare_pre_expansion_lint!for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
UseEarlyLintPasswithrustc_astfor pre-expansion lints that need to see#[derive(...)]attributes directly before macro expansion
Usedeclare_early_lint!for checking syntax and structure after macro expansion but before type resolution without type information
Usedeclare_late_lint!withLateLintPassandrustc_hirwhen type information or semantic analysis is required
Useui_test_exampleswhen tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Useui_testwhen all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Every lint crate MUST include atest_comment_annotations_match_stderrtest usinglint_utils::test_comment_annotations_match_stderrto validate comment/stderr alignment
Useis_in_contract_module_ast()helper fromlint_utilscrate to check if an AST item is in a contract/ directory
Document lint behavior in doc comments within the lint implementation
Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : In `Cargo.toml` for lint crates, reference common workspace dependencies (`clippy_utils`, `dylint_linting`, `lint_utils`) using `.workspace = true` to avoid duplication
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Organize Rust lint crates with structure: `src/lib.rs` for implementation, `ui/` directory for UI test files with corresponding `.stderr` files
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Verify tests pass with explicit `-p` flag to ensure correct package tests are running: `cargo test --lib -p <lint_name>`
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.stderr : Generate `.stderr` files by running tests (`cargo test --lib ui_examples`), copying normalized stderr output, and using `$DIR/` placeholder for file paths with exact line number matching
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_late_lint!` with `LateLintPass` and `rustc_hir` when type information or semantic analysis is required
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/README.md
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_early_lint!` for checking syntax and structure after macro expansion but before type resolution without type information
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/README.md
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `is_in_contract_module_ast()` helper from `lint_utils` crate to check if an AST item is in a contract/ directory
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `EarlyLintPass` with `rustc_ast` for pre-expansion lints that need to see `#[derive(...)]` attributes directly before macro expansion
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Organize Rust lint crates with structure: `src/lib.rs` for implementation, `ui/` directory for UI test files with corresponding `.stderr` files
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : For struct/enum/function multiline spans in UI tests, place error comments on the first line of the item declaration (e.g., `pub struct Name`), not on the `#[derive(...)]` attribute line
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
🧬 Code graph analysis (1)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (1)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (4)
use_tree_to_string(76-99)tree(79-81)tree(85-87)check_use_in_domain(101-118)
🪛 markdownlint-cli2 (0.18.1)
dylint_lints/de07_security/de0706_no_direct_sqlx/README.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (4)
dylint_lints/de07_security/de0706_no_direct_sqlx/README.md (4)
7-15: LGTM! Clear and comprehensive rationale.The security and architectural concerns are well-articulated, covering enforcement layers, type safety, maintainability, audit logging, and multi-tenant isolation.
18-66: LGTM! Examples clearly demonstrate bad vs. good patterns.The progression from direct sqlx usage → sea-orm → SecureConn effectively illustrates the architectural layers and security benefits.
68-75: LGTM! Configuration clearly documented.The "deny by default" severity and detection patterns (imports, nested imports, extern crate) are well-specified.
79-80: The "See Also" lint references are valid. Both DE0301 (de0301_no_infra_in_domain) and DE0308 (de0308_no_http_in_domain) directories exist in the de03_domain_layer, and the relative paths from the README correctly resolve to these lints.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
Outdated
Show resolved
Hide resolved
Critical bug fix: use_tree_to_string was returning only the FIRST nested
import path, causing false negatives for patterns like:
use crate::{Foo, http::StatusCode\}\;
Changed to use_tree_to_strings() that returns Vec<String> with ALL nested
paths, ensuring complete detection of violations in both:
- DE0301 (no infrastructure in domain)
- DE0308 (no HTTP in domain)
Fixes CodeRabbit major issue in PR hypernetix#253
Signed-off-by: KvizadSaderah <kvizad@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs`:
- Around line 75-88: The code in UseTreeKind::Nested builds paths with a leading
"::" when prefix is empty, causing strings like "::http::StatusCode" to miss
starts_with("http::") checks; update the loop in UseTreeKind::Nested (the block
that computes prefix and pushes into paths in use_tree_to_strings) to explicitly
handle empty prefix: if nested_str.is_empty() push prefix.clone(); else if
prefix.is_empty() push nested_str.clone(); otherwise push format!("{}::{}",
prefix, nested_str) so no leading :: is produced and empty-prefix cases are
normalized.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
dylint_lints/**/src/lib.rs
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/src/lib.rs: Usedeclare_pre_expansion_lint!for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
UseEarlyLintPasswithrustc_astfor pre-expansion lints that need to see#[derive(...)]attributes directly before macro expansion
Usedeclare_early_lint!for checking syntax and structure after macro expansion but before type resolution without type information
Usedeclare_late_lint!withLateLintPassandrustc_hirwhen type information or semantic analysis is required
Useui_test_exampleswhen tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Useui_testwhen all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Every lint crate MUST include atest_comment_annotations_match_stderrtest usinglint_utils::test_comment_annotations_match_stderrto validate comment/stderr alignment
Useis_in_contract_module_ast()helper fromlint_utilscrate to check if an AST item is in a contract/ directory
Document lint behavior in doc comments within the lint implementation
Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Place test comment annotations (`// Should trigger DEXXX - description` or `// Should not trigger DEXXX - description`) on the line immediately before where the error is reported, not on attribute lines
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : In `Cargo.toml` for lint crates, reference common workspace dependencies (`clippy_utils`, `dylint_linting`, `lint_utils`) using `.workspace = true` to avoid duplication
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Include at least one test case with no violations (empty or near-empty `.stderr` file) in UI tests
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Organize Rust lint crates with structure: `src/lib.rs` for implementation, `ui/` directory for UI test files with corresponding `.stderr` files
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_late_lint!` with `LateLintPass` and `rustc_hir` when type information or semantic analysis is required
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_early_lint!` for checking syntax and structure after macro expansion but before type resolution without type information
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `EarlyLintPass` with `rustc_ast` for pre-expansion lints that need to see `#[derive(...)]` attributes directly before macro expansion
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `is_in_contract_module_ast()` helper from `lint_utils` crate to check if an AST item is in a contract/ directory
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : For struct/enum/function multiline spans in UI tests, place error comments on the first line of the item declaration (e.g., `pub struct Name`), not on the `#[derive(...)]` attribute line
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
🧬 Code graph analysis (1)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (2)
dylint_lints/lint_utils/src/lib.rs (1)
is_in_domain_path(12-14)dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (3)
use_tree_to_strings(76-103)tree(85-87)check_use_in_domain(105-123)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Run Dylint Lints
- GitHub Check: Test Suite (windows-latest, rust=stable)
- GitHub Check: Test Suite (ubuntu-latest, rust=stable)
- GitHub Check: Test Suite (macos-latest, rust=stable)
- GitHub Check: UI macro tests (trybuild)
- GitHub Check: Code Coverage (cargo-llvm-cov)
- GitHub Check: users_info integration (Postgres)
- GitHub Check: Dylint Lints Tests
🔇 Additional comments (2)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (2)
11-50: Clear, well-scoped lint documentation.Doc comments and examples are concise and aligned with the lint intent.
126-137: Tests are in good shape.UI examples plus comment-annotation validation cover the lint nicely.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
Outdated
Show resolved
Hide resolved
Critical bug fix: use_tree_to_string was returning only the FIRST nested
import path, causing false negatives for patterns like:
use crate::{Foo, http::StatusCode\}\;
Changed to use_tree_to_strings() that returns Vec<String> with ALL nested
paths, ensuring complete detection of violations in both:
- DE0301 (no infrastructure in domain)
- DE0308 (no HTTP in domain)
Fixes CodeRabbit major issue in PR hypernetix#253
Signed-off-by: KvizadSaderah <kvizad@gmail.com>
Reviewer feedback: lints only checked 'use' statements but missed direct
type usage in struct fields, function parameters, and return types.
Extended both DE0301 and DE0308 to check:
- Struct field types
- Enum variant field types
- Function parameter types
- Function return types
- Type aliases
Now catches violations like:
struct Hello { field: http::StatusCode }
Added UI test bad_http_type_in_struct.rs
Addresses reviewer comment on PR hypernetix#253
Signed-off-by: KvizadSaderah <kvizad@gmail.com>
7a6c64a to
542cfcc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs`:
- Around line 118-145: type_to_string only inspects TyKind::Path and misses
nested/composite types (e.g., Option<http::StatusCode>, &http::Request>); update
type_to_string (or add a recursive helper used by check_type_in_domain) to
traverse TyKind::Ref, TyKind::Slice, TyKind::Array, TyKind::Ptr and inspect
generic arguments on TyKind::Path segments so any inner path segments are
converted to strings and checked against HTTP_PATTERNS, then keep
check_type_in_domain using the recursive result to emit
DE0308_NO_HTTP_IN_DOMAIN; consider extracting this traversal into a shared
lint_utils function for reuse by de0301.
♻️ Duplicate comments (1)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (1)
54-74: Missingmodkit_*infrastructure patterns.Per reviewer feedback, the following patterns should be added to cover internal infrastructure modules:
modkit_dbmodkit_db_macrosmodkit_transport_grpcSuggested additions
const INFRA_PATTERNS: &[&str] = &[ // Infrastructure layer "crate::infra", "crate::infrastructure", + // Internal infrastructure modules + "modkit_db", + "modkit_db_macros", + "modkit_transport_grpc", // Database frameworks (direct access forbidden, use modkit_db abstractions instead) "sea_orm", "sqlx",
🧹 Nitpick comments (1)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (1)
127-154: Limited type coverage for complex types.The
type_to_stringfunction only extracts paths from directTyKind::Pathtypes. Infra types wrapped in references (&sea_orm::Entity), smart pointers (Arc<sqlx::Pool>), or generics won't be detected.The import checking provides the primary enforcement, so this is acceptable as a secondary defense. Consider enhancing in a follow-up to recurse into generic arguments and reference types if needed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.stderr
🧰 Additional context used
📓 Path-based instructions (2)
dylint_lints/**/src/lib.rs
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/src/lib.rs: Usedeclare_pre_expansion_lint!for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
UseEarlyLintPasswithrustc_astfor pre-expansion lints that need to see#[derive(...)]attributes directly before macro expansion
Usedeclare_early_lint!for checking syntax and structure after macro expansion but before type resolution without type information
Usedeclare_late_lint!withLateLintPassandrustc_hirwhen type information or semantic analysis is required
Useui_test_exampleswhen tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Useui_testwhen all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Every lint crate MUST include atest_comment_annotations_match_stderrtest usinglint_utils::test_comment_annotations_match_stderrto validate comment/stderr alignment
Useis_in_contract_module_ast()helper fromlint_utilscrate to check if an AST item is in a contract/ directory
Document lint behavior in doc comments within the lint implementation
Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
dylint_lints/**/Cargo.toml
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/Cargo.toml: InCargo.tomlfor lint crates, reference common workspace dependencies (clippy_utils,dylint_linting,lint_utils) using.workspace = trueto avoid duplication
Define example targets in lint crateCargo.tomlwith[[example]]sections for each UI test case
Register new lint crates by adding them to thememberslist in the workspace rootCargo.toml
Files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Place test comment annotations (`// Should trigger DEXXX - description` or `// Should not trigger DEXXX - description`) on the line immediately before where the error is reported, not on attribute lines
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Organize Rust lint crates with structure: `src/lib.rs` for implementation, `ui/` directory for UI test files with corresponding `.stderr` files
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : In `Cargo.toml` for lint crates, reference common workspace dependencies (`clippy_utils`, `dylint_linting`, `lint_utils`) using `.workspace = true` to avoid duplication
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.stderr : Generate `.stderr` files by running tests (`cargo test --lib ui_examples`), copying normalized stderr output, and using `$DIR/` placeholder for file paths with exact line number matching
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Include at least one test case with no violations (empty or near-empty `.stderr` file) in UI tests
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : For struct/enum/function multiline spans in UI tests, place error comments on the first line of the item declaration (e.g., `pub struct Name`), not on the `#[derive(...)]` attribute line
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_early_lint!` for checking syntax and structure after macro expansion but before type resolution without type information
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_late_lint!` with `LateLintPass` and `rustc_hir` when type information or semantic analysis is required
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `EarlyLintPass` with `rustc_ast` for pre-expansion lints that need to see `#[derive(...)]` attributes directly before macro expansion
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `is_in_contract_module_ast()` helper from `lint_utils` crate to check if an AST item is in a contract/ directory
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : In `Cargo.toml` for lint crates, reference common workspace dependencies (`clippy_utils`, `dylint_linting`, `lint_utils`) using `.workspace = true` to avoid duplication
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Place test comment annotations (`// Should trigger DEXXX - description` or `// Should not trigger DEXXX - description`) on the line immediately before where the error is reported, not on attribute lines
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Define example targets in lint crate `Cargo.toml` with `[[example]]` sections for each UI test case
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Organize Rust lint crates with structure: `src/lib.rs` for implementation, `ui/` directory for UI test files with corresponding `.stderr` files
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
🧬 Code graph analysis (2)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (2)
dylint_lints/lint_utils/src/lib.rs (2)
is_in_domain_path(12-14)test_comment_annotations_match_stderr(208-311)dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (6)
use_tree_to_strings(67-96)tree(76-78)check_use_in_domain(98-116)path(121-123)ui_examples(197-199)test_comment_annotations_match_stderr(202-205)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (1)
dylint_lints/lint_utils/src/lib.rs (2)
is_in_domain_path(12-14)test_comment_annotations_match_stderr(208-311)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Run Dylint Lints
- GitHub Check: Test Suite (ubuntu-latest, rust=stable)
- GitHub Check: Test Suite (windows-latest, rust=stable)
- GitHub Check: Test Suite (macos-latest, rust=stable)
- GitHub Check: Code Coverage (cargo-llvm-cov)
- GitHub Check: UI macro tests (trybuild)
- GitHub Check: Dylint Lints Tests
- GitHub Check: users_info integration (Postgres)
🔇 Additional comments (11)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (4)
1-52: LGTM! Well-structured lint declaration with comprehensive documentation.The use of
declare_early_lint!withEarlyLintPassis appropriate for syntactic path checking without requiring type resolution. The doc comments provide clear guidance on what the lint detects and how to fix violations. Based on coding guidelines, this follows the recommended pattern for syntax-level lints.
76-105: LGTM!The recursive path building logic correctly handles all
UseTreeKindvariants and properly concatenates nested paths.
156-201: LGTM! Comprehensive item coverage.The implementation checks the key locations where infrastructure dependencies would appear: imports, struct/enum fields, function signatures, and type aliases. The early bail-out via
is_in_domain_pathensures the lint only runs on domain code.
203-215: LGTM! Tests follow required patterns.Both the
ui_examplestest and thetest_comment_annotations_match_stderrtest are present as required by coding guidelines. This ensures UI test files properly align with their.stderrcounterparts.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml (1)
1-36: Manifest scaffolding looks solid.
Examples and workspace dependencies are wired cleanly.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.stderr (1)
1-11: Expected diagnostic output matches the UI case.
Location and message align with the offending field.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rs (1)
1-11: UI example is well-annotated.
The trigger comment sits directly above the violating field, which should keep stderr mapping stable. Based on learnings, this placement matches the test-annotation conventions.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (4)
11-50: Lint docs and example are clear.
The motivation and examples make the rule easy to understand.
67-116: Use-tree handling and lint emission look good.
Recursion with early return should catch nested imports without extra noise.
147-191: Item filtering by domain path is straightforward.
The match dispatch keeps type checks centralized.
194-205: Test module added.
UI example coverage is wired in.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
Outdated
Show resolved
Hide resolved
CodeRabbit major issue: type_to_string only checked outer type path, missing nested types in generics like Option<http::StatusCode\>, Vec<http::Request\>, &http::Response. Refactored check_type_in_domain to recursively traverse: - Generic arguments (Option<T>, Vec<T>, Result<T,E>) - References (&T, &mut T) - Slices ([T]) - Arrays ([T; N]) - Raw pointers (*const T, *mut T) - Tuples ((T1, T2)) Applied to both DE0301 (infrastructure) and DE0308 (HTTP). Added UI test nested_generic_http_type.rs with 3 nested type cases. Fixes CodeRabbit major issue in PR hypernetix#253 Signed-off-by: KvizadSaderah <kvizad@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs`:
- Around line 127-184: check_type_in_domain currently skips TyKind::TraitObject
and TyKind::ImplTrait allowing infra traits in dyn/impl positions to escape;
update check_type_in_domain to handle TyKind::TraitObject and TyKind::ImplTrait
by traversing their trait bounds (extract each trait reference/path from the
trait object or impl-trait bounds and run the same infra-pattern checks you use
for TyKind::Path, and recurse into any generic arguments on those trait paths
just like the existing Path and AngleBracketed handling in
check_type_in_domain). Ensure you call the same lint emission
(DE0301_NO_INFRA_IN_DOMAIN) when a trait path matches INFRA_PATTERNS and add UI
tests for: an impl Trait return (e.g., fn f() -> impl sea_orm::EntityTrait), dyn
trait fields/params (e.g., Arc<dyn infra::Trait>, Box<dyn sqlx::Database>), and
wrapped cases (Arc<dyn infra::Trait>) to verify detection.
♻️ Duplicate comments (1)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (1)
54-66: Catch barehttpimports when matching infra patterns.
"http::"combined withstarts_withskipsuse http;and similar bare-crate paths. Consider boundary-aware matching to catch bothhttpandhttp::...without overmatching.✅ Proposed fix (boundary-aware matcher)
const INFRA_PATTERNS: &[&str] = &[ @@ - "http::", + "http", @@ ]; +fn path_matches(path: &str, pattern: &str) -> bool { + let pat = pattern.trim_end_matches("::"); + path == pat || path.starts_with(&format!("{pat}::")) +} @@ - if path_str.starts_with(pattern) { + if path_matches(&path_str, pattern) { @@ - if path_str.starts_with(pattern) { + if path_matches(&path_str, pattern) {You can confirm coverage with a UI-case search:
#!/bin/bash rg -n --type=rust '\buse\s+http\b' dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/uiAlso applies to: 112-118, 136-139
🧹 Nitpick comments (2)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (2)
52-65: Consider removing redundant patterns for clarity.The
"http::"pattern at index 0 already matches all subsequenthttp::StatusCode,http::Method, etc. patterns. While not incorrect (the loop breaks on first match), the redundancy could be confusing.♻️ Suggested simplification
const HTTP_PATTERNS: &[&str] = &[ "http::", - "http::StatusCode", - "http::Method", - "http::HeaderMap", - "http::Request", - "http::Response", "axum::http", "hyper::StatusCode", "hyper::Method", "reqwest::StatusCode", ];Alternatively, keep them as documentation but add a comment explaining they're for clarity.
67-96: Extractuse_tree_to_stringstolint_utilsto reduce duplication.This function is identical to the one in
de0301_no_infra_in_domain/src/lib.rs(confirmed in relevant code snippets). Consider extracting it tolint_utilsfor reuse across domain-layer lints.♻️ Proposed refactor
In
lint_utils/src/lib.rs:pub fn use_tree_to_strings(tree: &rustc_ast::UseTree) -> Vec<String> { // ... implementation }Then in both lint crates:
-fn use_tree_to_strings(tree: &UseTree) -> Vec<String> { - // ... 30 lines -} +use lint_utils::use_tree_to_strings;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.stderr
🧰 Additional context used
📓 Path-based instructions (2)
dylint_lints/**/src/lib.rs
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/src/lib.rs: Usedeclare_pre_expansion_lint!for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
UseEarlyLintPasswithrustc_astfor pre-expansion lints that need to see#[derive(...)]attributes directly before macro expansion
Usedeclare_early_lint!for checking syntax and structure after macro expansion but before type resolution without type information
Usedeclare_late_lint!withLateLintPassandrustc_hirwhen type information or semantic analysis is required
Useui_test_exampleswhen tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Useui_testwhen all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Every lint crate MUST include atest_comment_annotations_match_stderrtest usinglint_utils::test_comment_annotations_match_stderrto validate comment/stderr alignment
Useis_in_contract_module_ast()helper fromlint_utilscrate to check if an AST item is in a contract/ directory
Document lint behavior in doc comments within the lint implementation
Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
dylint_lints/**/Cargo.toml
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/Cargo.toml: InCargo.tomlfor lint crates, reference common workspace dependencies (clippy_utils,dylint_linting,lint_utils) using.workspace = trueto avoid duplication
Define example targets in lint crateCargo.tomlwith[[example]]sections for each UI test case
Register new lint crates by adding them to thememberslist in the workspace rootCargo.toml
Files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Place test comment annotations (`// Should trigger DEXXX - description` or `// Should not trigger DEXXX - description`) on the line immediately before where the error is reported, not on attribute lines
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : For struct/enum/function multiline spans in UI tests, place error comments on the first line of the item declaration (e.g., `pub struct Name`), not on the `#[derive(...)]` attribute line
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Organize Rust lint crates with structure: `src/lib.rs` for implementation, `ui/` directory for UI test files with corresponding `.stderr` files
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Include at least one test case with no violations (empty or near-empty `.stderr` file) in UI tests
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.stderr : Generate `.stderr` files by running tests (`cargo test --lib ui_examples`), copying normalized stderr output, and using `$DIR/` placeholder for file paths with exact line number matching
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : In `Cargo.toml` for lint crates, reference common workspace dependencies (`clippy_utils`, `dylint_linting`, `lint_utils`) using `.workspace = true` to avoid duplication
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_late_lint!` with `LateLintPass` and `rustc_hir` when type information or semantic analysis is required
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_early_lint!` for checking syntax and structure after macro expansion but before type resolution without type information
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `EarlyLintPass` with `rustc_ast` for pre-expansion lints that need to see `#[derive(...)]` attributes directly before macro expansion
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `is_in_contract_module_ast()` helper from `lint_utils` crate to check if an AST item is in a contract/ directory
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.stderr
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : In `Cargo.toml` for lint crates, reference common workspace dependencies (`clippy_utils`, `dylint_linting`, `lint_utils`) using `.workspace = true` to avoid duplication
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : For struct/enum/function multiline spans in UI tests, place error comments on the first line of the item declaration (e.g., `pub struct Name`), not on the `#[derive(...)]` attribute line
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.stderr
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Place test comment annotations (`// Should trigger DEXXX - description` or `// Should not trigger DEXXX - description`) on the line immediately before where the error is reported, not on attribute lines
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Define example targets in lint crate `Cargo.toml` with `[[example]]` sections for each UI test case
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test` when all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rs
🧬 Code graph analysis (2)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (2)
dylint_lints/lint_utils/src/lib.rs (1)
is_in_domain_path(12-14)dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (5)
use_tree_to_strings(67-96)tree(76-78)check_use_in_domain(98-116)check_type_in_domain(118-176)path(122-124)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (2)
dylint_lints/lint_utils/src/lib.rs (1)
is_in_domain_path(12-14)dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (3)
use_tree_to_strings(76-105)tree(85-87)path(131-133)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Dylint Lints Tests
- GitHub Check: Run Dylint Lints
- GitHub Check: Test Suite (ubuntu-latest, rust=stable)
- GitHub Check: Code Coverage (cargo-llvm-cov)
- GitHub Check: Test Suite (macos-latest, rust=stable)
- GitHub Check: modkit-db integration (backend=pg)
- GitHub Check: Test Suite (windows-latest, rust=stable)
- GitHub Check: UI macro tests (trybuild)
- GitHub Check: users_info integration (Postgres)
🔇 Additional comments (8)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.stderr (1)
1-27: LGTM!The stderr file correctly captures the three DE0308 violations for nested HTTP types. The error format follows dylint conventions with
$DIR/placeholders andLLline markers. The recursive type detection forOption<http::StatusCode>,Vec<http::HeaderMap>, and&'static http::Response<String>is properly validated.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml (1)
1-40: LGTM!The Cargo.toml follows the coding guidelines correctly:
- Workspace dependencies (
.workspace = true) forclippy_utils,dylint_linting, andlint_utils- Example targets defined for each UI test file
rustc_private = truefor rust-analyzer supportcdylibcrate type for dylint integrationdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rs (1)
1-15: LGTM!The UI test correctly covers the three nested HTTP type scenarios addressed in the commit (fixing the recursive type traversal issue). Comment annotations are properly placed on the line immediately before each violation, matching the stderr expectations. The
simulated_dircomment correctly places this in a domain path to trigger the lint.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (4)
11-50: Well-documented lint declaration.The doc comments clearly explain the lint's purpose, rationale, and provide good/bad examples. Using
declare_early_lint!is appropriate since this lint only needs AST-level analysis without type resolution. Based on learnings, this follows the guideline to document lint behavior in doc comments.
118-176: Recursive type traversal correctly implemented.The
check_type_in_domainfunction now properly handles nested HTTP types in:
- Generic arguments (
Option<http::StatusCode>)- References (
&http::Response)- Slices, arrays, raw pointers, and tuples
This addresses the major issue raised in past reviews about missing nested type detection.
225-237: LGTM!The test module includes both required tests per coding guidelines:
ui_examplesusingdylint_testing::ui_test_examplestest_comment_annotations_match_stderrusing thelint_utilshelper to validate comment/stderr alignment
178-223: Implementation correctly covers primary domain code patterns forEarlyLintPass.The
check_itemhook effectively handles all top-level ItemKind variants where HTTP types commonly appear: imports, struct fields, enum variants, function signatures, and type aliases. The recursive type checking properly handles generic arguments, references, slices, arrays, pointers, and tuples.Note: Methods within
implblocks areAssocItemvariants, notItemvariants, so checking them would require thecheck_assoc_itemhook. This is a consistent architectural pattern across similar lints in the codebase and is appropriate for domain layer validation where module-level declarations are the primary concern.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (1)
234-245: Tests wiring looks solid.UI examples + stderr annotation check provide the right safety net for diagnostics.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
CodeRabbit minor issue: TyKind::TraitObject and TyKind::ImplTrait were not inspected, allowing infrastructure/HTTP traits in trait object and impl Trait positions to escape detection. Examples that now trigger: - impl sqlx::Executor (impl Trait in return position) - Box<dyn http::Service\> (trait object) - Arc<dyn sea_orm::EntityTrait> (wrapped trait object) Added handling for: - TyKind::TraitObject: checks all trait bounds in dyn Trait - TyKind::ImplTrait: checks all trait bounds in impl Trait Applied to both DE0301 (infrastructure) and DE0308 (HTTP). Added UI test trait_bounds_http.rs with 2 impl Trait cases. Fixes CodeRabbit minor issue in PR hypernetix#253 Signed-off-by: KvizadSaderah <kvizad@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs`:
- Around line 148-232: The lint misses checking trait-bound
generic/associated-type arguments (e.g., impl Iterator<Item = sqlx::PgPool> or
dyn Service<Response = http::Response>) because TyKind::Path only inspects
AngleBracketedArg::Arg(Type) and trait bounds in TyKind::TraitObject /
TyKind::ImplTrait aren't recursing into their generic args; add a helper (e.g.,
walk_angle_bracketed_args) that accepts an &rustc_ast::AngleBracketedArgs and
calls check_type_in_domain for every GenericArg::Type and also recursively
visits associated-type bindings and constraint types (handle
AssocType/Constraint/binding variants), then call this helper from
check_type_in_domain when handling TyKind::Path segments' args and when
iterating trait_ref.trait_ref.path in TyKind::TraitObject and TyKind::ImplTrait
so INFRA_PATTERNS matching and DE0301_NO_INFRA_IN_DOMAIN diagnostics cover those
cases too.
In `@dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml`:
- Around line 1-44: The crate de0308_no_http_in_domain Cargo.toml currently
hardcodes edition and isn't registered in the workspace; update it to use
workspace-inherited edition by replacing the edition = "2024" entry with
edition.workspace = true in the [package] table, and register the crate in the
workspace root by adding "de0308_no_http_in_domain" to the workspace members
list so the workspace builds and inherits edition and other workspace settings.
In `@dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs`:
- Around line 139-223: The traversal misses trait-bound generics and
associated-type constraints (e.g., impl Iterator<Item = http::StatusCode>, dyn
Service<Response = http::Response>); update check_type_in_domain to recurse into
angle-bracketed generic args and associated-type constraints by extracting the
angle-arg handling into a helper used wherever path.segment.args are inspected
and also used when iterating GenericBound::Trait in TyKind::TraitObject and
TyKind::ImplTrait; correctly handle rustc_ast::AngleBracketedArg variants
including Constraint (not AssocType) and match its AssocItemConstraintKind to
extract types from Equality { term } and Bound { bounds } so you call
check_type_in_domain on those inner types and detect HTTP_PATTERNS there as
well.
In `@libs/modkit-bootstrap/src/config_provider.rs`:
- Around line 30-50: get_config_raw in AppConfigProvider (impl ConfigProvider
for AppConfigProvider) doesn't handle "api" (and optionally "tracing"), so
ModuleCtx::get_config("api") returns None; update the match in get_config_raw to
include "api" (and "tracing" if needed) and return
serde_json::to_value(&self.0.api).ok() (and similarly for tracing using
self.0.tracing.as_ref().and_then(|v| serde_json::to_value(v).ok()) if tracing is
optional) so global config lookups succeed.
In `@libs/modkit-bootstrap/src/config.rs`:
- Around line 1388-1463: Tests test_env_expansion_password and
test_env_expansion_in_dsn modify process-global environment variables causing
flakiness when run in parallel; fix by making them run serially: add serial_test
as a dev-dependency and annotate the test functions (test_env_expansion_password
and test_env_expansion_in_dsn) with #[serial] (or alternatively document running
tests with a single thread via cargo test -- --test-threads=1), ensuring the
crate import (use serial_test::serial) is added to the test module so the
annotations resolve.
In `@modules/api_ingress/src/config.rs`:
- Around line 11-50: ApiIngressConfig currently derives Default which sets
require_auth_by_default to false (Rust Default) but serde uses
default_require_auth_by_default (true), causing a mismatch; replace the
auto-derived Default with a manual impl for ApiIngressConfig that constructs the
same defaults as serde (or calls Default::default() on subfields) and explicitly
sets require_auth_by_default to default_require_auth_by_default() so the
programmatic Default aligns with the serde default; update/remove the
#[derive(Default)] and add an impl Default for ApiIngressConfig that initializes
bind_addr, enable_docs, cors_enabled, cors, defaults, auth_disabled, jwks_uri,
issuer, audience, and sets require_auth_by_default =
default_require_auth_by_default().
In `@modules/api_ingress/src/cors.rs`:
- Around line 5-8: The build_cors_layer function currently unconditionally
constructs a CorsLayer using cfg.cors.clone().unwrap_or_default(), which enables
CORS even when missing or disabled; change build_cors_layer (and related logic
that reads ApiIngressConfig) to first check cfg.cors_enabled (and/or
cfg.cors.is_some()) and return None when CORS is not enabled or absent,
otherwise unwrap the cfg.cors and build the CorsLayer; update the doc comment to
reflect that the function returns None when cors_enabled is false or the cors
section is missing.
In `@modules/api_ingress/src/rate_limit.rs`:
- Around line 54-78: The middleware builds keys from req.uri().path(), which
yields concrete paths and won't match templated keys stored in RateLimiterMap
(e.g., map.buckets and map.inflight); update rate_limit_middleware to first
attempt to read Axum's MatchedPath from req.extensions() (use MatchedPath as the
route template like "/users/{id}") and use that string for the key (falling back
to req.uri().path() if MatchedPath is absent), and add the necessary import for
MatchedPath so lookups against map.buckets and map.inflight succeed for
parameterized routes.
In `@modules/api_ingress/src/request_id.rs`:
- Around line 24-35: push_req_id_to_extensions currently only reads the incoming
header so when SetRequestIdLayer generates an ID (and stores it as a RequestId
extension) but no header exists, business logic never gets XRequestId; either
reorder middleware so SetRequestIdLayer is applied before
push_req_id_to_extensions in the middleware stack, or update
push_req_id_to_extensions to fallback to reading the generated RequestId
extension (e.g., req.extensions().get::<RequestId>()) and insert XRequestId(rid)
into req.extensions_mut() when the header is missing; modify the code within
push_req_id_to_extensions to perform header lookup first and then the
RequestId-extension fallback and record to the span as before.
In `@modules/api_ingress/src/router_cache.rs`:
- Around line 131-143: The compare_and_swap implementation uses Arc::ptr_eq
incorrectly by passing &*result (a &T) instead of &Arc<T>; update the equality
check in compare_and_swap to call Arc::ptr_eq(&result, &expected) so both
arguments are &Arc<T>, leaving the rest (creating new_arc, calling
self.inner.compare_and_swap, and returning Ok/Err with result.clone())
unchanged; ensure you reference the variables result and expected in the
compare_and_swap function and remove the dereference that produced &*result.
In `@modules/api_ingress/src/web.rs`:
- Around line 29-46: Update the HTML returned by the serve_docs function to stop
using the `@latest` CDN tags: pin both the script and stylesheet URLs to a
specific Stoplight Elements version (e.g., `@9.0.2`), add integrity attributes
with the actual SRI hashes computed from the CDN files, and include
crossorigin="anonymous" on both tags for proper SRI/CORS handling; fetch the
exact files for web-components.min.js and styles.min.css from the CDN, compute
their sha384 (or appropriate) SRI hashes, and place those hash strings into the
integrity attributes in the Html returned by serve_docs.
In `@modules/api_ingress/tests/auth_disabled_root_context.rs`:
- Around line 16-27: Replace the non-existent SecurityCtx usage with the real
SecurityContext API and pass a PolicyEngineRef into its scope calls: change the
import from use modkit_security::SecurityCtx; to use
modkit_security::SecurityContext (and import PolicyEngineRef or the project’s
policy engine constructor), then update ctx.scope() calls to
ctx.scope(policy_engine_ref) where you create/obtain a PolicyEngineRef in the
test, and replace SecurityCtx::root_ctx() with the SecurityContext variant that
creates a root context using the policy engine (e.g., construct a root
SecurityContext via the library API that accepts PolicyEngineRef and use that in
assertions).
In `@modules/grpc_hub/src/lib.rs`:
- Around line 68-76: The method listen_addr_tcp currently does match on
*self.listen_cfg.read(), which attempts to move a non-Copy ListenConfig out of
the RwLockReadGuard; change the match to operate on a reference to the guard
(e.g., let cfg = self.listen_cfg.read(); match &*cfg { ... }) so you don't move
out of the guard, and return the TCP address by cloning or copying the
referenced SocketAddr in the ListenConfig::Tcp arm; update uses of ListenConfig
and listen_cfg in listen_addr_tcp accordingly.
In `@modules/nodes_registry/src/api/rest/error.rs`:
- Around line 42-49: The match arm handling DomainError::Internal(msg) currently
sends the internal message back to clients; instead, log the original msg (using
the existing logger in scope or the chosen logging macro) and replace the
client-facing message in Problem::new with a generic string like "Internal
server error" or "An internal error occurred"; ensure you capture/log the error
message before returning (either by logging inside the DomainError::Internal arm
or logging prior to calling domain_error_to_problem) and keep the Problem::new
call using the generic text rather than the raw msg.
In `@modules/nodes_registry/src/domain/node_storage.rs`:
- Around line 18-21: The NodeStorage struct currently uses std::sync::RwLock
which can block the Tokio runtime; replace it with tokio::sync::RwLock in the
nodes field (nodes: RwLock<HashMap<Uuid, CachedNodeData>>) and update all
imports/usages accordingly so methods on NodeStorage (e.g., any
get/insert/remove or other functions that call nodes.read()/nodes.write()) use
the async-aware tokio::sync::RwLock and its async read/write guards to avoid
blocking executor threads.
In `@modules/nodes_registry/src/module.rs`:
- Around line 43-50: The Service is being created twice because Service::new()
is wrapped in two separate Arcs (one from a clone and one for the client);
instead create a single Arc<Service> and share it: call Service::new(), wrap
that instance once with Arc::new(...) (assign to a variable like service_arc),
then call self.service.store(Some(service_arc.clone())) and use
service_arc.clone() when constructing the NodesRegistryLocalClient
(NodesRegistryLocalClient::new(service_arc.clone())) before calling
expose_nodes_registry_client; this ensures both self.service and the local
client reference the same Arc<Service>.
♻️ Duplicate comments (1)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (1)
103-111: Handle bare/glob HTTP imports to avoid missed violations.
use http::*;oruse http;yields apath_strofhttp, which doesn’t matchstarts_with("http::"), so the lint won’t fire. Consider normalizing bare prefixes before matching.🩹 Proposed fix
- for pattern in HTTP_PATTERNS { - if path_str.starts_with(pattern) { + for pattern in HTTP_PATTERNS { + let bare = pattern.strip_suffix("::").unwrap_or(pattern); + if path_str == bare || path_str.starts_with(pattern) { cx.span_lint(DE0308_NO_HTTP_IN_DOMAIN, item.span, |diag| { diag.primary_message( "domain module imports HTTP type (DE0308)" );
🟡 Minor comments (13)
docs/ROADMAP.md-55-55 (1)
55-55: Empty section: "Nodes management".This section header has no content. If this is a placeholder for future work, consider adding a TODO comment or removing it until content is ready.
docs/ROADMAP.md-29-29 (1)
29-29: Fix heading levels and numbering for consistency.Phase 2 and Phase 3 use
###(level 3) headings with "2.2" and "2.3" prefixes, while Phase 1 uses a##(level 2) heading without a numeric prefix. This creates an inconsistent hierarchy where Phases 2 and 3 appear as subsections of Phase 1.📝 Proposed fix for consistent document structure
-### 2.2. Phase 2 - Extended Capabilities +## Phase 2 - Extended Capabilities-### 2.3. Phase 3 - Advanced Features +## Phase 3 - Advanced FeaturesAlso applies to: 43-43
modules/nodes_registry/src/domain/node_storage.rs-119-147 (1)
119-147: Don’t treat “empty capabilities” as a failure.If system collection returns an empty capability list,
get_syscapreturnsNone, which bubbles up asSysCapCollectionFailed. That treats a valid-but-empty response as an error. Consider returning an emptyNodeSysCapwhen system or custom syscaps exist, even if the merged map is empty.🐛 Proposed fix
- if cap_map.is_empty() { - None - } else { - Some(NodeSysCap { - node_id, - capabilities: cap_map.into_values().collect(), - collected_at: chrono::Utc::now(), - }) - } + let has_any = data.syscap_system.is_some() || !data.syscap_custom.is_empty(); + if !has_any { + None + } else { + Some(NodeSysCap { + node_id, + capabilities: cap_map.into_values().collect(), + collected_at: chrono::Utc::now(), + }) + }modules/nodes_registry/README.md-206-216 (1)
206-216: Surround the table with blank lines.Markdown linting requires blank lines before and after tables.
✏️ Suggested fix
### TTL Values + | Capability | TTL | Reason | |------------|-----|--------| | Architecture | 1 hour | Never changes | | RAM | 5 seconds | Changes frequently | | CPU | 10 minutes | Rarely changes | | OS | 2 minutes | Rarely changes | | GPU | 10 seconds | Can change | | Battery | 3 seconds | Very dynamic | | Custom Software | 60 seconds | Module-defined | +modules/nodes_registry/README.md-15-38 (1)
15-38: Add a language to the fenced code block.Markdown linting flags the unlabeled fence. Use a neutral language like
textfor the directory tree.✏️ Suggested fix
-``` +```text modules/nodes_registry/ ├── src/ │ ├── contract/ # Public API for other modules │ │ ├── client.rs # NodesRegistryApi trait │ │ ├── model.rs # Re-exports from modkit-node-info │ │ └── error.rs # Transport-agnostic errors │ ├── domain/ # Business logic │ │ ├── service.rs # Node management service with caching │ │ ├── node_storage.rs # In-memory storage with cache metadata │ │ └── error.rs # Domain errors │ ├── api/rest/ # REST API layer │ │ ├── dto.rs # DTOs with serde/ToSchema │ │ ├── handlers.rs # Axum handlers with cache refresh │ │ ├── routes.rs # Route registration with OpenAPI docs │ │ ├── mapper.rs # Domain to DTO mapping │ │ └── error.rs # Problem response mapping │ ├── gateways/ # Client implementations │ │ └── local.rs # Local client for ClientHub │ ├── config.rs # Module configuration │ ├── module.rs # Module registration │ └── lib.rs # Public exports └── Cargo.toml</details> </blockquote></details> <details> <summary>modules/nodes_registry/src/contract/client.rs-19-20 (1)</summary><blockquote> `19-20`: **Consider adding `force_refresh` parameter to contract API for feature parity.** The domain service's `get_node_syscap` method accepts a `force_refresh: bool` parameter, and the REST API exposes this via query parameter. However, the `NodesRegistryApi` trait omits this parameter, with the local gateway implementation hardcoding `false` when calling the domain service. This means contract API consumers cannot force-refresh capabilities, while REST API consumers can. If this limitation is intentional for simplicity, add documentation explaining the design decision; otherwise, adding the parameter would provide full feature parity. </blockquote></details> <details> <summary>docs/MODULE_CREATION_PROMT.md-4-4 (1)</summary><blockquote> `4-4`: **Add language identifiers to fenced code blocks (MD040)** markdownlint flags these fences without a language. Please add explicit language tags (e.g., `text`, `rust`, `toml`) or disable MD040 for this file if intentional. <details> <summary>💡 Example fix</summary> ```diff -``` +```textAlso applies to: 89-89, 102-102, 117-117, 139-139, 177-177, 192-192, 204-204, 220-220, 232-232, 291-291, 310-310, 336-336
docs/MODULE_CREATION_PROMT.md-23-29 (1)
23-29: Add the required NEW_MODULE guideline stepThe prompt should explicitly instruct the generator to open and follow
@/guidelines/NEW_MODULE.mdbefore scaffolding to align with repo requirements. Based on learnings, please add this step.✏️ Suggested addition
ABSOLUTE RULES — HYPERSPOT/MODKIT +- Always open `@/guidelines/NEW_MODULE.md` and follow it before scaffolding. - Hyperspot already has the single REST host: `api_ingress`. DO NOT implement rest_host or server startup.libs/modkit-bootstrap/src/paths/home_dir.rs-116-137 (1)
116-137: Restore HOME/APPDATA environment variables after each test to prevent cross-test interferenceMultiple test helpers (
unix_resolve_with_tilde,windows_default_uses_appdata, etc.) modify global environment variables without restoring them. This can cause failures when tests run in parallel. The pattern for proper restoration is already demonstrated inunix_will_fallback_when_home_missing()— save the original value before mutation and restore it in a cleanup phase, even if tests run sequentially withintests_secuential().modules/api_ingress/src/error.rs-46-50 (1)
46-50: Plumb the real request_id from span context into AppError responsesResponses and logs currently emit
"unknown"for request_id (line 48), making correlation difficult. The middleware already records the request_id to the tracing span; extract it there instead of using a hardcoded value. This enables proper request tracing across logs and error responses.modules/api_ingress/tests/auth_disabled_root_context.rs-121-156 (1)
121-156: Test is incomplete: missing "normal scope" comparison.The test is named
test_auth_disabled_vs_normal_scopebut only tests the root context (auth disabled) case. The "normal scope" test case that the function name implies is absent.Either complete the test by adding a comparison with normal (non-root) scope, or rename the function to accurately reflect what it tests (e.g.,
test_auth_disabled_root_scope_properties).libs/modkit-bootstrap/src/logging.rs-64-81 (1)
64-81:try_lockin Write impl may silently drop log data under contention.When
try_lock()fails, the error is propagated up, but the tracing infrastructure may not handle this gracefully. Under high contention, log entries could be lost. Consider usinglock()(blocking) instead, or implementing a bounded retry.💡 Alternative: use blocking lock
impl RotWriterHandle { - fn try_lock( + fn lock( &mut self, - ) -> std::io::Result<std::sync::MutexGuard<'_, FileRotate<AppendTimestamp>>> { - self.0 - .try_lock() - .map_err(|e| io::Error::other(format!("Lock failed: {}", e))) + ) -> std::sync::MutexGuard<'_, FileRotate<AppendTimestamp>> { + self.0.lock().unwrap_or_else(|e| e.into_inner()) } } impl Write for RotWriterHandle { fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> { - self.try_lock()?.write(buf) + self.lock().write(buf) } fn flush(&mut self) -> std::io::Result<()> { - self.try_lock()?.flush() + self.lock().flush() } }libs/modkit-bootstrap/src/logging.rs-326-333 (1)
326-333: Integer overflow possible inmax_size_bytescalculation.The calculation
mb * 1024 * 1024could overflow for largemax_size_mbvalues before theusize::try_fromcheck. Consider using checked arithmetic.🔧 Suggested fix with checked arithmetic
impl HasMaxSizeBytes for Section { fn max_size_bytes(&self) -> usize { self.max_size_mb - .map(|mb| mb * 1024 * 1024) - .and_then(|b| usize::try_from(b).ok()) + .and_then(|mb| mb.checked_mul(1024 * 1024)) + .and_then(|b| usize::try_from(b).ok()) .unwrap_or(DEFAULT_SECTION_MAX_SIZE_MB * 1024 * 1024) } }
🧹 Nitpick comments (26)
docs/ROADMAP.md (1)
46-46: Consider hyphenating "Server-side scripting".When "server side" is used as a compound adjective modifying a noun, it should be hyphenated: "Server-side scripting".
✏️ Proposed grammar fix
-- [ ] Server side scripting (FaaS, Workflows) +- [ ] Server-side scripting (FaaS, Workflows)libs/modkit-bootstrap/Cargo.toml (1)
18-39: Movetempfileto[dev-dependencies].
tempfileis only used in test-only code (#[cfg(test)]modules inhome_dir.rsandconfig.rs, and inconfig/tests.rs). Move it from[dependencies]to[dev-dependencies]to prevent it from being included in downstream builds.modules/directory_service/proto/directory/v1/directory.proto (1)
1-8: Proto definitions are well-structured.The service and message definitions follow proto3 best practices with proper field numbering.
Regarding the Buf lint warning about package directory mismatch: the package
modkit.directory.v1conventionally expects the file to reside inmodkit/directory/v1/relative to the proto root. The current placement undermodules/directory_service/proto/directory/v1/works for tonic-prost-build but violates Buf'sPACKAGE_DIRECTORY_MATCHrule. If Buf is used for linting or breaking change detection, consider either reorganizing the proto structure or adding a buf.yaml exclusion.modules/directory_service/src/config.rs (1)
7-8: LGTM!The placeholder config struct is appropriately designed for future expansion with correct derives for a deserializable configuration type.
Optional: Consider adding
Serializeif you anticipate needing to export or log the configuration.modules/directory_service/src/grpc_client.rs (1)
81-84: Consider preserving gRPC status details in error chain.Using
anyhow::anyhow!loses the structuredtonic::Status(code, metadata). Using.context()would preserve the original error for better debugging:- .map_err(|e| anyhow::anyhow!("gRPC call failed: {}", e))?; + .map_err(|e| anyhow::Error::from(e).context("resolve_grpc_service failed"))?;This applies to Line 101 as well.
modules/nodes_registry/src/api/rest/handlers.rs (1)
37-54: Sequential async calls may be slow for large node lists.The loop awaits
get_node_sysinfoandget_node_syscapsequentially for each node. For many nodes, this could be slow. Consider parallelizing withfutures::future::join_allor similar if performance becomes an issue.Also,
.ok()silently discards errors. Consider logging failures at debug/warn level so operators can diagnose issues.♻️ Optional: Parallel fetching example
use futures::future::join_all; if query.details { let detailed_futures = nodes.into_iter().map(|node| { let svc = svc.clone(); let force_refresh = query.force_refresh; async move { let node_id = node.id; let sysinfo = svc.get_node_sysinfo(node_id).await.ok().map(Into::into); let syscap = svc.get_node_syscap(node_id, force_refresh).await.ok().map(Into::into); let mut node_dto: NodeDto = node.into(); node_dto.sysinfo = sysinfo; node_dto.syscap = syscap; node_dto } }); let detailed_nodes = join_all(detailed_futures).await; Ok(Json(detailed_nodes)) }modules/grpc_hub/src/lib.rs (1)
147-214: ReadySignal fires before bind; consider notifying after bind.Right now
ready.notify()happens before the socket is bound, so callers can see “ready” even if bind fails. If ReadySignal is meant to indicate a live listener, move the notification into the per-transport serve paths after bind/initialization.♻️ Possible restructuring to notify after bind
- async fn serve_tcp( - addr: SocketAddr, - routes: Routes, - cancel: CancellationToken, - ) -> anyhow::Result<()> { - tracing::info!(%addr, transport = "tcp", "gRPC hub listening"); - Server::builder() - .add_routes(routes) - .serve_with_shutdown(addr, async move { - cancel.cancelled().await; - }) - .await?; - Ok(()) - } + async fn serve_tcp( + addr: SocketAddr, + routes: Routes, + cancel: CancellationToken, + ready: ReadySignal, + ) -> anyhow::Result<()> { + use tokio::net::TcpListener; + use tokio_stream::wrappers::TcpListenerStream; + + let listener = TcpListener::bind(addr) + .await + .with_context(|| format!("failed to bind TCP listener at '{addr}'"))?; + tracing::info!(%addr, transport = "tcp", "gRPC hub listening"); + ready.notify(); + let incoming = TcpListenerStream::new(listener); + Server::builder() + .add_routes(routes) + .serve_with_incoming_shutdown(incoming, async move { + cancel.cancelled().await; + }) + .await?; + Ok(()) + } - async fn serve_uds( - path: PathBuf, - routes: Routes, - cancel: CancellationToken, - ) -> anyhow::Result<()> { + async fn serve_uds( + path: PathBuf, + routes: Routes, + cancel: CancellationToken, + ready: ReadySignal, + ) -> anyhow::Result<()> { use tokio::net::UnixListener; use tokio_stream::wrappers::UnixListenerStream; @@ let uds = UnixListener::bind(&path) .with_context(|| format!("failed to bind UDS listener at '{}'", path.display()))?; + ready.notify(); let incoming = UnixListenerStream::new(uds); @@ - async fn serve_named_pipe( - pipe_name: String, - routes: Routes, - cancel: CancellationToken, - ) -> anyhow::Result<()> { + async fn serve_named_pipe( + pipe_name: String, + routes: Routes, + cancel: CancellationToken, + ready: ReadySignal, + ) -> anyhow::Result<()> { tracing::info!(name = %pipe_name, transport = "named_pipe", "gRPC hub listening"); let incoming = create_named_pipe_incoming(pipe_name, cancel.clone()); + ready.notify(); @@ - ready.notify(); - let listen_cfg = self.listen_cfg.read().clone(); match listen_cfg { - ListenConfig::Tcp(addr) => Self::serve_tcp(addr, routes, cancel).await?, + ListenConfig::Tcp(addr) => Self::serve_tcp(addr, routes, cancel, ready).await?, #[cfg(unix)] - ListenConfig::Uds(path) => Self::serve_uds(path, routes, cancel).await?, + ListenConfig::Uds(path) => Self::serve_uds(path, routes, cancel, ready).await?, #[cfg(windows)] ListenConfig::NamedPipe(pipe_name) => { - Self::serve_named_pipe(pipe_name, routes, cancel).await?; + Self::serve_named_pipe(pipe_name, routes, cancel, ready).await?; } }Also applies to: 216-249
libs/modkit-bootstrap/src/paths/home_dir.rs (1)
83-101: Avoid deprecated std::env::home_dir
env::home_dir()is deprecated and may trip warning/deny-warnings policies. Considerdirs::home_dir()/home::home_dir()or explicit env var lookups with platform fallbacks.modules/api_ingress/build.rs (1)
49-57: Add a timeout to avoid hanging builds.
reqwest::blocking::getcan wait indefinitely if the network stalls. Using a client with a timeout makes the build more resilient.🔧 Proposed fix (blocking client with timeout)
fn download_to(url: &str, dest: &Path) -> Result<(), Box<dyn std::error::Error>> { println!("cargo:rerun-if-changed={}", dest.display()); - let resp = reqwest::blocking::get(url)?; + let client = reqwest::blocking::Client::builder() + .timeout(std::time::Duration::from_secs(30)) + .build()?; + let resp = client.get(url).send()?; if !resp.status().is_success() { return Err(format!("HTTP {} for {}", resp.status(), url).into()); }modules/api_ingress/tests/integration_router.rs (1)
156-187: Consider adding meaningful assertions to these tests.Both tests only verify that the code runs without panicking. While the comments acknowledge this limitation, the tests would be more valuable with actual assertions.
Possible improvements:
- Verify specific routes exist on the router
- Check that OpenAPI spec contains the registered operations
- Make actual HTTP requests using
tower::ServiceExt::oneshotThis is fine for an initial implementation, but consider enhancing test coverage in a follow-up.
modules/api_ingress/src/assets.rs (2)
1-6: Consider scoping theunused_importsallow more narrowly.The blanket
#![allow(unused_imports)]suppresses warnings for the entire module. A more precise approach is to conditionally compile the imports:Suggested change
-#![allow(unused_imports)] -use axum::http::StatusCode; -use axum::response::IntoResponse; - -#[cfg(feature = "embed_elements")] -use rust_embed::RustEmbed; +#[cfg(feature = "embed_elements")] +use axum::http::StatusCode; +#[cfg(feature = "embed_elements")] +use axum::response::IntoResponse; +#[cfg(feature = "embed_elements")] +use rust_embed::RustEmbed;This keeps compile-time warnings active for actual unused imports while avoiding false positives.
30-42: Consider adding common web asset MIME types.The current set covers Stoplight Elements assets well. If this module may serve additional assets in the future, consider adding:
"ttf" => "font/ttf", + "html" => "text/html; charset=utf-8", + "json" => "application/json; charset=utf-8", + "ico" => "image/x-icon", + "woff" => "font/woff", _ => "application/octet-stream",If the scope is strictly Stoplight Elements assets, the current implementation is sufficient.
modules/api_ingress/tests/body_limit_tests.rs (2)
104-127: Consider asserting actual 413 behavior.This test only inspects config; adding a request that exceeds the limit and asserting a 413 would validate enforcement end‑to‑end.
155-190: Avoid hard‑coding the default size.Derive the expected default from the config defaults (e.g.,
ApiIngressConfig::default().defaults.body_limit_bytes) to prevent drift if the default changes.libs/modkit-bootstrap/src/config/tests.rs (1)
233-264: Make this test assertive.Right now it never fails; consider asserting the expected outcome so the test provides a signal instead of just printing.
modules/api_ingress/tests/auth_middleware.rs (1)
210-281: Consider adding assertions for response body content.The test verifies status codes but doesn't validate the actual response content. For the health and OpenAPI endpoints, consider adding body assertions to ensure the responses contain expected data.
💡 Optional enhancement for response validation
assert_eq!( response.status(), StatusCode::OK, "Health endpoint should be accessible" ); + + // Optionally verify health response content + let body = axum::body::to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert!(!body.is_empty(), "Health endpoint should return a body");modules/api_ingress/tests/cors_tests.rs (3)
101-106: Redundantget()wrapper on handler.
OperationBuilder::get("/cors-test")already specifies the HTTP method. Wrapping the handler withget(test_handler)is redundant since the builder knows it's a GET endpoint. Compare with line 113 which correctly usesaxum::routing::post(post_handler)for the POST endpoint.♻️ Suggested fix
let router = OperationBuilder::get("/cors-test") .operation_id("cors:test") .summary("CORS test endpoint") .json_response(200, "Success") - .handler(get(test_handler)) + .handler(test_handler) .register(router, openapi);
5-5: Unused import:routing::get.The
getimport fromaxum::routingappears unused after fixing the redundant wrapper issue above, or if kept as-is, the import should be reviewed for consistency.-use axum::{extract::Json, routing::get, Router}; +use axum::{extract::Json, Router};
130-149: Tests validate router construction but lack CORS header verification.The current tests confirm the router builds successfully with CORS enabled, but don't verify actual CORS behavior (e.g.,
Access-Control-Allow-Originheaders on preflight requests). The comment on line 148 acknowledges this limitation.Consider adding actual header verification in a follow-up, or add a TODO comment to track this gap.
Would you like me to generate a test that sends an OPTIONS preflight request and verifies CORS headers are correctly set?
modules/api_ingress/src/model.rs (1)
31-42: Consider removing#[allow(dead_code)]annotations.These methods (
contains,len,is_empty) are standard collection accessors. If they're not currently used, they may be needed soon for OpenAPI document assembly. Consider removing the dead_code annotations if these will be used, or remove the methods entirely if they're truly unnecessary.libs/modkit-bootstrap/src/logging.rs (1)
182-186: Potential panic fromchrono::Duration::dayswith large values.If
max_age_daysis extremely large (e.g., > i64::MAX / 86400),chrono::Duration::days()could panic. While unlikely with typical config values, consider adding validation.- let age = chrono::Duration::days(max_age_days.unwrap_or(1) as i64); + let days = max_age_days.unwrap_or(1).min(36500) as i64; // Cap at ~100 years + let age = chrono::Duration::days(days);modules/api_ingress/src/auth.rs (1)
190-220: Consider making OIDC configuration values configurable.Several values are currently hardcoded in the auth configuration:
leeway_seconds: 60refresh_interval_seconds: 300max_backoff_seconds: 3600- Claim names:
"tenants","roles"These may need adjustment for different deployments. Consider exposing them as optional fields in
ApiIngressConfig, leveraging the underlying modkit-auth library's built-in support for these configuration parameters. Note that the same hardcoding pattern exists inmodules/system/api_gateway/src/auth.rs.modules/api_ingress/tests/rate_limit_tests.rs (1)
213-243: Potential issue:api_ingressnot initialized before use intest_rate_limit_metadata_stored.In
test_rate_limit_metadata_stored,ApiIngress::default()is used directly without callinginit()with aModuleCtx. While this works for the current test (sincebuild_openapidoesn't require initialization), it differs from the pattern in the other two tests. For consistency and to ensure the test reflects production behavior:🔧 Suggested improvement for test consistency
#[tokio::test] async fn test_rate_limit_metadata_stored() { + let config = serde_json::json!({ + "bind_addr": "127.0.0.1:0", + "cors_enabled": false, + "auth_disabled": true + }); + let api_ingress = api_ingress::ApiIngress::default(); + let ctx = create_test_module_ctx_with_config(config); + api_ingress.init(&ctx).await.expect("Failed to init"); + let router = Router::<()>::new();modules/api_ingress/src/lib.rs (2)
111-119: Consider consolidatingget_configandget_cached_config.Both methods have identical implementations:
pub fn get_config(&self) -> ApiIngressConfig { (**self.config.load()).clone() } pub fn get_cached_config(&self) -> ApiIngressConfig { (**self.config.load()).clone() }If they're intended to have different behaviors, the names suggest
get_cached_configmight avoid the clone, but they're identical. Consider consolidating or documenting the intended distinction.
499-506: Consider logging or warning on unknown HTTP method fallback.The fallback to
HttpMethod::Getfor unknown methods could silently mask configuration errors:let method = match spec.method { Method::GET => HttpMethod::Get, // ... other methods _ => HttpMethod::Get, // Silent fallback };🔧 Suggested improvement to surface unknown methods
let method = match spec.method { Method::GET => HttpMethod::Get, Method::POST => HttpMethod::Post, Method::PUT => HttpMethod::Put, Method::DELETE => HttpMethod::Delete, Method::PATCH => HttpMethod::Patch, - _ => HttpMethod::Get, + other => { + tracing::warn!( + method = %other.as_str(), + path = %spec.path, + "Unknown HTTP method in operation spec, defaulting to GET in OpenAPI" + ); + HttpMethod::Get + } };libs/modkit-bootstrap/src/config.rs (1)
346-405: Consider extracting repeated DSN format logic.The Windows vs Unix DSN format logic is repeated multiple times in this file (here and in several other functions). Consider extracting to a helper:
🔧 Suggested helper to reduce duplication
/// Formats a SQLite DSN from a normalized path. /// Windows: sqlite:C:/path | Unix: sqlite://path fn format_sqlite_dsn(normalized_path: &str) -> String { if normalized_path.len() > 1 && normalized_path.chars().nth(1) == Some(':') { // Windows absolute path like C:/... format!("sqlite:{}", normalized_path) } else { // Unix absolute path format!("sqlite://{}", normalized_path) } }This helper could replace the repeated logic at lines 370-376, 394-400, 491-497, 538-546, 556-564, 573-581.
Add three new dylint rules: - DE0301: no_infra_in_domain - prohibits infrastructure imports in domain layer - DE0308: no_http_in_domain - prohibits HTTP types in domain layer - DE0706: no_direct_sqlx - prohibits direct sqlx usage, enforces Sea-ORM/SecORM Also updates lint_utils with helper functions for path checking. Signed-off-by: KvizadSaderah <kvizad@gmail.com>
- DE0301: remove unused DB/HTTP frameworks, use starts_with - DE0308: remove actix_web, use starts_with - DE0706: update rust-toolchain to TOML format with components - lint_utils: add explanatory comments for path checking logic Note: modkit_db provides abstractions (traits) that domain layer SHOULD use. Direct database access (sqlx, sea_orm) is forbidden, but modkit_db abstractions are allowed. Resolves all 9 review comments from @Bechma in PR hypernetix#253 Signed-off-by: KvizadSaderah <kvizad@gmail.com>
Critical bug fix: use_tree_to_string was returning only the FIRST nested
import path, causing false negatives for patterns like:
use crate::{Foo, http::StatusCode\}\;
Changed to use_tree_to_strings() that returns Vec<String> with ALL nested
paths, ensuring complete detection of violations in both:
- DE0301 (no infrastructure in domain)
- DE0308 (no HTTP in domain)
Fixes CodeRabbit major issue in PR hypernetix#253
Signed-off-by: KvizadSaderah <kvizad@gmail.com>
Reviewer feedback: lints only checked 'use' statements but missed direct
type usage in struct fields, function parameters, and return types.
Extended both DE0301 and DE0308 to check:
- Struct field types
- Enum variant field types
- Function parameter types
- Function return types
- Type aliases
Now catches violations like:
struct Hello { field: http::StatusCode }
Added UI test bad_http_type_in_struct.rs
Addresses reviewer comment on PR hypernetix#253
Signed-off-by: KvizadSaderah <kvizad@gmail.com>
CodeRabbit major issue: type_to_string only checked outer type path, missing nested types in generics like Option<http::StatusCode\>, Vec<http::Request\>, &http::Response. Refactored check_type_in_domain to recursively traverse: - Generic arguments (Option<T>, Vec<T>, Result<T,E>) - References (&T, &mut T) - Slices ([T]) - Arrays ([T; N]) - Raw pointers (*const T, *mut T) - Tuples ((T1, T2)) Applied to both DE0301 (infrastructure) and DE0308 (HTTP). Added UI test nested_generic_http_type.rs with 3 nested type cases. Fixes CodeRabbit major issue in PR hypernetix#253 Signed-off-by: KvizadSaderah <kvizad@gmail.com>
CodeRabbit minor issue: TyKind::TraitObject and TyKind::ImplTrait were not inspected, allowing infrastructure/HTTP traits in trait object and impl Trait positions to escape detection. Examples that now trigger: - impl sqlx::Executor (impl Trait in return position) - Box<dyn http::Service\> (trait object) - Arc<dyn sea_orm::EntityTrait> (wrapped trait object) Added handling for: - TyKind::TraitObject: checks all trait bounds in dyn Trait - TyKind::ImplTrait: checks all trait bounds in impl Trait Applied to both DE0301 (infrastructure) and DE0308 (HTTP). Added UI test trait_bounds_http.rs with 2 impl Trait cases. Fixes CodeRabbit minor issue in PR hypernetix#253 Signed-off-by: KvizadSaderah <kvizad@gmail.com>
Eliminate code duplication by moving use_tree_to_strings function from DE0301, DE0308, and DE0706 lints to shared lint_utils crate. Changes: - Add use_tree_to_strings() to lint_utils with full documentation - Update DE0301/DE0308 to use shared function (removed ~30 lines each) - Update DE0706 to use shared function (removed ~24 lines) - Add LintContext import where needed for EarlyContext methods - Remove unused imports (EarlyContext, Crate, UseTree, UseTreeKind) All tests pass for all three lints. Addresses CodeRabbit code duplication feedback. Signed-off-by: KvizadSaderah <kvizad@gmail.com>
ba3099e to
571ad8e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@dylint_lints/de07_security/de0706_no_direct_sqlx/README.md`:
- Line 3: The README's heading hierarchy skips h2 (it uses "###" under the
top-level title); update the headings "What it does", "Why is this bad?",
"Example", "Configuration", and "See Also" from "###" to "##" so they are proper
h2s (maintaining consistent Markdown heading levels under the main h1).
♻️ Duplicate comments (4)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (1)
139-187: Trait bounds don't recurse into generic arguments — may miss associated type constraints.The
TraitObjectandImplTraithandlers check the trait path itself but don't inspect angle-bracketed arguments on the trait path. This means cases like:fn foo() -> impl Iterator<Item = http::StatusCode> { ... }Would not be caught because:
- The trait path "Iterator" doesn't match HTTP_PATTERNS
- The associated type constraint
Item = http::StatusCodeis insegment.argsbut isn't inspectedA past review comment flagged this issue. While the response indicated it was addressed, the current implementation still appears to have this gap.
🔧 Proposed fix — recurse into trait bound generic args
// Handle trait objects: dyn http::Service TyKind::TraitObject(bounds, _) => { for bound in bounds { if let rustc_ast::GenericBound::Trait(trait_ref) = bound { // Check the trait path itself let path = &trait_ref.trait_ref.path; let path_str = path.segments.iter() .map(|seg| seg.ident.name.as_str()) .collect::<Vec<_>>() .join("::"); for pattern in HTTP_PATTERNS { if path_str.starts_with(pattern) { cx.span_lint(DE0308_NO_HTTP_IN_DOMAIN, ty.span, |diag| { diag.primary_message( format!("domain module uses HTTP trait `{}` (DE0308)", path_str) ); diag.help("domain should be transport-agnostic; handle HTTP in api/ layer"); }); return; } } + + // Also check generic arguments on the trait (e.g., Iterator<Item = http::StatusCode>) + for segment in &path.segments { + if let Some(args) = &segment.args { + if let rustc_ast::GenericArgs::AngleBracketed(ref angle_args) = **args { + for arg in &angle_args.args { + match arg { + rustc_ast::AngleBracketedArg::Arg(rustc_ast::GenericArg::Type(inner_ty)) => { + check_type_in_domain(cx, inner_ty); + } + rustc_ast::AngleBracketedArg::Constraint(assoc) => { + if let rustc_ast::AssocItemConstraintKind::Equality { term } = &assoc.kind { + if let rustc_ast::Term::Ty(inner_ty) = term { + check_type_in_domain(cx, inner_ty); + } + } + } + _ => {} + } + } + } + } + } } } }Apply the same pattern to the
ImplTraithandler (lines 163-187).dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (3)
54-73: Missing additional infrastructure patterns.As noted previously, the following patterns should be added to
INFRA_PATTERNS:
modkit_dbmodkit_db_macrosmodkit_transport_grpcProposed fix
const INFRA_PATTERNS: &[&str] = &[ // Infrastructure layer "crate::infra", "crate::infrastructure", // Database frameworks (direct access forbidden, use modkit_db abstractions instead) "sea_orm", "sqlx", + // Internal infrastructure modules + "modkit_db", + "modkit_db_macros", + "modkit_transport_grpc", // HTTP/Web frameworks (only used ones: axum, hyper, http) "axum", "hyper",
116-127: MissingAssocTypeandConstrainthandling in angle-bracketed arguments.The current implementation only handles
AngleBracketedArg::Arg(GenericArg::Type(...)). This misses associated type bindings likeIterator<Item = sqlx::PgPool>where the infra type appears in anAssocTypeposition.Proposed fix
if let rustc_ast::GenericArgs::AngleBracketed(ref angle_args) = **args { for arg in &angle_args.args { - if let rustc_ast::AngleBracketedArg::Arg(rustc_ast::GenericArg::Type(inner_ty)) = arg { - check_type_in_domain(cx, inner_ty); + match arg { + rustc_ast::AngleBracketedArg::Arg(rustc_ast::GenericArg::Type(inner_ty)) => { + check_type_in_domain(cx, inner_ty); + } + rustc_ast::AngleBracketedArg::Constraint(constraint) => { + if let Some(ref ty) = constraint.ty { + check_type_in_domain(cx, ty); + } + } + _ => {} } } }
152-175: Trait bounds inTraitObjectandImplTraitdon't recurse into generic arguments.The lint checks the trait path itself (e.g.,
sea_orm::EntityTrait) but doesn't recurse into the trait's generic arguments. This allows patterns likedyn Service<Response = http::Response>orimpl Iterator<Item = sqlx::PgPool>to escape detection.Proposed fix - add generic args traversal to trait bounds
TyKind::TraitObject(bounds, _) => { for bound in bounds { if let rustc_ast::GenericBound::Trait(trait_ref) = bound { // Check the trait path itself let path = &trait_ref.trait_ref.path; let path_str = path.segments.iter() .map(|seg| seg.ident.name.as_str()) .collect::<Vec<_>>() .join("::"); for pattern in INFRA_PATTERNS { if path_str.starts_with(pattern) { cx.span_lint(DE0301_NO_INFRA_IN_DOMAIN, ty.span, |diag| { diag.primary_message( format!("domain module uses infrastructure trait `{}` (DE0301)", path_str) ); diag.help("domain should depend only on abstractions; move infrastructure code to infra/ layer"); }); return; } } + + // Recursively check generic arguments in trait bounds + for segment in &path.segments { + if let Some(args) = &segment.args { + if let rustc_ast::GenericArgs::AngleBracketed(ref angle_args) = **args { + for arg in &angle_args.args { + match arg { + rustc_ast::AngleBracketedArg::Arg(rustc_ast::GenericArg::Type(inner_ty)) => { + check_type_in_domain(cx, inner_ty); + } + rustc_ast::AngleBracketedArg::Constraint(constraint) => { + if let Some(ref inner_ty) = constraint.ty { + check_type_in_domain(cx, inner_ty); + } + } + _ => {} + } + } + } + } + } } } }Apply the same pattern to
TyKind::ImplTrait.Also applies to: 177-200
🧹 Nitpick comments (4)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/README.md (1)
1-3: Fix heading hierarchy for markdown consistency.The static analysis tool correctly identifies that line 3 jumps from h1 (
#) to h3 (###). Heading levels should increment by one level at a time.📝 Proposed fix
# DE0308: No HTTP in Domain -### What it does +## What it doesAnd similarly for the other
###headings (Why is this bad?,Example,Configuration,See Also) — change them all to##.dylint_lints/lint_utils/src/lib.rs (1)
138-174: Solid implementation ofuse_tree_to_stringswith proper edge case handling.The function correctly:
- Handles all
UseTreeKindvariants (Simple, Glob, Nested)- Normalizes empty prefixes (line 164) to avoid
::path::to::itemissues- Recursively processes nested imports
This addresses the past review comments about handling all nested use items and empty prefix normalization.
Optional: The relevant code snippets show similar prefix-segment collection in
de0103_no_http_types_in_contract/src/lib.rs(lines 77-85). Consider refactoring that lint to reuse this shared utility to reduce duplication.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/README.md (2)
1-5: Fix heading levels to increment sequentially.The heading structure jumps from
#(h1) to###(h3), violating MD001. Use##for subsections.Proposed fix
# DE0301: No Infrastructure in Domain -### What it does +## What it does Checks that domain modules do not import infrastructure dependencies. -### Why is this bad? +## Why is this bad?Apply the same change to all
###headings (lines 7, 15, 46, 54).
50-52: Consider noting the overlap with DE0706.The documentation mentions
sqlxas a checked pattern, but DE0706 (no_direct_sqlx) also specifically forbids direct sqlx usage. Consider adding a note clarifying the relationship—DE0301 is broader (any infra in domain), while DE0706 is specific to sqlx regardless of layer.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dylint_lints/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
dylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/README.mddylint_lints/de03_domain_layer/de0301_no_infra_in_domain/rust-toolchaindylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/README.mddylint_lints/de03_domain_layer/de0308_no_http_in_domain/rust-toolchaindylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/trait_bounds_http.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/trait_bounds_http.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.tomldylint_lints/de07_security/de0706_no_direct_sqlx/README.mddylint_lints/de07_security/de0706_no_direct_sqlx/rust-toolchaindylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderrdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rsdylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.stderrdylint_lints/lint_utils/src/lib.rs
✅ Files skipped from review due to trivial changes (2)
- dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.stderr
- dylint_lints/de03_domain_layer/de0308_no_http_in_domain/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (17)
- dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.stderr
- dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rs
- dylint_lints/de07_security/de0706_no_direct_sqlx/Cargo.toml
- dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/good_domain_0301.rs
- dylint_lints/de03_domain_layer/de0308_no_http_in_domain/.cargo/config.toml
- dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/Cargo.toml
- dylint_lints/de07_security/de0706_no_direct_sqlx/src/lib.rs
- dylint_lints/de07_security/de0706_no_direct_sqlx/ui/good_sea_orm.rs
- dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.rs
- dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.rs
- dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/rust-toolchain
- dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.stderr
- dylint_lints/de07_security/de0706_no_direct_sqlx/rust-toolchain
- dylint_lints/de07_security/de0706_no_direct_sqlx/ui/bad_direct_sqlx.stderr
- dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/mixed_imports_0301.rs
- dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.stderr
- dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_type_in_struct.rs
🧰 Additional context used
📓 Path-based instructions (2)
dylint_lints/**/src/lib.rs
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/src/lib.rs: Usedeclare_pre_expansion_lint!for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
UseEarlyLintPasswithrustc_astfor pre-expansion lints that need to see#[derive(...)]attributes directly before macro expansion
Usedeclare_early_lint!for checking syntax and structure after macro expansion but before type resolution without type information
Usedeclare_late_lint!withLateLintPassandrustc_hirwhen type information or semantic analysis is required
Useui_test_exampleswhen tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Useui_testwhen all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Every lint crate MUST include atest_comment_annotations_match_stderrtest usinglint_utils::test_comment_annotations_match_stderrto validate comment/stderr alignment
Useis_in_contract_module_ast()helper fromlint_utilscrate to check if an AST item is in a contract/ directory
Document lint behavior in doc comments within the lint implementation
Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
dylint_lints/**/Cargo.toml
📄 CodeRabbit inference engine (dylint_lints/AGENTS.md)
dylint_lints/**/Cargo.toml: InCargo.tomlfor lint crates, reference common workspace dependencies (clippy_utils,dylint_linting,lint_utils) using.workspace = trueto avoid duplication
Define example targets in lint crateCargo.tomlwith[[example]]sections for each UI test case
Register new lint crates by adding them to thememberslist in the workspace rootCargo.toml
Files:
dylint_lints/Cargo.toml
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Place test comment annotations (`// Should trigger DEXXX - description` or `// Should not trigger DEXXX - description`) on the line immediately before where the error is reported, not on attribute lines
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `is_in_contract_module_ast()` helper from `lint_utils` crate to check if an AST item is in a contract/ directory
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_pre_expansion_lint!` for lints that check derive attributes before macro expansion (e.g., detecting serde/utoipa derives)
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/README.md
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Select pre-expansion lint pass for detecting serde/utoipa derives that must be checked before proc macros expand
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/trait_bounds_http.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test_examples` when tests need external dependencies (serde, utoipa) or have logically distinct scenarios with explicit test organization
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.stderrdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `EarlyLintPass` with `rustc_ast` for pre-expansion lints that need to see `#[derive(...)]` attributes directly before macro expansion
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_early_lint!` for checking syntax and structure after macro expansion but before type resolution without type information
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `declare_late_lint!` with `LateLintPass` and `rustc_hir` when type information or semantic analysis is required
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Document lint behavior in doc comments within the lint implementation
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/trait_bounds_http.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/README.mddylint_lints/de03_domain_layer/de0308_no_http_in_domain/README.mddylint_lints/de07_security/de0706_no_direct_sqlx/README.md
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : In `Cargo.toml` for lint crates, reference common workspace dependencies (`clippy_utils`, `dylint_linting`, `lint_utils`) using `.workspace = true` to avoid duplication
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Use `ui_test` when all tests have no external dependencies and you have many small, similar test cases for simpler configuration
Applied to files:
dylint_lints/lint_utils/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rsdylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Include at least one test case with no violations (empty or near-empty `.stderr` file) in UI tests
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Organize Rust lint crates with structure: `src/lib.rs` for implementation, `ui/` directory for UI test files with corresponding `.stderr` files
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/src/lib.rs : Every lint crate MUST include a `test_comment_annotations_match_stderr` test using `lint_utils::test_comment_annotations_match_stderr` to validate comment/stderr alignment
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/Cargo.tomldylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : For struct/enum/function multiline spans in UI tests, place error comments on the first line of the item declaration (e.g., `pub struct Name`), not on the `#[derive(...)]` attribute line
Applied to files:
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rsdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.stderrdylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Register new lint crates by adding them to the `members` list in the workspace root `Cargo.toml`
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/**/Cargo.toml : Define example targets in lint crate `Cargo.toml` with `[[example]]` sections for each UI test case
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Use `cargo dylint new <lint_name>` in `dylint_lints/` directory to initialize new lint crates with proper scaffolding
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.tomldylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.tomldylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/*.rs : Place test comment annotations (`// Should trigger DEXXX - description` or `// Should not trigger DEXXX - description`) on the line immediately before where the error is reported, not on attribute lines
Applied to files:
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rsdylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Applies to dylint_lints/ui/main.rs : If generated `main.rs` and `main.stderr` are empty, remove them from the UI test directory
Applied to files:
dylint_lints/Cargo.toml
📚 Learning: 2026-01-14T17:18:31.902Z
Learnt from: CR
Repo: hypernetix/hyperspot PR: 0
File: dylint_lints/AGENTS.md:0-0
Timestamp: 2026-01-14T17:18:31.902Z
Learning: Verify tests pass with explicit `-p` flag to ensure correct package tests are running: `cargo test --lib -p <lint_name>`
Applied to files:
dylint_lints/Cargo.toml
🧬 Code graph analysis (3)
dylint_lints/lint_utils/src/lib.rs (1)
dylint_lints/de01_contract_layer/de0103_no_http_types_in_contract/src/lib.rs (2)
tree(78-80)tree(84-86)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rs (1)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/bad_http_import.rs (1)
main(12-12)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/trait_bounds_http.rs (1)
modules/system/api_gateway/src/middleware/request_id.rs (1)
header(9-11)
🪛 markdownlint-cli2 (0.18.1)
dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/README.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/README.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
dylint_lints/de07_security/de0706_no_direct_sqlx/README.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test Suite (macos-latest, rust=stable)
- GitHub Check: Run Dylint Lints
- GitHub Check: Code Coverage (cargo-llvm-cov)
- GitHub Check: Dylint Lints Tests
- GitHub Check: Test Suite (ubuntu-latest, rust=stable)
- GitHub Check: Test Suite (windows-latest, rust=stable)
- GitHub Check: UI macro tests (trybuild)
- GitHub Check: users_info integration (Postgres)
🔇 Additional comments (19)
dylint_lints/de07_security/de0706_no_direct_sqlx/.cargo/config.toml (1)
1-6: LGTM!Standard dylint linker configuration that follows the established pattern used in other lint crates (de0301, de0308). The commented alternative for Rust 1.74.0+ is a helpful note for future maintainability.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/rust-toolchain (1)
1-3: Verify the pinned nightly and components are still available.
Nightly snapshots and components (especiallyrustc-dev) can be pruned; please confirmnightly-2025-09-18and required components still install cleanly and align with any workspace-level toolchain pin to avoid drift.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/.cargo/config.toml (1)
1-6: LGTM!Standard dylint linker configuration. This boilerplate is required for all dylint lint crates and matches the pattern used in other crates in this workspace.
dylint_lints/Cargo.toml (2)
11-14: New lint crates correctly registered as workspace members.The three new lints (DE0301, DE0308, DE0706) are properly registered. As per coding guidelines, new lint crates must be added to the
memberslist in the workspace rootCargo.toml.Note: The AI summary indicates
de0503_plugin_client_suffixwas "Removed", but it remains present at line 13. This may be a summary discrepancy rather than a code issue.
47-54: Dev dependencies appropriately added for lint UI tests.These workspace-level dev dependencies enable lint test crates to compile code samples that use external libraries (e.g., testing detection of
sqlximports,httptypes,serdederives, etc.). Thethiserror = "2.0"constraint is appropriate and will resolve to the latest available version.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/trait_bounds_http.stderr (1)
1-18: LGTM!The stderr output correctly captures the expected DE0308 lint diagnostics. The lint properly detects HTTP traits (
IntoHeaderName) in return type bounds within domain modules, with clear guidance directing developers to handle HTTP in theapi/layer.dylint_lints/de07_security/de0706_no_direct_sqlx/README.md (1)
77-80: LGTM!The "See Also" section now correctly references DE0301 and DE0308, which are both part of this PR. The relative paths are accurate.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/nested_generic_http_type.rs (1)
1-15: Well-structured UI test for nested generic HTTP types.The test correctly exercises DE0308 detection of HTTP types wrapped in generic containers (
Option,Vec,&'static) within domain structs. Comment annotations are properly placed on the lines immediately before the expected error locations, the.stderrfile is correctly formatted with matching diagnostics and line numbers, and the lint crate includes the required test validation.dylint_lints/de03_domain_layer/de0308_no_http_in_domain/README.md (1)
36-82: Well-documented examples showing the correct domain pattern.The examples clearly demonstrate the transformation from HTTP-coupled domain code to transport-agnostic domain errors with API-layer mapping. This provides excellent guidance for developers.
dylint_lints/lint_utils/src/lib.rs (1)
33-55: Path exclusion helpers are well-documented.The doc comments and inline comments clearly explain the purpose of the multiple OR conditions, addressing the different path contexts (absolute, relative, simulated test paths). This provides good maintainability for future developers.
dylint_lints/de03_domain_layer/de0308_no_http_in_domain/src/lib.rs (3)
10-49: Excellent doc comments following coding guidelines.The lint declaration includes comprehensive documentation with:
- Clear explanation of what the lint checks
- Rationale for why HTTP types in domain are problematic
- Concrete code examples showing bad and good patterns
This follows the coding guidelines requirement to document lint behavior in doc comments.
66-80: Clean use of shareduse_tree_to_stringsutility.The check_use_item function correctly uses the shared utility from lint_utils to get all import paths and checks each against HTTP_PATTERNS. This addresses the past review about handling all nested use items.
239-251: Tests follow required conventions.The test module includes:
ui_examples()usingdylint_testing::ui_test_examplesfor external dependencies (http, axum)test_comment_annotations_match_stderr()as required by coding guidelinesdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/good_domain_0308.rs (1)
1-19: Good negative test case following UI test conventions.This file correctly:
- Includes a no-violations test case as required by coding guidelines
- Uses
simulated_dirto simulate domain path- Places "Should not trigger" comments before non-HTTP imports
- Demonstrates the recommended pattern with a transport-agnostic
DomainResultenumdylint_lints/de03_domain_layer/de0308_no_http_in_domain/ui/trait_bounds_http.rs (1)
1-17: Test file structure and annotation placement are correct.The test properly:
- Places "Should trigger" comments on the line before each function declaration
- Uses
simulated_dirto simulate a domain path- Tests HTTP types appearing in
impl Traitreturn type boundsThe stderr file correctly reports two DE0308 errors at lines 9 and 14, matching the annotation comments. The lint implementation successfully detects
axum::http::header::IntoHeaderNameas a direct HTTP trait in the bounds.Note: The current implementation checks trait paths but doesn't recurse into generic arguments within bounds (e.g.,
Iterator<Item = http::StatusCode>). This test covers only direct HTTP traits, which are caught correctly. Associated type constraints within generics remain outside the current scope.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/src/lib.rs (3)
10-51: LGTM - Lint declaration follows guidelines.The lint is correctly declared using
declare_early_lint!with comprehensive doc comments explaining what it does, why violations are bad, and providing good/bad examples. This follows the coding guidelines for documenting lint behavior in doc comments.
205-250: LGTM - EarlyLintPass implementation is comprehensive.The lint correctly scopes to domain paths using
is_in_domain_pathand handles the key item kinds:Use,Struct,Enum,Fn, andTyAlias. This covers the main vectors for infrastructure leakage.
252-264: LGTM - Required tests are present.The implementation includes both the UI test via
ui_test_examplesand the requiredtest_comment_annotations_match_stderrtest using thelint_utilshelper. Based on learnings, this is mandatory for lint crates.dylint_lints/de03_domain_layer/de0301_no_infra_in_domain/ui/bad_infra_import.stderr (1)
1-27: LGTM - Test artifact correctly captures expected diagnostics.The stderr file properly documents the expected lint output for infrastructure imports (sea_orm, sqlx, axum) in domain code. Error messages are clear, include the lint code (DE0301), and provide actionable help text pointing to the correct resolution.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Change ### to ## for proper h2 level headings under main h1 title. Addresses CodeRabbit review feedback. Signed-off-by: KvizadSaderah <kvizad@gmail.com>
nanoandrew4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just now realizing after Mikes comment on #247 how brittle a lot of our lints are, since we may ban sqlx and sea-orm but there are other alternatives that the LLM could use to skirt around these restrictions. I'm not sure there is anything that can be done about it, besides accepting that some of these lints should give LLMs a limited level of guidance, but won't stop them from going against the guidelines.
| let path_str = path.segments.iter() | ||
| .map(|seg| seg.ident.name.as_str()) | ||
| .collect::<Vec<_>>() | ||
| .join("::"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conversion from path to path_str is done repeatedly throughout the lints, and perhaps in other lints as well, it would be nice to move it to the lint_utils lib to avoid unnecessary repetition.
nanoandrew4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After further review I've found that all three lints miss some scenarios and can be circumvented. Here are some examples that should fail but pass:
// Test file for DE0706: No Direct sqlx Usage via Qualified Paths
// This file demonstrates BAD patterns using fully qualified paths without imports
#![allow(unused_imports)]
#![allow(dead_code)]
// Should trigger DE0706 - sqlx
struct Database {
pool: sqlx::pool::Pool<sqlx::Any>,
}
// Should trigger DE0706 - sqlx
async fn query_users(pool: &sqlx::pool::Pool<sqlx::Any>) -> Result<(), sqlx::Error> {
Ok(())
}
// Should trigger DE0706 - sqlx
type DbPool = sqlx::pool::Pool<sqlx::Any>;
// Should trigger DE0706 - sqlx
enum DatabaseConfig {
Any(sqlx::pool::PoolOptions<sqlx::Any>),
}
fn main() {}// simulated_dir=/hyperspot/modules/some_module/domain/service.rs
// Test file for DE0301: Circumvention via expressions in function bodies
// This file demonstrates potential circumvention using qualified paths in expressions
#![allow(unused_imports)]
#![allow(dead_code)]
// Should trigger DE0301 - infra in domain (via local variable type annotation)
fn use_sqlx_in_local() {
let pool: sqlx::pool::Pool<sqlx::Any> = todo!();
}
// Should trigger DE0301 - infra in domain (via turbofish)
fn use_sqlx_turbofish() {
let _result = some_generic_fn::<sqlx::pool::Pool<sqlx::Any>>();
}
// Should trigger DE0301 - infra in domain (via as cast)
fn use_sqlx_cast() {
let _val = some_value() as sqlx::Error;
}
fn some_generic_fn<T>() -> T {
todo!()
}
fn some_value() -> i32 {
42
}
fn main() {}// simulated_dir=/hyperspot/modules/some_module/domain/service.rs
// Test file for DE0308: Circumvention via expressions in function bodies
// This file demonstrates potential circumvention using qualified paths in expressions
#![allow(unused_imports)]
#![allow(dead_code)]
// Should trigger DE0308 - HTTP in domain (via local variable type annotation)
fn use_http_in_local() {
let status: http::StatusCode = todo!();
}
// Should trigger DE0308 - HTTP in domain (via turbofish)
fn use_http_turbofish() {
let _result = some_generic_fn::<http::StatusCode>();
}
// Should trigger DE0308 - HTTP in domain (via as cast)
fn use_http_cast() {
let _val = some_value() as http::Method;
}
// Should trigger DE0308 - HTTP in domain (via nested generic in local)
fn use_http_nested() {
let _opt: Option<http::HeaderMap> = None;
}
fn some_generic_fn<T>() -> T {
todo!()
}
fn some_value() -> i32 {
42
}
fn main() {}Please review these examples to tweak the lints to be more robust. AI can help you identify possible holes in the implementation that should be caught by the lint but aren't.
|
We stop the PR for now until we discuss a proper implementation of safeguards by using traits, generics and macros. |
Overview
Add three new dylint rules for architectural enforcement.
Relates to #68 (DyLint Top-10 Implementation)
New Lints
Changes
de03_domain_layer/de0301_no_infra_in_domain/de03_domain_layer/de0308_no_http_in_domain/de07_security/de0706_no_direct_sqlx/lint_utils/src/lib.rswith helper functionsCargo.tomlwith new workspace membersTesting
make dylint-test✅make dylint✅This is a clean PR from upstream/main, replacing #224 which had merge conflicts.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.