From b0cf253e8218ab23dc0d638d32446a11454dfc67 Mon Sep 17 00:00:00 2001 From: James Spencer Date: Mon, 14 Sep 2026 14:25:02 -0500 Subject: [PATCH 1/3] feat: report unknown rule names in diagnosticSeverity A `diagnosticSeverity` key naming a rule the running build does not provide was accepted in silence, so a project pinning a rule at `error` believed it was enforced while nothing ran. The same plugin block is read by the JS `@effect/language-service`, whose rule set differs, which is how a config drifts into naming rules this build has never had. Validate the configured names when `compilerOptions.plugins` is parsed, so the diagnostic is anchored on the offending key in the config file that declares it and reaches both `tsc` and the language server through the config-file parsing diagnostics. Both the top-level map and the map inside each `overrides` entry are covered, and a near miss carries the intended name. The diagnostic defaults to `warning` and is configured through `diagnosticSeverity.unknownRuleName`, alongside `unusedDirective` as a severity key that is not a rule in the registry. Co-Authored-By: Claude Fable 5.1 --- .changeset/unknown-rule-name-diagnostic.md | 25 ++ ...0-tsoptions-effect-plugin-validation.patch | 34 +++ etscheckerhooks/init.go | 2 + .../diagnostics/effectDiagnosticMessages.json | 8 + .../effectconfigcheck/effectconfigcheck.go | 176 +++++++++++++ .../effectconfigcheck_test.go | 238 ++++++++++++++++++ internal/effecttest/runner.go | 5 + internal/rule/rule.go | 15 ++ internal/rulerunner/diagnostics.go | 2 +- shim/diagnostics/shim.go | 2 + shim/tsoptions/shim.go | 3 + .../effect-v4/unknownRuleName.errors.txt | 35 +++ .../unknownRuleName.flows.test.mermaid | 6 + .../effect-v4/unknownRuleName.flows.txt | 1 + .../effect-v4/unknownRuleName.layers.txt | 1 + .../effect-v4/unknownRuleName.pipings.txt | 15 ++ .../effect-v4/unknownRuleName.quickfixes.txt | 5 + .../unknownRuleName_overrides.errors.txt | 37 +++ ...knownRuleName_overrides.flows.test.mermaid | 6 + .../unknownRuleName_overrides.flows.txt | 1 + .../unknownRuleName_overrides.layers.txt | 1 + .../unknownRuleName_overrides.pipings.txt | 15 ++ .../unknownRuleName_overrides.quickfixes.txt | 5 + .../unknownRuleName_valid.errors.txt | 28 +++ .../unknownRuleName_valid.flows.test.mermaid | 6 + .../effect-v4/unknownRuleName_valid.flows.txt | 1 + .../unknownRuleName_valid.layers.txt | 1 + .../unknownRuleName_valid.pipings.txt | 15 ++ .../unknownRuleName_valid.quickfixes.txt | 5 + testdata/tests/effect-v4/unknownRuleName.ts | 22 ++ .../effect-v4/unknownRuleName_overrides.ts | 27 ++ .../tests/effect-v4/unknownRuleName_valid.ts | 21 ++ 32 files changed, 763 insertions(+), 1 deletion(-) create mode 100644 .changeset/unknown-rule-name-diagnostic.md create mode 100644 _patches/typescript/030-tsoptions-effect-plugin-validation.patch create mode 100644 internal/effectconfigcheck/effectconfigcheck.go create mode 100644 internal/effectconfigcheck/effectconfigcheck_test.go create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName.errors.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName.flows.test.mermaid create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName.flows.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName.layers.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName.pipings.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName.quickfixes.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_overrides.errors.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_overrides.flows.test.mermaid create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_overrides.flows.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_overrides.layers.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_overrides.pipings.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_overrides.quickfixes.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_valid.errors.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_valid.flows.test.mermaid create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_valid.flows.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_valid.layers.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_valid.pipings.txt create mode 100644 testdata/baselines/reference/effect-v4/unknownRuleName_valid.quickfixes.txt create mode 100644 testdata/tests/effect-v4/unknownRuleName.ts create mode 100644 testdata/tests/effect-v4/unknownRuleName_overrides.ts create mode 100644 testdata/tests/effect-v4/unknownRuleName_valid.ts diff --git a/.changeset/unknown-rule-name-diagnostic.md b/.changeset/unknown-rule-name-diagnostic.md new file mode 100644 index 00000000..6943718c --- /dev/null +++ b/.changeset/unknown-rule-name-diagnostic.md @@ -0,0 +1,25 @@ +--- +"@effect/tsgo": minor +--- + +Report a diagnostic when `diagnosticSeverity` names a rule this build does not provide. A name that does not resolve was previously accepted in silence, so a project pinning a rule at `error` believed it was enforced while nothing ran. The check covers the top-level `diagnosticSeverity` map and the map inside each `overrides` entry, is anchored on the offending key in the config file that declares it, and carries a spelling suggestion when one is close enough. + +```jsonc +{ + "compilerOptions": { + "plugins": [ + { + "name": "@effect/language-service", + "diagnosticSeverity": { + "floatingEfect": "error" + // warning TS377135: Unknown Effect diagnostic rule `floatingEfect` in + // `diagnosticSeverity`. Did you mean `floatingEffect`? + // effect(unknownRuleName) + } + } + ] + } +} +``` + +The diagnostic is a `warning` by default and is configured through `diagnosticSeverity.unknownRuleName` like any other, so `"unknownRuleName": "off"` silences it. Note that with the default `ignoreEffectWarningsInTscExitCode: false` a warning already fails `tsc`. diff --git a/_patches/typescript/030-tsoptions-effect-plugin-validation.patch b/_patches/typescript/030-tsoptions-effect-plugin-validation.patch new file mode 100644 index 00000000..3de8a321 --- /dev/null +++ b/_patches/typescript/030-tsoptions-effect-plugin-validation.patch @@ -0,0 +1,34 @@ +diff --git a/tsc/internal/tsoptions/parsinghelpers.go b/tsc/internal/tsoptions/parsinghelpers.go +--- a/tsc/internal/tsoptions/parsinghelpers.go ++++ b/tsc/internal/tsoptions/parsinghelpers.go +@@ -24,6 +24,17 @@ + MergeCompilerOptionsCallback = cb + } + ++// ValidateEffectPluginOptionsCallback is invoked after compilerOptions.plugins is ++// parsed from a tsconfig file, allowing external integrations to report ++// configuration diagnostics anchored on nodes inside that config file. ++var ValidateEffectPluginOptionsCallback func(sourceFile *ast.SourceFile, pluginsNode *ast.Node, options *core.CompilerOptions) []*ast.Diagnostic ++ ++// RegisterValidateEffectPluginOptionsCallback registers a callback invoked after ++// compilerOptions.plugins is parsed from a tsconfig file. ++func RegisterValidateEffectPluginOptionsCallback(cb func(sourceFile *ast.SourceFile, pluginsNode *ast.Node, options *core.CompilerOptions) []*ast.Diagnostic) { ++ ValidateEffectPluginOptionsCallback = cb ++} ++ + func ParseTristate(value any) core.Tristate { + if value == nil { + return core.TSUnknown +diff --git a/tsc/internal/tsoptions/tsconfigparsing.go b/tsc/internal/tsoptions/tsconfigparsing.go +--- a/tsc/internal/tsoptions/tsconfigparsing.go ++++ b/tsc/internal/tsoptions/tsconfigparsing.go +@@ -204,6 +204,9 @@ + switch parentOption.Name { + case "compilerOptions": + parseDiagnostics = ParseCompilerOptions(option.Name, value, compilerOptions) ++ if option.Name == "plugins" && ValidateEffectPluginOptionsCallback != nil { ++ parseDiagnostics = append(parseDiagnostics, ValidateEffectPluginOptionsCallback(sourceFile, propertyAssignment.Initializer, compilerOptions)...) ++ } + case "typeAcquisition": + parseDiagnostics = ParseTypeAcquisition(option.Name, value, typeAcquisition) + } diff --git a/etscheckerhooks/init.go b/etscheckerhooks/init.go index 9201af62..d3572ac1 100644 --- a/etscheckerhooks/init.go +++ b/etscheckerhooks/init.go @@ -7,6 +7,7 @@ import ( "context" "github.com/effect-ts/tsgo/etscore" + "github.com/effect-ts/tsgo/internal/effectconfigcheck" "github.com/effect-ts/tsgo/internal/effectconfigraw" "github.com/effect-ts/tsgo/internal/rulerunner" "github.com/microsoft/TypeScript/tsc/shim/ast" @@ -19,6 +20,7 @@ func init() { // Set the version suffix so that core.Version() includes the Effect version core.SetVersionSuffix("+effect-tsgo." + etscore.EffectVersion) effectconfigraw.Register() + effectconfigcheck.Register() // Register the after check source file callback checker.RegisterAfterCheckSourceFileCallback(afterCheckSourceFile) } diff --git a/internal/diagnostics/effectDiagnosticMessages.json b/internal/diagnostics/effectDiagnosticMessages.json index 1db9e0fa..6b5f1614 100644 --- a/internal/diagnostics/effectDiagnosticMessages.json +++ b/internal/diagnostics/effectDiagnosticMessages.json @@ -526,5 +526,13 @@ "`Effect.andThen` expresses this sequencing more directly than `Effect.flatMap` with a zero-parameter callback. effect(flatMapIgnoredParamToAndThen)": { "category": "Suggestion", "code": 377132 + }, + "Unknown Effect diagnostic rule `{0}` in `diagnosticSeverity`. This version of @effect/tsgo does not provide it, so this entry has no effect. effect(unknownRuleName)": { + "category": "Warning", + "code": 377134 + }, + "Unknown Effect diagnostic rule `{0}` in `diagnosticSeverity`. Did you mean `{1}`? effect(unknownRuleName)": { + "category": "Warning", + "code": 377135 } } diff --git a/internal/effectconfigcheck/effectconfigcheck.go b/internal/effectconfigcheck/effectconfigcheck.go new file mode 100644 index 00000000..1e46db6f --- /dev/null +++ b/internal/effectconfigcheck/effectconfigcheck.go @@ -0,0 +1,176 @@ +// Package effectconfigcheck validates the @effect/language-service plugin block of +// a tsconfig file and reports configuration diagnostics anchored on the offending +// node inside that file. +package effectconfigcheck + +import ( + "slices" + "sync" + + "github.com/effect-ts/tsgo/etscore" + "github.com/effect-ts/tsgo/internal/directives" + "github.com/effect-ts/tsgo/internal/rule" + "github.com/effect-ts/tsgo/internal/rules" + "github.com/microsoft/TypeScript/tsc/shim/ast" + "github.com/microsoft/TypeScript/tsc/shim/core" + tsdiag "github.com/microsoft/TypeScript/tsc/shim/diagnostics" + "github.com/microsoft/TypeScript/tsc/shim/tsoptions" +) + +var ( + unknownRuleNameMessage = tsdiag.Unknown_Effect_diagnostic_rule_0_in_diagnosticSeverity_This_version_of_effect_Slashtsgo_does_not_provide_it_so_this_entry_has_no_effect_effect_unknownRuleName + didYouMeanMessage = tsdiag.Unknown_Effect_diagnostic_rule_0_in_diagnosticSeverity_Did_you_mean_1_effect_unknownRuleName +) + +// Register wires tsconfig plugin-option validation into TypeScript-Go. +func Register() { + tsoptions.RegisterValidateEffectPluginOptionsCallback(ValidatePluginsNode) +} + +// ConfigurableNames returns every name diagnosticSeverity accepts: the rules in the +// registry plus the diagnostics that are configurable without being rules. +var ConfigurableNames = sync.OnceValue(func() []string { + names := make([]string, 0, len(rules.All)+len(rule.NonRuleConfigurableNames)) + for i := range rules.All { + names = append(names, rules.All[i].Name) + } + names = append(names, rule.NonRuleConfigurableNames...) + slices.Sort(names) + return names +}) + +// ValidatePluginsNode reports a diagnostic for every diagnosticSeverity key in the +// @effect/language-service plugin entry that this build does not provide, covering +// both the top-level map and the map inside each overrides entry. +func ValidatePluginsNode(sourceFile *ast.SourceFile, pluginsNode *ast.Node, options *core.CompilerOptions) []*ast.Diagnostic { + if sourceFile == nil || pluginsNode == nil || options == nil || options.Effect == nil { + return nil + } + if !options.Effect.Diagnostics { + return nil + } + + severity, configured := options.Effect.DiagnosticSeverity[rule.UnknownRuleNameName] + if !configured { + severity = etscore.SeverityWarning + } + if severity.IsOff() { + return nil + } + + pluginEntry := findEffectPluginEntry(pluginsNode) + if pluginEntry == nil { + return nil + } + + known := ConfigurableNames() + var diags []*ast.Diagnostic + diags = appendUnknownNames(diags, sourceFile, severity, known, propertyValue(pluginEntry, "diagnosticSeverity")) + for _, override := range arrayElements(propertyValue(pluginEntry, "overrides")) { + overrideOptions := propertyValue(override, "options") + diags = appendUnknownNames(diags, sourceFile, severity, known, propertyValue(overrideOptions, "diagnosticSeverity")) + } + return diags +} + +func appendUnknownNames( + diags []*ast.Diagnostic, + sourceFile *ast.SourceFile, + severity etscore.Severity, + known []string, + diagnosticSeverity *ast.Node, +) []*ast.Diagnostic { + if diagnosticSeverity == nil || !ast.IsObjectLiteralExpression(diagnosticSeverity) { + return diags + } + for _, property := range diagnosticSeverity.Properties() { + if !ast.IsPropertyAssignment(property) { + continue + } + name := property.Name() + if name == nil || !ast.IsStringLiteralLike(name) { + continue + } + text := ast.GetTextOfPropertyName(name) + if text == "" || slices.Contains(known, text) { + continue + } + diags = append(diags, unknownNameDiagnostic(sourceFile, name, text, severity, known)) + } + return diags +} + +func unknownNameDiagnostic( + sourceFile *ast.SourceFile, + name *ast.Node, + text string, + severity etscore.Severity, + known []string, +) *ast.Diagnostic { + var diagnostic *ast.Diagnostic + if suggestion := core.GetSpellingSuggestionForStrings(text, slices.Values(known)); suggestion != "" { + diagnostic = tsoptions.CreateDiagnosticForNodeInSourceFile(sourceFile, name, didYouMeanMessage, text, suggestion) + } else { + diagnostic = tsoptions.CreateDiagnosticForNodeInSourceFile(sourceFile, name, unknownRuleNameMessage, text) + } + + category := directives.ToCategory(severity) + if diagnostic.Category() == category { + return diagnostic + } + return ast.NewDiagnosticFromSerialized( + diagnostic.File(), + core.NewTextRange(diagnostic.Pos(), diagnostic.End()), + diagnostic.Code(), + category, + diagnostic.MessageKey(), + diagnostic.MessageArgs(), + diagnostic.MessageChain(), + diagnostic.RelatedInformation(), + diagnostic.ReportsUnnecessary(), + diagnostic.ReportsDeprecated(), + diagnostic.SkippedOnNoEmit(), + ) +} + +// findEffectPluginEntry returns the object literal in the plugins array whose name +// is the Effect language service plugin, or nil when the block is absent or is not +// written as a literal. +func findEffectPluginEntry(pluginsNode *ast.Node) *ast.Node { + for _, entry := range arrayElements(pluginsNode) { + name := propertyValue(entry, "name") + if name == nil || !ast.IsStringLiteralLike(name) { + continue + } + if name.Text() == etscore.EffectPluginName { + return entry + } + } + return nil +} + +func arrayElements(node *ast.Node) []*ast.Node { + if node == nil || !ast.IsArrayLiteralExpression(node) { + return nil + } + return node.Elements() +} + +func propertyValue(objectLiteral *ast.Node, key string) *ast.Node { + if objectLiteral == nil || !ast.IsObjectLiteralExpression(objectLiteral) { + return nil + } + for _, property := range objectLiteral.Properties() { + if !ast.IsPropertyAssignment(property) { + continue + } + name := property.Name() + if name == nil || !ast.IsStringLiteralLike(name) { + continue + } + if ast.GetTextOfPropertyName(name) == key { + return property.Initializer() + } + } + return nil +} diff --git a/internal/effectconfigcheck/effectconfigcheck_test.go b/internal/effectconfigcheck/effectconfigcheck_test.go new file mode 100644 index 00000000..cadedcc0 --- /dev/null +++ b/internal/effectconfigcheck/effectconfigcheck_test.go @@ -0,0 +1,238 @@ +package effectconfigcheck_test + +import ( + "testing" + "testing/fstest" + + "github.com/effect-ts/tsgo/internal/effectconfigcheck" + "github.com/effect-ts/tsgo/internal/effectconfigraw" + "github.com/effect-ts/tsgo/internal/rule" + "github.com/microsoft/TypeScript/tsc/shim/ast" + "github.com/microsoft/TypeScript/tsc/shim/core" + tsdiag "github.com/microsoft/TypeScript/tsc/shim/diagnostics" + "github.com/microsoft/TypeScript/tsc/shim/parser" + "github.com/microsoft/TypeScript/tsc/shim/tsoptions" + "github.com/microsoft/TypeScript/tsc/shim/tspath" + "github.com/microsoft/TypeScript/tsc/shim/vfs" + "github.com/microsoft/TypeScript/tsc/shim/vfs/vfstest" +) + +const currentDirectory = "/.src" + +const unknownRuleNameCode = 377134 + +const didYouMeanCode = 377135 + +type parseConfigHost struct { + fs vfs.FS +} + +func (h *parseConfigHost) FS() vfs.FS { return h.fs } + +func (h *parseConfigHost) GetCurrentDirectory() string { return currentDirectory } + +// configDiagnostics parses the named tsconfig out of files and returns the +// diagnostics the config parse produced. +func configDiagnostics(t *testing.T, files map[string]string, configName string) []*ast.Diagnostic { + t.Helper() + effectconfigraw.Register() + effectconfigcheck.Register() + + testfs := make(map[string]any, len(files)) + for name, content := range files { + testfs[tspath.GetNormalizedAbsolutePath(name, currentDirectory)] = &fstest.MapFile{Data: []byte(content)} + } + fs := vfstest.FromMap(testfs, true /*useCaseSensitiveFileNames*/) + + configFileName := tspath.GetNormalizedAbsolutePath(configName, currentDirectory) + sourceFile := parser.ParseSourceFile(ast.SourceFileParseOptions{ + FileName: configFileName, + Path: tspath.ToPath(configFileName, currentDirectory, true), + }, files[configName], core.ScriptKindJSON) + + parsed := tsoptions.ParseJsonSourceFileConfigFileContent( + &tsoptions.TsConfigSourceFile{SourceFile: sourceFile}, + &parseConfigHost{fs: fs}, + tspath.GetDirectoryPath(configFileName), + nil, + nil, + configFileName, + nil, + nil, + nil, + ) + // TS18003 (no inputs found) is inherent to a config fixture that ships no + // source files and is not what these tests are about. + var effectDiags []*ast.Diagnostic + for _, diag := range parsed.Errors { + if rule.IsEffectCode(diag.Code()) { + effectDiags = append(effectDiags, diag) + } + } + return effectDiags +} + +func pluginConfig(body string) string { + return `{ + "compilerOptions": { + "plugins": [ + { + "name": "@effect/language-service", + ` + body + ` + } + ] + } +}` +} + +func codesOf(diags []*ast.Diagnostic) []int32 { + codes := make([]int32, 0, len(diags)) + for _, diag := range diags { + codes = append(codes, diag.Code()) + } + return codes +} + +func TestUnknownRuleNameIsReported(t *testing.T) { + t.Parallel() + + diags := configDiagnostics(t, map[string]string{ + "tsconfig.json": pluginConfig(`"diagnosticSeverity": { "importFromBarrel": "error" }`), + }, "tsconfig.json") + + if len(diags) != 1 { + t.Fatalf("expected 1 diagnostic, got %d: %v", len(diags), codesOf(diags)) + } + if got := diags[0].Code(); got != unknownRuleNameCode { + t.Errorf("code = %d, want %d", got, unknownRuleNameCode) + } + if got := diags[0].Category(); got != tsdiag.CategoryWarning { + t.Errorf("category = %v, want warning", got) + } + if got := diags[0].File(); got == nil { + t.Error("diagnostic is not anchored on the config file") + } + if args := diags[0].MessageArgs(); len(args) != 1 || args[0] != "importFromBarrel" { + t.Errorf("message args = %v, want [importFromBarrel]", args) + } +} + +func TestKnownNamesAreSilent(t *testing.T) { + t.Parallel() + + diags := configDiagnostics(t, map[string]string{ + "tsconfig.json": pluginConfig(`"diagnosticSeverity": { "floatingEffect": "error", "unusedDirective": "warning", "unknownRuleName": "warning" }`), + }, "tsconfig.json") + + if len(diags) != 0 { + t.Fatalf("expected no diagnostics, got %v", codesOf(diags)) + } +} + +func TestOverridesAreChecked(t *testing.T) { + t.Parallel() + + diags := configDiagnostics(t, map[string]string{ + "tsconfig.json": pluginConfig(`"overrides": [ + { "include": ["src/**"], "options": { "diagnosticSeverity": { "notARule": "error" } } } + ]`), + }, "tsconfig.json") + + if len(diags) != 1 { + t.Fatalf("expected 1 diagnostic, got %d: %v", len(diags), codesOf(diags)) + } + if args := diags[0].MessageArgs(); len(args) != 1 || args[0] != "notARule" { + t.Errorf("message args = %v, want [notARule]", args) + } +} + +func TestNearMissSuggestsTheIntendedRule(t *testing.T) { + t.Parallel() + + diags := configDiagnostics(t, map[string]string{ + "tsconfig.json": pluginConfig(`"diagnosticSeverity": { "floatingEfect": "error" }`), + }, "tsconfig.json") + + if len(diags) != 1 { + t.Fatalf("expected 1 diagnostic, got %d: %v", len(diags), codesOf(diags)) + } + if got := diags[0].Code(); got != didYouMeanCode { + t.Fatalf("code = %d, want %d", got, didYouMeanCode) + } + if args := diags[0].MessageArgs(); len(args) != 2 || args[1] != "floatingEffect" { + t.Errorf("message args = %v, want suggestion floatingEffect", args) + } +} + +func TestSeverityIsConfigurable(t *testing.T) { + t.Parallel() + + t.Run("off suppresses the diagnostic", func(t *testing.T) { + t.Parallel() + diags := configDiagnostics(t, map[string]string{ + "tsconfig.json": pluginConfig(`"diagnosticSeverity": { "importFromBarrel": "error", "unknownRuleName": "off" }`), + }, "tsconfig.json") + if len(diags) != 0 { + t.Fatalf("expected no diagnostics, got %v", codesOf(diags)) + } + }) + + t.Run("error raises the category", func(t *testing.T) { + t.Parallel() + diags := configDiagnostics(t, map[string]string{ + "tsconfig.json": pluginConfig(`"diagnosticSeverity": { "importFromBarrel": "error", "unknownRuleName": "error" }`), + }, "tsconfig.json") + if len(diags) != 1 { + t.Fatalf("expected 1 diagnostic, got %v", codesOf(diags)) + } + if got := diags[0].Category(); got != tsdiag.CategoryError { + t.Errorf("category = %v, want error", got) + } + }) +} + +func TestUnrelatedPluginIsIgnored(t *testing.T) { + t.Parallel() + + diags := configDiagnostics(t, map[string]string{ + "tsconfig.json": `{ + "compilerOptions": { + "plugins": [ + { "name": "some-other-plugin", "diagnosticSeverity": { "importFromBarrel": "error" } } + ] + } +}`, + }, "tsconfig.json") + + if len(diags) != 0 { + t.Fatalf("expected no diagnostics, got %v", codesOf(diags)) + } +} + +func TestExtendedConfigIsChecked(t *testing.T) { + t.Parallel() + + diags := configDiagnostics(t, map[string]string{ + "tsconfig.base.json": pluginConfig(`"diagnosticSeverity": { "importFromBarrel": "error" }`), + "tsconfig.json": `{ "extends": "./tsconfig.base.json" }`, + }, "tsconfig.json") + + if len(diags) != 1 { + t.Fatalf("expected 1 diagnostic, got %d: %v", len(diags), codesOf(diags)) + } + if file := diags[0].File(); file == nil || tspath.GetBaseFileName(file.FileName()) != "tsconfig.base.json" { + t.Errorf("diagnostic is not anchored on the config that declares the plugin block") + } +} + +func TestDiagnosticsDisabledSuppressesTheCheck(t *testing.T) { + t.Parallel() + + diags := configDiagnostics(t, map[string]string{ + "tsconfig.json": pluginConfig(`"diagnostics": false, "diagnosticSeverity": { "importFromBarrel": "error" }`), + }, "tsconfig.json") + + if len(diags) != 0 { + t.Fatalf("expected no diagnostics, got %v", codesOf(diags)) + } +} diff --git a/internal/effecttest/runner.go b/internal/effecttest/runner.go index 56830105..0bf8180f 100644 --- a/internal/effecttest/runner.go +++ b/internal/effecttest/runner.go @@ -311,6 +311,11 @@ func RunEffectTest(t *testing.T, version bundledeffect.EffectVersion, testFile s // Get diagnostics ctx := context.Background() var diagnostics []*ast.Diagnostic + if parsedConfig != nil { + // Config-file diagnostics are reported against the tsconfig rather than a + // source file, so they never reach the per-file collections below. + diagnostics = append(diagnostics, parsedConfig.Errors...) + } diagnostics = append(diagnostics, program.GetProgramDiagnostics()...) diagnostics = append(diagnostics, program.GetSyntacticDiagnostics(ctx, nil)...) diagnostics = append(diagnostics, program.GetSemanticDiagnostics(ctx, nil)...) diff --git a/internal/rule/rule.go b/internal/rule/rule.go index a135bb5a..2ee8c83b 100644 --- a/internal/rule/rule.go +++ b/internal/rule/rule.go @@ -37,6 +37,21 @@ type Rule struct { Run func(ctx *Context) []*ast.Diagnostic } +// UnusedDirectiveName configures the diagnostic reported for an +// @effect-diagnostics directive that suppresses nothing. It is a diagnosticSeverity +// key but not a per-file rule, so it is absent from the rule registry. +const UnusedDirectiveName = "unusedDirective" + +// UnknownRuleNameName configures the diagnostic reported for a diagnosticSeverity +// entry naming a rule this build does not provide. Like UnusedDirectiveName it is a +// diagnosticSeverity key but not a per-file rule. +const UnknownRuleNameName = "unknownRuleName" + +// NonRuleConfigurableNames lists every diagnosticSeverity key that configures a +// diagnostic which is not a rule in the registry. Validation of configured names +// must accept these in addition to the registry. +var NonRuleConfigurableNames = []string{UnusedDirectiveName, UnknownRuleNameName} + // ByName finds a rule by name in a slice. Returns nil if not found. func ByName(rules []Rule, name string) *Rule { for i := range rules { diff --git a/internal/rulerunner/diagnostics.go b/internal/rulerunner/diagnostics.go index caa39c86..461887a9 100644 --- a/internal/rulerunner/diagnostics.go +++ b/internal/rulerunner/diagnostics.go @@ -229,7 +229,7 @@ func createTransformedDiagnostic(original *ast.Diagnostic, newCategory tsdiag.Ca } func unusedDirectiveDiagnostics(sf *ast.SourceFile, allDirectives []directives.Directive, directiveSet *directives.DirectiveSet, resolvedSeverity map[string]etscore.Severity) []*ast.Diagnostic { - severity, ok := severityFromMap(resolvedSeverity, "unusedDirective") + severity, ok := severityFromMap(resolvedSeverity, rule.UnusedDirectiveName) if !ok { severity = etscore.SeverityWarning } diff --git a/shim/diagnostics/shim.go b/shim/diagnostics/shim.go index b9d03044..948479ed 100644 --- a/shim/diagnostics/shim.go +++ b/shim/diagnostics/shim.go @@ -2061,6 +2061,8 @@ var Unexpected_token_expected = diagnostics.Unexpected_token_expected var Unicode_escape_sequence_cannot_appear_here = diagnostics.Unicode_escape_sequence_cannot_appear_here var Unicode_escape_sequences_are_only_available_when_the_Unicode_u_flag_or_the_Unicode_Sets_v_flag_is_set = diagnostics.Unicode_escape_sequences_are_only_available_when_the_Unicode_u_flag_or_the_Unicode_Sets_v_flag_is_set var Unicode_property_value_expressions_are_only_available_when_the_Unicode_u_flag_or_the_Unicode_Sets_v_flag_is_set = diagnostics.Unicode_property_value_expressions_are_only_available_when_the_Unicode_u_flag_or_the_Unicode_Sets_v_flag_is_set +var Unknown_Effect_diagnostic_rule_0_in_diagnosticSeverity_Did_you_mean_1_effect_unknownRuleName = diagnostics.Unknown_Effect_diagnostic_rule_0_in_diagnosticSeverity_Did_you_mean_1_effect_unknownRuleName +var Unknown_Effect_diagnostic_rule_0_in_diagnosticSeverity_This_version_of_effect_Slashtsgo_does_not_provide_it_so_this_entry_has_no_effect_effect_unknownRuleName = diagnostics.Unknown_Effect_diagnostic_rule_0_in_diagnosticSeverity_This_version_of_effect_Slashtsgo_does_not_provide_it_so_this_entry_has_no_effect_effect_unknownRuleName var Unknown_Unicode_property_name = diagnostics.Unknown_Unicode_property_name var Unknown_Unicode_property_name_or_value = diagnostics.Unknown_Unicode_property_name_or_value var Unknown_Unicode_property_value = diagnostics.Unknown_Unicode_property_value diff --git a/shim/tsoptions/shim.go b/shim/tsoptions/shim.go index f8405393..0364fda4 100644 --- a/shim/tsoptions/shim.go +++ b/shim/tsoptions/shim.go @@ -118,10 +118,13 @@ type ParsedCommandLine = tsoptions.ParsedCommandLine type ParsedOptions = tsoptions.ParsedOptions //go:linkname RegisterMergeCompilerOptionsCallback github.com/microsoft/TypeScript/tsc/internal/tsoptions.RegisterMergeCompilerOptionsCallback func RegisterMergeCompilerOptionsCallback(cb func(targetOptions *core.CompilerOptions, sourceOptions *core.CompilerOptions, rawSource any, sourceConfigPath string, basePath string)) +//go:linkname RegisterValidateEffectPluginOptionsCallback github.com/microsoft/TypeScript/tsc/internal/tsoptions.RegisterValidateEffectPluginOptionsCallback +func RegisterValidateEffectPluginOptionsCallback(cb func(sourceFile *ast.SourceFile, pluginsNode *ast.Node, options *core.CompilerOptions) []*ast.Diagnostic) type SourceOutputAndProjectReference = tsoptions.SourceOutputAndProjectReference type TSConfig = tsoptions.TSConfig //go:linkname TargetToLibMap github.com/microsoft/TypeScript/tsc/internal/tsoptions.TargetToLibMap func TargetToLibMap() map[core.ScriptTarget]string type TsConfigSourceFile = tsoptions.TsConfigSourceFile var TscBuildOption = tsoptions.TscBuildOption +var ValidateEffectPluginOptionsCallback = tsoptions.ValidateEffectPluginOptionsCallback var WatchNameMap = tsoptions.WatchNameMap diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName.errors.txt b/testdata/baselines/reference/effect-v4/unknownRuleName.errors.txt new file mode 100644 index 00000000..8f5e4d63 --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName.errors.txt @@ -0,0 +1,35 @@ +=== Metadata === +Effect version: 4.0.0 + +/.src/tsconfig.json(8,11): warning TS377134: Unknown Effect diagnostic rule `importFromBarrel` in `diagnosticSeverity`. This version of @effect/tsgo does not provide it, so this entry has no effect. effect(unknownRuleName) +/.src/tsconfig.json(9,11): warning TS377134: Unknown Effect diagnostic rule `outdatedEffectCodegen` in `diagnosticSeverity`. This version of @effect/tsgo does not provide it, so this entry has no effect. effect(unknownRuleName) + + +==== /.src/tsconfig.json (2 errors) ==== + { + "compilerOptions": { + "plugins": [ + { + "name": "@effect/language-service", + "diagnosticSeverity": { + "floatingEffect": "error", + "importFromBarrel": "error", + ~~~~~~~~~~~~~~~~~~ +!!! warning TS377134: Unknown Effect diagnostic rule `importFromBarrel` in `diagnosticSeverity`. This version of @effect/tsgo does not provide it, so this entry has no effect. effect(unknownRuleName) + "outdatedEffectCodegen": "error" + ~~~~~~~~~~~~~~~~~~~~~~~ +!!! warning TS377134: Unknown Effect diagnostic rule `outdatedEffectCodegen` in `diagnosticSeverity`. This version of @effect/tsgo does not provide it, so this entry has no effect. effect(unknownRuleName) + } + } + ] + } + } + + +==== /.src/test.ts (0 errors) ==== + import { Effect } from "effect" + + // The rule names above that this build does not provide are reported on the + // tsconfig; floatingEffect is provided, so it is not. + export const program = Effect.succeed(1) + diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName.flows.test.mermaid b/testdata/baselines/reference/effect-v4/unknownRuleName.flows.test.mermaid new file mode 100644 index 00000000..e5c22a8b --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName.flows.test.mermaid @@ -0,0 +1,6 @@ +flowchart TB + 0[/"type: 1
node: 1"/] + 1["type: Effect#lt;number, never, never#gt;
callee: Effect.succeed
args: #91;#93;"] + 2[/"type: #lt;A#gt;#40;value: A#41; =#gt; Effect#lt;A, never, never#gt;
node: Effect.succeed"/] + 0 -->|"kind: pipe"| 1 + 2 -->|"kind: transformCallee"| 1 \ No newline at end of file diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName.flows.txt b/testdata/baselines/reference/effect-v4/unknownRuleName.flows.txt new file mode 100644 index 00000000..14e30c5d --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName.flows.txt @@ -0,0 +1 @@ +/.src/test.ts -> unknownRuleName.flows.test.mermaid diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName.layers.txt b/testdata/baselines/reference/effect-v4/unknownRuleName.layers.txt new file mode 100644 index 00000000..39e24226 --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName.layers.txt @@ -0,0 +1 @@ +==== /.src/test.ts (0 layer exports) ==== diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName.pipings.txt b/testdata/baselines/reference/effect-v4/unknownRuleName.pipings.txt new file mode 100644 index 00000000..35c1234e --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName.pipings.txt @@ -0,0 +1,15 @@ +==== /.src/test.ts (1 flows) ==== + +=== Piping Flow === +Location: 5:23 - 5:41 +Node: Effect.succeed(1) +Node Kind: KindCallExpression + +Subject: 1 +Subject Type: 1 + +Transformations (1): + [0] kind: call + callee: Effect.succeed + args: (constant) + outType: Effect diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName.quickfixes.txt b/testdata/baselines/reference/effect-v4/unknownRuleName.quickfixes.txt new file mode 100644 index 00000000..dad05bfc --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName.quickfixes.txt @@ -0,0 +1,5 @@ +=== Quick Fix Inventory === +(no diagnostics) + +=== Quick Fix Application Results === +(no quick fixes to apply) diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.errors.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.errors.txt new file mode 100644 index 00000000..77a25131 --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.errors.txt @@ -0,0 +1,37 @@ +=== Metadata === +Effect version: 4.0.0 + +/.src/tsconfig.json(11,17): warning TS377135: Unknown Effect diagnostic rule `floatingEfect` in `diagnosticSeverity`. Did you mean `floatingEffect`? effect(unknownRuleName) + + +==== /.src/tsconfig.json (1 errors) ==== + { + "compilerOptions": { + "plugins": [ + { + "name": "@effect/language-service", + "overrides": [ + { + "include": ["**/*.ts"], + "options": { + "diagnosticSeverity": { + "floatingEfect": "error" + ~~~~~~~~~~~~~~~ +!!! warning TS377135: Unknown Effect diagnostic rule `floatingEfect` in `diagnosticSeverity`. Did you mean `floatingEffect`? effect(unknownRuleName) + } + } + } + ] + } + ] + } + } + + +==== /.src/test.ts (0 errors) ==== + import { Effect } from "effect" + + // diagnosticSeverity inside an overrides entry is checked the same way, and a + // near miss carries the intended name. + export const program = Effect.succeed(1) + diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.flows.test.mermaid b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.flows.test.mermaid new file mode 100644 index 00000000..e5c22a8b --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.flows.test.mermaid @@ -0,0 +1,6 @@ +flowchart TB + 0[/"type: 1
node: 1"/] + 1["type: Effect#lt;number, never, never#gt;
callee: Effect.succeed
args: #91;#93;"] + 2[/"type: #lt;A#gt;#40;value: A#41; =#gt; Effect#lt;A, never, never#gt;
node: Effect.succeed"/] + 0 -->|"kind: pipe"| 1 + 2 -->|"kind: transformCallee"| 1 \ No newline at end of file diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.flows.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.flows.txt new file mode 100644 index 00000000..e745e92f --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.flows.txt @@ -0,0 +1 @@ +/.src/test.ts -> unknownRuleName_overrides.flows.test.mermaid diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.layers.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.layers.txt new file mode 100644 index 00000000..39e24226 --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.layers.txt @@ -0,0 +1 @@ +==== /.src/test.ts (0 layer exports) ==== diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.pipings.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.pipings.txt new file mode 100644 index 00000000..35c1234e --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.pipings.txt @@ -0,0 +1,15 @@ +==== /.src/test.ts (1 flows) ==== + +=== Piping Flow === +Location: 5:23 - 5:41 +Node: Effect.succeed(1) +Node Kind: KindCallExpression + +Subject: 1 +Subject Type: 1 + +Transformations (1): + [0] kind: call + callee: Effect.succeed + args: (constant) + outType: Effect diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.quickfixes.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.quickfixes.txt new file mode 100644 index 00000000..dad05bfc --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_overrides.quickfixes.txt @@ -0,0 +1,5 @@ +=== Quick Fix Inventory === +(no diagnostics) + +=== Quick Fix Application Results === +(no quick fixes to apply) diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_valid.errors.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.errors.txt new file mode 100644 index 00000000..255f86d0 --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.errors.txt @@ -0,0 +1,28 @@ +=== Metadata === +Effect version: 4.0.0 + + + +==== /.src/tsconfig.json (0 errors) ==== + { + "compilerOptions": { + "plugins": [ + { + "name": "@effect/language-service", + "diagnosticSeverity": { + "floatingEffect": "error", + "unusedDirective": "warning", + "unknownRuleName": "warning" + } + } + ] + } + } + + +==== /.src/test.ts (0 errors) ==== + import { Effect } from "effect" + + // Every configured name resolves, so the configuration is reported clean. + export const program = Effect.succeed(1) + diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_valid.flows.test.mermaid b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.flows.test.mermaid new file mode 100644 index 00000000..e5c22a8b --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.flows.test.mermaid @@ -0,0 +1,6 @@ +flowchart TB + 0[/"type: 1
node: 1"/] + 1["type: Effect#lt;number, never, never#gt;
callee: Effect.succeed
args: #91;#93;"] + 2[/"type: #lt;A#gt;#40;value: A#41; =#gt; Effect#lt;A, never, never#gt;
node: Effect.succeed"/] + 0 -->|"kind: pipe"| 1 + 2 -->|"kind: transformCallee"| 1 \ No newline at end of file diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_valid.flows.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.flows.txt new file mode 100644 index 00000000..3c33dd85 --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.flows.txt @@ -0,0 +1 @@ +/.src/test.ts -> unknownRuleName_valid.flows.test.mermaid diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_valid.layers.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.layers.txt new file mode 100644 index 00000000..39e24226 --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.layers.txt @@ -0,0 +1 @@ +==== /.src/test.ts (0 layer exports) ==== diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_valid.pipings.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.pipings.txt new file mode 100644 index 00000000..f6f2a37e --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.pipings.txt @@ -0,0 +1,15 @@ +==== /.src/test.ts (1 flows) ==== + +=== Piping Flow === +Location: 4:23 - 4:41 +Node: Effect.succeed(1) +Node Kind: KindCallExpression + +Subject: 1 +Subject Type: 1 + +Transformations (1): + [0] kind: call + callee: Effect.succeed + args: (constant) + outType: Effect diff --git a/testdata/baselines/reference/effect-v4/unknownRuleName_valid.quickfixes.txt b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.quickfixes.txt new file mode 100644 index 00000000..dad05bfc --- /dev/null +++ b/testdata/baselines/reference/effect-v4/unknownRuleName_valid.quickfixes.txt @@ -0,0 +1,5 @@ +=== Quick Fix Inventory === +(no diagnostics) + +=== Quick Fix Application Results === +(no quick fixes to apply) diff --git a/testdata/tests/effect-v4/unknownRuleName.ts b/testdata/tests/effect-v4/unknownRuleName.ts new file mode 100644 index 00000000..ca26e21a --- /dev/null +++ b/testdata/tests/effect-v4/unknownRuleName.ts @@ -0,0 +1,22 @@ +// @filename: tsconfig.json +{ + "compilerOptions": { + "plugins": [ + { + "name": "@effect/language-service", + "diagnosticSeverity": { + "floatingEffect": "error", + "importFromBarrel": "error", + "outdatedEffectCodegen": "error" + } + } + ] + } +} + +// @filename: test.ts +import { Effect } from "effect" + +// The rule names above that this build does not provide are reported on the +// tsconfig; floatingEffect is provided, so it is not. +export const program = Effect.succeed(1) diff --git a/testdata/tests/effect-v4/unknownRuleName_overrides.ts b/testdata/tests/effect-v4/unknownRuleName_overrides.ts new file mode 100644 index 00000000..8e7ec8ac --- /dev/null +++ b/testdata/tests/effect-v4/unknownRuleName_overrides.ts @@ -0,0 +1,27 @@ +// @filename: tsconfig.json +{ + "compilerOptions": { + "plugins": [ + { + "name": "@effect/language-service", + "overrides": [ + { + "include": ["**/*.ts"], + "options": { + "diagnosticSeverity": { + "floatingEfect": "error" + } + } + } + ] + } + ] + } +} + +// @filename: test.ts +import { Effect } from "effect" + +// diagnosticSeverity inside an overrides entry is checked the same way, and a +// near miss carries the intended name. +export const program = Effect.succeed(1) diff --git a/testdata/tests/effect-v4/unknownRuleName_valid.ts b/testdata/tests/effect-v4/unknownRuleName_valid.ts new file mode 100644 index 00000000..af56d754 --- /dev/null +++ b/testdata/tests/effect-v4/unknownRuleName_valid.ts @@ -0,0 +1,21 @@ +// @filename: tsconfig.json +{ + "compilerOptions": { + "plugins": [ + { + "name": "@effect/language-service", + "diagnosticSeverity": { + "floatingEffect": "error", + "unusedDirective": "warning", + "unknownRuleName": "warning" + } + } + ] + } +} + +// @filename: test.ts +import { Effect } from "effect" + +// Every configured name resolves, so the configuration is reported clean. +export const program = Effect.succeed(1) From 557f36ff357c253c151c67c357fb169b981b4501 Mon Sep 17 00:00:00 2001 From: James Spencer Date: Mon, 14 Sep 2026 15:01:31 -0500 Subject: [PATCH 2/3] fix: resolve unknown-rule-name suppression against the merged config The enable and severity gate was read from the options of the file being parsed, which is built before any extends hop is merged. Three measured consequences: an inherited `diagnostics: false` was ignored, an inherited `diagnosticSeverity.unknownRuleName` did not silence a name declared in a child, and a package could not silence a name declared in a base config it does not own. Split the work across the two seams where each half is actually available. The offending node exists only while its own file is parsed, so that phase now reports unconditionally and keeps the anchor. The setting that governs the report belongs to the merged configuration, so a second callback runs from parseJsonConfigFileContentWorker once the chain is merged and drops or recategorizes what the first phase produced. Register the callbacks in the tests through the etscheckerhooks init rather than assigning the globals from parallel subtests, which `go test -race` reported as 25 data races. Co-Authored-By: Claude Fable 5.1 --- ...0-tsoptions-effect-plugin-validation.patch | 49 +++++-- .../effectconfigcheck/effectconfigcheck.go | 133 +++++++++++------- .../effectconfigcheck_test.go | 71 +++++++++- shim/tsoptions/shim.go | 5 +- 4 files changed, 194 insertions(+), 64 deletions(-) diff --git a/_patches/typescript/030-tsoptions-effect-plugin-validation.patch b/_patches/typescript/030-tsoptions-effect-plugin-validation.patch index 3de8a321..d9d25ef6 100644 --- a/_patches/typescript/030-tsoptions-effect-plugin-validation.patch +++ b/_patches/typescript/030-tsoptions-effect-plugin-validation.patch @@ -1,24 +1,39 @@ diff --git a/tsc/internal/tsoptions/parsinghelpers.go b/tsc/internal/tsoptions/parsinghelpers.go --- a/tsc/internal/tsoptions/parsinghelpers.go +++ b/tsc/internal/tsoptions/parsinghelpers.go -@@ -24,6 +24,17 @@ +@@ -22,6 +22,32 @@ + // standard compiler option merging. + func RegisterMergeCompilerOptionsCallback(cb func(targetOptions, sourceOptions *core.CompilerOptions, rawSource any, sourceConfigPath string, basePath string)) { MergeCompilerOptionsCallback = cb - } - -+// ValidateEffectPluginOptionsCallback is invoked after compilerOptions.plugins is -+// parsed from a tsconfig file, allowing external integrations to report -+// configuration diagnostics anchored on nodes inside that config file. -+var ValidateEffectPluginOptionsCallback func(sourceFile *ast.SourceFile, pluginsNode *ast.Node, options *core.CompilerOptions) []*ast.Diagnostic ++} ++ ++// ValidateEffectPluginOptionsCallback lets the Effect integration report ++// configuration diagnostics for its own plugin block, anchored on nodes inside the ++// config file that declares it. It is invoked while compilerOptions.plugins is ++// parsed, which is the only point at which those nodes exist, and therefore runs ++// before any extends hop is merged: it must not decide whether a diagnostic is ++// enabled, because the setting that governs that may be inherited. That decision ++// belongs to FinalizeEffectPluginDiagnosticsCallback. ++var ValidateEffectPluginOptionsCallback func(sourceFile *ast.SourceFile, pluginsNode *ast.Node) []*ast.Diagnostic + -+// RegisterValidateEffectPluginOptionsCallback registers a callback invoked after ++// RegisterValidateEffectPluginOptionsCallback registers a callback invoked when +// compilerOptions.plugins is parsed from a tsconfig file. -+func RegisterValidateEffectPluginOptionsCallback(cb func(sourceFile *ast.SourceFile, pluginsNode *ast.Node, options *core.CompilerOptions) []*ast.Diagnostic) { ++func RegisterValidateEffectPluginOptionsCallback(cb func(sourceFile *ast.SourceFile, pluginsNode *ast.Node) []*ast.Diagnostic) { + ValidateEffectPluginOptionsCallback = cb +} + ++// FinalizeEffectPluginDiagnosticsCallback lets the Effect integration drop or ++// recategorize the diagnostics it contributed earlier, now that the whole extends ++// chain has been merged and the config's final compiler options are known. ++var FinalizeEffectPluginDiagnosticsCallback func(diagnostics []*ast.Diagnostic, options *core.CompilerOptions) []*ast.Diagnostic ++ ++// RegisterFinalizeEffectPluginDiagnosticsCallback registers a callback invoked ++// after the whole extends chain has been merged. ++func RegisterFinalizeEffectPluginDiagnosticsCallback(cb func(diagnostics []*ast.Diagnostic, options *core.CompilerOptions) []*ast.Diagnostic) { ++ FinalizeEffectPluginDiagnosticsCallback = cb + } + func ParseTristate(value any) core.Tristate { - if value == nil { - return core.TSUnknown diff --git a/tsc/internal/tsoptions/tsconfigparsing.go b/tsc/internal/tsoptions/tsconfigparsing.go --- a/tsc/internal/tsoptions/tsconfigparsing.go +++ b/tsc/internal/tsoptions/tsconfigparsing.go @@ -27,8 +42,18 @@ diff --git a/tsc/internal/tsoptions/tsconfigparsing.go b/tsc/internal/tsoptions/ case "compilerOptions": parseDiagnostics = ParseCompilerOptions(option.Name, value, compilerOptions) + if option.Name == "plugins" && ValidateEffectPluginOptionsCallback != nil { -+ parseDiagnostics = append(parseDiagnostics, ValidateEffectPluginOptionsCallback(sourceFile, propertyAssignment.Initializer, compilerOptions)...) ++ parseDiagnostics = append(parseDiagnostics, ValidateEffectPluginOptionsCallback(sourceFile, propertyAssignment.Initializer)...) + } case "typeAcquisition": parseDiagnostics = ParseTypeAcquisition(option.Name, value, typeAcquisition) } +@@ -1261,6 +1264,9 @@ + parsedConfig, errors := parseConfig(json, sourceFile, host, basePath, configFileName, resolutionStack, extendedConfigCache) + mergeCompilerOptions(parsedConfig.options, existingOptions, existingOptionsRaw, configFileName, basePath) + handleOptionConfigDirTemplateSubstitution(parsedConfig.options, basePathForFileNames) ++ if FinalizeEffectPluginDiagnosticsCallback != nil { ++ errors = FinalizeEffectPluginDiagnosticsCallback(errors, parsedConfig.options) ++ } + rawConfig := parseJsonToStringKey(parsedConfig.raw) + if configFileName != "" && parsedConfig.options != nil { + parsedConfig.options.ConfigFilePath = tspath.NormalizeSlashes(configFileName) diff --git a/internal/effectconfigcheck/effectconfigcheck.go b/internal/effectconfigcheck/effectconfigcheck.go index 1e46db6f..9a5180af 100644 --- a/internal/effectconfigcheck/effectconfigcheck.go +++ b/internal/effectconfigcheck/effectconfigcheck.go @@ -1,6 +1,13 @@ // Package effectconfigcheck validates the @effect/language-service plugin block of // a tsconfig file and reports configuration diagnostics anchored on the offending // node inside that file. +// +// Validation runs in two phases because the two things it needs become available at +// different points. The offending node exists only while the config file that +// declares it is being parsed, before any extends hop is merged; whether the +// diagnostic is enabled, and at which severity, is a property of the fully merged +// configuration. ValidatePluginsNode therefore reports every unresolved name it +// finds, and FinalizeDiagnostics drops or recategorizes them once the merge is done. package effectconfigcheck import ( @@ -25,6 +32,7 @@ var ( // Register wires tsconfig plugin-option validation into TypeScript-Go. func Register() { tsoptions.RegisterValidateEffectPluginOptionsCallback(ValidatePluginsNode) + tsoptions.RegisterFinalizeEffectPluginDiagnosticsCallback(FinalizeDiagnostics) } // ConfigurableNames returns every name diagnosticSeverity accepts: the rules in the @@ -41,20 +49,11 @@ var ConfigurableNames = sync.OnceValue(func() []string { // ValidatePluginsNode reports a diagnostic for every diagnosticSeverity key in the // @effect/language-service plugin entry that this build does not provide, covering -// both the top-level map and the map inside each overrides entry. -func ValidatePluginsNode(sourceFile *ast.SourceFile, pluginsNode *ast.Node, options *core.CompilerOptions) []*ast.Diagnostic { - if sourceFile == nil || pluginsNode == nil || options == nil || options.Effect == nil { - return nil - } - if !options.Effect.Diagnostics { - return nil - } - - severity, configured := options.Effect.DiagnosticSeverity[rule.UnknownRuleNameName] - if !configured { - severity = etscore.SeverityWarning - } - if severity.IsOff() { +// both the top-level map and the map inside each overrides entry. It reports +// unconditionally: the config file being parsed does not yet know what it inherits, +// so FinalizeDiagnostics owns the decision to keep, drop or recategorize. +func ValidatePluginsNode(sourceFile *ast.SourceFile, pluginsNode *ast.Node) []*ast.Diagnostic { + if sourceFile == nil || pluginsNode == nil { return nil } @@ -65,18 +64,84 @@ func ValidatePluginsNode(sourceFile *ast.SourceFile, pluginsNode *ast.Node, opti known := ConfigurableNames() var diags []*ast.Diagnostic - diags = appendUnknownNames(diags, sourceFile, severity, known, propertyValue(pluginEntry, "diagnosticSeverity")) + diags = appendUnknownNames(diags, sourceFile, known, propertyValue(pluginEntry, "diagnosticSeverity")) for _, override := range arrayElements(propertyValue(pluginEntry, "overrides")) { overrideOptions := propertyValue(override, "options") - diags = appendUnknownNames(diags, sourceFile, severity, known, propertyValue(overrideOptions, "diagnosticSeverity")) + diags = appendUnknownNames(diags, sourceFile, known, propertyValue(overrideOptions, "diagnosticSeverity")) } return diags } +// FinalizeDiagnostics applies the merged configuration to the diagnostics +// ValidatePluginsNode contributed. Diagnostics from any other source pass through +// untouched. This is what makes `diagnostics: false` and +// `diagnosticSeverity.unknownRuleName` work when they are inherited through +// extends rather than declared in the file that carries the offending name. +func FinalizeDiagnostics(diags []*ast.Diagnostic, options *core.CompilerOptions) []*ast.Diagnostic { + if !slices.ContainsFunc(diags, isUnknownRuleNameDiagnostic) { + return diags + } + + severity, enabled := resolveSeverity(options) + category := directives.ToCategory(severity) + + result := make([]*ast.Diagnostic, 0, len(diags)) + for _, diag := range diags { + if !isUnknownRuleNameDiagnostic(diag) { + result = append(result, diag) + continue + } + if !enabled { + continue + } + result = append(result, withCategory(diag, category)) + } + return result +} + +// resolveSeverity reads the severity of the unknown-rule-name diagnostic from the +// merged configuration, and reports whether it should be surfaced at all. +func resolveSeverity(options *core.CompilerOptions) (etscore.Severity, bool) { + if options == nil || options.Effect == nil || !options.Effect.Diagnostics { + return etscore.SeverityOff, false + } + severity, configured := options.Effect.DiagnosticSeverity[rule.UnknownRuleNameName] + if !configured { + severity = etscore.SeverityWarning + } + return severity, !severity.IsOff() +} + +func isUnknownRuleNameDiagnostic(diag *ast.Diagnostic) bool { + if diag == nil { + return false + } + code := diag.Code() + return code == unknownRuleNameMessage.Code() || code == didYouMeanMessage.Code() +} + +func withCategory(diag *ast.Diagnostic, category tsdiag.Category) *ast.Diagnostic { + if diag.Category() == category { + return diag + } + return ast.NewDiagnosticFromSerialized( + diag.File(), + core.NewTextRange(diag.Pos(), diag.End()), + diag.Code(), + category, + diag.MessageKey(), + diag.MessageArgs(), + diag.MessageChain(), + diag.RelatedInformation(), + diag.ReportsUnnecessary(), + diag.ReportsDeprecated(), + diag.SkippedOnNoEmit(), + ) +} + func appendUnknownNames( diags []*ast.Diagnostic, sourceFile *ast.SourceFile, - severity etscore.Severity, known []string, diagnosticSeverity *ast.Node, ) []*ast.Diagnostic { @@ -95,42 +160,16 @@ func appendUnknownNames( if text == "" || slices.Contains(known, text) { continue } - diags = append(diags, unknownNameDiagnostic(sourceFile, name, text, severity, known)) + diags = append(diags, unknownNameDiagnostic(sourceFile, name, text, known)) } return diags } -func unknownNameDiagnostic( - sourceFile *ast.SourceFile, - name *ast.Node, - text string, - severity etscore.Severity, - known []string, -) *ast.Diagnostic { - var diagnostic *ast.Diagnostic +func unknownNameDiagnostic(sourceFile *ast.SourceFile, name *ast.Node, text string, known []string) *ast.Diagnostic { if suggestion := core.GetSpellingSuggestionForStrings(text, slices.Values(known)); suggestion != "" { - diagnostic = tsoptions.CreateDiagnosticForNodeInSourceFile(sourceFile, name, didYouMeanMessage, text, suggestion) - } else { - diagnostic = tsoptions.CreateDiagnosticForNodeInSourceFile(sourceFile, name, unknownRuleNameMessage, text) + return tsoptions.CreateDiagnosticForNodeInSourceFile(sourceFile, name, didYouMeanMessage, text, suggestion) } - - category := directives.ToCategory(severity) - if diagnostic.Category() == category { - return diagnostic - } - return ast.NewDiagnosticFromSerialized( - diagnostic.File(), - core.NewTextRange(diagnostic.Pos(), diagnostic.End()), - diagnostic.Code(), - category, - diagnostic.MessageKey(), - diagnostic.MessageArgs(), - diagnostic.MessageChain(), - diagnostic.RelatedInformation(), - diagnostic.ReportsUnnecessary(), - diagnostic.ReportsDeprecated(), - diagnostic.SkippedOnNoEmit(), - ) + return tsoptions.CreateDiagnosticForNodeInSourceFile(sourceFile, name, unknownRuleNameMessage, text) } // findEffectPluginEntry returns the object literal in the plugins array whose name diff --git a/internal/effectconfigcheck/effectconfigcheck_test.go b/internal/effectconfigcheck/effectconfigcheck_test.go index cadedcc0..e7e37376 100644 --- a/internal/effectconfigcheck/effectconfigcheck_test.go +++ b/internal/effectconfigcheck/effectconfigcheck_test.go @@ -4,8 +4,6 @@ import ( "testing" "testing/fstest" - "github.com/effect-ts/tsgo/internal/effectconfigcheck" - "github.com/effect-ts/tsgo/internal/effectconfigraw" "github.com/effect-ts/tsgo/internal/rule" "github.com/microsoft/TypeScript/tsc/shim/ast" "github.com/microsoft/TypeScript/tsc/shim/core" @@ -15,6 +13,11 @@ import ( "github.com/microsoft/TypeScript/tsc/shim/tspath" "github.com/microsoft/TypeScript/tsc/shim/vfs" "github.com/microsoft/TypeScript/tsc/shim/vfs/vfstest" + + // etscheckerhooks registers the config-validation callbacks in its init, which + // is the same path the compiler uses. Registering from the tests themselves + // would race, because every test here runs in parallel. + _ "github.com/effect-ts/tsgo/etscheckerhooks" ) const currentDirectory = "/.src" @@ -35,8 +38,6 @@ func (h *parseConfigHost) GetCurrentDirectory() string { return currentDirectory // diagnostics the config parse produced. func configDiagnostics(t *testing.T, files map[string]string, configName string) []*ast.Diagnostic { t.Helper() - effectconfigraw.Register() - effectconfigcheck.Register() testfs := make(map[string]any, len(files)) for name, content := range files { @@ -236,3 +237,65 @@ func TestDiagnosticsDisabledSuppressesTheCheck(t *testing.T) { t.Fatalf("expected no diagnostics, got %v", codesOf(diags)) } } + +func TestInheritedSuppressionAcrossExtends(t *testing.T) { + t.Parallel() + + t.Run("unknownRuleName off in a base silences a name declared in a child", func(t *testing.T) { + t.Parallel() + diags := configDiagnostics(t, map[string]string{ + "tsconfig.base.json": pluginConfig(`"diagnosticSeverity": { "unknownRuleName": "off" }`), + "tsconfig.json": `{ "extends": "./tsconfig.base.json", "compilerOptions": { "plugins": [ + { "name": "@effect/language-service", "diagnosticSeverity": { "importFromBarrel": "error" } } + ] } }`, + }, "tsconfig.json") + if len(diags) != 0 { + t.Fatalf("expected no diagnostics, got %v", codesOf(diags)) + } + }) + + t.Run("diagnostics false in a base silences a name declared in a child", func(t *testing.T) { + t.Parallel() + diags := configDiagnostics(t, map[string]string{ + "tsconfig.base.json": pluginConfig(`"diagnostics": false`), + "tsconfig.json": `{ "extends": "./tsconfig.base.json", "compilerOptions": { "plugins": [ + { "name": "@effect/language-service", "diagnosticSeverity": { "importFromBarrel": "error" } } + ] } }`, + }, "tsconfig.json") + if len(diags) != 0 { + t.Fatalf("expected no diagnostics, got %v", codesOf(diags)) + } + }) + + t.Run("a child silences a name declared in a base it does not own", func(t *testing.T) { + t.Parallel() + diags := configDiagnostics(t, map[string]string{ + "tsconfig.base.json": pluginConfig(`"diagnosticSeverity": { "importFromBarrel": "error" }`), + "tsconfig.json": `{ "extends": "./tsconfig.base.json", "compilerOptions": { "plugins": [ + { "name": "@effect/language-service", "diagnosticSeverity": { "unknownRuleName": "off" } } + ] } }`, + }, "tsconfig.json") + if len(diags) != 0 { + t.Fatalf("expected no diagnostics, got %v", codesOf(diags)) + } + }) + + t.Run("a child raises the severity of a name declared in a base", func(t *testing.T) { + t.Parallel() + diags := configDiagnostics(t, map[string]string{ + "tsconfig.base.json": pluginConfig(`"diagnosticSeverity": { "importFromBarrel": "error" }`), + "tsconfig.json": `{ "extends": "./tsconfig.base.json", "compilerOptions": { "plugins": [ + { "name": "@effect/language-service", "diagnosticSeverity": { "unknownRuleName": "error" } } + ] } }`, + }, "tsconfig.json") + if len(diags) != 1 { + t.Fatalf("expected 1 diagnostic, got %v", codesOf(diags)) + } + if got := diags[0].Category(); got != tsdiag.CategoryError { + t.Errorf("category = %v, want error", got) + } + if file := diags[0].File(); file == nil || tspath.GetBaseFileName(file.FileName()) != "tsconfig.base.json" { + t.Error("diagnostic should stay anchored on the config that declares the name") + } + }) +} diff --git a/shim/tsoptions/shim.go b/shim/tsoptions/shim.go index 0364fda4..1ca4c69a 100644 --- a/shim/tsoptions/shim.go +++ b/shim/tsoptions/shim.go @@ -49,6 +49,7 @@ func CreateDiagnosticForNodeInSourceFileOrCompilerDiagnostic(sourceFile *ast.Sou type DidYouMeanOptionsDiagnostics = tsoptions.DidYouMeanOptionsDiagnostics type ExtendedConfigCache = tsoptions.ExtendedConfigCache type ExtendedConfigCacheEntry = tsoptions.ExtendedConfigCacheEntry +var FinalizeEffectPluginDiagnosticsCallback = tsoptions.FinalizeEffectPluginDiagnosticsCallback //go:linkname ForEachCompilerOptionValue github.com/microsoft/TypeScript/tsc/internal/tsoptions.ForEachCompilerOptionValue func ForEachCompilerOptionValue(options *core.CompilerOptions, declFilter func(*tsoptions.CommandLineOption) bool, fn func(option *tsoptions.CommandLineOption, value reflect.Value, i int) bool) bool //go:linkname GetCallbackForFindingPropertyAssignmentByValue github.com/microsoft/TypeScript/tsc/internal/tsoptions.GetCallbackForFindingPropertyAssignmentByValue @@ -116,10 +117,12 @@ func ParseWatchOptions(key string, value any, allOptions *core.WatchOptions) []* type ParsedBuildCommandLine = tsoptions.ParsedBuildCommandLine type ParsedCommandLine = tsoptions.ParsedCommandLine type ParsedOptions = tsoptions.ParsedOptions +//go:linkname RegisterFinalizeEffectPluginDiagnosticsCallback github.com/microsoft/TypeScript/tsc/internal/tsoptions.RegisterFinalizeEffectPluginDiagnosticsCallback +func RegisterFinalizeEffectPluginDiagnosticsCallback(cb func(diagnostics []*ast.Diagnostic, options *core.CompilerOptions) []*ast.Diagnostic) //go:linkname RegisterMergeCompilerOptionsCallback github.com/microsoft/TypeScript/tsc/internal/tsoptions.RegisterMergeCompilerOptionsCallback func RegisterMergeCompilerOptionsCallback(cb func(targetOptions *core.CompilerOptions, sourceOptions *core.CompilerOptions, rawSource any, sourceConfigPath string, basePath string)) //go:linkname RegisterValidateEffectPluginOptionsCallback github.com/microsoft/TypeScript/tsc/internal/tsoptions.RegisterValidateEffectPluginOptionsCallback -func RegisterValidateEffectPluginOptionsCallback(cb func(sourceFile *ast.SourceFile, pluginsNode *ast.Node, options *core.CompilerOptions) []*ast.Diagnostic) +func RegisterValidateEffectPluginOptionsCallback(cb func(sourceFile *ast.SourceFile, pluginsNode *ast.Node) []*ast.Diagnostic) type SourceOutputAndProjectReference = tsoptions.SourceOutputAndProjectReference type TSConfig = tsoptions.TSConfig //go:linkname TargetToLibMap github.com/microsoft/TypeScript/tsc/internal/tsoptions.TargetToLibMap From 10e8c34f2f0afbd214f914e6ab2c8372d2f0012f Mon Sep 17 00:00:00 2001 From: James Spencer Date: Mon, 14 Sep 2026 15:18:58 -0500 Subject: [PATCH 3/3] test: pin that finalizing leaves foreign config diagnostics alone FinalizeDiagnostics receives the whole config-error slice and returns a replacement, so it holds drop authority over diagnostics it does not own, restrained only by a code predicate. The rest of the suite reads the Effect diagnostics through a filter that would hide the loss of that predicate, so a regression dropping every other config error in the file could land green. Assert on the unfiltered slice instead, with an unknown compiler option alongside an unknown rule name at `unknownRuleName: "off"`, which is the arm that rebuilds the slice. Removing the predicate turns this test red. Co-Authored-By: Claude Fable 5.1 --- .../effectconfigcheck_test.go | 59 +++++++++++++++++-- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/internal/effectconfigcheck/effectconfigcheck_test.go b/internal/effectconfigcheck/effectconfigcheck_test.go index e7e37376..5ffa3fa9 100644 --- a/internal/effectconfigcheck/effectconfigcheck_test.go +++ b/internal/effectconfigcheck/effectconfigcheck_test.go @@ -34,9 +34,9 @@ func (h *parseConfigHost) FS() vfs.FS { return h.fs } func (h *parseConfigHost) GetCurrentDirectory() string { return currentDirectory } -// configDiagnostics parses the named tsconfig out of files and returns the -// diagnostics the config parse produced. -func configDiagnostics(t *testing.T, files map[string]string, configName string) []*ast.Diagnostic { +// allConfigDiagnostics parses the named tsconfig out of files and returns every +// diagnostic the config parse produced, unfiltered. +func allConfigDiagnostics(t *testing.T, files map[string]string, configName string) []*ast.Diagnostic { t.Helper() testfs := make(map[string]any, len(files)) @@ -62,10 +62,17 @@ func configDiagnostics(t *testing.T, files map[string]string, configName string) nil, nil, ) - // TS18003 (no inputs found) is inherent to a config fixture that ships no - // source files and is not what these tests are about. + return parsed.Errors +} + +// configDiagnostics returns only the Effect diagnostics. TS18003 (no inputs found) +// is inherent to a config fixture that ships no source files and is not what most +// of these tests are about. Anything asserting on what FinalizeDiagnostics must +// leave alone has to use allConfigDiagnostics instead. +func configDiagnostics(t *testing.T, files map[string]string, configName string) []*ast.Diagnostic { + t.Helper() var effectDiags []*ast.Diagnostic - for _, diag := range parsed.Errors { + for _, diag := range allConfigDiagnostics(t, files, configName) { if rule.IsEffectCode(diag.Code()) { effectDiags = append(effectDiags, diag) } @@ -299,3 +306,43 @@ func TestInheritedSuppressionAcrossExtends(t *testing.T) { } }) } + +// FinalizeDiagnostics is handed the whole config-error slice and returns a +// replacement, so it holds drop authority over diagnostics it does not own. This +// asserts on the unfiltered slice, because the Effect-only filter used elsewhere +// would hide exactly the regression this guards: a lost ownership predicate +// silently dropping every other config error in the file. +func TestFinalizeLeavesForeignDiagnosticsAlone(t *testing.T) { + t.Parallel() + + files := map[string]string{ + "tsconfig.json": `{ + "compilerOptions": { + "bogusOption": true, + "plugins": [ + { + "name": "@effect/language-service", + "diagnosticSeverity": { "importFromBarrel": "error", "unknownRuleName": "off" } + } + ] + } +}`, + } + + // "off" takes FinalizeDiagnostics down its drop arm, which is the arm that + // rebuilds the slice and so the only one that can lose a foreign diagnostic. + diags := allConfigDiagnostics(t, files, "tsconfig.json") + + var foundUnknownOption bool + for _, diag := range diags { + if rule.IsEffectCode(diag.Code()) { + t.Errorf("expected every Effect diagnostic to be dropped, got %d", diag.Code()) + } + if diag.Code() == 5023 { + foundUnknownOption = true + } + } + if !foundUnknownOption { + t.Fatalf("TS5023 was dropped by FinalizeDiagnostics; got %v", codesOf(diags)) + } +}