Forge-Store/Forge-Engine
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects 1 Releases Wiki Activity
Files
55e81c0fbb9e99323a426380a0b87d2d8d3cc5b1
Forge-Engine/CODE_QUALITY.md
T
williamandClaude Sonnet 5 55e81c0fbb Ajoute CODE_QUALITY.md et corrige S7632 a la racine (helper assert_not_none) 
- Nouveau db/assert_not_none.py : centralise l'unique suppression
  Bandit/Ruff (# nosec B101 / # noqa: S101) de narrowing de type dans
  tout le moteur. Remplace 11 sites disperses (ai/chat.py, ai/tools.py,
  routes/scenes/scene_object_{add,collision,geometry,personnage_data,
  quiz_config}.py, screens/payload/full_game_payload.py,
  scripts/build_demo_dialogues.py) qui repetaient chacun le meme
  commentaire empile - Sonar (python:S7632) ne parse pas deux
  commentaires # sur une ligne, meme si Ruff et Bandit les acceptent
  chacun tres bien (contrainte structurelle documentee dans
  CODE_QUALITY.md : chaque outil exige son propre mot-cle immediatement
  apres un #, aucun format a un seul # ne peut satisfaire les deux a la
  fois). Les 6 sites # nosec B608 (SQL dynamique) restent inchanges,
  nature differente, hors perimetre de ce refactor.
  Verifie : mypy --strict propre (389 fichiers), ruff/bandit/import-linter
  clean, suite complete verte (591 tests), scan SonarQube local relance
  confirmant S7632 a 7 (1 seul site restant dans le helper lui-meme + les
  6 B608), 0 bug (une regression S8371 trouvee et corrigee en route).

- 4 sites |safe repositionnes sur leur ligne exacte (scene_edit.html,
  register_2fa.html, onboarding_new.html, play.html) - le marqueur
  NOSONAR etait sur la ligne precedente par erreur (meme piege que celui
  documente pour S8371 ci-dessus). Confirme par scan que meme corrige,
  l'analyseur Web de Sonar ne supporte aucune syntaxe de suppression
  inline testee pour la regle Web:S5247 - documente comme limitation
  technique connue dans CODE_QUALITY.md plutot que force.

- CODE_QUALITY.md (nouveau) : reference complete du dispositif qualite -
  vue d'ensemble par outil, configuration de chacun, commandes de lancement
  local, procedure de justification d'une exception (avec le format exact
  attendu par Bandit/Ruff/Sonar, verifie empiriquement), table des 10
  exceptions documentees, backlog (djLint H021, code smells Sonar).

djLint (H021, styles inline) volontairement saute pour ce commit - meme
backlog assume que les commits precedents, aucun rapport avec ce changement.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-16 12:41:02 +02:00

