# Nix Hygiene Review Lens

Review Nix/NixOS configurations for **code quality, anti-patterns, and maintainability**.

## Linter Integration

This lens is backed by static analysis tools. Run first:

```bash
# Dead code detection
deadnix --fail .

# Anti-pattern detection
statix check .
```

Focus LLM review on semantic issues linters can't catch.

## What to Look For

### Dead Code (deadnix)
- Unused let bindings
- Unused function arguments (use `_` prefix if intentional)
- Unused imports in module args
- Unreachable code paths

### Anti-Patterns (statix)
- `with pkgs;` or `with lib;` abuse (prefer explicit references)
- `rec {}` when `let ... in` would be cleaner
- Manual `callPackage` instead of overlay
- IFD (import-from-derivation) without clear justification
- `builtins.toPath` (deprecated)
- Empty patterns in conditionals

### Module Design
- Options without type annotations (`types.str`, `types.listOf`, etc.)
- Missing `mkEnableOption` for boolean enable flags
- Options without descriptions or examples
- `config` references in `options` (evaluation order issues)
- Circular imports between modules
- Missing `lib.mkIf cfg.enable` guards around config blocks
- Misuse of `lib.mkDefault` vs `lib.mkForce` (prefer mkDefault for overridable)
- Missing `assertions` for invalid configuration detection

### Flake Hygiene
- Missing `flake.lock` (should be committed)
- Stale lock file (very old nixpkgs revision)
- Unused flake inputs
- `follows` mismatches causing duplicate nixpkgs
- Missing `nixConfig` for substituters

### Overlay & Override Issues
- `override` vs `overrideAttrs` confusion
- Overlays that don't compose (reference `final` vs `prev` correctly)
- `callPackage` in overlay without proper scoping
- Fixed-point (overlay) evaluated too early

### Reproducibility & Purity
- `import <nixpkgs> {}` (impure, non-reproducible)
- `builtins.getEnv` or `builtins.currentTime` (impure)
- Missing `hash` on `fetchTarball`, `fetchGit`, `fetchurl`
- Reliance on `NIX_PATH` in flake-based projects

### Build Efficiency
- `src = ./.` without filtering (rebuilds on README changes)
- Missing `lib.cleanSource` or `nix-gitignore`
- `builtins.readDir` on large trees at eval time

### NixOS-Specific
- `environment.systemPackages` bloat (prefer per-user or per-service)
- `system.stateVersion` modifications (should never change after install)
- `nixpkgs.config.allowUnfree = true` globally (prefer per-package)
- Imperative state in `system.activationScripts` without guards

## Output Format

```
[NIX-HYGIENE] <severity:HIGH|MED|LOW> <file:line>
Issue: <anti-pattern or code quality issue>
Linter: <deadnix|statix|manual> if applicable
Suggest: <cleaner alternative>
Evidence: <pattern found>
```

## Common Fixes

```nix
# Bad: with abuse
{ pkgs, ... }: with pkgs; [ git vim curl ]

# Good: explicit references (no 'with')
{ pkgs, ... }: [ pkgs.git pkgs.vim pkgs.curl ]

# Bad: missing type
options.myOption = mkOption { default = ""; };

# Good: typed option
options.myOption = mkOption {
  type = types.str;
  default = "";
  description = "What this option does";
};

# Bad: unused binding
let
  foo = "unused";  # deadnix will flag
  bar = "used";
in bar

# Good: remove or prefix with _
let
  _foo = "intentionally unused";
  bar = "used";
in bar
```

## False Positive Risks

- `_` prefixed args are intentionally unused (function signature compatibility)
- Some `with` usage is acceptable in small scopes (e.g., `meta = with lib; { ... }`)
- IFD may be justified for code generation or complex derivations
- Flake inputs may be used transitively via `follows`
- `rec {}` is standard in `mkDerivation` for referencing `version` in `src`
- `allowUnfree = true` globally may be deliberate policy for workstations
- `systemPackages` bloat is acceptable for single-user immutable systems

## Guidelines

- **HIGH** = evaluation errors, circular deps, type mismatches, stale security-critical lock
- **MED** = anti-patterns, missing types, dead code, module design issues
- **LOW** = style preferences, minor cleanup, verbose but working code
- Run deadnix and statix first - don't duplicate their findings
- Focus on semantic issues: module boundaries, design patterns, maintainability
- `with` is not inherently evil - context matters
