Skip to content

Commit c83ee5e

Browse files
akoclaude
andcommitted
feat(lsp): widget property typos surface as real-time diagnostics
Wires widget property validation into the LSP's on-keystroke diagnostic publisher so unknown property keys on pluggable widgets appear as squiggles while the author types — not only when they save and trigger `mxcli check`. Implementation: - `ValidateWidgetPropertiesForStatement(stmt, registry)` exposed as a per-statement entry so the LSP can attach per-statement line numbers (instead of dumping all violations at line 0). - `LoadWidgetRegistry(projectPath)` extracted so callers can manage registry lifetime — the LSP loads once via `ensureWidgetRegistry()` rather than rebuilding it per keystroke. - `runSemanticValidation` promoted to a method on `*mdlServer` so it can reuse `s.widgetRegistry`. Nil-safe — widget checks are skipped when the registry hasn't loaded (e.g. no project context). - Consolidated the two duplicate `widgetCompletionsOnce.Do` blocks in lsp_completion.go into a single `ensureWidgetRegistry()` helper. Fixes a latent bug where `s.widgetRegistry` was nil if the first Once caller was the completion path (only the second block set it). Tests: - `TestWidgetPropertyDiagnostics` covers create-page, alter-page-insert, and the happy path (no false positives on valid properties). Verified end-to-end against test5-app: `combobox cb1 (optionsSourcType: 'enumeration')` produces `MDL-WIDGET01: widget cb1 (combobox) has no property optionsSourcType — did you mean optionsSourceType?` through the same code path the LSP now fires inline. Issue: #540 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3d4bedd commit c83ee5e

4 files changed

Lines changed: 141 additions & 36 deletions

File tree

