Skip to content

fix: remove unexpected space in output format string#281

Merged
hardfist merged 2 commits intoweb-infra-dev:mainfrom
ulivz:fix/unexpected-space-after-fix
Aug 18, 2025
Merged

fix: remove unexpected space in output format string#281
hardfist merged 2 commits intoweb-infra-dev:mainfrom
ulivz:fix/unexpected-space-after-fix

Conversation

@ulivz
Copy link
Copy Markdown
Member

@ulivz ulivz commented Aug 15, 2025

Summary

Improves the autofix for no-unnecessary-type-assertion rule to properly remove whitespace before 'as' keyword, preventing extra spaces in output.

Close: #280

  • Before: foo() as number -> foo() ;
  • After: foo() as number -> foo();

Edge Case

This Pull Request does not solve an edge case:

  • Source:
const foo = (  3 + 5
  ) /*as*/ as //as
  (
    number
  );
  • Expected:
const foo = (  3 + 5
  ) /*as*/  //as
;
  • Current workaround:
const foo = (  3 + 5
  ) /*as*/;

Copilot AI review requested due to automatic review settings August 15, 2025 15:54
@netlify
Copy link
Copy Markdown

netlify bot commented Aug 15, 2025

Deploy Preview for rslint canceled.

Name Link
🔨 Latest commit 60ba29e
🔍 Latest deploy log https://app.netlify.com/projects/rslint/deploys/68a0b0b25481930008d812a1

Copy link
Copy Markdown
Contributor

Copilot AI left a 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 formatting issue in the autofix functionality of the no-unnecessary-type-assertion rule where extra spaces were being left in the output after removing unnecessary type assertions. The fix ensures that when removing type assertions like foo() as number, the result is properly formatted as foo(); instead of foo() ;.

Key changes:

  • Improved whitespace handling in the autofix logic to include preceding spaces when removing 'as' keyword
  • Simplified the fix generation to use a single removal range instead of multiple separate fixes
  • Updated test expectations to reflect the corrected output format

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
no_unnecessary_type_assertion.go Enhanced autofix logic to properly handle whitespace removal before 'as' keyword and simplified fix generation
no_unnecessary_type_assertion_test.go Updated test case expectation to reflect the corrected output format without extra spaces

@ulivz ulivz changed the title fix: remove unexpected space in output format string WIP: fix: remove unexpected space in output format string Aug 15, 2025
@ulivz ulivz force-pushed the fix/unexpected-space-after-fix branch from e0e5af3 to 9a5f9f4 Compare August 15, 2025 16:00
@ulivz ulivz changed the title WIP: fix: remove unexpected space in output format string fix: remove unexpected space in output format string Aug 15, 2025
@ulivz
Copy link
Copy Markdown
Member Author

ulivz commented Aug 15, 2025

Updated: I tested the original issue (#280) locally with the fixed build bin and it was verified to be resolved:

Before

image

After

image

ulivz added 2 commits August 17, 2025 00:24
Improves the autofix for no-unnecessary-type-assertion rule to properly
remove whitespace before 'as' keyword, preventing extra spaces in output.

Before: foo() as number -> foo()  ;
After:  foo() as number -> foo();

refactor: remove redundant comments from no_unnecessary_type_assertion

Remove duplicate explanatory comments that were repeating
the obvious functionality of the code below them.
Add bounds check before accessing sourceText[startPos-2] to prevent
potential panic when startPos-2 < 0.
@zoolsher zoolsher force-pushed the fix/unexpected-space-after-fix branch from 88a844e to 60ba29e Compare August 16, 2025 16:24
@zoolsher zoolsher requested a review from hardfist August 16, 2025 16:24
sourceText := ctx.SourceFile.Text()
startPos := asKeywordRange.Pos()

if startPos > expression.End() && sourceText[startPos-1] == ' ' {
Copy link
Copy Markdown
Contributor

@hardfist hardfist Aug 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems can't handle following case well

const foo = 3               as 3; // multi space
const foo = 3 /* some comment here*/ as 3 // contains comment

the origin typescript-eslint implementation use a util to handle space and comment https://github.com/typescript-eslint/typescript-eslint/blob/b2ee794265c4c727009e65a4eb5f06fad9686cf8/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts#L304

@hardfist
Copy link
Copy Markdown
Contributor

I'm gonna merge this first and provide utils to simplify format logic(align with typeccript-eslint) in following pr

@hardfist hardfist merged commit c5d9af3 into web-infra-dev:main Aug 18, 2025
12 checks passed
@ulivz
Copy link
Copy Markdown
Member Author

ulivz commented Aug 18, 2025

@hardfist Thanks! I am still working on some releases this week and haven't had time to follow up. If this problem still exists by then, I will be happy to contribute.

ScriptedAlchemy pushed a commit to ScriptedAlchemy/rslint that referenced this pull request Aug 19, 2025
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.

[Bug]: An unexpected space after --fix

3 participants