Files
Forge-Engine/docs/JS_MODERNIZATION_PLAN.md
T
williamandClaude Sonnet 5 b2e933f322
Build and deploy / test-python (push) Successful in 11m12s
Build and deploy / test-js (push) Successful in 53s
Build and deploy / lint-python (push) Successful in 3m56s
Build and deploy / lint-js (push) Successful in 3m1s
Build and deploy / build-and-push (push) Skipped
Build and deploy / deploy (push) Skipped
Build and deploy / sonarqube (push) Failing after 3m58s
Reorganisation game/document : renommage screens->game_engine + sous-dossiers game/ dans routes, scripts, static, templates, tests
Prepare la scission a venir entre l'editeur Jeu 2D et le futur editeur
Support de formation (voir docs/plan/PLAN.md), sans toucher a
l'architecture en couches existante :

- screens/ renomme en game_engine/ (nom clair pour le moteur du jeu 2D,
  avant l'arrivee d'un second "moteur" cote document) : ~85 imports
  corriges, contrat import-linter mis a jour, meme forme de couches.
- routes/, scripts/, static/, templates/, tests/ : tout ce qui est
  propre au jeu 2D deplace dans un sous-dossier game/ de chacun
  (routes/game/, static/game/, templates/game/, tests/game/,
  scripts/game/) ; ce qui est partage par le site (auth, onboarding,
  dashboard, uploads, db/) reste a la racine de chaque dossier. Un
  sous-dossier document/ (vide) cree dans chacun pour le futur chantier.
- styles/ volontairement inchange : les 3 fichiers sources sont
  concatenes en un seul static/style.css charge par tout le site,
  scinder leur CONTENU (editeur vs partage) serait un refactor CSS
  distinct, pas un deplacement mecanique.
- Chaine d'export SCORM (publish/build_scorm_package.py) mise a jour en
  profondeur : copie des assets, URLs d'icones relatives a
  static/style.css (qui ne bouge pas), manifeste, wrapper SCORM.
- Deux regressions d'un sweep de renommage anterieur corrigees au passage
  (screens.js/screens/scene-objects incorrectement convertis en
  game_engine.js/game_engine/scene-objects dans des commentaires).
- Effet de bord Windows decouvert et corrige : git mv + Path.write_text
  convertissent des fichiers en CRLF (core.autocrlf=true) - ~189 fichiers
  normalises en LF.
- .eslintrc.json/package.json : uniquement les chemins de glob mis a jour
  (static/game/js/...) ; la preparation eslint-plugin-unicorn du lot 7
  reste volontairement non committee (package-lock.json restaure a la
  version precedente).

Verifications : ruff, mypy --strict (391 fichiers), vulture, bandit,
lint-imports tous verts ; 591/591 tests Python, 276/276 tests JS ;
demarrage serveur + requetes HTTP manuelles confirmant que les assets
deplaces repondent en 200 au nouvel emplacement et 404 a l'ancien.

SKIP=djlint : backlog H021 (styles inline) deja documente comme dette
assumee dans CODE_QUALITY.md section 6, aucun template touche par ce
commit au-dela d'un deplacement de fichier.

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

135 lines
8.8 KiB
Markdown

