Skip to content

Commit adaffb1

Browse files
JohannesHoppeJohannesHoppe
committed
docs: Tone down exaggerated claims and improve accuracy
CHANGES: 1. Replaced "PERFECT PARITY" with accurate edge case language (Test A10) - Changed header from "PERFECT PARITY" to "Edge Case" - Acknowledged this is INVALID TypeScript that compiler would reject - Clarified import organizers don't validate semantics (compiler's job) - Changed assertion from "PERFECT PARITY" to "edge case behavior matching" 2. Replaced "must produce correct output" with "must match expected output" (255 instances) - Old: "must produce correct output" (implies settings are respected) - New: "must match expected output" (neutral, accurate for test verification) - This is more honest when settings are silently ignored (legacy mode bugs) 3. Updated README.md claims about legacy mode - Changed: "match original behavior exactly" → "match output format" - Changed: "100% backward compatibility" → "maximum backward compatibility" - Added: "Crash handling — Gracefully handles cases that crashed old extension" - Changed: "IGNORED" → "SILENTLY IGNORED" for clarity - Changed: "Bug in old extension" → "Bug replication" (more humble) - Added prominent warning: Config settings silently ignored in legacy mode - Changed: "exact old output" → "consistent output format" RATIONALE: This addresses valid audit findings: - "PERFECT PARITY" overstates things for invalid TypeScript - "correct output" is contradictory when configs are ignored - "exactly" and "100%" are technically false (we DO fix crashes) We now accurately describe: - Legacy mode matches OUTPUT FORMAT (not exact behavior) - We gracefully handle crashes (silent fix) - Config settings are SILENTLY IGNORED (prominent warning added) - Invalid TS is an edge case (not our job to validate) TEST RESULTS: ✅ All 191 tests passing
1 parent 9d16f5c commit adaffb1

12 files changed

Lines changed: 273 additions & 270 deletions

‎README.md‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,19 +180,22 @@ Once your settings are migrated, you have two options:
180180

181181
If the old TypeScript Hero extension is still active, you'll see a reminder in the migration notification suggesting you can disable it.
182182

183-
**Legacy Mode:** For migrated users, `legacyMode` is automatically set to `true` to match the original TypeScript Hero behavior exactly. When enabled, legacy mode replicates ALL old behaviors (including bugs) for 100% backward compatibility:
183+
**Legacy Mode:** For migrated users, `legacyMode` is automatically set to `true` to match the original TypeScript Hero output format. When enabled, legacy mode replicates old behaviors (including bugs) for maximum backward compatibility:
184184

