Skip to content

Commit c379689

Browse files
committed
fix: use Unicode-aware regex for identifier matching and fix misleading test claims
- Replace [$\w]+ with [\p{ID_Continue}$]+ (with u flag) in extractSpecifierComments to correctly match Unicode identifiers (α, β, Ω, café, etc.) not just ASCII - Add B6c test verifying Unicode specifiers with trailing comments are preserved - Fix INDENT-L1 test name: old extension reads editor.tabSize, not hardcoded 4 - Fix broken reference to non-existent 999-manual-proof.test.ts - Fix type-only-imports-comparison.test.ts: clarify it only tests new extension - Fix re-export test comments: old extension doesn't touch re-exports at all
1 parent 220b564 commit c379689

5 files changed

Lines changed: 63 additions & 15 deletions

File tree

src/imports/import-manager.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,8 @@ export class ImportManager {
367367
const rest = leadingBlockMatch[2];
368368

369369
// Extract specifier name from rest (might be "A," or "A as B," etc.)
370-
const specMatch = rest.match(/^(?:type\s+)?([$\w]+)/);
370+
// Use \p{ID_Continue} to match all valid ECMAScript identifier characters (including Unicode)
371+
const specMatch = rest.match(/^(?:type\s+)?([\p{ID_Continue}$]+)/u);
371372
if (specMatch) {
372373
const specName = specMatch[1];
373374
if (specifierNames.includes(specName)) {
@@ -380,7 +381,8 @@ export class ImportManager {
380381
}
381382

382383
// Check for trailing line comment: specifier // comment
383-
const trailingLineMatch = trimmed.match(/^(?:type\s+)?([$\w]+)(?:\s+as\s+[$\w]+)?\s*,?\s*(\/\/.*?)$/);
384+
// Use \p{ID_Continue} to match all valid ECMAScript identifier characters (including Unicode)
385+
const trailingLineMatch = trimmed.match(/^(?:type\s+)?([\p{ID_Continue}$]+)(?:\s+as\s+[\p{ID_Continue}$]+)?\s*,?\s*(\/\/.*?)$/u);
384386
if (trailingLineMatch) {
385387
const specName = trailingLineMatch[1];
386388
const comment = trailingLineMatch[2];
@@ -394,7 +396,8 @@ export class ImportManager {
394396
}
395397

396398
// Regular specifier without comments
397-
const regularMatch = trimmed.match(/^(?:type\s+)?([$\w]+)/);
399+
// Use \p{ID_Continue} to match all valid ECMAScript identifier characters (including Unicode)
400+
const regularMatch = trimmed.match(/^(?:type\s+)?([\p{ID_Continue}$]+)/u);
398401
if (regularMatch) {
399402
const specName = regularMatch[1];
400403
if (specifierNames.includes(specName) && !result.has(specName)) {

tests/comparison/test-cases/type-only-imports-comparison.test.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,24 @@
11
/**
2-
* Type-Only Import Handling - Legacy vs Modern Mode Comparison
2+
* Type-Only Import Handling - Legacy vs Modern Mode (NEW EXTENSION ONLY)
3+
*
4+
* NOTE: Despite being in the comparison test directory, this file does NOT run the
5+
* old TypeScript Hero extension. It only tests the new extension's behavior in both
6+
* legacy and modern modes. The old extension cannot be tested here because its
7+
* typescript-parser predates TypeScript 3.8 and cannot parse `import type` syntax.
8+
*
9+
* For VERIFIED old extension behavior with `import type`, see:
10+
* - tests/comparison/test-cases/06-edge-cases.test.ts (test 086)
11+
* - tests/comparison/test-cases/11-true-compatibility.test.ts (TC7a, TC7b)
312
*
413
* This test demonstrates the key difference in type-only import handling between:
514
* - Legacy mode (legacyMode: true): Strips `import type` keywords, allows merging with value imports
615
* - Modern mode (legacyMode: false): Preserves `import type` keywords, keeps type/value imports separate
7-
*
8-
* **Why the difference?**
9-
* - Old extension: Pre-TypeScript 3.8, before `import type` was introduced
10-
* - Modern mode: Respects TypeScript 3.8+ semantics for type-only imports
1116
*/
1217

1318
import * as assert from 'assert';
1419
import { organizeImportsNew } from '../new-extension/adapter';
1520

16-
suite('Type-Only Import Handling: Legacy vs Modern', () => {
21+
suite('Type-Only Import Handling: Legacy vs Modern (new extension only)', () => {
1722

1823
test('LEGACY MODE: Strips import type keywords', async () => {
1924
// SCENARIO: import type statement

tests/unit/import-manager.edge-cases-audit.test.ts

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ const x = A + B + Z;
223223
`;
224224

225225
// LEGACY MODE: Strip comments to match old TypeScript Hero extension
226-
// (Proven in tests/comparison/test-cases/999-manual-proof.test.ts)
226+
// (Verified in tests/comparison/test-cases/00-shared-behavior.test.ts and 11-indentation-behavior.test.ts L4)
227227
const expected = `import { A, B, Z } from 'lib';
228228
229229
const x = A + B + Z;
@@ -354,6 +354,46 @@ const x = A + a + B + b + Ä + ä + α + β + Ω;
354354
}
355355
});
356356

357+
// ============================================================================
358+
// B6c: Unicode specifiers WITH comments (regression test for \p{ID_Continue} regex)
359+
// ============================================================================
360+
361+
test('B6c: Unicode specifiers with trailing comments are preserved', async () => {
362+
// This tests that the extractSpecifierComments regex handles Unicode identifier
363+
// characters (not just ASCII [$\w]). ECMAScript allows Unicode letters in identifiers.
364+
const content = `import {
365+
Ω, // Greek capital letter omega
366+
α, // Greek small letter alpha
367+
café, // French word with accent
368+
} from 'lib';
369+
370+
const x = Ω + α + café;
371+
`;
372+
373+
// Sorted alphabetically (case-insensitive), comments must travel with their specifiers
374+
const expected = `import {
375+
café, // French word with accent
376+
α, // Greek small letter alpha
377+
Ω, // Greek capital letter omega
378+
} from 'lib';
379+
380+
const x = Ω + α + café;
381+
`;
382+
383+
const doc = await createTempDocument(content, 'ts');
384+
try {
385+
const config = new ImportsConfig();
386+
const manager = new ImportManager(doc, config);
387+
const edits = await manager.organizeImports();
388+
await applyEditsToDocument(doc, edits);
389+
390+
const result = doc.getText();
391+
assert.strictEqual(result, expected, 'Unicode specifier comments must be preserved when sorting');
392+
} finally {
393+
await deleteTempDocument(doc);
394+
}
395+
});
396+
357397
// ============================================================================
358398
// B7: Path case sensitivity
359399
// ============================================================================
@@ -804,7 +844,7 @@ import { Y } from './m';
804844
const foo = Y;
805845
`;
806846

807-
// Expected: Re-exports are moved AFTER imports (matching old TypeScript Hero behavior)
847+
// Expected: Re-exports are placed AFTER organized imports (old extension doesn't touch re-exports at all)
808848
const expected = `import { Y } from './m';
809849
810850
export { X } from './m';

tests/unit/import-manager.edge-cases.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ import { Y } from './m';
783783
const foo = Y;
784784
`;
785785

786-
// Expected: Re-exports are moved AFTER imports (matching old TypeScript Hero behavior)
786+
// Expected: Re-exports are placed AFTER organized imports (old extension doesn't touch re-exports at all)
787787
const expected = `import { Y } from './m';
788788
789789
export { X } from './m';

tests/unit/import-manager.indentation.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import { createTempDocument, deleteTempDocument } from './test-helpers';
77
* ImportManager - Indentation Tests
88
*
99
* These tests verify that our indentation implementation:
10-
* 1. Legacy mode: Matches old TypeScript Hero (always spaces, 4-space default)
10+
* 1. Legacy mode: Matches old TypeScript Hero (always spaces, reads from editor.tabSize)
1111
* 2. Modern mode: Enhanced UX (supports tabs, 2-space default)
1212
* 3. useOnlyExtensionSettings: Provides full control
1313
*/
1414

1515
suite('ImportManager - Indentation - Legacy Mode', () => {
1616

17-
test('INDENT-L1: Default 4 spaces in legacy mode', async () => {
17+
test('INDENT-L1: Uses editor.tabSize in legacy mode (VS Code default is 4)', async () => {
1818
const content = `import { VeryLongComponentName, AnotherLongName, ThirdName, FourthName, FifthName } from 'library';
1919
2020
const x = VeryLongComponentName;
@@ -56,7 +56,7 @@ const b = FifthName;
5656
await (await import('vscode')).workspace.applyEdit(edit);
5757

5858
const result = doc.getText();
59-
assert.strictEqual(result, expected, 'Legacy mode must use 4-space indentation by default');
59+
assert.strictEqual(result, expected, 'Legacy mode uses editor.tabSize for indentation (VS Code general default is 4)');
6060
} finally {
6161
await deleteTempDocument(doc);
6262
}

0 commit comments

Comments
 (0)