Skip to content

Conversation

@gabritto
Copy link
Member

Follow up to #2079.

In Strada fourslash, we obtained compiler diagnostics directly and reused the compiler test harness functions for producing a baseline file for diagnostics. In Corsa fourslash, we get LSP diagnostics instead of ast.Diagnostic, and so to reuse the existing diagnostic baseline functions I had to refactor them to accept an interface with common methods for both the LSP and internal diagnostic types.

@gabritto gabritto marked this pull request as ready for review November 14, 2025 23:09
Copilot AI review requested due to automatic review settings November 14, 2025 23:09
Copilot finished reviewing on behalf of gabritto November 14, 2025 23:11
Copy link
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 ports baseline diagnostics tests from Strada fourslash to Corsa fourslash. To enable reusing compiler test baseline functions for LSP diagnostics, it refactors diagnostic formatting code to work with a common interface rather than concrete ast.Diagnostic types.

Key changes:

  • Introduces Diagnostic and FileLike interfaces to abstract diagnostic formatting
  • Wraps ast.Diagnostic in an adapter type to implement the new interface
  • Implements fourslash LSP diagnostic baseline testing via VerifyBaselineNonSuggestionDiagnostics

Reviewed Changes

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

Show a summary per file
File Description
internal/diagnosticwriter/diagnosticwriter.go Adds Diagnostic and FileLike interfaces; wraps ast.Diagnostic to implement these interfaces; updates formatting functions to accept interface types
internal/fourslash/fourslash.go Implements Diagnostic and DiagnosticFile types for LSP diagnostics; adds VerifyBaselineNonSuggestionDiagnostics method
internal/fourslash/baselineutil.go Adds "Syntax and Semantic Diagnostics" to baseline extension mapping
internal/testutil/tsbaseline/error_baseline.go Makes error baseline functions generic over the Diagnostic interface
internal/testutil/tsbaseline/js_emit_baseline.go Updates calls to use wrapped diagnostics
internal/scanner/scanner.go Changes GetECMAPositionOfLineAndCharacter to accept ast.SourceFileLike interface
internal/ls/diagnostics.go Updates to use new WriteFlattenedASTDiagnosticMessage function
internal/execute/tsc/diagnostics.go Wraps ast.Diagnostic instances before passing to formatting functions
Multiple test/baseline files Adds new fourslash tests and baseline files for JavaScript syntactic diagnostics
internal/fourslash/_scripts/convertFourslash.mts Adds support for converting baseline diagnostic test commands
internal/tsoptions/*_test.go Updates to use FromASTDiagnostics wrapper
internal/testutil/parsetestutil/parsetestutil.go Updates to use FromASTDiagnostics wrapper

@gabritto gabritto added this pull request to the merge queue Nov 17, 2025
Merged via the queue into main with commit 08bc24d Nov 17, 2025
22 checks passed
@gabritto gabritto deleted the gabritto/testdiagnostics2 branch November 17, 2025 16:43
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.

3 participants