Forge-Store/Forge-Engine
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects 1 Releases Wiki Activity
Files
ccf836f2c59b79b7f533b62489831f9bc9218c6a
Forge-Engine/docs/JS_MODERNIZATION_PLAN.md
T
williamandClaude Sonnet 5 66d8eaeae8 Lots 1-3 modernisation JS (S8786/S2703/S2486) + retrait Sonar CI/prod 
- Lot 1 (S8786, ReDoS) : 5 sites documentes NOSONAR apres preuve empirique
  (script reproductible docs/redos_probe_s8786.js), aucune reecriture
  defensive necessaire.
- Lot 2 (S2703, variable globale implicite) : bug reel trouve et corrige
  (SCENE_OBJECT_NAMES en const au lieu de let, cassait la reassignation
  cross-script depuis scene-editor.js) + test de non-regression ; 4 autres
  sites confirmes surs et documentes.
- Lot 3 (S2486, exceptions avalees) : 6 sites confirmes surs et
  documentes ; 2 sites (config sprite JSON invalide) corriges avec un
  console.warn devtools, comportement joueur inchange, couverts par un
  nouveau test.
- Retrait du job CI sonarqube (.gitea/workflows/deploy.yml) et du service
  prod sonarqube/sonar-postgres (docker-compose.prod.yml) : acces dashboard
  bloque par des soucis d'infrastructure reseau (WSL2/pare-feu Hyper-V en
  local, reseau Docker partage avec Caddy pas en place en prod), sans lien
  avec le code du moteur - mis de cote plutot que de continuer a bloquer
  sur de l'infra. Les lots 4+ de modernisation JS dependent de scores
  Sonar exacts et sont donc egalement en pause (voir CODE_QUALITY.md).

SKIP=djlint : H021 (styles inline, 49 occurrences) est un backlog deja
documente et assume (CODE_QUALITY.md section 6), sur des templates non
touches par ce commit - deja exclu de la CI pour la meme raison.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-18 08:46:24 +02:00

135 lines
8.8 KiB
Markdown
Raw Blame History

# 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/js/play/__tests__/*.test.js
static/js/play/offline/__tests__/*.test.js
static/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`.
Reference in New Issue View Git Blame Copy Permalink