185185
- **`blankLinesAfterImports`** — Always preserves existing blank lines (ignores configured value)
186-
- **`organizeSortsByFirstSpecifier`** — **IGNORED** (always sorts by library name within groups) ⚠️ This is a bug in the old extension
187-
- **`disableImportsSorting`** — **IGNORED** (always sorts imports within groups) ⚠️ This is a bug in the old extension
186+
- **`organizeSortsByFirstSpecifier`** — **SILENTLY IGNORED** (always sorts by library name within groups) ⚠️ Bug replication
187+
- **`disableImportsSorting`** — **SILENTLY IGNORED** (always sorts imports within groups) ⚠️ Bug replication
188188
- **Merge timing** — Merges BEFORE removeTrailingIndex (old bug: `./lib/index` and `./lib` won't merge)
189189
- **Type-only imports** — Strips `import type` keywords (old TS <3.8 behavior)
190190
- **Indentation** — Always uses spaces (ignores `insertSpaces` setting)
191+
- **Crash handling** — Gracefully handles cases that crashed old extension (silent fix)
191192

192-
**Why replicate bugs?** Migrated users depend on exact old output. Any change would create massive diffs across their codebase on first run, breaking trust.
193+
**Why replicate bugs?** Migrated users depend on consistent output format. Any change would create massive diffs across their codebase on first run, breaking trust.
193194

194195
**Modern mode fixes these bugs:** New users get `legacyMode: false` by default for correct behavior. You can toggle this setting anytime via the command palette or your configuration.
195196

197+
> **⚠️ IMPORTANT**: When `legacyMode: true`, certain config settings are silently ignored (see above). The extension does NOT warn about this - it's intentional for backward compatibility. If you need these settings to work, set `legacyMode: false`.
198+
196199
### No Old Settings?
197200

198201
If you've never used TypeScript Hero before, the migration simply won't run — no action needed!
@@ -318,7 +321,7 @@ Mini TypeScript Hero respects your editor's indentation settings for multiline i
318321
- Reads VS Code's resolved editor settings (usually **2 spaces** for TypeScript)
319322
- Falls back to **4 spaces** when no editor context is available
320323
- Always uses spaces (never tabs)
321-
- Matches old TypeScript Hero behavior exactly
324+
- Matches old TypeScript Hero output format
322325

323326
**Note:** VS Code automatically applies `.editorconfig` settings to `editor.tabSize` and `editor.insertSpaces`. The extension reads these resolved values, so EditorConfig integration works automatically.
324327

‎comparison-test-harness/test-cases/01-sorting.test.ts‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ const z: OnInit = null as any;
3333
const oldResult = await organizeImportsOld(input);
3434
const newResult = await organizeImportsNew(input);
3535

36-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
37-
assert.equal(newResult, expected, 'New extension must produce correct output');
36+
assert.equal(oldResult, expected, 'Old extension must match expected output');
37+
assert.equal(newResult, expected, 'New extension must match expected output');
3838
});
3939

4040
test('002. All capitals specifiers (Component, OnInit)', async () => {
@@ -53,8 +53,8 @@ const z: OnInit = null as any;
5353
const oldResult = await organizeImportsOld(input);
5454
const newResult = await organizeImportsNew(input);
5555

56-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
57-
assert.equal(newResult, expected, 'New extension must produce correct output');
56+
assert.equal(oldResult, expected, 'Old extension must match expected output');
57+
assert.equal(newResult, expected, 'New extension must match expected output');
5858
});
5959

6060
test('003. All lowercase specifiers (map, filter, tap)', async () => {
@@ -75,8 +75,8 @@ const z = tap;
7575
const oldResult = await organizeImportsOld(input);
7676
const newResult = await organizeImportsNew(input);
7777

78-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
79-
assert.equal(newResult, expected, 'New extension must produce correct output');
78+
assert.equal(oldResult, expected, 'Old extension must match expected output');
79+
assert.equal(newResult, expected, 'New extension must match expected output');
8080
});
8181

8282
test('004. Mixed lower and upper start (inject, Component, map, OnInit)', async () => {
@@ -99,8 +99,8 @@ const z = map;
9999
const oldResult = await organizeImportsOld(input);
100100
const newResult = await organizeImportsNew(input);
101101

102-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
103-
assert.equal(newResult, expected, 'New extension must produce correct output');
102+
assert.equal(oldResult, expected, 'Old extension must match expected output');
103+
assert.equal(newResult, expected, 'New extension must match expected output');
104104
});
105105

106106
test('005. Library name sorting (alphabetical)', async () => {
@@ -125,8 +125,8 @@ const w = z;
125125
const oldResult = await organizeImportsOld(input);
126126
const newResult = await organizeImportsNew(input);
127127

128-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
129-
assert.equal(newResult, expected, 'New extension must produce correct output');
128+
assert.equal(oldResult, expected, 'Old extension must match expected output');
129+
assert.equal(newResult, expected, 'New extension must match expected output');
130130
});
131131

132132
test('006. Sort by first specifier (enabled)', async () => {
@@ -148,8 +148,8 @@ const y = zoo;
148148
const oldResult = await organizeImportsOld(input, { organizeSortsByFirstSpecifier: true });
149149
const newResult = await organizeImportsNew(input, { organizeSortsByFirstSpecifier: true });
150150

151-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
152-
assert.equal(newResult, expected, 'New extension must produce correct output');
151+
assert.equal(oldResult, expected, 'Old extension must match expected output');
152+
assert.equal(newResult, expected, 'New extension must match expected output');
153153
});
154154

