From f4fe326473ecc5439b041b86984c16921a052556 Mon Sep 17 00:00:00 2001 From: "Sean R. Abraham" Date: Thu, 8 May 2025 11:33:18 -0400 Subject: [PATCH 1/2] Improve the missing CompilerOptions check This mucks with the real code a bit, but it works better and is simpler. See https://github.com/microsoft/typescript-go/pull/839 https://github.com/microsoft/typescript-go/issues/842 --- internal/tsoptions/parsinghelpers.go | 12 ++++++- internal/tsoptions/parsinghelpers_test.go | 43 ++++++++++++----------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/internal/tsoptions/parsinghelpers.go b/internal/tsoptions/parsinghelpers.go index 75662a7056..0e12c2418a 100644 --- a/internal/tsoptions/parsinghelpers.go +++ b/internal/tsoptions/parsinghelpers.go @@ -140,6 +140,11 @@ func ParseCompilerOptions(key string, value any, allOptions *core.CompilerOption if allOptions == nil { return nil } + parseCompilerOptions(key, value, allOptions) + return nil +} + +func parseCompilerOptions(key string, value any, allOptions *core.CompilerOptions) (foundKey bool) { switch key { case "allowJs": allOptions.AllowJs = parseTristate(value) @@ -255,6 +260,8 @@ func ParseCompilerOptions(key string, value any, allOptions *core.CompilerOption allOptions.MapRoot = parseString(value) case "module": allOptions.ModuleKind = value.(core.ModuleKind) + case "moduleDetectionKind": + allOptions.ModuleDetection = value.(core.ModuleDetectionKind) case "moduleResolution": allOptions.ModuleResolution = value.(core.ModuleResolutionKind) case "moduleSuffixes": @@ -393,8 +400,11 @@ func ParseCompilerOptions(key string, value any, allOptions *core.CompilerOption allOptions.NewLine = value.(core.NewLineKind) case "watch": allOptions.Watch = parseTristate(value) + default: + // different than any key above + return false } - return nil + return true } func ParseWatchOptions(key string, value any, allOptions *core.WatchOptions) []*ast.Diagnostic { diff --git a/internal/tsoptions/parsinghelpers_test.go b/internal/tsoptions/parsinghelpers_test.go index 8d356e7814..4add2fe9e6 100644 --- a/internal/tsoptions/parsinghelpers_test.go +++ b/internal/tsoptions/parsinghelpers_test.go @@ -1,6 +1,7 @@ package tsoptions import ( + "fmt" "reflect" "strings" "testing" @@ -8,34 +9,36 @@ import ( "github.com/microsoft/typescript-go/internal/core" ) -func TestParseCompilerOptionNoMissingTristates(t *testing.T) { +func TestParseCompilerOptionNoMissingFields(t *testing.T) { t.Parallel() - var missingKeys []string for _, field := range reflect.VisibleFields(reflect.TypeFor[core.CompilerOptions]()) { - keyName := field.Name - // use the JSON key from the tag, if present - // e.g. `json:"dog[,anythingelse]"` --> dog - if jsonTag, ok := field.Tag.Lookup("json"); ok { - keyName = strings.SplitN(jsonTag, ",", 2)[0] - } - - isTristate := field.Type == reflect.TypeFor[core.Tristate]() - if isTristate { - // Set the field on a CompilerOptions to something other than the - // default (i.e. not TSUnknown), then check whether - // ParseCompilerOptions does actually update the value for that key. - testValue := core.TSTrue + func() { + defer func() { + if r := recover(); r != nil { + if strings.Contains(fmt.Sprint(r), "interface conversion") { + // this is a success case, amazingly + return + } + t.Errorf("unexpected panic: %v", r) + } + }() + keyName := field.Name + // use the JSON key from the tag, if present + // e.g. `json:"dog[,anythingelse]"` --> dog + if jsonTag, ok := field.Tag.Lookup("json"); ok { + keyName = strings.SplitN(jsonTag, ",", 2)[0] + } + var something any co := core.CompilerOptions{} - ParseCompilerOptions(keyName, testValue, &co) - newSetValue := reflect.ValueOf(co).FieldByName(field.Name) - if !newSetValue.Equal(reflect.ValueOf(testValue)) { + found := parseCompilerOptions(keyName, something, &co) + if !found { missingKeys = append(missingKeys, keyName) } - } + }() } if len(missingKeys) > 0 { - t.Errorf("The following Tristate keys are missing entries in the ParseCompilerOptions"+ + t.Errorf("The following keys are missing entries in the ParseCompilerOptions"+ " switch statement:\n%v", missingKeys) } } From 58eecf00e2705db99f806090fb60a0aee84e305e Mon Sep 17 00:00:00 2001 From: "Sean R. Abraham" Date: Thu, 8 May 2025 11:53:35 -0400 Subject: [PATCH 2/2] No need to panic --- internal/tsoptions/parsinghelpers_test.go | 36 ++++++++--------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/internal/tsoptions/parsinghelpers_test.go b/internal/tsoptions/parsinghelpers_test.go index 4add2fe9e6..6a99e35b50 100644 --- a/internal/tsoptions/parsinghelpers_test.go +++ b/internal/tsoptions/parsinghelpers_test.go @@ -1,7 +1,6 @@ package tsoptions import ( - "fmt" "reflect" "strings" "testing" @@ -13,29 +12,18 @@ func TestParseCompilerOptionNoMissingFields(t *testing.T) { t.Parallel() var missingKeys []string for _, field := range reflect.VisibleFields(reflect.TypeFor[core.CompilerOptions]()) { - func() { - defer func() { - if r := recover(); r != nil { - if strings.Contains(fmt.Sprint(r), "interface conversion") { - // this is a success case, amazingly - return - } - t.Errorf("unexpected panic: %v", r) - } - }() - keyName := field.Name - // use the JSON key from the tag, if present - // e.g. `json:"dog[,anythingelse]"` --> dog - if jsonTag, ok := field.Tag.Lookup("json"); ok { - keyName = strings.SplitN(jsonTag, ",", 2)[0] - } - var something any - co := core.CompilerOptions{} - found := parseCompilerOptions(keyName, something, &co) - if !found { - missingKeys = append(missingKeys, keyName) - } - }() + keyName := field.Name + // use the JSON key from the tag, if present + // e.g. `json:"dog[,anythingelse]"` --> dog + if jsonTag, ok := field.Tag.Lookup("json"); ok { + keyName = strings.SplitN(jsonTag, ",", 2)[0] + } + val := reflect.Zero(field.Type).Interface() + co := core.CompilerOptions{} + found := parseCompilerOptions(keyName, val, &co) + if !found { + missingKeys = append(missingKeys, keyName) + } } if len(missingKeys) > 0 { t.Errorf("The following keys are missing entries in the ParseCompilerOptions"+