Skip to content

Commit a83fa37

Browse files
Ajit Pratap Singhclaude
authored andcommitted
feat(linter,lsp,parser): visitor migration, config loader, LSP dispatch, structured parser errors
Addresses 5 issues from the 2026-04-16 architect review: C5: Linter rules now use ast.Walk instead of flat top-level traversal - 14 rules migrated (L009, L011-L014, L016, L017, L020, L023-L026, L028, L030) - 6 already-walking rules reinforced (L018, L019, L021, L022, L027, L029) - L022 FunctionOnColumn now descends into FROM subqueries - L029 SubqueryCanBeJoin traversal bug fixed (was skipping WITH/FROM/JOIN) - 22 new nested-case tests across performance, naming, safety, style H8: .gosqlx.yml config loader in pkg/linter/config/ - YAML schema: rules{id:{enabled,severity,params}}, ignore[], default_severity - Load(path), LoadDefault() (walks from cwd), Config.Apply() - linter.NewWithIgnore wires in IgnoreMatcher without circular dep - All 30 rules respect enable/disable and severity overrides via wrapper - yaml.v3 already in go.mod - 7 testdata fixtures, 18 tests H9: LSP statement dispatch in pkg/lsp/handler.go - Replaced fmt.Sprintf("%T")+strings.Contains with proper ast.Statement type switch - 15 statement kinds: SelectStatement, InsertStatement, UpdateStatement, DeleteStatement, MergeStatement, CreateTableStatement, CreateViewStatement, CreateIndexStatement, CreateMaterializedViewStatement, CreateSequenceStatement, DropStatement, DropSequenceStatement, AlterStatement, AlterTableStatement, AlterSequenceStatement, TruncateStatement, WithClause - Meaningful symbol names ("SELECT from users" vs "SELECT #1") H10: LSP documentSymbol ranges - Real statement ranges derived from AST position info via walkNode+nodeLocation - Fallback to earliestChildLocation + semicolon-or-EOF scan for nodes lacking Pos - Ranges bounded by next statement start (never overlap) - New TestHandler_DocumentSymbol_MultiStatementRanges verifies distinct ranges H7: 37 fmt.Errorf sites in parser converted to structured errors - Constructor distribution: 16 InvalidSyntaxError, 7 ExpectedTokenError, 5 UnsupportedFeatureError, 6 WrapError, 1 IncompleteStatementError - Context cancellation sites now return ctx.Err() directly (preserves errors.Is(err, context.Canceled) semantics) - INSERT...SELECT ErrUnexpectedStatement sentinel preserved via WithCause - New parser_errors_test.go covers 6 categories / 12 subtests go test -race ./... passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1866e27 commit a83fa37

42 files changed

Lines changed: 2718 additions & 399 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

pkg/linter/config/config.go

