Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 11, 2025

Add Document Formatting Support for MIDL3

This PR adds document formatting capability to the MIDL3 language server, similar to clang-format for C++.

✅ Completed:

  • Core formatter implementation with configurable options
  • Support for C preprocessor directives (#ifdef, #define, etc.)
  • Configurable brace placement style (newLine/sameLine)
  • Configurable indent size (default: 4 spaces)
  • Format entire document or selection
  • Preserves comments and attributes
  • Handles invalid/incomplete code gracefully
  • Fixed build pipeline errors by upgrading TypeScript
  • Character-by-character state machine parser for robust formatting
  • Handles compact single-line code
  • Correctly preserves braces within comments
  • Fixed critical bug where properties were being lost
  • All spacing normalization issues fixed
  • Context-aware attribute formatting
  • Blank line handling and import separation
  • Semicolon placement with closing braces
  • All 35 unit tests passing

Configuration Options:

  • midl3.format.braceStyle: "newLine" (default) or "sameLine"
  • midl3.format.indentSize: Default 4 spaces per indent level

Implementation Details:

Latest changes:

  • ✅ Context-aware attribute formatting:
    • Attributes on separate lines for type declarations (namespace, runtimeclass, interface, struct, enum, delegate)
    • Attributes stay on same line for members (methods, properties, parameters)
    • Multiple consecutive attributes each get their own line for types
    • Space added after attributes that stay inline
  • ✅ Blank line handling:
    • Extra blank lines removed from formatted output
    • Single blank line automatically added between imports and first non-import line
  • ✅ Semicolon placement: Semicolons after closing braces kept on same line (e.g., interface Foo { };)
  • ✅ Track nested brackets within attributes (e.g., [attr(arr[0])])
  • ✅ Added 8 comprehensive tests for new features

Test Results:

  • 35 of 35 tests passing (13 parser + 22 formatter) ✅
  • 100% test pass rate
  • All formatting rules working correctly
  • No data loss issues
  • No security vulnerabilities

Files changed:

  • server/src/formatter.ts:
    • Added context-aware attribute handling with type vs member detection
    • Added import/code separation logic
    • Added semicolon placement with closing braces
    • Enhanced blank line handling
  • server/src/test/formatter.test.ts: Added 8 comprehensive tests

Key Features:

  1. Spacing Normalization: Colons, commas, multiple spaces
  2. Brace Styles: Configurable newLine/sameLine placement
  3. Comment Preservation: Line comments, block comments handled correctly
  4. Preprocessor Support: #ifdef, #define, etc. preserved at column 0
  5. Property Accessors: { get; set; } properly formatted
  6. Compact Code: Single-line code expanded correctly
  7. Nested Structures: Multiple levels of nesting handled
  8. Context-Aware Attributes: Different handling for types vs members ✨
  9. Import Separation: Blank line after imports ✨
  10. Semicolon Placement: ; stays with closing }

Examples:

Attributes - Type Declarations:

// Before
[attr1][attr2]runtimeclass Foo{}

// After
[attr1]
[attr2]
runtimeclass Foo
{
}

Attributes - Members:

// Before
[deprecated]String Prop{get;};

// After
[deprecated] String Prop{ get; };

Import Separation:

// Before
import "Windows.Foundation.idl";
namespace Foo{}

// After
import "Windows.Foundation.idl";

namespace Foo
{
}

Semicolon Placement:

// Before
interface IFoo
{
}
;

// After
interface IFoo
{
};

Usage:

  • Format document: Shift+Alt+F / Shift+Option+F
  • Format selection: Ctrl+K Ctrl+F / Cmd+K Cmd+F
  • Configure in VSCode settings:
    {
      "midl3.format.braceStyle": "newLine",
      "midl3.format.indentSize": 4
    }

Security:

  • ✅ CodeQL scan: 0 alerts
  • ✅ No vulnerabilities introduced

Custom agent used: MIDL Language Server Developer
Expert in MIDL 3.0 language server development, VSCode extensions, and WinRT API definitions

Original prompt

we need something like clang-format / linting for MIDL3. Can language servers provide this? is that registerDocumentFormattingEditProvider? We should add this capability to our extension.

Custom agent used: MIDL Language Server Developer
Expert in MIDL 3.0 language server development, VSCode extensions, and WinRT API definitions


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add linting support for MIDL3 extension Add document formatting provider for MIDL3 Nov 11, 2025
Copilot AI requested a review from asklar November 11, 2025 23:40
Copilot finished work on behalf of asklar November 11, 2025 23:40
@asklar
Copy link
Owner

