Skip to content

Commit 892896b

Browse files
iisaduaniisaduan
authored
add error handling to fourslash parsing (#1179)
1 parent 64f9931 commit 892896b

File tree

2 files changed

+62
-30
lines changed

2 files changed

+62
-30
lines changed

‎internal/fourslash/test_parser.go

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package fourslash
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"strings"
67
"testing"
78
"unicode/utf8"
@@ -56,11 +57,15 @@ func ParseTestData(t *testing.T, contents string, fileName string) TestData {
5657
markerPositions := make(map[string]*Marker)
5758
var markers []*Marker
5859
var ranges []*RangeMarker
59-
filesWithMarker, symlinks, _, globalOptions := testrunner.ParseTestFilesAndSymlinks(
60+
61+
filesWithMarker, symlinks, _, globalOptions, e := testrunner.ParseTestFilesAndSymlinks(
6062
contents,
6163
fileName,
6264
parseFileContent,
6365
)
66+
if e != nil {
67+
t.Fatalf("Error parsing fourslash data: %s", e.Error())
68+
}
6469

6570
hasTSConfig := false
6671
for _, file := range filesWithMarker {
@@ -138,7 +143,7 @@ const (
138143
stateInObjectMarker
139144
)
140145

141-
func parseFileContent(fileName string, content string, fileOptions map[string]string) *testFileWithMarkers {
146+
func parseFileContent(fileName string, content string, fileOptions map[string]string) (*testFileWithMarkers, error) {
142147
fileName = tspath.GetNormalizedAbsolutePath(fileName, "/")
143148

144149
// The file content (minus metacharacters) so far
@@ -159,7 +164,7 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
159164
column := 1
160165

161166
// The current marker (or maybe multi-line comment?) we're parsing, possibly
162-
var openMarker locationInformation
167+
var openMarker *locationInformation
163168

164169
// The latest position of the start of an unflushed plain text area
165170
lastNormalCharPosition := 0
@@ -197,7 +202,7 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
197202
} else if previousCharacter == '|' && currentCharacter == ']' {
198203
// found a range end
199204
if len(openRanges) == 0 {
200-
reportError(fileName, line, column, "Found range end with no matching start.")
205+
return nil, reportError(fileName, line, column, "Found range end with no matching start.")
201206
}
202207
rangeStart := openRanges[len(openRanges)-1]
203208
openRanges = openRanges[:len(openRanges)-1]
@@ -219,7 +224,7 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
219224
} else if previousCharacter == '/' && currentCharacter == '*' {
220225
// found a possible marker start
221226
state = stateInSlashStarMarker
222-
openMarker = locationInformation{
227+
openMarker = &locationInformation{
223228
position: (i - 1) - difference,
224229
sourcePosition: i - 1,
225230
sourceLine: line,
@@ -228,7 +233,7 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
228233
} else if previousCharacter == '{' && currentCharacter == '|' {
229234
// found an object marker start
230235
state = stateInObjectMarker
231-
openMarker = locationInformation{
236+
openMarker = &locationInformation{
232237
position: (i - 1) - difference,
233238
sourcePosition: i - 1,
234239
sourceLine: line,
@@ -240,7 +245,10 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
240245
// Object markers are only ever terminated by |} and have no content restrictions
241246
if previousCharacter == '|' && currentCharacter == '}' {
242247
objectMarkerData := strings.TrimSpace(content[openMarker.sourcePosition+2 : i-1])
243-
marker := getObjectMarker(fileName, openMarker, objectMarkerData)
248+
marker, e := getObjectMarker(fileName, openMarker, objectMarkerData)
249+
if e != nil {
250+
return nil, e
251+
}
244252

245253
if len(openRanges) > 0 {
246254
openRanges[len(openRanges)-1].marker = marker
@@ -252,7 +260,7 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
252260
difference += i + 1 - openMarker.sourcePosition
253261

254262
// Reset the state
255-
openMarker = locationInformation{}
263+
openMarker = nil
256264
state = stateNone
257265
}
258266
case stateInSlashStarMarker:
@@ -276,7 +284,7 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
276284
difference += i + 1 - openMarker.sourcePosition
277285

278286
// Reset the state
279-
openMarker = locationInformation{}
287+
openMarker = nil
280288
state = stateNone
281289
} else if !(stringutil.IsDigit(currentCharacter) ||
282290
stringutil.IsASCIILetter(currentCharacter) ||
@@ -289,7 +297,7 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
289297
// Bail out the text we've gathered so far back into the output
290298
flush(i)
291299
lastNormalCharPosition = i
292-
openMarker = locationInformation{}
300+
openMarker = nil
293301
state = stateNone
294302
}
295303
}
@@ -309,6 +317,15 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
309317
// Add the remaining text
310318
flush(-1)
311319

320+
if len(openRanges) > 0 {
321+
openRange := openRanges[0]
322+
return nil, reportError(fileName, openRange.sourceLine, openRange.sourceColumn, "Unterminated range.")
323+
}
324+
325+
if openMarker != nil {
326+
return nil, reportError(fileName, openMarker.sourceLine, openMarker.sourceColumn, "Unterminated marker.")
327+
}
328+
312329
outputString := output.String()
313330
// Set LS positions for markers
314331
lineMap := ls.ComputeLineStarts(outputString)
@@ -338,22 +355,20 @@ func parseFileContent(fileName string, content string, fileOptions map[string]st
338355
file: testFileInfo,
339356
markers: markers,
340357
ranges: rangeMarkers,
341-
}
358+
}, nil
342359
}
343360

344-
func getObjectMarker(fileName string, location locationInformation, text string) *Marker {
361+
func getObjectMarker(fileName string, location *locationInformation, text string) (*Marker, error) {
345362
// Attempt to parse the marker value as JSON
346363
var v interface{}
347364
e := json.Unmarshal([]byte("{ "+text+" }"), &v)
348365

349366
if e != nil {
350-
reportError(fileName, location.sourceLine, location.sourceColumn, "Unable to parse marker text "+text)
351-
return nil
367+
return nil, reportError(fileName, location.sourceLine, location.sourceColumn, "Unable to parse marker text "+text)
352368
}
353369
markerValue, ok := v.(map[string]interface{})
354370
if !ok || len(markerValue) == 0 {
355-
reportError(fileName, location.sourceLine, location.sourceColumn, "Object markers can not be empty")
356-
return nil
371+
return nil, reportError(fileName, location.sourceLine, location.sourceColumn, "Object markers can not be empty")
357372
}
358373

359374
marker := &Marker{
@@ -369,10 +384,17 @@ func getObjectMarker(fileName string, location locationInformation, text string)
369384
}
370385
}
371386

372-
return marker
387+
return marker, nil
388+
}
389+
390+
func reportError(fileName string, line int, col int, message string) error {
391+
return &fourslashError{fmt.Sprintf("%v (%v,%v): %v", fileName, line, col, message)}
392+
}
393+
394+
type fourslashError struct {
395+
err string
373396
}
374397

375-
func reportError(fileName string, line int, col int, message string) {
376-
// !!! not implemented
377-
// errorMessage := fileName + "(" + string(line) + "," + string(col) + "): " + message;
398+
func (e *fourslashError) Error() string {
399+
return e.err
378400
}

‎internal/testrunner/test_case_parser.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ var fourslashDirectives = []string{"emitthisfile"}
4747
// Given a test file containing // @FileName directives,
4848
// return an array of named units of code to be added to an existing compiler instance.
4949
func makeUnitsFromTest(code string, fileName string) testCaseContent {
50-
testUnits, symlinks, currentDirectory, _ := ParseTestFilesAndSymlinks(
50+
testUnits, symlinks, currentDirectory, _, _ := ParseTestFilesAndSymlinks(
5151
code,
5252
fileName,
53-
func(filename string, content string, fileOptions map[string]string) *testUnit {
54-
return &testUnit{content: content, name: filename}
53+
func(filename string, content string, fileOptions map[string]string) (*testUnit, error) {
54+
return &testUnit{content: content, name: filename}, nil
5555
},
5656
)
5757
if currentDirectory == "" {
@@ -108,8 +108,8 @@ func makeUnitsFromTest(code string, fileName string) testCaseContent {
108108
func ParseTestFilesAndSymlinks[T any](
109109
code string,
110110
fileName string,
111-
parseFile func(filename string, content string, fileOptions map[string]string) T,
112-
) (units []T, symlinks map[string]string, currentDir string, globalOptions map[string]string) {
111+
parseFile func(filename string, content string, fileOptions map[string]string) (T, error),
112+
) (units []T, symlinks map[string]string, currentDir string, globalOptions map[string]string, e error) {
113113
// List of all the subfiles we've parsed out
114114
var testUnits []T
115115

@@ -119,6 +119,7 @@ func ParseTestFilesAndSymlinks[T any](
119119
var currentFileContent strings.Builder
120120
var currentFileName string
121121
var currentDirectory string
122+
var parseError error
122123
currentFileOptions := make(map[string]string)
123124
symlinks = make(map[string]string)
124125
globalOptions = make(map[string]string)
@@ -153,7 +154,11 @@ func ParseTestFilesAndSymlinks[T any](
153154
// New metadata statement after having collected some code to go with the previous metadata
154155
if currentFileName != "" {
155156
// Store result file
156-
newTestFile := parseFile(currentFileName, currentFileContent.String(), currentFileOptions)
157+
newTestFile, e := parseFile(currentFileName, currentFileContent.String(), currentFileOptions)
158+
if e != nil {
159+
parseError = e
160+
break
161+
}
157162
testUnits = append(testUnits, newTestFile)
158163

159164
// Reset local data
@@ -184,11 +189,16 @@ func ParseTestFilesAndSymlinks[T any](
184189
currentFileName = tspath.GetBaseFileName(fileName)
185190
}
186191

187-
// EOF, push whatever remains
188-
newTestFile2 := parseFile(currentFileName, currentFileContent.String(), currentFileOptions)
189-
testUnits = append(testUnits, newTestFile2)
192+
// if there are no parse errors so far, parse the rest of the file
193+
if parseError == nil {
194+
// EOF, push whatever remains
195+
newTestFile2, e := parseFile(currentFileName, currentFileContent.String(), currentFileOptions)
196+
197+
parseError = e
198+
testUnits = append(testUnits, newTestFile2)
199+
}
190200

191-
return testUnits, symlinks, currentDirectory, globalOptions
201+
return testUnits, symlinks, currentDirectory, globalOptions, parseError
192202
}
193203

194204
func extractCompilerSettings(content string) rawCompilerSettings {

0 commit comments

Comments
 (0)