144 lines
15 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Qualité de code — Forge Engine
Référence de fonctionnement du dispositif qualité mis en place (Phases 1
à 4). Pour le compte-rendu chronologique de ce qui a été fait/trouvé/
décidé pendant la mise en place, voir `docs/SESSION_RECAP.md` — ce
fichier-ci documente uniquement **l'état actuel** et **comment
l'utiliser au quotidien**.
Règle de base, non négociable : **aucune règle de lint/typage n'est
désactivée globalement dans un fichier de config sans validation
explicite.** Une exception ponctuelle est toujours une ligne de code
(`# noqa`, `# nosec`, `// NOSONAR`) avec une raison précise, jamais un
ignore de fichier entier ou de règle globale — voir section 4.
## 1. Vue d'ensemble
| Outil | Vérifie | Portée | Obligatoire / Avertissement |
|---|---|---|---|
| **Ruff** (lint) | Erreurs Python, imports inutilisés, style | `*.py` | Obligatoire (bloquant CI + pre-commit) |
| **Ruff** (format) | Formatage Python | `*.py` | Obligatoire |
| **Mypy** `--strict` | Typage statique Python | `*.py` | Obligatoire |
| **Bandit** | Sécurité Python (injections, primitives faibles) | `ai`, `auth`, `core`, `db`, `filters`, `publish`, `routes`, `screens`, `scripts`, `app.py`, `build_css.py` | Obligatoire |
| **Vulture** | Code mort Python | mêmes dossiers que Bandit | Obligatoire |
| **import-linter** | Contrat de couches applicatives | tout le code Python | Obligatoire |
| **ESLint** (`airbnb-base`) | Lint JavaScript | `static/js/**/*.js` | Obligatoire |
| **Stylelint** (`stylelint-config-standard`) | Lint CSS | `styles/**/*.css` | Obligatoire |
| **djLint** | Lint des templates Jinja | `templates/**/*.html` | **Avertissement** — H021 (styles inline) en backlog assumé, voir section 6 |
| **SonarQube** (local + CI) | Bugs/vulnérabilités/code smells/hotspots agrégés | tout le dépôt | **Avertissement** — non-bloquant en CI pour l'instant, voir section 3 |
## 2. Configuration de chaque outil
### Ruff (`pyproject.toml`, `[tool.ruff]`)
- `line-length = 120`, `target-version = "py313"`.
- Règles activées : `F` (pyflakes), `E`/`W` (pycodestyle), `B` (bugbear), `C4` (comprehensions), `SIM` (simplification), `S` (bandit-équivalent Ruff), `I` (imports).
- `per-file-ignores` : `S101`/`S106` désactivées pour `tests/` uniquement (assert et mots de passe en dur sont la norme dans les tests, jamais en dehors).
- Exclusions : `projects/`, `user_assets/`, `data/`, `Bug/`, `regles/` (données runtime, jamais du code).
### Mypy (`pyproject.toml`, `[tool.mypy]`)
- `strict = true`, `python_version = "3.13"`.
- `explicit_package_bases = true` + `mypy_path = "."` : nécessaire car `tests/` n'a pas de `__init__.py` — sans ça mypy refuse de démarrer ("Source file found twice under different module names"). **Ce n'est pas un assouplissement de `strict`**, juste la résolution des chemins de module.
- Mêmes exclusions que Ruff.
### Bandit (`pyproject.toml`, `[tool.bandit]`)
- `exclude_dirs` : `projects`, `user_assets`, `data`, `Bug`, `regles`, `tests` (les tests contiennent des mots de passe/tokens de test en dur, jamais un vrai risque).
- Portée explicite (`-r ai auth core db filters publish routes screens scripts app.py build_css.py`) plutôt que tout le dépôt.
### Vulture (`pyproject.toml`, `[tool.vulture]`)
- `min_confidence = 80`, `paths` = mêmes dossiers que Bandit + `vulture_whitelist.py`.
- `exclude = ["*/tests/*"]`.
- Faux positifs structurels (routes Flask enregistrées par décorateur, hooks appelés par convention de nom) : whitelist dédiée dans `vulture_whitelist.py`, **jamais** un `min_confidence` abaissé globalement.
### import-linter (`pyproject.toml`, `[tool.importlinter]`)
- Un seul contrat de type `layers`, du haut vers le bas : `routes` → `ai | publish | core` → `screens` → `auth | filters` → `db`.
- Une couche ne peut importer qu'une couche strictement en dessous d'elle, jamais au-dessus, jamais une couche sœur du même niveau.
- `app.py` (point d'entrée, pas un paquet) reste hors contrat — c'est lui qui importe `routes`/`core`, jamais l'inverse.
### ESLint (`.eslintrc.json`)
- Base `airbnb-base` + plugin `unused-imports`.
- `max-len` porté à 120 (aligné sur Ruff) au lieu du 80 par défaut d'Airbnb.
- Plusieurs règles Airbnb désactivées **après vérification individuelle que le code existant les respecte déjà différemment** (voir section 5 pour le détail et le raisonnement de chacune) : `no-param-reassign`, `no-use-before-define` (assoupli pour les fonctions, gardé strict pour variables/classes), `func-names`, `no-underscore-dangle`, `no-console`, `no-plusplus`, `no-continue`, `no-bitwise`, `guard-for-in`, `no-restricted-syntax`, `prefer-destructuring`, `consistent-return`, `no-return-assign`, `no-nested-ternary`, `no-void`, `implicit-arrow-linebreak`, `no-unused-expressions`, `no-useless-concat`.
- `unused-imports/no-unused-vars` avec une liste blanche (`varsIgnorePattern`) d'environ 95 noms : fonctions invoquées uniquement depuis des attributs `onclick`/`onchange` inline dans les templates Jinja, invisibles pour l'analyse statique d'ESLint.
- Bloc `globals` documentant les variables cross-fichiers volontaires (ex. `gameData`, `screensData` — voir section 5).
### Stylelint (`.stylelintrc.json`)
- Base `stylelint-config-standard`.
- `selector-class-pattern`/`selector-id-pattern` désactivées : le projet utilise du camelCase pour ses classes/id CSS depuis le début (`.canvasElement`, `#scormProgress`...) — imposer le kebab-case du preset aurait demandé de renommer des milliers de sélecteurs et leurs usages JS/HTML pour un gain nul.
- `no-descending-specificity` désactivée : le CSS existant est organisé par composant/fonctionnalité, pas par ordre strict de spécificité.
- `ignoreFiles` : `static/vendor/**`, `static/style.css` (bundle généré, contient du Bulma vendored).
### djLint (`pyproject.toml`, `[tool.djlint]`)
- `profile = "jinja"`, `max_line_length = 160`, `indent = 2`.
- H021 (styles inline) : **backlog assumé, pas une exception corrigée** — voir section 6, ne pas confondre avec les vraies exceptions de la section 5.
### SonarQube
- **Local (développement continu)** : Docker Community Build + PostgreSQL (jamais la base H2 embarquée), tourne en permanence sur cette machine via `~/sonarqube-stack/docker-compose.yml` (WSL2/Ubuntu). Dashboard : **http://localhost:9000** — identifiants personnels, jamais consignés ici.
- **CI (`sonar.forgebase.fr`)** : instance séparée, self-hébergée, scan à chaque push via `.gitea/workflows/deploy.yml` (job `sonarqube`), **non-bloquant** (`continue-on-error: true`) le temps que le rapport soit entièrement trié.
## 3. Lancer les checks en local
```bash
# Un outil a la fois (memes commandes que .pre-commit-config.yaml)
ruff check .
ruff format --check .
mypy .
vulture
bandit -c pyproject.toml -r ai auth core db filters publish routes screens scripts app.py build_css.py
lint-imports
djlint templates
npx eslint "static/js/**/*.js"
npx stylelint "styles/**/*.css"
# Tout d'un coup (ce que fait un commit)
pre-commit run --all-files
# Suite de tests
python -m pytest tests/ -q
node --test static/js/play/__tests__/*.test.js static/js/scenes/__tests__/*.test.js
# Scan SonarQube local (rapport seul - jamais de correction automatique
# a partir de son seul resultat sans triage prealable, voir section 4)
# Necessite le stack Docker local demarre (voir section 2) et un token
# genere sur http://localhost:9000 (Mon compte > Security > Generate Token).
docker run --rm --network sonarqube-stack_default \
-v <chemin-absolu-du-repo>:/usr/src \
sonarsource/sonar-scanner-cli \
-Dsonar.host.url=http://sonarqube:9000 \
-Dsonar.token=<votre-token>
```
## 4. Procédure de justification d'une exception
1. **Jamais de correction mécanique sans comprendre la cause.** Avant de corriger un finding, déterminer s'il s'agit d'un vrai problème ou d'un faux positif dû au contexte du projet (fixture pytest, dispatch dynamique JS/Jinja, whitelist codée en dur, architecture volontaire).
2. **Une exception ponctuelle, jamais globale.** Le commentaire de suppression va **sur la ligne exacte** signalée par l'outil — pas la ligne au-dessus, pas la ligne en dessous (piège vécu à plusieurs reprises cette session : un commentaire mal placé ne supprime rien du tout, silencieusement).
3. **La raison est précise, jamais vague.** "Faux positif" seul ne suffit pas — expliquer *pourquoi* (ex. "table_name vient de slugify()+prefixe obj_, jamais d'une entrée brute").
4. **Format attendu par outil** (vérifié empiriquement cette session, chaque outil a ses propres exigences de position du mot-clé) :
- Bandit : `# nosec <CODE>` — le mot `nosec` doit être immédiatement précédé d'un `#` sur la ligne.
- Ruff : `# noqa: <CODE>` — le mot `noqa` doit être immédiatement précédé d'un `#` sur la ligne. **Bandit et Ruff peuvent coexister sur une même ligne physique** en utilisant deux `#` distincts (`# nosec B101 # noqa: S101 - raison`) — c'est structurel, pas un choix de style : chaque outil cherche son propre mot-clé juste après un `#`, et aucun format à un seul `#` ne peut satisfaire les deux à la fois (testé empiriquement).
- SonarQube (Python) : `# NOSONAR <règle> - raison`, sur la ligne exacte.
- SonarQube (JS) : `// NOSONAR <règle> - raison`, sur la ligne exacte.
- SonarQube (templates Jinja/HTML, règle `Web:*`) : **aucune syntaxe trouvée qui fonctionne** malgré plusieurs tentatives (commentaire Jinja `{# #}`, commentaire JS natif dans un `<script>`, bonne position de ligne) — voir la limitation documentée en section 5 (`Web:S5247`).
5. **Toute exception validée est répercutée ici** (section 5), avec la date/le contexte. Une exception non documentée ici n'est pas considérée comme validée.
6. **Qui valide** : aucune exception n'est appliquée sans validation explicite de l'utilisateur — présenter le choix avec ses compromis, jamais trancher seul quand plusieurs options légitimes existent.
## 5. Exceptions et faux positifs documentés
| Site(s) | Outil / règle | Raison | Contexte |
|---|---|---|---|
| `core/flask_app.py:22` | `python:S4502` (Sonar) | CSRF géré par `core/csrf_guard.py` — garde maison globale (`@app.before_request`), testée dans `test_csrf.py`, jamais Flask-WTF. Sonar ne reconnaît pas cette implémentation custom. | Phase 3 |
| `screens/data_actions/compute_operation.py` (×2), `static/js/play/offline/compute-operation.js` (×2), `static/js/scenes/collision-rules-editor.js` (×2), `static/js/triggers/trigger-editor.js` | `B311`/`S311`/`python:S2245`/`javascript:S2245` | Tirage aléatoire de jeu (dé, id local d'UI) — jamais un usage cryptographique. | Phase 3 |
| `static/js/play/offline/xapi-client.js` (18 sites) + `static/js/play/offline/__tests__/xapi-client.test.js` (2 sites) | `javascript:S5332` | Identifiants du vocabulaire xAPI standard ADL (`http://adlnet.gov/expapi/...`), jamais déréférencés en réseau — simples chaînes comparées/embarquées, le `http://` fait partie du texte fixé par la spec. Le vrai endpoint réseau (`config.endpoint`) est toujours saisi par le créateur, jamais un littéral de ce fichier. | Phase 3 |
| `publish/scorm_manifest.py` | `B406` (Bandit) | Seul fichier du dépôt qui touche du XML — uniquement en génération (`xml.sax.saxutils.escape`), jamais en parsing d'XML externe. | Phase 3 |
| `scripts/build_demo_dialogues.py:61-63` | `python:S8371` (Sonar) | Accès direct `resp.headers["Location"]` volontaire : script d'usage unique jamais exécuté en production, un `KeyError` cru est un échec au moins aussi clair qu'un `.get()` renvoyant `None`. | Phase 3 |
| `static/js/scenes/collision-rules-editor.js` (`leafAction.then = nextLeaf`) | `javascript:S7739` | `then` est un champ métier ("action suivante de la chaîne"), jamais une promesse — vérifié qu'aucun site d'appel ne le passe à `await`/`Promise.resolve()`. Risque latent documenté plutôt que renommage (le nom est ancré dans le schéma JSON persisté en base et côté Python). | Phase 3 |
| `db/assert_not_none.py:14` | `B101`/`S101`/`python:S7632` | Unique `assert` de narrowing de type restant dans tout le moteur, après centralisation de 11 sites dispersés (`ai/`, `routes/scenes/`, `screens/payload/`, `scripts/`) dans ce helper unique. Voir section 4 pour l'impossibilité structurelle de satisfaire Bandit+Ruff avec un seul `#`. | Session du 16/09/2026 |
| `db/rows/delete_row.py`, `db/rows/get_row.py`, `db/rows/list_rows.py`, `screens/animations/update_animation_clip.py`, `screens/flow/add_flow_node.py`, `screens/scenes/add_scene_object.py` | `B608`/`S608`/`python:S7632` | Noms de table/colonnes construits uniquement à partir de `slugify()`/whitelists codées en dur (`_UPDATABLE_FIELDS`, `FLOW_NODE_FIELDS`), jamais d'une entrée arbitraire — valeurs toujours paramétrées (`?`). Distinct du cas `assert_not_none` ci-dessus (nature différente : construction de SQL, pas narrowing de type) — non couvert par ce refactor. | Session du 16/09/2026 |
| `static/js/play/bindings.js:179` (`gameData = newData`) | `javascript:S2703` | Pattern volontaire de scripts globaux (pas des modules ES) : `gameData`/`screensData` sont déclarés une fois par `let` dans le `<script>` inline de `templates/play.html`, chargé avant tous les `static/js/play/*.js` (ordre séquentiel vérifié, pas de `defer`/`async`). Réassignation légitime d'une variable déjà déclarée dans un scope global partagé — déjà documenté dans `.eslintrc.json` (`"gameData": "writable"`). | Session du 16/09/2026 |
| 31 sites `\|safe` (`templates/scene_edit.html`, `templates/play.html`, `templates/auth/register_2fa.html`, `templates/onboarding/onboarding_new.html`, `templates/game_dashboard_simple.html`) | `Web:S5247` (Sonar) | **Faux positif confirmé sur le fond, mais non-supprimable techniquement pour l'instant.** 3 sous-groupes : (1) `rendered_html`/`qr_svg`/`description` — HTML déjà échappé côté Python (`html.escape()`) ou généré sans texte libre utilisateur ; (2) 25 sites `*_json` — `db.json_for_script()` échappe déjà `</script>` (voir Phase 3). Plusieurs syntaxes de suppression testées (commentaire Jinja `{# #}`, commentaire JS natif dans un `<script>`, bonne position de ligne) : **aucune ne fonctionne** avec l'analyseur Web de cette version de SonarQube. La résolution "Faux positif" via l'API est bloquée par le système de permissions de session. `sonar.issue.ignore.multicriteria` existe mais sans sélecteur de ligne (exclusion fichier entier uniquement) — écarté pour `scene_edit.html`/`play.html` (masquerait un futur `\|safe` réellement dangereux). Ces 31 sites restent visibles dans le rapport Sonar en l'état ; traités et compris, pas un point ouvert côté code. | Phase 3 + investigation du 16/09/2026 |
## 6. Backlog qualité
- **djLint H021 (49 occurrences, 6 templates)** — styles inline à remplacer par un système de modèles/classes CSS réutilisables. **Priorité immédiate** après la clôture de ce dispositif, avant ou après la Phase 4 CI selon décision à prendre le moment venu. Ce n'est **pas** une exception documentée en section 5 : c'est une dette assumée et non traitée, à corriger, pas à justifier indéfiniment.
- **Code smells SonarQube (421 restants)** — analysés en volume (modernisation JS, complexité cognitive Python, duplication) mais pas encore triés catégorie par catégorie. Prochain lot après ce document.
- **Trajectoire SonarQube CI bloquant** — une fois (a) le rapport de code smells trié et (b) une solution trouvée pour `Web:S5247` (ou acceptée comme limitation permanente), retirer `continue-on-error: true` du job `sonarqube` dans `.gitea/workflows/deploy.yml`.
Reference in New Issue View Git Blame Copy Permalink