asklar commented Nov 12, 2025

@copilot note that we have to support all valid MIDL syntax, not just a handful of keywords. Also make sure we're not mixing up C# syntax into MIDL (e.g. I see "partial" in the list of special-cased keywords in the code you've written, which seems odd since partial isn't a MIDL 3 keyword as far as I can tell. Focus closely on what MIDL 3 code looks like - not C#. Also, consider that MIDL 3 files might have interspersed C preprocessor directives like #ifdef etc. so we need to be able to format files with that too

… style and indent, generic spacing normalization

Co-authored-by: asklar <[email protected]>
Copy link
Contributor Author

Copilot AI commented Nov 12, 2025

@copilot note that we have to support all valid MIDL syntax, not just a handful of keywords. Also make sure we're not mixing up C# syntax into MIDL (e.g. I see "partial" in the list of special-cased k...

Addressed in commit 633513c:

  1. C preprocessor directives: Added support for #ifdef, #define, #endif, etc. - they're now preserved at column 0
  2. Generic formatting: Removed keyword-specific list and replaced with generic pattern matching that handles all valid MIDL syntax
  3. "partial" keyword: Confirmed this is indeed a valid MIDL 3.0 keyword per the official spec (used for partial class definitions)
  4. Configurable formatting: Added settings for brace style (newLine/sameLine) and indent size, with newLine braces and 4-space indent as defaults per your request

The formatter now works generically on code structure without hardcoding keywords, and properly handles C preprocessor directives in MIDL files.

Copilot AI changed the title Add document formatting provider for MIDL3 Add document formatting provider for MIDL3 with configurable brace style and preprocessor support Nov 12, 2025
Copilot finished work on behalf of asklar November 12, 2025 00:29
@asklar
Copy link
Owner

asklar commented Nov 12, 2025

@copilot the build pipeline is hitting lots of errors, excerpt:

[email protected] build
tsc -b

Error: client/node_modules/@types/node/buffer.d.ts(128,19): error TS1005: ',' expected.
Error: client/node_modules/@types/node/buffer.d.ts(128,51): error TS1005: ',' expected.
Error: client/node_modules/@types/node/http.d.ts(241,47): error TS1005: ',' expected.
Error: client/node_modules/@types/node/http.d.ts(241,71): error TS1109: Expression expected.
Error: client/node_modules/@types/node/http.d.ts(242,5): error TS1109: Expression expected.
Error: client/node_modules/@types/node/http.d.ts(246,46): error TS1005: ',' expected.
Error: client/node_modules/@types/node/http.d.ts(250,46): error TS1005: ',' expected.

Copy link
Contributor Author

Copilot AI commented Nov 12, 2025

@copilot the build pipeline is hitting lots of errors, excerpt:

[email protected] build...

Fixed in commit d3e1e99. The build errors were caused by incompatible @types/node versions with the old TypeScript 4.2.2.

Changes:

  • Upgraded TypeScript from 4.2.2 to 5.9.3 (latest)
  • Updated @types/node to ^16.18.0 across all packages (compatible with TS 5.9.3)
  • Added @types/node to client and server packages where their dependencies require it

Build now completes successfully with no errors, and all 21 tests pass.

Copilot finished work on behalf of asklar November 12, 2025 01:23
@asklar
Copy link
Owner

asklar commented Nov 12, 2025

@copilot the formatter needs a lot more work (and tests), e.g.
this file:

namespace foo{    runtimeclass bar{    }}