cmd/mxcli/lsp_completion.go

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,16 @@ func (s *mdlServer) mdlCompletionItems(linePrefixUpper string) []protocol.Comple
105105
// NOTE: Cached via sync.Once — new .def.json files added while the LSP server is
106106
// running will not appear until the server is restarted.
107107
func (s *mdlServer) widgetRegistryCompletions() []protocol.CompletionItem {
108+
s.ensureWidgetRegistry()
109+
return s.widgetCompletionItems
110+
}
111+
112+
// ensureWidgetRegistry loads the widget registry once per server lifetime,
113+
// caching the result on s.widgetRegistry and s.widgetCompletionItems.
114+
// Safe to call from any goroutine.
115+
// NOTE: new .def.json files added while the LSP server is running will not
116+
// appear until the server is restarted.
117+
func (s *mdlServer) ensureWidgetRegistry() {
108118
s.widgetCompletionsOnce.Do(func() {
109119
registry, err := executor.NewWidgetRegistry()
110120
if err != nil {
@@ -113,6 +123,7 @@ func (s *mdlServer) widgetRegistryCompletions() []protocol.CompletionItem {
113123
if err := registry.LoadUserDefinitions(s.mprPath); err != nil {
114124
log.Printf("warning: loading user widget definitions for LSP: %v", err)
115125
}
126+
s.widgetRegistry = registry
116127
for _, def := range registry.All() {
117128
s.widgetCompletionItems = append(s.widgetCompletionItems, protocol.CompletionItem{
118129
Label: def.MDLName,
@@ -121,7 +132,6 @@ func (s *mdlServer) widgetRegistryCompletions() []protocol.CompletionItem {
121132
})
122133
}
123134
})
124-
return s.widgetCompletionItems
125135
}
126136

127137
// mdlCreateContextKeywords are object types suggested after CREATE.
@@ -781,23 +791,7 @@ func scanEnclosingWidget(text string, cursorLine, cursorCol int, s *mdlServer) *
781791
}
782792

783793
// Lazy-load the registry (re-use the widget-completions cache).
784-
s.widgetCompletionsOnce.Do(func() {
785-
registry, err := executor.NewWidgetRegistry()
786-
if err != nil {
787-
return
788-
}
789-
if err := registry.LoadUserDefinitions(s.mprPath); err != nil {
790-
log.Printf("warning: loading user widget definitions for LSP: %v", err)
791-
}
792-
s.widgetRegistry = registry
793-
for _, def := range registry.All() {
794-
s.widgetCompletionItems = append(s.widgetCompletionItems, protocol.CompletionItem{
795-
Label: def.MDLName,
796-
Kind: protocol.CompletionItemKindClass,
797-
Detail: "Pluggable widget: " + def.WidgetID,
798-
})
799-
}
800-
})
794+
s.ensureWidgetRegistry()
801795
if s.widgetRegistry == nil {
802796
return nil
803797
}

cmd/mxcli/lsp_completion_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,84 @@ func mapKeys(m map[string]bool) []string {
262262
return out
263263
}
264264

265+
// TestWidgetPropertyDiagnostics verifies that unknown property keys on a
266+
// pluggable widget surface as LSP diagnostics through runSemanticValidation
267+
// (the path that fires on every keystroke).
268+
func TestWidgetPropertyDiagnostics(t *testing.T) {
269+
s := &mdlServer{}
270+
271+
tests := []struct {
272+
name string
273+
text string
274+
wantRule string // RuleID expected in at least one diagnostic
275+
wantInMsg []string // substrings expected somewhere in the diagnostics
276+
wantNoMatch bool // when no widget diagnostic should fire
277+
}{
278+
{
279+
name: "typo on combobox property surfaces MDL-WIDGET01",
280+
text: "create page Mod.X (Title: 'T') {\n" +
281+
" combobox cb1 (\n" +
282+
" optionsSourcType: 'enumeration'\n" +
283+
" )\n" +
284+
"}",
285+
wantRule: "MDL-WIDGET01",
286+
wantInMsg: []string{"optionsSourcType", "combobox"},
287+
},
288+
{
289+
name: "valid property — no widget diagnostic",
290+
text: "create page Mod.X (Title: 'T') {\n" +
291+
" combobox cb1 (\n" +
292+
" optionsSourceType: 'enumeration'\n" +
293+
" )\n" +
294+
"}",
295+
wantNoMatch: true,
296+
},
297+
{
298+
name: "alter page insert — typo flagged",
299+
text: "ALTER PAGE Mod.X {\n" +
300+
" INSERT AFTER target1 {\n" +
301+
" combobox cb1 (badprop: 'x')\n" +
302+
" }\n" +
303+
"};",
304+
wantRule: "MDL-WIDGET01",
305+
wantInMsg: []string{"badprop"},
306+
},
307+
}
308+
309+
for _, tt := range tests {
310+
t.Run(tt.name, func(t *testing.T) {
311+
diags := s.runSemanticValidation(tt.text)
312+
var widgetDiags []protocol.Diagnostic
313+
for _, d := range diags {
314+
if code, ok := d.Code.(string); ok && code == "MDL-WIDGET01" {
315+
widgetDiags = append(widgetDiags, d)
316+
}
317+
}
318+
if tt.wantNoMatch {
319+
if len(widgetDiags) != 0 {
320+
t.Errorf("expected no widget diagnostics, got: %v", widgetDiags)
321+
}
322+
return
323+
}
324+
if len(widgetDiags) == 0 {
325+
t.Fatalf("expected at least one MDL-WIDGET01 diagnostic, got none (all diags: %v)", diags)
326+
}
327+
for _, sub := range tt.wantInMsg {
328+
found := false
329+
for _, d := range widgetDiags {
330+
if strings.Contains(d.Message, sub) {
331+
found = true
332+
break
333+
}
334+
}
335+
if !found {
336+
t.Errorf("expected substring %q in a widget diagnostic; messages: %v", sub, widgetDiags)
337+
}
338+
}
339+
})
340+
}
341+
}
342+
265343
// TestWidgetPropertyHover verifies LSP hover surfaces widget property
266344
// descriptions / type / default from the .def.json content.
267345
func TestWidgetPropertyHover(t *testing.T) {

cmd/mxcli/lsp_diagnostics.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func (s *mdlServer) publishDiagnostics(ctx context.Context, docURI uri.URI, text
6363
diags := parseMDLDiagnostics(text)
6464
// If no parse errors, run semantic validation inline
6565
if len(diags) == 0 {
66-
diags = append(diags, runSemanticValidation(text)...)
66+
diags = append(diags, s.runSemanticValidation(text)...)
6767
}
6868
if diags == nil {
6969
diags = []protocol.Diagnostic{} // send empty array to clear diagnostics
@@ -250,14 +250,19 @@ func mapStatementLines(text string) []uint32 {
250250

251251
// runSemanticValidation runs the same validators as cmd_check.go directly on parsed text,
252252
// returning LSP diagnostics with structured rule IDs.
253-
func runSemanticValidation(text string) []protocol.Diagnostic {
253+
func (s *mdlServer) runSemanticValidation(text string) []protocol.Diagnostic {
254254
prog, errs := visitor.Build(text)
255255
if len(errs) > 0 || prog == nil {
256256
return nil
257257
}
258258

259259
stmtLines := mapStatementLines(text)
260260

261+
// Widget validation reuses the LSP's cached widget registry so the
262+
// filesystem isn't walked on every keystroke. Nil-safe: when the registry
263+
// hasn't loaded (no project context, etc.) widget validation is skipped.
264+
s.ensureWidgetRegistry()
265+
261266
var diags []protocol.Diagnostic
262267
for i, stmt := range prog.Statements {
263268
var violations []linter.Violation
@@ -276,6 +281,9 @@ func runSemanticValidation(text string) []protocol.Diagnostic {
276281
violations = append(violations, executor.ValidateOQLTypes(viewStmt.Query.RawQuery, viewStmt.Attributes)...)
277282
}
278283
}
284+
if s.widgetRegistry != nil {
285+
violations = append(violations, executor.ValidateWidgetPropertiesForStatement(stmt, s.widgetRegistry)...)
286+
}
279287

280288
lineNum := uint32(0)
281289
if i < len(stmtLines) {

mdl/executor/validate_widgets.go

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,33 +40,58 @@ var universalWidgetProperties = map[string]bool{
4040
// project-installed widgets get validated too; otherwise only built-in
4141
// definitions are consulted.
4242
func ValidateWidgetProperties(prog *ast.Program, projectPath string) []linter.Violation {
43+
registry := LoadWidgetRegistry(projectPath)
44+
if registry == nil {
45+
return nil
46+
}
47+
var violations []linter.Violation
48+
for _, stmt := range prog.Statements {
49+
violations = append(violations, ValidateWidgetPropertiesForStatement(stmt, registry)...)
50+
}
51+
return violations
52+
}
53+
54+
// LoadWidgetRegistry returns a widget registry loaded with both the built-in
55+
// definitions and (when projectPath is non-empty) project-level .def.json
56+
// files. Returns nil if registry initialization fails. The LSP uses this to
57+
// load the registry once per session rather than per validation pass.
58+
func LoadWidgetRegistry(projectPath string) *WidgetRegistry {
4359
registry, err := NewWidgetRegistry()
4460
if err != nil || registry == nil {
4561
return nil
4662
}
4763
if projectPath != "" {
4864
_ = registry.LoadUserDefinitions(projectPath)
4965
}
66+
return registry
67+
}
5068

51-
var violations []linter.Violation
52-
for _, stmt := range prog.Statements {
53-
switch s := stmt.(type) {
54-
case *ast.CreatePageStmtV3:
55-
violations = append(violations, validateWidgetTree(s.Widgets, registry, "page "+s.Name.String())...)
56-
case *ast.CreateSnippetStmtV3:
57-
violations = append(violations, validateWidgetTree(s.Widgets, registry, "snippet "+s.Name.String())...)
58-
case *ast.AlterPageStmt:
59-
for _, op := range s.Operations {
60-
switch o := op.(type) {
61-
case *ast.InsertWidgetOp:
62-
violations = append(violations, validateWidgetTree(o.Widgets, registry, "alter "+s.PageName.String())...)
63-
case *ast.ReplaceWidgetOp:
64-
violations = append(violations, validateWidgetTree(o.NewWidgets, registry, "alter "+s.PageName.String())...)
65-
}
69+
// ValidateWidgetPropertiesForStatement runs widget property validation on a
70+
// single statement using a pre-loaded registry. Returns no violations for
71+
// statements that don't carry pluggable widgets (everything except
72+
// CreatePageStmtV3, CreateSnippetStmtV3, AlterPageStmt).
73+
func ValidateWidgetPropertiesForStatement(stmt ast.Statement, registry *WidgetRegistry) []linter.Violation {
74+
if registry == nil {
75+
return nil
76+
}
77+
switch s := stmt.(type) {
78+
case *ast.CreatePageStmtV3:
79+
return validateWidgetTree(s.Widgets, registry, "page "+s.Name.String())
80+
case *ast.CreateSnippetStmtV3:
81+
return validateWidgetTree(s.Widgets, registry, "snippet "+s.Name.String())
82+
case *ast.AlterPageStmt:
83+
var out []linter.Violation
84+
for _, op := range s.Operations {
85+
switch o := op.(type) {
86+
case *ast.InsertWidgetOp:
87+
out = append(out, validateWidgetTree(o.Widgets, registry, "alter "+s.PageName.String())...)
88+
case *ast.ReplaceWidgetOp:
89+
out = append(out, validateWidgetTree(o.NewWidgets, registry, "alter "+s.PageName.String())...)
6690
}
6791
}
92+
return out
6893
}
69-
return violations
94+
return nil
7095
}
7196

7297
// validateWidgetTree recursively walks the AST widget tree and validates

0 commit comments

Comments
 (0)