Skip to content

Port some more truncation check related code #1373

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 3 commits into
base: main
Choose a base branch
from

Conversation

jakebailey
Copy link
Member

These are things I found while trying to work through #988.

@jakebailey jakebailey requested review from weswigham and Copilot July 8, 2025 23:48
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

Ports additional truncation-check features and refactors type-to-string logic, while updating submodule compiler snapshots.

  • Refactors mapToTypeNodes to support truncation flags and adds corresponding truncation handling.
  • Switches from typeToStringEx to a simpler typeToString helper and replaces visitedTypes map with collections.Set.
  • Updates submodule compiler baselines for namespace disambiguation and computed-name declaration emit tests, and adds AST pooling for intersection-type nodes.

Reviewed Changes

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

Show a summary per file
File Description
internal/collections/multimap.go Added Values() method to MultiMap
internal/checker/relater.go Replaced typeToStringEx calls with typeToString
internal/checker/printer.go Introduced typeToString helper and updated TypeToString
internal/checker/nodebuilderimpl.go Refactored mapToTypeNodes, added truncation logic, changed visitedTypes type
internal/checker/nodebuilder.go Removed visitedTypes map initialization
internal/ast/ast.go Added pooling support for IntersectionTypeNode
testdata/baselines/reference/submodule/compiler/namespaceDisambiguationInUnion.errors.txt Updated expected error messages
testdata/baselines/reference/submodule/compiler/declarationEmitSimpleComputedNames1.js Fixed computed-names declaration emit baselines
testdata/baselines/reference/submodule/compiler/declarationEmitClassMemberWithComputedPropertyName.js Adjusted computed-property declaration emit baselines
Comments suppressed due to low confidence (3)

internal/checker/nodebuilder.go:28

  • The visitedTypes Set field is never initialized in enterContext, leading to nil-pointer panics when calling .Has or .Add. Initialize it, e.g., visitedTypes: collections.NewSet[TypeId](),.
		inferTypeParameters:      make([]*Type, 0),

internal/collections/multimap.go:63

  • Consider adding unit tests for the new Values method to verify it returns the expected sequence of value slices.
func (s *MultiMap[K, V]) Values() iter.Seq[[]V] {

internal/checker/nodebuilderimpl.go:242

  • Calling fmt.Sprintf here requires importing the fmt package. Add import "fmt" to the file's import block.
				text := fmt.Sprintf("... %d more ...", len(list)-2)

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.

1 participant