Lines changed: 395 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,395 @@
1+
// Copyright 2026 GoSQLX Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Package config provides .gosqlx.yml configuration loading for the linter.
16+
//
17+
// The configuration file controls which rules run, their severity overrides,
18+
// optional per-rule parameters, and file ignore patterns. It is typically
19+
// committed to a project root so a team shares a consistent lint policy.
20+
//
21+
// Example .gosqlx.yml:
22+
//
23+
// rules:
24+
// L001:
25+
// enabled: true
26+
// severity: error
27+
// L005:
28+
// enabled: true
29+
// params:
30+
// max_length: 120
31+
// L011:
32+
// enabled: false
33+
// ignore:
34+
// - "migrations/*.sql"
35+
// - "vendor/**"
36+
// default_severity: warning
37+
//
38+
// Typical use:
39+
//
40+
// cfg, err := config.LoadDefault()
41+
// if err != nil && !errors.Is(err, config.ErrNotFound) {
42+
// log.Fatal(err)
43+
// }
44+
// rules := cfg.Apply(allRules)
45+
// l := linter.NewWithConfig(cfg, rules...)
46+
//
47+
// Unknown rule IDs in the config file are reported via Config.Warnings but do
48+
// not cause Load to fail. This allows forward compatibility when older
49+
// installations read configs that reference newer rules.
50+
package config
51+
52+
import (
53+
"errors"
54+
"fmt"
55+
"os"
56+
"path/filepath"
57+
"strings"
58+
59+
"github.com/ajitpratap0/GoSQLX/pkg/linter"
60+
"gopkg.in/yaml.v3"
61+
)
62+
63+
// DefaultFilename is the conventional name for the linter configuration file.
64+
const DefaultFilename = ".gosqlx.yml"
65+
66+
// ErrNotFound is returned by LoadDefault when no .gosqlx.yml is found while
67+
// walking up from the current working directory. Callers that want to treat a
68+
// missing config as "use built-in defaults" should check with errors.Is.
69+
var ErrNotFound = errors.New("gosqlx: no .gosqlx.yml found")
70+
71+
// RuleConfig represents per-rule configuration entries.
72+
//
73+
// A rule entry may set Enabled (default true if the rule appears at all),
74+
// override Severity (error | warning | info), and supply rule-specific Params.
75+
// Unknown params are preserved and may be consumed by rules that understand
76+
// them; rules that don't read params simply ignore them.
77+
type RuleConfig struct {
78+
// Enabled is an optional pointer so the zero value ("not set") differs
79+
// from an explicit `enabled: false`.
80+
Enabled *bool `yaml:"enabled"`
81+
82+
// Severity overrides the rule's default severity. Empty means "no override".
83+
Severity string `yaml:"severity"`
84+
85+
// Params are rule-specific parameters (e.g. max_length for L005).
86+
// The types are whatever YAML produces (int, string, bool, []interface{}, map).
87+
Params map[string]any `yaml:"params"`
88+
}
89+
90+
// Config represents the parsed contents of a .gosqlx.yml file.
91+
//
92+
// Config is immutable after Load returns. Methods on *Config do not mutate it.
93+
type Config struct {
94+
// Rules maps rule IDs (e.g. "L001") to per-rule configuration.
95+
Rules map[string]RuleConfig `yaml:"rules"`
96+
97+
// Ignore is a list of glob patterns for file paths to skip during linting.
98+
// Patterns support "*" (single path segment) and "**" (any number of
99+
// segments). Patterns are matched against the filename passed to the
100+
// linter (LintFile, LintString's filename arg, or LintDirectory entries).
101+
Ignore []string `yaml:"ignore"`
102+
103+
// DefaultSeverity, if set, is applied to rules whose RuleConfig.Severity
104+
// is empty. Valid values: "error", "warning", "info". Empty means the
105+
// rule's built-in severity is used.
106+
DefaultSeverity string `yaml:"default_severity"`
107+
108+
// Path is the absolute filesystem path the config was loaded from.
109+
// Empty for configs constructed in memory.
110+
Path string `yaml:"-"`
111+
112+
// Warnings are non-fatal diagnostics produced during Load (e.g. unknown
113+
// rule IDs, unknown severity values). They are forward-compatibility
114+
// signals, not errors.
115+
Warnings []string `yaml:"-"`
116+
}
117+
118+
// Load reads and parses the configuration file at the given path.
119+
//
120+
// Returns an error if the file cannot be read or contains invalid YAML.
121+
// Unknown rule IDs and unknown severity values become Warnings rather than
122+
// errors so the linter can keep running against newer configs.
123+
func Load(path string) (*Config, error) {
124+
abs, err := filepath.Abs(path)
125+
if err != nil {
126+
return nil, fmt.Errorf("gosqlx config: resolve path %q: %w", path, err)
127+
}
128+
129+
data, err := os.ReadFile(filepath.Clean(abs)) // #nosec G304 -- config path is user-provided by design
130+
if err != nil {
131+
return nil, fmt.Errorf("gosqlx config: read %s: %w", abs, err)
132+
}
133+
134+
return parse(data, abs)
135+
}
136+
137+
// LoadDefault searches for .gosqlx.yml starting at the current working
138+
// directory and walking up toward the filesystem root. It returns the first
139+
// match found, or ErrNotFound if none exists.
140+
//
141+
// Use errors.Is(err, ErrNotFound) to distinguish "no config" from a real
142+
// I/O or parse error.
143+
func LoadDefault() (*Config, error) {
144+
cwd, err := os.Getwd()
145+
if err != nil {
146+
return nil, fmt.Errorf("gosqlx config: getwd: %w", err)
147+
}
148+
return loadDefaultFrom(cwd)
149+
}
150+
151+
// loadDefaultFrom is the testable version of LoadDefault that walks up from
152+
// the given start directory instead of the process cwd.
153+
func loadDefaultFrom(start string) (*Config, error) {
154+
dir, err := filepath.Abs(start)
155+
if err != nil {
156+
return nil, fmt.Errorf("gosqlx config: abs %q: %w", start, err)
157+
}
158+
159+
for {
160+
candidate := filepath.Join(dir, DefaultFilename)
161+
if info, statErr := os.Stat(candidate); statErr == nil && !info.IsDir() {
162+
return Load(candidate)
163+
}
164+
parent := filepath.Dir(dir)
165+
if parent == dir {
166+
break
167+
}
168+
dir = parent
169+
}
170+
return nil, ErrNotFound
171+
}
172+
173+
// parse parses YAML bytes into a *Config. It attaches Path for diagnostics
174+
// and populates Warnings for unknown rule IDs / unknown severities. Invalid
175+
// YAML or unknown top-level keys produce a hard error.
176+
func parse(data []byte, path string) (*Config, error) {
177+
cfg := &Config{Path: path}
178+
179+
dec := yaml.NewDecoder(strings.NewReader(string(data)))
180+
dec.KnownFields(true) // reject unknown top-level keys to catch typos
181+
182+
if err := dec.Decode(cfg); err != nil {
183+
// Empty file is fine; yaml returns io.EOF on empty input.
184+
if strings.TrimSpace(string(data)) == "" {
185+
return cfg, nil
186+
}
187+
return nil, fmt.Errorf("gosqlx config: parse %s: %w", path, err)
188+
}
189+
190+
// Validate default_severity.
191+
if cfg.DefaultSeverity != "" && !isValidSeverity(cfg.DefaultSeverity) {
192+
cfg.Warnings = append(cfg.Warnings,
193+
fmt.Sprintf("unknown default_severity %q (valid: error, warning, info)", cfg.DefaultSeverity))
194+
cfg.DefaultSeverity = ""
195+
}
196+
197+
// Warn on unknown rule IDs and unknown per-rule severities.
198+
for id, rc := range cfg.Rules {
199+
if !linter.IsValidRuleID(id) {
200+
cfg.Warnings = append(cfg.Warnings,
201+
fmt.Sprintf("unknown rule ID %q (forward-compat: ignored)", id))
202+
}
203+
if rc.Severity != "" && !isValidSeverity(rc.Severity) {
204+
cfg.Warnings = append(cfg.Warnings,
205+
fmt.Sprintf("rule %s: unknown severity %q (valid: error, warning, info)", id, rc.Severity))
206+
}
207+
}
208+
209+
// Validate ignore patterns compile as globs. We test via matchGlob against
210+
// a probe string so malformed patterns get surfaced as warnings now rather
211+
// than later.
212+
for _, pat := range cfg.Ignore {
213+
if _, err := matchGlob(pat, "probe.sql"); err != nil {
214+
cfg.Warnings = append(cfg.Warnings,
215+
fmt.Sprintf("invalid ignore pattern %q: %v", pat, err))
216+
}
217+
}
218+
219+
return cfg, nil
220+
}
221+
222+
// isValidSeverity reports whether s is one of the three accepted severity
223+
// strings. Empty strings are NOT considered valid by this function (callers
224+
// that treat empty as "no override" should check that separately).
225+
func isValidSeverity(s string) bool {
226+
switch linter.Severity(s) {
227+
case linter.SeverityError, linter.SeverityWarning, linter.SeverityInfo:
228+
return true
229+
}
230+
return false
231+
}
232+
233+
// Apply filters and configures the given rules per this config.
234+
//
235+
// Behavior:
236+
// - A rule entry with Enabled == &false drops the rule from the result.
237+
// - A rule entry with a valid Severity wraps the rule so Severity()
238+
// returns the override, and emitted Violations carry that severity.
239+
// - A rule with no entry uses DefaultSeverity if set and valid; otherwise
240+
// the rule's built-in severity.
241+
// - Rules in the config that don't appear in the input slice are ignored
242+
// (no rule is instantiated from nothing).
243+
//
244+
// The returned slice has the same order as the input, minus disabled rules.
245+
// The input slice is not mutated.
246+
func (c *Config) Apply(rules []linter.Rule) []linter.Rule {
247+
if c == nil {
248+
return rules
249+
}
250+
251+
out := make([]linter.Rule, 0, len(rules))
252+
for _, r := range rules {
253+
rc, hasEntry := c.Rules[r.ID()]
254+
255+
// Explicit disable.
256+
if hasEntry && rc.Enabled != nil && !*rc.Enabled {
257+
continue
258+
}
259+
260+
// Determine effective severity override.
261+
sev := ""
262+
switch {
263+
case hasEntry && rc.Severity != "" && isValidSeverity(rc.Severity):
264+
sev = rc.Severity
265+
case c.DefaultSeverity != "" && isValidSeverity(c.DefaultSeverity):
266+
// Only apply default if rule has no explicit override.
267+
if !(hasEntry && rc.Severity != "") {
268+
sev = c.DefaultSeverity
269+
}
270+
}
271+
272+
if sev != "" && linter.Severity(sev) != r.Severity() {
273+
r = wrapWithSeverity(r, linter.Severity(sev))
274+
}
275+
out = append(out, r)
276+
}
277+
return out
278+
}
279+
280+
// ShouldIgnore reports whether filename matches any Ignore pattern.
281+
//
282+
// Matching is performed against both the raw filename and its cleaned form.
283+
// Patterns use "**" for "any number of path segments" and "*" for a single
284+
// path segment. A nil or empty config returns false.
285+
func (c *Config) ShouldIgnore(filename string) bool {
286+
if c == nil || len(c.Ignore) == 0 || filename == "" {
287+
return false
288+
}
289+
// Normalize path separators so Windows-style inputs still match POSIX
290+
// patterns. filepath.ToSlash is a no-op on non-Windows, so we replace
291+
// backslashes explicitly for cross-platform robustness.
292+
target := strings.ReplaceAll(filepath.ToSlash(filename), `\`, "/")
293+
cleanTarget := strings.ReplaceAll(filepath.ToSlash(filepath.Clean(filename)), `\`, "/")
294+
295+
for _, pat := range c.Ignore {
296+
for _, candidate := range []string{target, cleanTarget} {
297+
match, err := matchGlob(pat, candidate)
298+
if err == nil && match {
299+
return true
300+
}
301+
}
302+
}
303+
return false
304+
}
305+
306+
// configuredRule wraps a Rule to override its reported severity. All
307+
// Violations returned by Check are rewritten to carry the override so reports
308+
// are consistent with what Severity() reports.
309+
type configuredRule struct {
310+
linter.Rule
311+
sev linter.Severity
312+
}
313+
314+
func wrapWithSeverity(r linter.Rule, sev linter.Severity) linter.Rule {
315+
return &configuredRule{Rule: r, sev: sev}
316+
}
317+
318+
// Severity returns the override severity.
319+
func (c *configuredRule) Severity() linter.Severity { return c.sev }
320+
321+
// Check runs the wrapped rule and rewrites each violation's Severity field
322+
// to the override, keeping all other fields intact.
323+
func (c *configuredRule) Check(ctx *linter.Context) ([]linter.Violation, error) {
324+
vs, err := c.Rule.Check(ctx)
325+
if err != nil {
326+
return vs, err
327+
}
328+
for i := range vs {
329+
vs[i].Severity = c.sev
330+
}
331+
return vs, nil
332+
}
333+
334+
// matchGlob implements a minimal glob matcher supporting "**" (any number of
335+
// path segments, including zero) and "*" (a single path segment with no "/").
336+
// It differs from filepath.Match which treats "**" the same as "*".
337+
//
338+
// Returns (matched, error). Errors indicate malformed patterns.
339+
func matchGlob(pattern, name string) (bool, error) {
340+
pattern = filepath.ToSlash(pattern)
341+
name = filepath.ToSlash(name)
342+
343+
// Exact match short-circuit.
344+
if pattern == name {
345+
return true, nil
346+
}
347+
348+
// Split pattern on "/" but preserve "**" segments.
349+
patSegs := strings.Split(pattern, "/")
350+
nameSegs := strings.Split(name, "/")
351+
return matchSegments(patSegs, nameSegs)
352+
}
353+
354+
// matchSegments matches path segments with "**" wildcard semantics.
355+
func matchSegments(pat, name []string) (bool, error) {
356+
for len(pat) > 0 {
357+
p := pat[0]
358+
if p == "**" {
359+
// Skip consecutive "**" segments.
360+
for len(pat) > 0 && pat[0] == "**" {
361+
pat = pat[1:]
362+
}
363+
if len(pat) == 0 {
364+
return true, nil // trailing ** matches everything remaining
365+
}
366+
// Try to match the remaining pattern at every suffix of name.
367+
for i := 0; i <= len(name); i++ {
368+
ok, err := matchSegments(pat, name[i:])
369+
if err != nil {
370+
return false, err
371+
}
372+
if ok {
373+
return true, nil
374+
}
375+
}
376+
return false, nil
377+
}
378+
379+
if len(name) == 0 {
380+
return false, nil
381+
}
382+
383+
// Non-** segment must match a single name segment via filepath.Match.
384+
ok, err := filepath.Match(p, name[0])
385+
if err != nil {
386+
return false, err
387+
}
388+
if !ok {
389+
return false, nil
390+
}
391+
pat = pat[1:]
392+
name = name[1:]
393+
}
394+
return len(name) == 0, nil
395+
}

0 commit comments

Comments
 (0)