Skip to content

fix(printer): map source maps to MostOriginal source to avoid out-of-range positions #1539

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

relsunkaev
Copy link

@relsunkaev relsunkaev commented Aug 7, 2025

Fixes a panic during declaration map emission by mapping source-map positions against the original parse tree source.

Changes:

  • printer: use MostOriginal(node) to select mapping source in emitSourceMaps*
  • test: add isolated sourcemap mismatch repro under internal/printer

All tests pass locally: 9155 tests, 506 skipped.

…range positions during emit

test(printer): add sourcemap mismatch repro exercising original-node mapping

chore: remove temporary debug logging
@Copilot Copilot AI review requested due to automatic review settings August 7, 2025 23:33
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 a panic during declaration map emission that occurs when source map positions reference different files or position spaces than the current file being processed. The fix ensures source maps use the original parse tree source file for position mappings instead of the current file.

Key changes:

  • Modified printer source map emission functions to use MostOriginal(node) for selecting the correct source file
  • Added comprehensive test coverage for the source map mismatch scenario
  • Removed temporary debug logging

Reviewed Changes

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

Show a summary per file
File Description
internal/scanner/utilities.go Removed debug comment line
internal/scanner/scanner.go Fixed variable scoping in GetLineAndCharacterOfPosition function
internal/printer/utilities.go Removed debug comment line
internal/printer/sourcemap_mismatch_panic_test.go Added isolated test reproducing the source map panic scenario
internal/printer/printer.go Updated source map emission to use original node's source file for position mappings
internal/execute/blackbox_panic_test.go Added blackbox test for declaration map panic with destructured parameters

@@ -19,6 +19,7 @@ package printer

import (
"fmt"
"os"
Copy link
Preview

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

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

The os import is added but not used in the visible code. This appears to be leftover debug code that should be removed.

Suggested change
"os"

Copilot uses AI. Check for mistakes.

@relsunkaev
Copy link
Author

@relsunkaev please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Uplinq Inc."

@jakebailey
Copy link
Member

This PR contains a whole lot of unrelated changes / formatting issues, and adds tests in formats we don't use for testing.

I'm not sure if this is just AI generated or what, but it would need changes to be able to accept (and at least formatting to review)

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