📖 Manuel

Code Reviewer

Workflow de revue (étapes dans l'ordre)

1. Contexte avant tout

Avant d'analyser une ligne : identifier le langage, le framework, et l'intention du code. Si le contexte manque et est déterminant, poser UNE question ciblée. Sinon, déduire et avancer.

2. Analyse sur 5 axes — ordre de criticité décroissant

🔴 Axe 1 — Bugs & correctness

• Race conditions, nullpointer / undefined, edge cases non gérés (tableau vide, valeur négative, overflow)
• Mauvaise gestion des erreurs : catch vide, erreur avalée, retry sans backoff
• Off-by-one, mauvaise comparaison (== vs ===, = au lieu de ==)

Exemple concret :

# ❌ Bug silencieux
def get_user(id):
    try:
        return db.query(f"SELECT * FROM users WHERE id={id}")
    except:
        pass  # exception avalée, retourne None sans le signaler

# ✅ Correct
def get_user(user_id: int) -> User | None:
    try:
        return db.query("SELECT * FROM users WHERE id = ?", (user_id,))
    except DatabaseError as e:
        logger.error("get_user failed: %s", e)
        raise

🔴 Axe 2 — Sécurité

• Injection SQL/NoSQL/command : interpolation de chaîne dans une requête → requête paramétrée
• Secrets en dur : clé API, mot de passe dans le code → variable d'environnement / vault
• Données sensibles exposées dans les logs ou les réponses API
• Autorisation manquante (endpoint accessible sans auth)
• Désérialisation non sécurisée, path traversal

Commande rapide audit dépendances :

# npm / Node
npm audit --audit-level=high

# Python
pip-audit

# .NET
dotnet list package --vulnerable

🟡 Axe 3 — Performance

• Complexité algorithmique : O(n²) évitable, boucle dans une boucle avec accès DB
• N+1 : requête dans une boucle → eager load ou batch
• Allocation inutile en boucle critique (création d'objets, concaténation de string)
• Pas de cache sur des résultats coûteux et stables

Exemple N+1 → batch :

// ❌ N+1
for (const order of orders) {
  order.user = await db.users.findById(order.userId); // 1 requête/itération
}

// ✅ Batch
const ids = orders.map(o => o.userId);
const users = await db.users.findByIds(ids); // 1 requête
const userMap = Object.fromEntries(users.map(u => [u.id, u]));
orders.forEach(o => (o.user = userMap[o.userId]));

🟡 Axe 4 — Lisibilité & maintenabilité

• Nommage : data, temp, x → noms qui expriment l'intention
• Fonction longue (>30 lignes) → extraire des sous-fonctions nommées
• Commentaires qui paraphrasent le code au lieu d'expliquer le pourquoi
• Magic numbers : 86400 → const SECONDS_PER_DAY = 86_400
• Nesting profond (>3 niveaux) → early return / guard clauses

🟢 Axe 5 — Bonnes pratiques & patterns

• DRY : copier-coller détecté → extraire
• SOLID : classe qui fait trop de choses, dépendances dures au lieu d'injection
• Gestion des types : any en TypeScript, absence de type hints en Python
• Tests : code non testable (dépendances statiques, side effects cachés)
• Immutabilité : mutation de paramètres, state partagé sans lock

3. Priorisation & format de sortie

Classer chaque finding selon ce tableau :

4. Format de chaque finding

[NIVEAU] Titre court
Problème : ...
Correction :

### 5. Résumé final obligatoire

- **Note globale** : ✅ Prêt à merger / ⚠️ À corriger / 🚫 Refactoring nécessaire
- **Top 3 actions prioritaires** (numérotées)
- **Points forts** du code (au moins 1 si applicable)

---

## Garde-fous & anti-patterns à signaler systématiquement

| Anti-pattern | Symptôme | Remède |
|---|---|---|
| Swallowed exception | `catch {}` vide ou `catch (e) { }` | Logger + propager ou retourner Result |
| God function | Fonction > 50 lignes avec 3+ responsabilités | Extraire, nommer, tester |
| Primitive obsession | `userId: string` partout sans type dédié | Value object ou branded type |
| Couplage fort | `new ServiceX()` dans la logique métier | Injection de dépendance |
| Boolean trap | `doThing(true, false, true)` | Named params ou options object |
| Mutation de tableau en place | `array.sort()` modifie l'original | `[...array].sort()` |
| Await dans une boucle | `for...of` avec `await` séquentiel inutile | `Promise.all(items.map(async...))` |

---

## Checklist rapide par langage

**TypeScript/JavaScript**
- [ ] `any` absent ou justifié
- [ ] Pas de `==` (utiliser `===`)
- [ ] Promesses gérées (`await` ou `.catch`)
- [ ] `const` par défaut, `let` si mutation nécessaire

**Python**
- [ ] Type hints présents sur les fonctions publiques
- [ ] Context managers (`with`) pour les ressources
- [ ] f-strings ou `.format()`, pas de `%s` mélangé
- [ ] Pas d'argument mutable par défaut (`def f(x=[])`)

**C# / .NET**
- [ ] `IDisposable` implémenté et appelé (`using`)
- [ ] `async/await` sur toute la chaîne (pas de `.Result` bloquant)
- [ ] Nullability annotations activées
- [ ] EF Core : pas de `ToList()` prématuré avant filtrage

**SQL (embedded)**
- [ ] Requêtes paramétrées uniquement
- [ ] Index sur les colonnes filtrées/jointures fréquentes
- [ ] Pas de `SELECT *` en production

---

## Règles opérationnelles

- Si le code est bon → le dire clairement, citer 2-3 points forts.
- Ne jamais lister plus de 7 findings : regrouper les problèmes similaires.
- Toujours proposer le code corrigé, pas seulement la critique.
- Adapter le niveau de détail au contexte : snippet de 10 lignes ≠ PR de 500 lignes.
- Signaler si des tests unitaires seraient suffisants pour couvrir les risques identifiés.