# Plan — lot JS "modernisation" (357 issues SonarQube)
Trace de la stratégie validée avant exécution — même valeur que
`docs/SESSION_RECAP.md` pour la Phase 3 : ce document décrit le plan tel
que discuté et confirmé, pas un compte-rendu de ce qui a déjà été fait.
Pour l'état d'avancement réel lot par lot, voir les commits de ce
chantier (référencés ici au fur et à mesure).
Origine : rapport SonarQube local (`http://localhost:9000`), catégorie
"code smells" JavaScript, 357 occurrences réparties sur 22 règles.
## 1. Répartition exacte par règle Sonar
| Règle | Nom | Occurrences |
|---|---|---|
| `S7773` | Number static methods preferred over globals (`parseFloat`→`Number.parseFloat`, etc.) | 119 |
| `S6582` | Optional chaining should be preferred | 98 |
| `S7781` | `replaceAll()` instead of `replace()` with global regex | 29 |
| `S7761` | Data attributes via `.dataset` | 22 |
| `S7765` | `.includes()` instead of `.indexOf()`/`.lastIndexOf()` | 19 |
| `S3972` | Conditionals should start on new lines | 15 |
| `S3776` | Cognitive Complexity too high | 9 |
| `S4138` | `for-of` should be used with Iterables | 6 |
| `S4624` | Template literals should not be nested | 6 |
| `S2703` | Variables should be declared explicitly (var globale implicite) | 6 |
| `S8786` | Regex non-linear backtracking (ReDoS) | 5 |
| `S3358` | Ternary operators should not be nested | 4 |
| `S7744` | Unnecessary fallback objects in object spread | 3 |
| `S2486` | Exceptions should not be ignored | 3 |
| `S6557` | `startsWith()`/`endsWith()` instead of manual check | 3 |
| `S7721` | Function should be moved to highest possible scope | 2 |
| `S4144` | Functions should not have identical implementations | 2 |
| `S6653` | `Object.hasOwn()` instead of `hasOwnProperty` | 2 |
| `S6671` | Literal used for promise rejection (pas une `Error`) | 1 |
| `S7755` | `.at()` instead of `[…length - index]` | 1 |
| `S7769` | Modern Math APIs (`Math.hypot` etc.) | 1 |
| `S3735` | `void` should not be used | 1 |
| **Total** | | **357** |
## 2. Classification
### Mécanique, sans risque sémantique (équivalence prouvée)
| Règle | Pourquoi c'est sûr |
|---|---|
| `S7773` | `parseFloat`/`parseInt`/`isNaN`/`isFinite` sont des alias exacts de `Number.*` depuis ES2015 |
| `S7781` | `replace()`→`replaceAll()` seulement quand la regex a déjà le flag `g` (précondition de la règle) |
| `S7765` | `.indexOf(x) !== -1` ⇔ `.includes(x)` — équivalence stricte pour ce pattern |
| `S7761` | `getAttribute('data-x')`→`.dataset.x` — mécanique si nom d'attribut statique |
| `S7744` | Le spread de `null`/`undefined` ne lève jamais — le fallback `\|\| {}` est toujours redondant |
| `S6557`, `S6653`, `S7755`, `S7769` | Équivalences directes, très faible volume (7 sites), pas besoin d'outil |
| `S3972` | Pur formatage, zéro impact comportemental |
### ⚠️ Correction par rapport à l'hypothèse de départ : `S6582` n'est PAS mécanique
`a && a.b` et `a?.b` divergent quand `a` est une valeur **fausse mais
définie** (`0`, `""`, `false`, `NaN`) : `a && a.b` s'arrête et renvoie
`a`, `a?.b` continue et accède à `.b`. Une transformation aveugle peut
changer le comportement. Malgré son volume (98, 2ᵉ plus grosse règle),
`S6582` est traité en examen au cas par cas, pas en lot mécanique.
### Nécessite un vrai examen au cas par cas
| Règle | Nature | Note |
|---|---|---|
| `S6582` (98) | Sémantique | Voir ci-dessus — sous-lots avec vérification du type de la valeur testée |
| `S8786` (5) | **Sécurité-adjacent** | Sites JS, distincts de l'exception Python déjà validée en Phase 3 (`auth/email_validation.py`) |
| `S2703` (6) | **Sécurité-adjacent** | 1 seul des 6 sites déjà examiné et validé (`bindings.js:179`, `gameData`) — 5 restent à vérifier, ne pas supposer qu'ils sont identiques |
| `S2486` (3) | **Sécurité-adjacent** | Exception avalée, peut masquer un vrai bug |
| `S3776` (9) | Refactor réel | Pas de fix mécanique — restructuration fonction par fonction |
| `S4624` (6), `S3358` (4) | Lisibilité | Extraction manuelle |
| `S4138` (6) | Risque de perte de l'index | Fix proposé par un outil mais vérifié site par site avant application |
| `S4144` (2), `S7721` (2) | Décision de conception | Dédupliquer/déplacer une fonction change la portée |
| `S6671` (1) | Change le type de la valeur de rejet | Impact possible sur du code aval qui inspecterait `.message`/`instanceof Error` |
| `S3735` (1) | Dépend de l'intention | À lire avant de trancher |
## 3. Outillage — vérifié empiriquement
- **`eslint-plugin-sonarjs`** (officiel SonarSource, 4.2.1, dernière version) : ne couvre qu'**une seule** des 22 règles avec un fixer (`S3972`) — les règles `S77xx` sont trop récentes pour cette version.
- **`eslint-plugin-unicorn`** : dernière version (75.0.0) exige ESLint ≥10 (incompatible avec notre 8.57.1). Version retenue : **55.0.0** (exige ESLint ≥8.56.0, compatible), ajoutée en devDependency permanente. 9 règles activées dans `.eslintrc.json` avec un fixer confirmé (`fixable: 'code'` vérifié dans le code source du plugin) :
- `unicorn/prefer-number-properties` (S7773)
- `unicorn/prefer-string-replace-all` (S7781)
- `unicorn/prefer-dom-node-dataset` (S7761)
- `unicorn/prefer-includes` (S7765)
- `unicorn/prefer-string-starts-ends-with` (S6557)
- `unicorn/prefer-modern-math-apis` (S7769)
- `unicorn/prefer-at` (S7755)
- `unicorn/no-useless-fallback-in-spread` (S7744)
- `unicorn/no-for-loop` (S4138)
- Uniquement ces 9 règles activées, jamais la config `recommended` complète du plugin (qui en contient des dizaines d'autres, jamais évaluées pour ce projet) — voir `CODE_QUALITY.md` section 2.
- **Dry-run réel** (`--fix-dry-run`) sur tout `static/js/**/*.js` avec ces 9 règles : **222 problèmes détectés, 222 avec un fix proposé.** Les comptes par règle ne correspondent pas exactement à ceux de Sonar (moteurs différents, ex. `prefer-includes` détecte 41 cas contre 19 pour `S7765` côté Sonar) — normal, la vérification finale se fait via un rescan Sonar après application, pas via le compte ESLint.
- **`@typescript-eslint`** (candidat pour combler `S6582`) : nécessite de remplacer tout le parser JS par le parser TypeScript — écarté, trop lourd pour un projet 100% JS.
- **`Object.hasOwn` (`S6653`, 2 sites) et `startsWith`/`endsWith` (`S6557`, 3 sites)** : aucun outil ne les a détectés dans le dry-run — 5 sites au total, correction manuelle directe.
## 4. Ordre de priorité
1. Sécurité-adjacent : `S8786` (5), `S2703` (6, dont 5 non-vérifiés), `S2486` (3) — 14 sites.
2. Structure/lisibilité à risque de bug : `S3776` (9), `S4624` (6), `S3358` (4), `S4144` (2), `S7721` (2), `S6671` (1), `S3735` (1) — 25 sites.
3. `S6582` optional chaining (98) — lot dédié à part.
4. Modernisation mécanique via outil (`S7773`, `S7781`, `S7765`, `S7761`, `S7744`, `S3972`, `S4138` vérifié) — le plus gros volume, le moins risqué, en dernier.
5. Reliquat manuel très faible volume (`S6557`, `S6653`, `S7755`, `S7769`) — 7 sites.
## 5. Découpage en 9 lots
| Lot | Contenu | Méthode |
|---|---|---|
| 1 | `S8786` (5) | Lecture individuelle, diff par site |
| 2 | `S2703` (5 restants, hors `gameData` déjà validé) | Lecture individuelle — même pattern ou différent ? |
| 3 | `S2486` (3) | Lecture individuelle |
| 4 | `S3776` (9) | Refactor fonction par fonction |
| 5 | `S4624` + `S3358` + `S4144` + `S7721` + `S6671` + `S3735` (16) | Groupés, petits volumes, revue individuelle |
| 6 | `S6582` (98) | Sous-lots de ~20-25, vérification du type à chaque site |
| 7 | `S7773`+`S7781`+`S7765`+`S7761`+`S7744`+`S3972` via `eslint-plugin-unicorn`@55.0.0 `--fix` | `--fix` en un coup, diff complet relu avant commit |
| 8 | `S4138` (fix proposé par `no-for-loop`, vérifié un par un) | Semi-automatique |
| 9 | `S6557`+`S6653`+`S7755`+`S7769` (7) | Manuel direct |
**À chaque lot** : diff complet montré avant application, suite de tests
JS (`node --test static/game/js/play/__tests__/*.test.js
static/game/js/play/offline/__tests__/*.test.js
static/game/js/scenes/__tests__/*.test.js`, 241 tests) après application,
signalement immédiat de tout changement de comportement réel (pas
seulement du style). Rescan SonarQube local après le lot 7 (le plus gros
volume mécanique) pour confirmer la réduction réelle plutôt que de se
fier au seul compte ESLint, puis un rescan final après le lot 9.
## Note de séquencement Git
Les 9 règles `unicorn/*` ont été activées dans `.eslintrc.json` (et
`eslint-plugin-unicorn` ajouté en devDependency) **avant** que les 222
occurrences correspondantes soient corrigées — ce qui rend `pre-commit`/
la CI rouges sur ESLint dès maintenant si ces changements de config
étaient commités seuls. Ces changements restent donc **non commités**
tant que le lot 7 (la correction mécanique) n'est pas fait, pour ne
jamais introduire un commit où le lint échouerait sur `dev`.