155155
test('007. String imports come first', async () => {
@@ -173,8 +173,8 @@ const y = map;
173173
const oldResult = await organizeImportsOld(input);
174174
const newResult = await organizeImportsNew(input);
175175

176-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
177-
assert.equal(newResult, expected, 'New extension must produce correct output');
176+
assert.equal(oldResult, expected, 'Old extension must match expected output');
177+
assert.equal(newResult, expected, 'New extension must match expected output');
178178
});
179179

180180
test('008. Multiple string imports sorted', async () => {
@@ -195,8 +195,8 @@ import 'zebra';
195195
// old extension adds EOF blank line (ends with \n\n), new extension doesn't (ends with \n).
196196
// We normalize the old output for comparison since both behaviors are valid.
197197
const oldResultWithoutTrailingBlank = oldResult.replace(/\n\n$/, '\n');
198-
assert.equal(oldResultWithoutTrailingBlank, expected, 'Old extension must produce correct output');
199-
assert.equal(newResult, expected, 'New extension must produce correct output');
198+
assert.equal(oldResultWithoutTrailingBlank, expected, 'Old extension must match expected output');
199+
assert.equal(newResult, expected, 'New extension must match expected output');
200200
});
201201

202202
test('009. Specifiers with aliases', async () => {
@@ -217,8 +217,8 @@ const z: Init = null as any;
217217
const oldResult = await organizeImportsOld(input);
218218
const newResult = await organizeImportsNew(input);
219219

220-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
221-
assert.equal(newResult, expected, 'New extension must produce correct output');
220+
assert.equal(oldResult, expected, 'Old extension must match expected output');
221+
assert.equal(newResult, expected, 'New extension must match expected output');
222222
});
223223

224224
test('010. Default + named imports', async () => {
@@ -239,8 +239,8 @@ const z = useEffect;
239239
const oldResult = await organizeImportsOld(input);
240240
const newResult = await organizeImportsNew(input);
241241

242-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
243-
assert.equal(newResult, expected, 'New extension must produce correct output');
242+
assert.equal(oldResult, expected, 'Old extension must match expected output');
243+
assert.equal(newResult, expected, 'New extension must match expected output');
244244
});
245245

246246
test('011. Namespace imports', async () => {
@@ -261,8 +261,8 @@ const y = RxJS;
261261
const oldResult = await organizeImportsOld(input);
262262
const newResult = await organizeImportsNew(input);
263263

264-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
265-
assert.equal(newResult, expected, 'New extension must produce correct output');
264+
assert.equal(oldResult, expected, 'Old extension must match expected output');
265+
assert.equal(newResult, expected, 'New extension must match expected output');
266266
});
267267

268268
test('012. Disable sorting', async () => {
@@ -283,8 +283,8 @@ const w = z;
283283
const oldResult = await organizeImportsOld(input, { disableImportsSorting: true });
284284
const newResult = await organizeImportsNew(input, { disableImportsSorting: true });
285285

286-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
287-
assert.equal(newResult, expected, 'New extension must produce correct output');
286+
assert.equal(oldResult, expected, 'Old extension must match expected output');
287+
assert.equal(newResult, expected, 'New extension must match expected output');
288288
});
289289

290290
test('013. Case-insensitive library sorting', async () => {
@@ -309,8 +309,8 @@ const z = c;
309309
const oldResult = await organizeImportsOld(input);
310310
const newResult = await organizeImportsNew(input);
311311

312-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
313-
assert.equal(newResult, expected, 'New extension must produce correct output');
312+
assert.equal(oldResult, expected, 'Old extension must match expected output');
313+
assert.equal(newResult, expected, 'New extension must match expected output');
314314
});
315315

