-
Notifications
You must be signed in to change notification settings - Fork 732
Fix formatter adding extra space at end of line without trailing newline #1933
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
Conversation
Co-authored-by: jakebailey <[email protected]>
The issue was that EndOfFileToken was incorrectly included in the `allTokens` array, which caused the "SpaceAfterSemicolon" rule to match between semicolon and EOF tokens. This resulted in a space being inserted at the end of files without trailing newlines. Also fixed GetECMAEndLinePosition to correctly return the index of the last character (length-1) for the last line, matching the TypeScript implementation. Co-authored-by: jakebailey <[email protected]>
Co-authored-by: jakebailey <[email protected]>
Added test cases for various code patterns to ensure the formatter doesn't add trailing spaces before EOF. Co-authored-by: jakebailey <[email protected]>
|
This probably fixes the extra newlines that @andrewbranch found when working on autoimports formatting? |
| if token != ast.KindEndOfFile { | ||
| allTokens = append(allTokens, token) | ||
| } |
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.
This is correct, reading Strada.
Moved the test from api_test.go to a new format_test.go file for better organization. The api_test.go file should contain tests for the main API/integration tests, while specific formatting edge case tests belong in their own file. Co-authored-by: jakebailey <[email protected]>
The changes to GetECMAEndLinePosition were not needed to fix the issue. The fix in rules.go to exclude EOF from allTokens is sufficient on its own. Co-authored-by: jakebailey <[email protected]>
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 fixes a bug where the formatter incorrectly added a trailing space to files without a trailing newline. The root cause was that EndOfFileToken was being included in the allTokens array, causing the "SpaceAfterSemicolon" formatting rule to trigger between semicolons and EOF.
Key changes:
- Excluded
ast.KindEndOfFilefrom theallTokensarray in the rule builder - Added comprehensive test coverage for formatting files without trailing newlines
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/format/rules.go | Modified token array building to exclude EOF token, preventing incorrect spacing rules |
| internal/format/format_test.go | Added new test file with test cases verifying no trailing spaces are added to files without trailing newlines |
Problem
The formatter was incorrectly adding a space at the end of files that don't have a trailing newline. For example, formatting a file containing just
1;(with no trailing newline) would result in1;(with a space added after the semicolon).Root Cause
The
EndOfFileTokenwas incorrectly included in theallTokensarray, which caused the "SpaceAfterSemicolon" formatting rule to match between semicolon and EOF tokens. TypeScript explicitly excludes EOF from itsallTokensarray to prevent this.Solution
internal/format/rules.go: Modified
getAllRules()to excludeast.KindEndOfFilewhen building theallTokensarray, matching TypeScript's behavior.internal/format/format_test.go: Added comprehensive test cases to verify the fix works for various code patterns including simple statements, function calls, variable declarations, and multiple statements without trailing newlines.
Testing
All tests pass, including:
The fix is minimal and focused - it only changes the token rules to exclude EOF from
allTokens, preventing the "SpaceAfterSemicolon" rule from incorrectly applying between semicolon and EOF tokens.Fixes #1926
Original prompt
Fixes #1926
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.