Prompts de revue de code : 16 contrôles prêts
Seize extraits portant chacun un vrai défaut, avec la question qu'un relecteur poserait dessus. Chaque carte règle déjà le langage, le point de revue de code visé et l'environnement d'exécution : remplacez l'exemple par votre code et appliquez.
Bugs et logique
Du code qui compile et ment quand même : trouver un bug qui ne lève aucune erreur, un argument mutable par défaut en Python, un zéro pris pour une absence, des heures sans fuseau et une régression glissée dans une pull request.
Argument mutable par défaut
Le piège classique de Python : la liste de la signature survit à l’appel et accumule les données des autres.
Relis cette fonction de panier : des clients trouvent dans leur commande des articles qu’ils n’ont jamais ajoutés.def add_item(item, cart=[]): cart.append(item) return cart
Ligne 1 · critique · bugcart=[] n’est créé qu’une fois, à l’import du module, et non à chaque appel : le deuxième client continue de remplir le panier du premier, et ainsi de suite jusqu’au redémarrage du processus.Correctif : cart: list | None = None, puis en première ligne du corps if cart is None: cart = [].
Un zéro pris pour une absence
Le test de vérité masque un zéro légitime, et le client au solde vide voit un texte de remplacement.
Regarde ce test : distingue-t-il vraiment une valeur absente d’un zéro ?def render_balance(user): balance = user.get("balance") if not balance: return "aucune donnée" return format_money(balance)
Ligne 3 · majeur · bugif not balance attrape aussi bien None que 0 et 0.0 : celui qui a dépensé jusqu’au dernier centime voit « aucune donnée » au lieu d’un zéro honnête.Correctif : if balance is None — l’absence se teste explicitement et le zéro part au formatage comme n’importe quel nombre.
Comparer des heures sans fuseau
datetime.now() renvoie l’heure locale du serveur alors que l’expiration du jeton est stockée en UTC.
Vérifie le calcul d’expiration du jeton. Le service tourne sur des serveurs de plusieurs fuseaux.def is_expired(token): return datetime.now() > token.expires_at
Ligne 2 · critique · bugdatetime.now() est une heure locale naïve, expires_at sort de la base en UTC : sur un serveur à Paris les jetons vivent deux heures de trop et, face à une valeur aware, la comparaison échoue tout de suite en TypeError.Correctif : datetime.now(timezone.utc) et une règle pour tout le projet — on ne stocke que de l’aware en UTC.
Régression dans une pull request
Une relecture du changement plutôt que du fichier : ce que ce commit emporte en production.
Relis ce diff de pull request : qu’est-ce qui part exactement en production avec lui ?@@ -12,8 +12,7 @@ def apply_discount(order, percent): if order.paid: raise ValueError("order already paid")- if percent > 90:- raise ValueError("discount too large") order.total = order.total * (100 - percent) / 100+ order.discount_percent = percent
Lignes 15-16 · critique · bugLe contrôle supprimé était la seule borne sur percent : à 150 le total de la commande devient négatif et la caisse en fait un remboursement.Correctif : remettre le contrôle ou le déplacer dans la validation de la requête, et ajouter le cas percent=150 aux tests.
Sécurité
Là où une saisie extérieure atteint la base et les fichiers : repérer une faille de sécurité avant la mise en production, une injection SQL dans un champ de recherche, un jeton en dur et un chemin qui sort du dossier d'upload.
Injection SQL dans la recherche
L’adresse venue du formulaire est collée dans le SQL : celui qui remplit le formulaire pilote désormais la base.
Relis cette recherche d’utilisateur. L’e-mail vient directement d’un formulaire du site.def find_user(conn, email): query = "SELECT * FROM users WHERE email = '" + email + "'" return conn.execute(query).fetchone()
Ligne 2 · critique · sécuritéemail est collé dans le texte SQL : une valeur contenant une apostrophe referme la condition et tout ce qui suit est exécuté par la base comme ta propre requête, DROP TABLE compris.Correctif : un paramètre plutôt qu’une concaténation — conn.execute("SELECT id, email FROM users WHERE email = %s", [email]) ; au passage disparaît le SELECT * qui ramène aussi le hash du mot de passe.
Jeton en dur dans le script
Une clé de production dort dans le dépôt et s’imprime au passage dans le journal de déploiement.
Relis ce script de déploiement : y a-t-il ici quelque chose de dangereux côté sécurité ?#!/usr/bin/env bashAPI_TOKEN="sk_live_9f3c1ad84b22"echo "deploying with $API_TOKEN"curl -H "Authorization: Bearer $API_TOKEN" -X POST https://api.example.com/deploy
Lignes 2-3 · critique · sécuritéUn jeton de production est écrit dans un fichier versionné : tous ceux qui ont cloné le dépôt l’ont dans leur historique, et echo le recopie dans le journal de déploiement, que bien plus de monde lit.Correctif : le lire dans l’environnement ($API_TOKEN sans valeur par défaut), supprimer le echo et considérer la clé comme fuitée — donc la faire tourner.
Sortie du dossier d’upload
Le nom de fichier est repris tel quel de la requête, et deux points suivis d’une barre mènent à n’importe quel fichier du serveur.
Relis ce point d’entrée de téléchargement : le nom du fichier arrive dans la query.def download(request): name = request.args.get("file") path = os.path.join("/var/app/uploads", name) return send_file(path)
Ligne 3 · critique · sécuritéos.path.join quitte sans broncher /var/app/uploads quand le nom vaut ../../etc/passwd, et un chemin absolu écrase carrément le premier argument : tout fichier lisible par le processus devient téléchargeable.Correctif : os.path.basename(name), puis comparer os.path.realpath du résultat au dossier d’upload et ne servir le fichier que s’il est à l’intérieur.
Performance
Ce qui tient en test et s'écroule ensuite : optimiser les performances d'un code lent, sortir les requêtes d'une boucle, repérer celle qui n'utilise pas l'index et lire un gros journal sans le charger d'un bloc en mémoire.
Requêtes dans la boucle
Le rapport fait deux allers-retours en base par utilisateur : mille lignes deviennent deux mille requêtes.
Regarde ce rapport : il met une minute pour mille utilisateurs. Où passe le temps ?def orders_report(user_ids): rows = [] for user_id in user_ids: user = db.query("SELECT name FROM users WHERE id = %s", user_id) orders = db.query("SELECT total FROM orders WHERE user_id = %s", user_id) rows.append((user.name, sum(o.total for o in orders))) return rows
Lignes 3-5 · majeur · performanceDeux requêtes par utilisateur : mille utilisateurs, donc deux mille allers-retours, et le temps part dans le réseau, pas dans le calcul.Correctif : une requête avec JOIN et GROUP BY users.id, ou deux requêtes avec IN et le rapprochement fait en mémoire.
Requête qui rate l’index
Une fonction sur la colonne et un pourcentage en tête de LIKE éteignent les index : la base lit toute la table.
Relis cette requête : sur une table de dix millions de lignes elle met vingt secondes.SELECT *FROM ordersWHERE date_trunc('day', created_at) = '2026-09-01' AND lower(email) LIKE '%@example.com'ORDER BY created_at DESC
Lignes 3-4 · majeur · performancedate_trunc sur created_at rend l’index inutile, et un LIKE commençant par un pourcentage ne peut de toute façon pas s’en servir : il reste un seq scan sur la table entière, et SELECT * ramène en plus des colonnes que personne ne lit.Correctif : comparer created_at à un intervalle (à partir de minuit le 1er septembre et plus petit que minuit le 2), indexer le domaine à part ou le stocker dans sa propre colonne, et ne sélectionner que les champs utilisés.
Le journal entier en mémoire
Le fichier est lu d’un seul coup : la taille du journal devient la taille du processus.
Relis ce compteur d’erreurs dans un journal. Les fichiers font plusieurs gigaoctets.def count_errors(path): lines = open(path).read().split("\n") return len([line for line in lines if "ERROR" in line])
Ligne 2 · majeur · performanceread() charge tout le fichier en mémoire, split double la note et la liste dans len() en garde une troisième copie : sur huit gigaoctets de journal, l’OOM killer arrive avant le résultat. Le fichier n’est d’ailleurs jamais fermé.Correctif : with open(path) as f et sum(1 for line in f if "ERROR" in line) — ligne à ligne, à mémoire constante.
Lisibilité et conventions
Le code tourne et reste illisible : aplatir quatre conditions imbriquées, rendre un code plus lisible sans le réécrire, suivre les conventions de nommage du langage et sortir le taux de TVA recopié dans deux fonctions.
Quatre conditions imbriquées
La règle d’envoi du message se cache au cinquième niveau d’indentation et ne se lit que d’un bloc.
Évalue la lisibilité de cette fonction : il faut la lire jusqu’au bout pour connaître la condition.def notify(user): if user is not None: if user.email is not None: if user.subscribed: if not user.banned: send_email(user.email) return True return False
Lignes 2-5 · mineur · lisibilitéQuatre if imbriqués, c’est une seule règle étalée en escalier : pour savoir qui reçoit le message il faut tenir les quatre conditions en tête à la fois, alors que PEP 8 demande la forme plate.Correctif : des retours anticipés — if user is None: return False et ainsi de suite — et le corps reste sur un seul niveau d’indentation.
Des noms contre la convention
Une méthode avec une majuscule et des variables d’une lettre : RuboCop râle, et le prochain lecteur aussi.
Relis cette méthode avec RuboCop : qu’est-ce qui va ici contre le style Ruby habituel ?def CalcTotal(o) t = 0 o.each do |i| t = t + i.price * i.qty end tend
Lignes 1-4 · mineur · conventionsNaming/MethodName : en Ruby un nom de méthode s’écrit en snake_case, et le CamelCase se lit ici comme une constante. Les noms o, t et i ne disent rien de leur contenu, et cumuler à la main est exactement le travail de sum.Correctif : def calc_total(items) et items.sum do |item| item.price * item.qty end — quatre lignes tiennent en une.
La TVA à deux endroits
Une même règle recopiée dans deux fonctions, et déjà divergente : 20 pour cent sur la facture, 19 sur le reçu.
Relis ces deux fonctions : calculent-elles la même chose ?def invoice_total(order): return round(order.subtotal * 1.2, 2)def receipt_total(order): return round(order.subtotal * 1.19, 2)
Lignes 2 et 5 · majeur · architectureUne règle métier écrite deux fois a déjà divergé : la facture applique 20 pour cent, le reçu 19, et le client voit deux totaux différents pour une seule commande.Correctif : une constante VAT_RATE et une fonction unique appelée par les deux — le taux ne se change plus qu’à un seul endroit.
Tests et fiabilité
Ce qui se passe le jour où ça casse : améliorer la couverture de tests au-delà du seul cas heureux, retrouver une exception attrapée puis ignorée en silence et vérifier qu'une sauvegarde s'est réellement écrite.
Test du seul cas heureux
Un unique cas au vert donne l’impression d’une couverture qui n’existe pas.
Évalue ces tests d’apply_discount : que leur manque-t-il ?def test_apply_discount(): order = Order(total=100) apply_discount(order, 10) assert order.total == 90
Ligne 1 · majeur · testsUn seul cas est couvert : une remise ordinaire sur une commande non payée. Rien sur zéro, sur cent pour cent, sur une valeur négative, sur une commande déjà payée ni sur l’arrondi de 33,33 — chacune de ces branches peut casser sans bruit.Correctif : parametrize sur les valeurs limites et un test dédié avec pytest.raises pour percent=150 et pour la commande déjà payée.
Exception avalée
except Exception: pass transforme une panne en « ok » et efface la trace des journaux.
Relis cet enregistrement de profil : des utilisateurs disent que leurs modifications disparaissent parfois.def save_profile(user, data): try: db.update(user.id, data) search.reindex(user.id) except Exception: pass return "ok"
Lignes 5-6 · critique · bugexcept Exception: pass avale tout et renvoie "ok" même quand l’écriture n’a pas eu lieu : l’utilisateur voit un succès, la donnée manque et le journal est vide. Avec deux opérations sous un même try, une réindexation en échec ne se distingue plus d’une base en échec.Correctif : attraper les exceptions précises, journaliser avec logger.exception et renvoyer un statut honnête ; sortir la réindexation pour que son échec n’annule pas l’enregistrement.
Sauvegarde sans vérification
Le script ignore les codes de retour et supprime les anciennes copies avant qu’on découvre que la nouvelle est vide.
Relis ce script de sauvegarde nocturne : la restauration depuis la dernière copie a échoué.#!/usr/bin/env bashpg_dump "$DATABASE_URL" > /backup/db.sqlgzip -f /backup/db.sqlfind /backup -name "db.sql.gz" -mtime +7 -delete
Lignes 2-4 · critique · bugIl manque set -euo pipefail : si pg_dump échoue le fichier est créé quand même, vide, gzip le compresse sans broncher et find supprime les copies valides de plus d’une semaine ; au bout de sept jours il ne reste plus une seule sauvegarde intacte. Un $DATABASE_URL vide passe tout aussi inaperçu.Correctif : set -euo pipefail en première ligne, vérifier la taille du dump après pg_dump et ne supprimer les anciennes copies qu’une fois la nouvelle réussie.