316316
test('014. Numbers in specifier names', async () => {
@@ -333,8 +333,8 @@ const d = test10;
333333
const oldResult = await organizeImportsOld(input);
334334
const newResult = await organizeImportsNew(input);
335335

336-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
337-
assert.equal(newResult, expected, 'New extension must produce correct output');
336+
assert.equal(oldResult, expected, 'Old extension must match expected output');
337+
assert.equal(newResult, expected, 'New extension must match expected output');
338338
});
339339

340340
test('015. Special characters in library names', async () => {
@@ -359,7 +359,7 @@ const z = c;
359359
const oldResult = await organizeImportsOld(input);
360360
const newResult = await organizeImportsNew(input);
361361

362-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
363-
assert.equal(newResult, expected, 'New extension must produce correct output');
362+
assert.equal(oldResult, expected, 'Old extension must match expected output');
363+
assert.equal(newResult, expected, 'New extension must match expected output');
364364
});
365365
});

‎comparison-test-harness/test-cases/02-merging.test.ts‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ const y = B;
2525
const oldResult = await organizeImportsOld(input);
2626
const newResult = await organizeImportsNew(input);
2727

28-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
29-
assert.equal(newResult, expected, 'New extension must produce correct output');
28+
assert.equal(oldResult, expected, 'Old extension must match expected output');
29+
assert.equal(newResult, expected, 'New extension must match expected output');
3030
});
3131

3232
test('017. Three imports from same module', async () => {
@@ -49,8 +49,8 @@ const z = C;
4949
const oldResult = await organizeImportsOld(input);
5050
const newResult = await organizeImportsNew(input);
5151

52-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
53-
assert.equal(newResult, expected, 'New extension must produce correct output');
52+
assert.equal(oldResult, expected, 'Old extension must match expected output');
53+
assert.equal(newResult, expected, 'New extension must match expected output');
5454
});
5555

5656
test('018. Same library, default + named', async () => {
@@ -72,8 +72,8 @@ const z = B;
7272
const oldResult = await organizeImportsOld(input);
7373
const newResult = await organizeImportsNew(input);
7474

75-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
76-
assert.equal(newResult, expected, 'New extension must produce correct output');
75+
assert.equal(oldResult, expected, 'Old extension must match expected output');
76+
assert.equal(newResult, expected, 'New extension must match expected output');
7777
});
7878

7979
test('019. Duplicate specifiers - graceful error handling', async () => {
@@ -130,8 +130,8 @@ const y = Lib2;
130130
const oldResult = await organizeImportsOld(input);
131131
const newResult = await organizeImportsNew(input);
132132

133-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
134-
assert.equal(newResult, expected, 'New extension must produce correct output');
133+
assert.equal(oldResult, expected, 'Old extension must match expected output');
134+
assert.equal(newResult, expected, 'New extension must match expected output');
135135
});
136136

137137
test('021. String imports cannot merge', async () => {
@@ -151,8 +151,8 @@ import './lib';
151151
const oldResult = await organizeImportsOld(input);
152152
const newResult = await organizeImportsNew(input);
153153

154-
assert.equal(oldResult, expectedOld, 'Old extension must produce correct output');
155-
assert.equal(newResult, expectedNew, 'New extension must produce correct output');
154+
assert.equal(oldResult, expectedOld, 'Old extension must match expected output');
155+
assert.equal(newResult, expectedNew, 'New extension must match expected output');
156156
});
157157

158158
test('022. Merging after removeTrailingIndex', async () => {
@@ -174,8 +174,8 @@ const y = B;
174174
const oldResult = await organizeImportsOld(input);
175175
const newResult = await organizeImportsNew(input);
176176

177-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
178-
assert.equal(newResult, expected, 'New extension must produce correct output');
177+
assert.equal(oldResult, expected, 'Old extension must match expected output');
178+
assert.equal(newResult, expected, 'New extension must match expected output');
179179
});
180180

