diff --git a/CHANGELOG.md b/CHANGELOG.md index d6a2662f2..4f402ecb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 benchmarks that publish diagnostics no longer time a growing buffer. ### Performance +- **Linter**: rule L007 (keyword case consistency) scans words without allocating (substring words, ASCII upper-casing into a stack buffer before the keyword lookup). Output is unchanged. On the canonical benchmarks (Apple M4, local), `BenchmarkLintRule/L007` allocates about 98% fewer objects (72565 to 1194 allocs/op) and about 97% fewer bytes, and `BenchmarkLintRW` allocates about 12.5% fewer objects. [W6-03] - **gosqlx**: errors returned by `Parse`, `Validate`, `Lint` and the tree helpers now wrap the sentinel and the underlying error in an unexported `wrapErr` that builds its message lazily, instead of `fmt.Errorf("%w: %w", ...)` on every failure. Error text, `errors.Is` and `errors.As` behaviour are unchanged; only the unexported concrete error type differs. The saving applies to callers that only check `errors.Is`/`errors.As` or drop the message: when the message is built (`err.Error()`, as the LSP and CLI do) the gain is about 2 allocs/op and 3-4% B/op per error (syntax_mid 21 -> 19 allocs). Because the message is now built on demand, `Error()` reflects any later mutation of the inner `*errors.Error` reached through `errors.As`; previously the text was fixed at creation. [W1-03] - **Linter L010**: scans bytes in a single pass instead of rebuilding substrings and running a regular expression; `BenchmarkLintRule/L010` is 67.5% faster on the diff --git a/pkg/linter/rules/keywords/keyword_case.go b/pkg/linter/rules/keywords/keyword_case.go index b76f02ea4..8e600f7cd 100644 --- a/pkg/linter/rules/keywords/keyword_case.go +++ b/pkg/linter/rules/keywords/keyword_case.go @@ -132,63 +132,80 @@ func (r *KeywordCaseRule) Check(ctx *linter.Context) ([]linter.Violation, error) violations := []linter.Violation{} for lineNum, line := range ctx.Lines { - // Tokenize the line to find keywords - words := tokenizeLine(line) - - for _, word := range words { - upperWord := strings.ToUpper(word.text) - if sqlKeywords[upperWord] { - // Check if case matches preferred style - hasViolation := false - var expectedCase string - - if r.preferredStyle == CaseUpper { - hasViolation = word.text != upperWord - expectedCase = upperWord - } else { - lowerWord := strings.ToLower(word.text) - hasViolation = word.text != lowerWord - expectedCase = lowerWord - } + scanWords(line, func(text string, column int) { + if !isSQLKeyword(text) { + return + } + // Check if case matches preferred style + hasViolation := false + var expectedCase string + + if r.preferredStyle == CaseUpper { + expectedCase = strings.ToUpper(text) + hasViolation = text != expectedCase + } else { + expectedCase = strings.ToLower(text) + hasViolation = text != expectedCase + } - if hasViolation { - violations = append(violations, linter.Violation{ - Rule: r.ID(), - RuleName: r.Name(), - Severity: r.Severity(), - Message: "Keyword '" + word.text + "' should be " + string(r.preferredStyle) + "case: '" + expectedCase + "'", - Location: models.Location{Line: lineNum + 1, Column: word.column}, - Line: line, - Suggestion: "Change '" + word.text + "' to '" + expectedCase + "'", - CanAutoFix: true, - }) - } + if hasViolation { + violations = append(violations, linter.Violation{ + Rule: r.ID(), + RuleName: r.Name(), + Severity: r.Severity(), + Message: "Keyword '" + text + "' should be " + string(r.preferredStyle) + "case: '" + expectedCase + "'", + Location: models.Location{Line: lineNum + 1, Column: column}, + Line: line, + Suggestion: "Change '" + text + "' to '" + expectedCase + "'", + CanAutoFix: true, + }) } - } + }) } return violations, nil } -// wordToken represents a word extracted from a line with its position. -type wordToken struct { - text string - column int // 1-indexed column position in the line +// maxKeywordLen bounds the length of ASCII entries in sqlKeywords. +const maxKeywordLen = 16 + +// isSQLKeyword reports whether word, compared case-insensitively, is in +// sqlKeywords. ASCII words are upper-cased into a stack buffer so the map +// lookup does not allocate; words containing non-ASCII bytes take the +// strings.ToUpper slow path (Unicode case folding can map e.g. U+017F to S). +func isSQLKeyword(word string) bool { + var buf [maxKeywordLen]byte + if len(word) <= maxKeywordLen { + for i := 0; i < len(word); i++ { + c := word[i] + if c >= 0x80 { + return sqlKeywords[strings.ToUpper(word)] + } + if c >= 'a' && c <= 'z' { + c -= 'a' - 'A' + } + buf[i] = c + } + return sqlKeywords[string(buf[:len(word)])] + } + // Defensive: unreachable with today's table (longest keyword is 12 bytes and + // only U+017F/U+0131 shrink on upper-casing, capping reachable words at 14 + // bytes), kept so a future table change cannot silently diverge. + for i := 0; i < len(word); i++ { + if word[i] >= 0x80 { + return sqlKeywords[strings.ToUpper(word)] + } + } + return false } -// tokenizeLine extracts words from a line with their column positions. -// -// Parses the line character by character, extracting sequences of letters, digits, -// and underscores as words. Skips content inside string literals (both single and -// double quoted) to avoid extracting keywords from SQL string values. -// -// Returns a slice of wordTokens representing each word and its position. -func tokenizeLine(line string) []wordToken { - words := []wordToken{} +// scanWords calls emit for each word in line (letters, digits and underscores, +// not starting with a digit) outside quoted strings, passing a substring of +// line and its 1-indexed byte column. It does not allocate. +func scanWords(line string, emit func(text string, column int)) { inString := false stringChar := rune(0) wordStart := -1 - currentWord := strings.Builder{} for i, ch := range line { // Handle string literals - skip keywords inside strings @@ -196,11 +213,7 @@ func tokenizeLine(line string) []wordToken { inString = true stringChar = ch if wordStart >= 0 { - words = append(words, wordToken{ - text: currentWord.String(), - column: wordStart + 1, // 1-indexed - }) - currentWord.Reset() + emit(line[wordStart:i], wordStart+1) wordStart = -1 } continue @@ -219,26 +232,16 @@ func tokenizeLine(line string) []wordToken { if wordStart < 0 { wordStart = i } - currentWord.WriteRune(ch) } else if wordStart >= 0 { - words = append(words, wordToken{ - text: currentWord.String(), - column: wordStart + 1, // 1-indexed - }) - currentWord.Reset() + emit(line[wordStart:i], wordStart+1) wordStart = -1 } } // Don't forget the last word if wordStart >= 0 { - words = append(words, wordToken{ - text: currentWord.String(), - column: wordStart + 1, - }) + emit(line[wordStart:], wordStart+1) } - - return words } // Fix converts all keywords to the preferred case in SQL content. diff --git a/pkg/linter/rules/keywords/keyword_case_differential_test.go b/pkg/linter/rules/keywords/keyword_case_differential_test.go new file mode 100644 index 000000000..8ed7122e2 --- /dev/null +++ b/pkg/linter/rules/keywords/keyword_case_differential_test.go @@ -0,0 +1,134 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package keywords_test + +import ( + "io/fs" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/linter" + "github.com/ajitpratap0/GoSQLX/pkg/linter/rules/keywords" +) + +// Walk every SQL file, including nested corpus directories. Fail on missing +// fixtures so a working-directory mistake cannot silently weaken coverage. +func walkSQL(tb testing.TB, visit func(path, dialect, sql string)) { + tb.Helper() + for _, root := range []string{"../../../../testdata/realworld", "../../../../testdata/corpus"} { + count := 0 + err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() || filepath.Ext(path) != ".sql" { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + dialect := "" + if strings.HasSuffix(root, "realworld") { + dialect = strings.Split(rel, string(filepath.Separator))[0] + } + visit(path, dialect, string(data)) + count++ + return nil + }) + if err != nil { + tb.Fatal(err) + } + if count == 0 { + tb.Fatalf("no SQL fixtures in %s", root) + } + } +} + +func TestKeywordCaseDifferential(t *testing.T) { + check := func(t *testing.T, sql string) { + t.Helper() + for _, style := range []keywords.CaseStyle{"", keywords.CaseUpper, keywords.CaseLower, "other"} { + rule := keywords.NewKeywordCaseRule(style) + preferred := style + if preferred == "" { + preferred = keywords.CaseUpper + } + old := &legacyKeywordCaseRule{KeywordCaseRule: rule, preferredStyle: preferred} + ctx := linter.NewContext(sql, "differential.sql") + want, wantErr := old.Check(ctx) + got, gotErr := rule.Check(ctx) + if wantErr != nil || gotErr != nil { + t.Fatalf("Check errors: old=%v new=%v", wantErr, gotErr) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("style %q SQL %q\nnew: %#v\nold: %#v", style, sql, got, want) + } + // Fix is unchanged, but verify that applying either violation set yields + // exactly the same output and that another application is stable. + wantFix, err := rule.Fix(sql, want) + if err != nil { + t.Fatal(err) + } + gotFix, err := rule.Fix(sql, got) + if err != nil { + t.Fatal(err) + } + again, err := rule.Fix(gotFix, got) + if err != nil { + t.Fatal(err) + } + if gotFix != wantFix || again != gotFix { + t.Fatalf("style %q: fixes differ or are unstable", style) + } + } + } + cases := []string{ + "", "SeLeCt id, name FrOm users WhErE active = TrUe", + "ſelect caſe when true then 1 end from café where 日本語٢ = 1", + "select selecté, éselect, _from, from2, 2where, select\u0301 from таблица", + "select \"from\", `where`, [select] from users", + "select 'from ''where'' select', \"join \"\"on\"\"\" from users", + "select 1 -- from WHERE\n/* select 'where' */ FrOm users", + "select 'unterminated from\nwhere id = 1\r\n\torder by id", + "\xffselect \xc0 from \xe2\x82 where\x00true", + "ſelectſelectſelectſelect from abcdefghijklmnopqrstuvwxyz", + "select'x'from\"t\"where\"a\"='b'", + "SELECT\"col\"FROM t", + "select`a`from`b`", + } + for _, sql := range cases { + t.Run(sql, func(t *testing.T) { check(t, sql) }) + } + statements := 0 + walkSQL(t, func(path, dialect, sql string) { + t.Run(path, func(t *testing.T) { + check(t, sql) + for _, statement := range gosqlx.SplitStatements(sql, dialect) { + check(t, statement) + statements++ + } + }) + }) + t.Logf("compared %d corpus and realworld statements plus full files and edge cases", statements) +} diff --git a/pkg/linter/rules/keywords/keyword_case_oracle_test.go b/pkg/linter/rules/keywords/keyword_case_oracle_test.go new file mode 100644 index 000000000..d0b9caef3 --- /dev/null +++ b/pkg/linter/rules/keywords/keyword_case_oracle_test.go @@ -0,0 +1,167 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// The original L007 Check and tokenizer are retained as a differential oracle. +package keywords_test + +import ( + "strings" + "unicode" + + "github.com/ajitpratap0/GoSQLX/pkg/linter" + "github.com/ajitpratap0/GoSQLX/pkg/linter/rules/keywords" + "github.com/ajitpratap0/GoSQLX/pkg/models" +) + +type legacyKeywordCaseRule struct { + *keywords.KeywordCaseRule + preferredStyle keywords.CaseStyle +} + +const CaseUpper = keywords.CaseUpper + +var sqlKeywords = map[string]bool{ + "SELECT": true, "FROM": true, "WHERE": true, "AND": true, "OR": true, + "NOT": true, "IN": true, "IS": true, "NULL": true, "LIKE": true, + "BETWEEN": true, "EXISTS": true, "CASE": true, "WHEN": true, "THEN": true, + "ELSE": true, "END": true, "AS": true, "ON": true, "JOIN": true, + "INNER": true, "LEFT": true, "RIGHT": true, "FULL": true, "OUTER": true, + "CROSS": true, "NATURAL": true, "USING": true, "GROUP": true, "BY": true, + "HAVING": true, "ORDER": true, "ASC": true, "DESC": true, "LIMIT": true, + "OFFSET": true, "UNION": true, "ALL": true, "EXCEPT": true, "INTERSECT": true, + "INSERT": true, "INTO": true, "VALUES": true, "UPDATE": true, "SET": true, + "DELETE": true, "CREATE": true, "TABLE": true, "INDEX": true, "VIEW": true, + "DROP": true, "ALTER": true, "ADD": true, "COLUMN": true, "CONSTRAINT": true, + "PRIMARY": true, "KEY": true, "FOREIGN": true, "REFERENCES": true, + "UNIQUE": true, "CHECK": true, "DEFAULT": true, "CASCADE": true, + "WITH": true, "RECURSIVE": true, "DISTINCT": true, "TRUE": true, "FALSE": true, + "OVER": true, "PARTITION": true, "ROWS": true, "RANGE": true, "UNBOUNDED": true, + "PRECEDING": true, "FOLLOWING": true, "CURRENT": true, "ROW": true, + "RETURNING": true, "COALESCE": true, "NULLIF": true, "CAST": true, + "MERGE": true, "MATCHED": true, "MATERIALIZED": true, "REFRESH": true, + "ROLLUP": true, "CUBE": true, "GROUPING": true, "SETS": true, +} + +func (r *legacyKeywordCaseRule) Check(ctx *linter.Context) ([]linter.Violation, error) { + violations := []linter.Violation{} + + for lineNum, line := range ctx.Lines { + // Tokenize the line to find keywords + words := tokenizeLine(line) + + for _, word := range words { + upperWord := strings.ToUpper(word.text) + if sqlKeywords[upperWord] { + // Check if case matches preferred style + hasViolation := false + var expectedCase string + + if r.preferredStyle == CaseUpper { + hasViolation = word.text != upperWord + expectedCase = upperWord + } else { + lowerWord := strings.ToLower(word.text) + hasViolation = word.text != lowerWord + expectedCase = lowerWord + } + + if hasViolation { + violations = append(violations, linter.Violation{ + Rule: r.ID(), + RuleName: r.Name(), + Severity: r.Severity(), + Message: "Keyword '" + word.text + "' should be " + string(r.preferredStyle) + "case: '" + expectedCase + "'", + Location: models.Location{Line: lineNum + 1, Column: word.column}, + Line: line, + Suggestion: "Change '" + word.text + "' to '" + expectedCase + "'", + CanAutoFix: true, + }) + } + } + } + } + + return violations, nil +} + +// wordToken represents a word extracted from a line with its position. +type wordToken struct { + text string + column int // 1-indexed column position in the line +} + +// tokenizeLine extracts words from a line with their column positions. +// +// Parses the line character by character, extracting sequences of letters, digits, +// and underscores as words. Skips content inside string literals (both single and +// double quoted) to avoid extracting keywords from SQL string values. +// +// Returns a slice of wordTokens representing each word and its position. +func tokenizeLine(line string) []wordToken { + words := []wordToken{} + inString := false + stringChar := rune(0) + wordStart := -1 + currentWord := strings.Builder{} + + for i, ch := range line { + // Handle string literals - skip keywords inside strings + if !inString && (ch == '\'' || ch == '"') { + inString = true + stringChar = ch + if wordStart >= 0 { + words = append(words, wordToken{ + text: currentWord.String(), + column: wordStart + 1, // 1-indexed + }) + currentWord.Reset() + wordStart = -1 + } + continue + } + + if inString { + if ch == stringChar { + inString = false + stringChar = 0 + } + continue + } + + // Handle identifiers and keywords + if unicode.IsLetter(ch) || ch == '_' || (wordStart >= 0 && unicode.IsDigit(ch)) { + if wordStart < 0 { + wordStart = i + } + currentWord.WriteRune(ch) + } else if wordStart >= 0 { + words = append(words, wordToken{ + text: currentWord.String(), + column: wordStart + 1, // 1-indexed + }) + currentWord.Reset() + wordStart = -1 + } + } + + // Don't forget the last word + if wordStart >= 0 { + words = append(words, wordToken{ + text: currentWord.String(), + column: wordStart + 1, + }) + } + + return words +} diff --git a/pkg/linter/rules/keywords/keyword_case_scan_test.go b/pkg/linter/rules/keywords/keyword_case_scan_test.go new file mode 100644 index 000000000..55b1814b9 --- /dev/null +++ b/pkg/linter/rules/keywords/keyword_case_scan_test.go @@ -0,0 +1,86 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package keywords + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/linter" +) + +func TestScanWordsNoAllocations(t *testing.T) { + const line = "SELECT café, 日本語٢, abcdefghijklmnopqrstuvwxyz FROM users WHERE id = 1 -- select 'from'" + words := 0 + allocs := testing.AllocsPerRun(100, func() { + scanWords(line, func(_ string, _ int) { words++ }) + }) + if allocs != 0 { + t.Fatalf("scanWords allocated %g times per run", allocs) + } + if words == 0 { + t.Fatal("scanWords did not emit any words") + } +} + +func TestKeywordCaseCleanASCIIHasNoAllocations(t *testing.T) { + for _, style := range []CaseStyle{CaseUpper, CaseLower} { + t.Run(string(style), func(t *testing.T) { + sql := "SELECT user_id FROM users WHERE active = TRUE" + if style == CaseLower { + sql = "select user_id from users where active = true" + } + rule := NewKeywordCaseRule(style) + ctx := linter.NewContext(sql, "allocation.sql") + allocs := testing.AllocsPerRun(100, func() { + violations, err := rule.Check(ctx) + if err != nil || len(violations) != 0 { + t.Fatalf("Check: violations=%v err=%v", violations, err) + } + }) + if allocs != 0 { + t.Fatalf("clean ASCII Check allocated %g times per run", allocs) + } + }) + } +} + +func TestKeywordLengthBound(t *testing.T) { + for keyword := range sqlKeywords { + if len(keyword) > maxKeywordLen { + t.Errorf("keyword %q exceeds stack buffer bound %d", keyword, maxKeywordLen) + } + } +} + +// wordToken represents a word extracted from a line with its position. +type wordToken struct { + text string + column int // 1-indexed column position in the line +} + +// tokenizeLine extracts words from a line with their column positions. +// +// Parses the line character by character, extracting sequences of letters, digits, +// and underscores as words. Skips content inside string literals (both single and +// double quoted) to avoid extracting keywords from SQL string values. +// +// Returns a slice of wordTokens representing each word and its position. +func tokenizeLine(line string) []wordToken { + words := []wordToken{} + scanWords(line, func(text string, column int) { + words = append(words, wordToken{text: text, column: column}) + }) + return words +}