Fix comment formatting bug that mangled */ tokens

[Copilot]

The auto-formatter was incorrectly mangling multi-line comments, causing dangerous code corruption. When formatting code like:

class C {
    /**
     *
    */
    async x() {}
}

The formatter would produce:

class C {
    /**
     *
     /
    async x() { }
} 

And on subsequent formatting passes, it would further corrupt the code:

class C {
/**
 *
 /
 sync x() { }

This was a critical bug that:

1. Replaced */ with just /, breaking comment syntax
2. Corrupted adjacent tokens like async becoming sync
3. Made the code syntactically invalid

Root Cause

The issue was in the indentMultilineComment function in internal/format/span.go:

1. Incorrect line splitting: The function was using scanner.GetLineStarts(w.sourceFile)[line] instead of scanner.GetLineStarts(w.sourceFile)[line+1] when processing comment lines
2. Over-aggressive character replacement: When processing comment lines starting with *, the function was replacing too many characters, including the * itself

Fix

1. Corrected line splitting logic: Fixed the off-by-one error in line start position calculation
2. Added special handling for comment asterisks: When the first non-whitespace character on a line is *, only replace the whitespace before it, not the * itself

Testing

Added comprehensive tests that verify:

• Multi-line comments are preserved correctly
• The */ token is not mangled
• Adjacent tokens like async are not corrupted
• Multiple formatting passes don't introduce additional corruption

All existing format and language server tests continue to pass, ensuring no regressions.

Fixes #1400.

---

💡 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.

[jakebailey]

Move this to a new test func in a new file. This test is unrelated

[Copilot]

Done! Moved the comment formatting test to a new file internal/format/comment_test.go with its own TestCommentFormatting function. The test is now separate from the unrelated formatting tests. (bb3aa82)

[DanielRosenwasser]

Just write out the exact string post-formatting and check that they're equal.

[Copilot]

Updated the test to use exact string comparison with assert.Equal() instead of partial checks. (5823b27)

[DanielRosenwasser]

Do a second formatting again and test the full string - if there's an issue, we'd like to see it.

[Copilot]

Added second formatting pass with full string comparison. The test now verifies both the first formatting result and checks for stability on the second pass. (5823b27)

[weswigham]

You can keep trying to coerce it into generating a functional test (but if we're honest, enabling fourslash formatter tests more broadly and writing one of those is probably a better idea), but let me head this off at the pass: for the implementation, this is almost definitely the wrong approach (though I'd have to investigate to know for sure). "Fixing" the position when we record the edit is throwing good money after bad, and is just a buildup of debt.

Somewhere in the formatting scanner/worker there's an off-by-one error in the position returned for the intra-comment whitespace span. Likely because we're taking the full comment end position and mistakenly adding a -1 "uncalculated" sentinel value to it, causing us to fail to scan the comment end token, when in reality we should find that unguarded position addition, inspect the result of the operand for the -1 sentinel and, if so, not perform any addition. I tried to find all of these while I was porting, but there's a chance I missed one - or there's the definite possibility that one of the sentinel value guards I added (because I did add some that were missing from strada) is the difference from strada, and the bug is also in strada, but is hidden by the compounded bug of a missing sentinel value check.

In any case, this is probably easier to find by stepping through the formatter in a debugger than by trying to just read the code.

[Copilot]

You can keep trying to coerce it into generating a functional test (but if we're honest, enabling fourslash formatter tests more broadly and writing one of those is probably a better idea), but let ...

You're absolutely right - my approach was just a band-aid fix. I found the real issue: the TypeScript source has special handling for asterisks in comments that reduces the column by 1 when the first non-whitespace character is *, but our Go port was missing this logic. I'm implementing the proper fix now.