181181
test('023. Merging preserves used specifiers only', async () => {
@@ -195,8 +195,8 @@ const y = B;
195195
const oldResult = await organizeImportsOld(input);
196196
const newResult = await organizeImportsNew(input);
197197

198-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
199-
assert.equal(newResult, expected, 'New extension must produce correct output');
198+
assert.equal(oldResult, expected, 'Old extension must match expected output');
199+
assert.equal(newResult, expected, 'New extension must match expected output');
200200
});
201201

202202
test('024. Merging sorts specifiers alphabetically', async () => {
@@ -219,8 +219,8 @@ const z = Z;
219219
const oldResult = await organizeImportsOld(input);
220220
const newResult = await organizeImportsNew(input);
221221

222-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
223-
assert.equal(newResult, expected, 'New extension must produce correct output');
222+
assert.equal(oldResult, expected, 'Old extension must match expected output');
223+
assert.equal(newResult, expected, 'New extension must match expected output');
224224
});
225225

226226
test('025. Default + named with aliases', async () => {
@@ -240,8 +240,8 @@ const y = AliasA;
240240
const oldResult = await organizeImportsOld(input);
241241
const newResult = await organizeImportsNew(input);
242242

243-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
244-
assert.equal(newResult, expected, 'New extension must produce correct output');
243+
assert.equal(oldResult, expected, 'Old extension must match expected output');
244+
assert.equal(newResult, expected, 'New extension must match expected output');
245245
});
246246

247247
test('026. Multiple modules with merging', async () => {
@@ -268,8 +268,8 @@ const d = B2;
268268
const oldResult = await organizeImportsOld(input);
269269
const newResult = await organizeImportsNew(input);
270270

271-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
272-
assert.equal(newResult, expected, 'New extension must produce correct output');
271+
assert.equal(oldResult, expected, 'Old extension must match expected output');
272+
assert.equal(newResult, expected, 'New extension must match expected output');
273273
});
274274

275275
test('027. Mixed import types from same module', async () => {
@@ -294,8 +294,8 @@ const z = A;
294294
const oldResult = await organizeImportsOld(input);
295295
const newResult = await organizeImportsNew(input);
296296

297-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
298-
assert.equal(newResult, expected, 'New extension must produce correct output');
297+
assert.equal(oldResult, expected, 'Old extension must match expected output');
298+
assert.equal(newResult, expected, 'New extension must match expected output');
299299
});
300300

301301
test('028. Real Angular example - merges @angular/core imports', async () => {
@@ -341,8 +341,8 @@ const user = UserDetail;
341341
const oldResult = await organizeImportsOld(input);
342342
const newResult = await organizeImportsNew(input);
343343

344-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
345-
assert.equal(newResult, expected, 'New extension must produce correct output');
344+
assert.equal(oldResult, expected, 'Old extension must match expected output');
345+
assert.equal(newResult, expected, 'New extension must match expected output');
346346
});
347347

348348
test('029. Merging with type imports', async () => {
@@ -370,8 +370,8 @@ const y = B;
370370
const oldResult = await organizeImportsOld(input);
371371
const newResult = await organizeImportsNew(input, { legacyMode: false }); // Enable modern mode
372372

373-
assert.equal(oldResult, expectedOld, 'Old extension must produce correct output');
374-
assert.equal(newResult, expectedNew, 'New extension must produce correct output (preserves type)');
373+
assert.equal(oldResult, expectedOld, 'Old extension must match expected output');
374+
assert.equal(newResult, expectedNew, 'New extension must match expected output (preserves type)');
375375
});
376376

377377
test('030. Merging with multiple aliases', async () => {
@@ -391,7 +391,7 @@ const y = AliasB;
391391
const oldResult = await organizeImportsOld(input);
392392
const newResult = await organizeImportsNew(input);
393393

394-
assert.equal(oldResult, expected, 'Old extension must produce correct output');
395-
assert.equal(newResult, expected, 'New extension must produce correct output');
394+
assert.equal(oldResult, expected, 'Old extension must match expected output');
395+
assert.equal(newResult, expected, 'New extension must match expected output');
396396
});
397397
});

0 commit comments

Comments
 (0)