-
Notifications
You must be signed in to change notification settings - Fork 740
Add namespace synchronization feature #8913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Implements command to sync C# namespaces with folder structure - Scans all .csproj files in workspace recursively - Calculates expected namespace based on file path relative to project root - Supports both file-scoped and block-scoped namespace syntax - Enhanced directory exclusions (bin, obj, .git, node_modules, etc.) - Includes user confirmation prompt with Apply/Preview/Cancel options - Preview mode shows changes grouped by project without applying them - Added comprehensive test coverage (20 unit + 4 integration tests) All 527 tests passing.
@dotnet-policy-service agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a new namespace synchronization feature that automatically aligns C# namespace declarations with their folder structure. The feature adds a csharp.syncNamespaces command that scans .csproj files, calculates expected namespaces based on file paths, and offers users options to apply changes, preview them, or cancel.
Changes:
- Added namespace calculation and extraction logic supporting both file-scoped and block-scoped syntax
- Implemented .csproj file analysis to extract root namespaces and find C# files
- Created main sync command with user prompts and workspace edit functionality
- Added 24 unit and integration tests (though several have quality issues)
- Integrated telemetry tracking for the new command
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test/lsptoolshost/unitTests/namespaceCalculator.test.ts | Unit tests for namespace calculation and extraction functions |
| test/lsptoolshost/unitTests/csprojAnalyzer.test.ts | Unit tests for .csproj file parsing (tests don't call actual implementation) |
| test/lsptoolshost/integrationTests/namespaceSync.integration.test.ts | Integration tests for sync workflow (mostly smoke tests with weak assertions) |
| src/shared/telemetryEventNames.ts | Added NamespaceSyncCommand telemetry event |
| src/lsptoolshost/refactoring/namespaceSync.ts | Main command implementation with preview and apply logic |
| src/lsptoolshost/refactoring/namespaceCalculator.ts | Core namespace calculation and extraction functions |
| src/lsptoolshost/refactoring/csprojAnalyzer.ts | Functions to find .csproj and .cs files, extract root namespace |
| src/lsptoolshost/commands.ts | Registered the new csharp.syncNamespaces command |
| package.nls.json | Added localization string for the command |
| package.json | Added command definition with workspace trust requirement |
- Extract EXCLUDED_DIRECTORIES constant to eliminate duplication (DRY principle) - Add .trim() to getRootNamespace() for whitespace handling - Rewrite sanitizeFolderName() to comply with C# namespace rules: * Prefix digits with underscore (e.g., '3rdParty' -> '_3rdParty') * Replace dots with underscores to avoid extra namespace segments - Remove incorrect @param dryRun from JSDoc comment - Update test to expect 'MyApp._3rdParty' per C# identifier rules All 527 unit tests passing.
- Use path.posix for consistent path operations across Windows/Mac/Linux - Update test to use vscode.Uri.parse() for cross-platform URI creation - Implement Uri.parse() mock with platform-aware path conversion - Remove 'wwwroot' from excluded directories (ASP.NET projects may need it) Fixes test failures on Linux and macOS platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
test/lsptoolshost/integrationTests/namespaceSync.integration.test.ts
Outdated
Show resolved
Hide resolved
test/lsptoolshost/integrationTests/namespaceSync.integration.test.ts
Outdated
Show resolved
Hide resolved
…tform compatibility
Description
Implements a new command to automatically synchronize C# namespace declarations with their folder structure.
Changes
csharp.syncNamespacescommand that scans all .csproj files in workspacenamespace Foo.Bar;) and block-scoped (namespace Foo.Bar { }) syntaxTesting