add unknown option error for TypeAcquisition #859
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds dedicated error handling for unknown type acquisition options during tsconfig parsing. Key changes include updating baseline test files to expect diagnostic TS17010 for type acquisition, modifying tsconfig parsing logic in internal/tsoptions/tsconfigparsing.go, and updating helper and error creation functions to use a new diagnostic message.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| testdata/baselines/reference/config/tsconfigParsing/Convert incorrect format tsconfig.json to typeAcquisition with jsonSourceFile api.js | Updated test baseline to reflect new error message for type acquisition option. |
| testdata/baselines/reference/config/tsconfigParsing/Convert incorrect format tsconfig.json to typeAcquisition with json api.js | Updated test baseline error message from TS5023 to TS17010. |
| testdata/baselines/reference/config/tsconfigParsing/Convert incorrect format jsconfig.json to typeAcquisition with jsonSourceFile api.js | Updated test baseline to reflect new diagnostic message for unknown type acquisition option. |
| testdata/baselines/reference/config/tsconfigParsing/Convert incorrect format jsconfig.json to typeAcquisition with json api.js | Updated test baseline error message from TS5023 to TS17010. |
| internal/tsoptions/tsconfigparsing.go | Modified unknown option error handling condition to only report when extra diagnostics are available and switched to use the new createUnknownOptionError function. |
| internal/tsoptions/parsinghelpers.go | Added UnknownOptionDiagnostic methods for different parser types to support the new error reporting. |
| internal/tsoptions/errors.go | Refactored error creation for unknown options, ensuring consistency by passing in the diagnostic message from the parser. Also introduced the extraKeyDiagnostics helper. |
Comments suppressed due to low confidence (1)
internal/tsoptions/errors.go:78
- [nitpick] Verify that the ordering and use of parameters (especially the newly introduced unknownOptionDiagnostic) are consistently applied across all error creation calls and document this behavior in the codebase if not already done.
return createDiagnosticForNodeInSourceFileOrCompilerDiagnostic(sourceFile, node, unknownOptionDiagnostic, unknownOptionErrorText)
| if !ok { | ||
| // !!! TODO?: support suggestion | ||
| errors = append(errors, ast.NewCompilerDiagnostic(diagnostics.Unknown_compiler_option_0, key)) | ||
| errors = append(errors, createUnknownOptionError(key, result.UnknownOptionDiagnostic(), "", nil, nil, nil)) |
There was a problem hiding this comment.
isnt this just result.UnknownOptionDiagnostic() after adding that method to optionParser
There was a problem hiding this comment.
result.UnknownOptionDiagnostic() only returns the diagnostic message. We need the call to create the full error (and format if needed)
…o unknownOptionError
| return ast.NewCompilerDiagnostic(message, args...) | ||
| } | ||
|
|
||
| func extraKeyDiagnostics(s string) *diagnostics.Message { |
There was a problem hiding this comment.
sorry i meant here shouldnt this be from parser.unknownDiagnostics ? i dont understand why we have two ways of doing same thing
There was a problem hiding this comment.
They are implemented in two different ways because of how it was implemented in strada, and then the two different code paths that use the messages.
When we are parsing from a map to a struct, we only have an optionParser, we don't know what type of optionParser it is, so we can't use extraKeyDiagnostics directly.
When we are parsing the tsconfig, we don't use optionParser, so we are not able call optionsParser.UnknownOptionDiagnostic().
I could refactor each of the optionsParser.UnknownOptionDiagnostic() to be like
func (o *typeAcquisitionParser) UnknownOptionDiagnostic() *diagnostics.Message {
return extraKeyDiagnostics("typeAcquisition")
}
but I'm not sure if it's worth the indirection
There was a problem hiding this comment.
I thought about it some more, and ended up doing that refactor
3 participants
This PR adds some of the error handling that was missed in #852 and wasn't ported over in the initial tsconfig parsing port