Handle numbers in tsconfig parsing#3502
Conversation
andrewbranch
left a comment
There was a problem hiding this comment.
Does this mean this is totally untested in actual compiler tests?? 😮
|
No, the feature works, it's just that they all go through |
There was a problem hiding this comment.
Pull request overview
Fixes tsconfig parsing so numeric compiler options (notably maxNodeModuleJsDepth) are correctly read from JSON and surfaced in the parsed CompilerOptions, aligning behavior with expectations for number-typed options in config files.
Changes:
- Extend numeric option parsing to accept
float64values (as produced by JSON parsing) when populating*intfields. - Update tsconfig parsing baselines to include
maxNodeModuleJsDepthinCompilerOptionsoutput for both json and jsonSourceFile APIs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/tsoptions/parsinghelpers.go |
Updates parseNumber to accept JSON numeric (float64) inputs. |
testdata/baselines/reference/config/tsconfigParsing/parses tsconfig with compilerOptions, files, include, and exclude with json api.js |
Baseline updated to reflect correct parsing of maxNodeModuleJsDepth. |
testdata/baselines/reference/config/tsconfigParsing/parses tsconfig with compilerOptions, files, include, and exclude with jsonSourceFile api.js |
Baseline updated to reflect correct parsing of maxNodeModuleJsDepth. |
| if num, ok := value.(float64); ok { | ||
| n := int(num) | ||
| return &n | ||
| } |
There was a problem hiding this comment.
Converting a JSON number (float64) to int without bounds checking can yield implementation-dependent results when the value is outside the target int range, and will also silently truncate fractional values. Consider validating the float64 is within [minInt, maxInt] (and, if desired, that it’s an integer value) before converting; otherwise return nil or emit a diagnostic so an invalid tsconfig value can’t become a surprising negative/garbled int (e.g., affecting maxDepth handling).
This has never worked and we have never noticed because
maxNodeModuleJsDepthis actually our onlyintcompiler option that isn't a new one like--checkers. Wow!Fixes #3493