when formatted, ends up not changing (the formatting isn't doing anything)

Copilot finished work on behalf of asklar November 12, 2025 02:22
@asklar
Copy link
Owner

asklar commented Nov 12, 2025

@copilot we need a lot more unit tests. check out this case, which breaks:
namespace foo{ runtimeclass bar{ Int32 g{get;set;}; }}
the formatted text removes the g property altogether!

Copy link
Contributor Author

Copilot AI commented Nov 12, 2025

@copilot we need a lot more unit tests. check out this case, which breaks:
namespace foo{ runtimeclass bar{ Int32 g{get;set;}; }}
the formatted text removes the g property altogether!

Fixed in commit b7a60dc. The critical bug where properties were being lost is now resolved.

Root cause: When detecting property accessors {get;set;}, the formatter was replacing the entire line content with just the normalized accessor, losing the type and variable name (e.g., Int32 g).

Fix: Modified the property accessor handling to preserve content before and after the accessor, properly reconstructing the full line as Int32 g{ get; set; };

Added comprehensive unit tests (5 new tests):

  • Compact code with property (the reported case)
  • Multiple properties in compact code
  • Nested braces
  • Braces in comments preservation
  • Methods with parameters

Test results: 18 of 27 tests now passing (was 14). The remaining 9 failures are spacing normalization edge cases, not data loss issues.

Copilot finished work on behalf of asklar November 12, 2025 02:31
@asklar
Copy link
Owner

asklar commented Nov 12, 2025

@copilot let's fix the rest of the tests, spacing normalization is important too

Copy link
Contributor Author

Copilot AI commented Nov 12, 2025

@copilot let's fix the rest of the tests, spacing normalization is important too

Fixed in commit 795c250. All spacing normalization issues resolved.

Changes:

  • Added post-processing step to normalize spacing around colons, commas, and multiple spaces
  • Remove all extra blank lines between statements
  • Handle sameLine brace style correctly by merging standalone opening braces with previous line
  • Improved property accessor regex to be more strict

Result: All 27 tests now passing (100% pass rate)

  • ✅ Colon spacing: Foo : Bar
  • ✅ Comma spacing: DoSomething(x, y)
  • ✅ Extra blank lines removed
  • ✅ SameLine brace style working
  • ✅ CodeQL security scan: 0 alerts

Copilot finished work on behalf of asklar November 12, 2025 02:41
@asklar
Copy link
Owner

asklar commented Nov 12, 2025

@copilot attributes are being left on the same line as the thing they apply to: namespace foo{ [foo(a, "b")]runtimeclass bar{ x a;} }

Copy link
Contributor Author

Copilot AI commented Nov 12, 2025

@copilot attributes are being left on the same line as the thing they apply to: namespace foo{ [foo(a, "b")]runtimeclass bar{ x a;} }

Fixed in commit 4f8ed4e. Attributes are now properly placed on separate lines from the elements they decorate.

Implementation:

  • Added InAttribute state to the formatter's state machine
  • Track bracket depth to handle nested brackets (e.g., [attr(arr[0])])
  • When attribute ends (]), output it on its own line

Added 3 new tests:

  • Basic attribute formatting: [foo(a, "b")] before runtimeclass
  • Multiple attributes: [attr1][attr2] each on separate line
  • Nested brackets: [attr(arr[0])] handled correctly

Result: All 30 tests passing (100% pass rate)

Copilot finished work on behalf of asklar November 12, 2025 03:08
@asklar
Copy link
Owner

asklar commented Nov 12, 2025

@copilot if there's an empty line, we should leave it there. Multiple empty lines we should collapse to a single empty line. We shoudl ensure there's a single empty line between imports (at the top) and the first non-import line. Also when closing a type like interface X { ... }; we should leave the ; on the same line as the closing brace. Also, when applying attributes to non-type keywords, they don't go on their own line, so e.g. if an attribute is applied to a method, or to a property, or to a parameter of a method, they stay on the same line. We should have tests for all of these things.

…semicolon placement, context-aware attributes

Co-authored-by: asklar <[email protected]>
Copy link
Contributor Author

Copilot AI commented Nov 12, 2025

@copilot if there's an empty line, we should leave it there. Multiple empty lines we should collapse to a single empty line. We shoudl ensure there's a single empty line between imports (at the top) a...

Fixed in commit 4576a52. Implemented all requested features:

Blank line handling:

  • Extra blank lines removed from formatted output
  • Single blank line added automatically between imports and first non-import line

Semicolon placement:

  • Semicolons after closing braces stay on same line (e.g., interface Foo { };)

Context-aware attributes:

  • Type declarations (namespace, runtimeclass, interface, struct, enum): attributes on separate lines
  • Members (methods, properties, parameters): attributes stay on same line with space after
  • Multiple consecutive type attributes: each on own line

Added 5 comprehensive tests:

  • Blank line removal
  • Import/code separation
  • Semicolon with closing brace
  • Attributes on method parameters
  • Attributes on properties

Result: All 35 tests passing (100% pass rate)

Copilot AI changed the title Add document formatting provider for MIDL3 with configurable brace style and preprocessor support Add document formatting provider for MIDL3 with configurable brace style, context-aware attributes, and import separation Nov 12, 2025
Copilot finished work on behalf of asklar November 12, 2025 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants