Skip to content

Conversation

andrewbranch
Copy link
Member

Discovered this while investigating something else

@andrewbranch andrewbranch requested review from Copilot, navya9singh and iisaduan and removed request for Copilot September 3, 2025 22:41
@andrewbranch andrewbranch marked this pull request as draft September 3, 2025 22:42
@andrewbranch
Copy link
Member Author

Wait this isn't quite right

@andrewbranch
Copy link
Member Author

Yes it is. Test modification proves it

@andrewbranch andrewbranch marked this pull request as ready for review September 3, 2025 22:44
@Copilot Copilot AI review requested due to automatic review settings September 3, 2025 22:44
Copy link
Contributor

@Copilot 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 an out-of-bounds panic when the tsconfig "files" array contains duplicate entries. The solution tracks the actual number of literal files separately from the total file count, preventing incorrect array slicing when duplicates cause the literal file count to be smaller than expected.

Key changes:

  • Modified getFileNamesFromConfigSpecs to return both file names and literal file count
  • Added literalFileNamesLen field to track the actual number of unique literal files
  • Updated LiteralFileNames() method to use the tracked count instead of the original file spec length

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
internal/tsoptions/tsconfigparsing.go Modified file name resolution functions to return literal file count and updated parsing logic to track this count
internal/tsoptions/parsedcommandline.go Added literalFileNamesLen field and updated LiteralFileNames() method to use the tracked count instead of file spec length
internal/tsoptions/parsedcommandline_test.go Added test case to verify duplicate files in tsconfig are handled correctly

Copy link
Member

@sheetalkamat sheetalkamat left a comment

Choose a reason for hiding this comment

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

I think fix should be at program and hence duplicate of #1549

The main blocking here is file casing and module resolutions that happen and discussion for that fix here #1535

@andrewbranch
Copy link
Member Author

I don’t think this is a duplicate; I wasn’t targeting a fix to programs at all. The test case added here, using only config file parsing and its methods, panics without the fix. LiteralFileNames is used by the LSP infrastructure without a program being involved.

@sheetalkamat
Copy link
Member

I don’t think this is a duplicate; I wasn’t targeting a fix to programs at all. The test case added here, using only config file parsing and its methods, panics without the fix. LiteralFileNames is used by the LSP infrastructure without a program being involved.

Oh i see.. I forgot that we use Literal file names stuff for watching and matching..

@andrewbranch andrewbranch added this pull request to the merge queue Sep 5, 2025
Merged via the queue into microsoft:main with commit 9347366 Sep 5, 2025
22 checks passed
@andrewbranch andrewbranch deleted the bug/literal-file-names branch September 5, 2025 17:32
Harsh1925 pushed a commit to Harsh1925/typescript-go that referenced this pull request Sep 9, 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.

2 participants