SuperAPI AI Directory
Partner with us中文AI newsFavorites
Skillsrules-review

rules-review

Coding & development

View repository

Updated 10-15 19:54

About this skill

altium-cli Rules Review

Audit Rust source files against the project rules below. For each violation found, report:

  1. Rule violated (by name from the checklist)
  2. File and line (e.g., crates/altium-format/src/schlib.rs:123)
  3. What's wrong (concrete description)
  4. How to fix (specific suggestion)

At the end, give a summary: total violations found, grouped by severity (CRITICAL / WARNING).

Review Checklist

CRITICAL Rules (must never be violated)

R1: Fail Fast — No Silent Skipping

The parser must never silently skip data it doesn't understand. If a stream, record, or field exists in the file, the parser MUST either fully parse it or return an error. Look for:

  • Any form of skip_known, ignore_remaining, or _ => {} catch-all match arms that discard data
  • Marking entries as consumed in TrackedCfbDocument without actually reading and parsing their contents
  • Deferring parsing to "future milestones" by suppressing errors
  • Circumventing assert_all_consumed() in any way
  • .ok(), .unwrap_or_default(), or let _ = on parse results that silently drop errors

R2: No Raw Primitive Types

Never use raw primitive integers where domain types exist in altium-format-types. Look for:

  • u8 where PcbObjectId should be used
  • i32 where SchRecordType, Coord, or other typed values should be used
  • Raw integer literals for constants (e.g., 0xD0 instead of INSTRUCTION_BINARY, 0x00FF_FFFF instead of BLOCK_SIZE_MASK)
  • Any struct field using a primitive integer type when a domain type exists or should be created
  • Note: Rust String is correct for decoded text — Altium's own C# code uses plain .NET string. The encoding is a property of the serialization context, not the type.

R2b: Strict Encoding at Parse Boundaries

All byte-to-string decoding MUST use strict (non-lossy) functions. Look for:

  • from_utf8_lossy() — ALWAYS a violation; use from_utf8() with error propagation or Windows-1252 decode
  • Discarded had_errors flag on encoding_rs::UTF_16LE.decode() calls — MUST be checked
  • Note: encoding_rs::WINDOWS_1252.decode() maps all 256 byte values and cannot error, so discarding its flags is safe

R3: No Silent Error Dropping

Everything fallible MUST return Result<T, AltiumFormatError> (in altium-format) or Result<T, SpecError> (in autopcb-spec). Look for:

  • .unwrap() in non-test code
  • .expect() in library code (acceptable in tests and CLI)
  • Swallowing errors with if let Ok(x) = ... without handling the error case
  • Using Option where Result is appropriate for fallible operations
  • Missing ? propagation where errors should bubble up

R4: Crate Privacy — No Implementation Leaks

altium-format implementation details must stay private to the crate. Look for:

  • pub visibility on internal parsing structs/functions that shouldn't be exposed
  • autopcb-spec or altium-cli directly accessing altium-format internals
  • Types or functions that should be pub(crate) but are pub

R5: No Unconsumed Data Suppression

Do NOT mark streams, records, or fields as "consumed" without actually parsing them. Look for:

  • Calls to consume/mark-consumed methods without preceding parse logic
  • Empty implementations that just mark things as handled
  • TODOs or comments like "parse later" with suppressed errors

WARNING Rules (should be followed, deviations need justification)

R6: Error Type Correctness

Each crate uses its own error type:

  • altium-format → AltiumFormatError
  • autopcb-spec → SpecError
  • altium-cli → anyhow Look for error types used in the wrong crate.

R7: Domain Types in altium-format-types

New domain types belong in altium-format-types, not inline in other crates. Look for:

  • New enums or type aliases defined in altium-format that represent Altium concepts
  • Constants defined outside of altium-format-types/src/constants/

R8: Constants from FileFormatConsts

All file format constants should come from altium_format_types::constants::* (mirroring FileFormatConsts.cs). Look for:

  • Hard-coded magic numbers that correspond to known Altium constants
  • String literals for stream names, record type names, or parameter keys that should be constants

R9: Dependency Direction

The dependency graph must be strictly:

altium-format-types → altium-format-derive → altium-format → autopcb-spec → altium-cli

Look for reverse dependencies or circular imports.

R10: Result Over Panic

Library code (everything except altium-cli) should return Result instead of panicking. Look for:

  • panic!(), unreachable!() (when the case IS reachable), todo!() in non-test code
  • assert!() in production code paths (acceptable in tests)

How to Review

1. Determine scope

If the user specifies a scope, use it. Otherwise, audit all Rust source files across the workspace:

crates/altium-format/src/**/*.rs
crates/altium-format-derive/src/**/*.rs
crates/altium-format-types/src/**/*.rs
crates/autopcb-spec/src/**/*.rs
crates/altium-cli/src/**/*.rs

Acceptable scope specifiers from the user:

  • A crate name: e.g., "altium-format" → audit all .rs files in that crate
  • A file or glob: e.g., "schlib.rs" or "src/param_*.rs"
  • "changes" or "diff": audit only git changes (git diff HEAD)
  • A commit or PR: audit that specific changeset
  • No argument: audit the entire workspace

2. Scan systematically

For each file in scope, use Grep and Read to check for violations. Efficient search patterns:

Rule Grep patterns to try
R1 _ => \{\}, _ => Ok\(\), skip, ignore_remaining, .ok() on parse results
R2 Struct fields with : u8, : i32, : u32, : i64, : u64; hex literals 0x[0-9a-fA-F]+
R2b from_utf8_lossy, decode_without_bom_handling with discarded had_errors on UTF-16
R3 .unwrap(), .expect(, if let Ok(, let _ =
R4 pub fn and pub struct in altium-format (check if they should be pub(crate))
R5 mark_consumed, set_consumed, consumed = true without adjacent parsing
R6 anyhow in library crates, AltiumFormatError in spec/ops crates
R7 enum definitions in altium-format that look like Altium domain concepts
R8 String literals that look like stream/record names, numeric magic constants
R9 Check Cargo.toml files for dependency direction violations
R10 panic!, unreachable!, todo!, assert! in non-test code

3. Use context to reduce false positives

  • .unwrap() and .expect() in #[cfg(test)] modules or test files are fine
  • assert! in test code is fine
  • pub on items that are part of the crate's intended public API is fine — focus on items that look like internal parsing machinery
  • Some _ => arms legitimately handle a finite set — check if data is being discarded
  • Raw types in proc macro code (altium-format-derive) may be acceptable since it generates code

4. Report findings

Order by severity (CRITICAL first, then WARNING), then by file path. Use this format:

## Findings

### CRITICAL

**R2: No Raw Primitive Types** — `crates/altium-format/src/schlib.rs:45`
Field `component_count: u32` uses raw primitive. Should use a domain type or at minimum document why a raw type is necessary here.
**Fix**: Define an appropriate type in `altium-format-types` or use an existing one.

### WARNING

**R8: Constants from FileFormatConsts** — `crates/altium-format/src/pcblib.rs:112`
Hard-coded string `"Board6"` should use a constant from `altium_format_types::constants`.
**Fix**: Add `pub const BOARD6: &str = "Board6";` to the appropriate constants module.

## Summary

| Severity | Count |
|----------|-------|
| CRITICAL | 1 |
| WARNING  | 1 |
| **Total** | **2** |

5. If no violations found

Explicitly state: "No rule violations found in the reviewed scope." and list which files were checked.