Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion USAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
- [Experimental Features](#experimental-features)
- [enable-experimental-multi-table-support](#enable-experimental-multi-table-support)
- [enable-experimental-buffered-copy](#enable-experimental-buffered-copy)
- [`move` command](#move-command)
- [native linting support](#native-linting-support)

## Getting Started

Expand Down Expand Up @@ -365,4 +367,15 @@ This feature provides a new top level binary `move`, which can copy whole schema

This command depends strongly on the experimental buffered copy and multi-table support, both which are currently experimental. There is not too much which is special to move on top of these two features, so once they become stable, so too can `move`.

It is anticipated that `move` will need to provide some pluggable method of cutover so external metadata systems can be updated. There is no current design for this.
It is anticipated that `move` will need to provide some pluggable method of cutover so external metadata systems can be updated. There is no current design for this.


### native linting support

**Feature Description**

This feature adds native linting support to Spirit, allowing for various rules to be applied to schema changes before they are executed.

**Current Status**

This feature is partially complete. It relies on new support for parsing CREATE TABLE statements (see `pkg/statetement/parse_create_table.go`). There are so far only a few linters implemented. This functionality is not currently exposed via command line flags.
20 changes: 20 additions & 0 deletions cmd/lint/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
The `lint` command is a simplistic, **experimental** interface to spirit's `lint` package.

It is not intended that this command be incorporated into your workflow at this time. The interface and
output of this command is likely to change drastically, and it's possible it will be removed entirely.

You can provide the statements via the `--statements` option in one of three ways:
1. In plaintext on the command line (`--statements="CREATE TABLE ..." --statements="ALTER TABLE ..."`)
2. Via a file or directory or glob pattern containing the statements (`--statements=file:/path/to/file.sql`)
3. Via standard input (`--statements=-`)

You can combine these however you like.

All statements provided are considered as a single group.

Because of implementation details of the `lint` package and in order to reduce the complexity of this CLI,
the `CREATE TABLE` and `ALTER TABLE` statements have to be provided in a pretty specific way:

* `CREATE TABLE` statements must be provided one-by-one. One per `--statements` argument, one per file, etc.
* `ALTER TABLE` statements can be provided one-by-one, or multiple `ALTER TABLE` statements can be provided in a single
`--statements` argument or file, separated by semicolons.
15 changes: 15 additions & 0 deletions cmd/lint/lint.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

import (
"github.com/alecthomas/kong"
"github.com/block/spirit/pkg/lint"
)

var cli struct {
lint.Lint `cmd:"" help:"Lint CREATE TABLE and ALTER TABLE statements."`
}

func main() {
ctx := kong.Parse(&cli)
ctx.FatalIfErrorf(ctx.Run())
}
303 changes: 303 additions & 0 deletions pkg/lint/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,303 @@
# Linter Framework

The `lint` package provides a framework for static analysis of MySQL schema definitions and DDL statements. It enables validation and best-practice enforcement beyond the runtime checks provided by the `check` package.

## Architecture

All built-in linters are automatically registered and enabled when the `lint` package is imported. The framework uses a flat package structure:

- **Core framework files**: `lint.go`, `linter.go`, `registry.go`, `violation.go`
- **Linter implementations**: `lint_*.go` (e.g., `lint_invisible_index.go`)

## Quick Start

### Using the Linter Framework

```go
import (
"github.com/block/spirit/pkg/lint"
)

// All built-in linters are automatically registered!
violations, err := lint.RunLinters(tables, stmts, lint.Config{})
if err != nil {
// Handle configuration errors
}

// Check for errors
if lint.HasErrors(violations) {
// Handle errors
}

// Filter violations
errors := lint.FilterBySeverity(violations, lint.SeverityError)
warnings := lint.FilterBySeverity(violations, lint.SeverityWarning)
```

### Creating a Custom Linter

Custom linters can be
1. added directly to the `lint` package (in new files with the `lint_` prefix, for consistency)
2. added to your own package and registered by blank import that relies on the `init()` function
3. added to your own code and registered explicitly using `lint.Register()`

```go
// lint_my_custom.go
package lint

import (
"github.com/block/spirit/pkg/statement"
)

// Register your linter in init()
func init() {
Register(&MyCustomLinter{})
}

// MyCustomLinter checks custom rules
type MyCustomLinter struct{}

func (l *MyCustomLinter) Name() string { return "my_custom" }
func (l *MyCustomLinter) Category() string { return "naming" }
func (l *MyCustomLinter) Description() string { return "Checks naming conventions" }
func (l *MyCustomLinter) String() string { return l.Name() }

func (l *MyCustomLinter) Lint(createTables []*statement.CreateTable, alterStatements []*statement.AbstractStatement) []Violation {
var violations []Violation

for _, ct := range createTables {
// Check table properties
if /* condition */ {
violations = append(violations, Violation{
Linter: l,
Severity: SeverityWarning,
Message: "Table name issue",
Location: &Location{
Table: ct.GetTableName(),
},
})
}
}

return violations
}
```

### Configuring Linters

#### Enabling/Disabling Linters

```go
// Disable specific linters
violations, err := lint.RunLinters(tables, stmts, lint.Config{
Enabled: map[string]bool{
"invisible_index_before_drop": false,
"primary_key_type": true,
},
})
if err != nil {
// Handle configuration errors
}
```

#### Configurable Linters

Some linters support additional configuration options via the `Settings` field. Linters that implement the `ConfigurableLinter` interface accept settings as `map[string]string`:

```go
violations, err := lint.RunLinters(tables, stmts, lint.Config{
Settings: map[string]map[string]string{
"invisible_index_before_drop": {
"raiseError": "true", // Make violations errors instead of warnings
},
},
})
if err != nil {
// Handle configuration errors (e.g., invalid settings)
}
```

Each configurable linter defines its own settings keys and values. See the individual linter documentation below for available options.

## Core Types

### Severity Levels

- **ERROR**: Will cause actual problems (data loss, inconsistency, MySQL limitations)
- **WARNING**: Best practice violations, potential issues
- **INFO**: Suggestions, style preferences

### Violation

```go
type Violation struct {
Linter Linter // The linter that produced this violation
Severity Severity // ERROR, WARNING, or INFO
Message string // Human-readable message
Location *Location // Where the violation occurred
Suggestion *string // Optional fix suggestion
Context map[string]any // Additional context
}
```

### Location

```go
type Location struct {
Table string // Table name
Column *string // Column name (if applicable)
Index *string // Index name (if applicable)
Constraint *string // Constraint name (if applicable)
}
```

## API Functions

### Registration

- `Register(l Linter)` - Register a linter (call from init())
- `Enable(name string)` - Enable a linter by name
- `Disable(name string)` - Disable a linter by name
- `List()` - Get all registered linter names
- `ListByCategory(category string)` - Get linters in a category
- `Get(name string)` - Get a linter by name

### Execution

- `RunLinters(createTables, alterStatements, config) ([]Violation, error)` - Run all enabled linters, returns violations and any configuration errors
- `HasErrors(violations)` - Check if any violations are errors
- `HasWarnings(violations)` - Check if any violations are warnings
- `FilterBySeverity(violations, severity)` - Filter by severity level
- `FilterByLinter(violations, name)` - Filter by linter name

## Built-in Linters

The `lint` package includes several linters:

### invisible_index_before_drop

**Category**: schema
**Severity**: Warning (default), Error (configurable)
**Configurable**: Yes

Requires indexes to be made invisible before dropping them as a safety measure. This ensures the index isn't needed before permanently removing it.

**Configuration Options:**

- `raiseError` (string): Set to `"true"` to make violations errors instead of warnings. Default: `false`.

**Example Usage:**

```go
// ❌ Violation
ALTER TABLE users DROP INDEX idx_email;

// ✅ Correct
ALTER TABLE users ALTER INDEX idx_email INVISIBLE;
-- Wait and monitor performance
ALTER TABLE users DROP INDEX idx_email;
```

**Configuration Example:**

```go
violations, err := lint.RunLinters(tables, stmts, lint.Config{
Settings: map[string]map[string]string{
"invisible_index_before_drop": {
"raiseError": "true", // Violations will be errors
},
},
})
```

### multiple_alter_table

**Category**: schema
**Severity**: Info

Detects multiple ALTER TABLE statements on the same table that could be combined into one for better performance and fewer table rebuilds.

```go
// ❌ Violation
ALTER TABLE users ADD COLUMN age INT;
ALTER TABLE users ADD INDEX idx_age (age);

// ✅ Better
ALTER TABLE users
ADD COLUMN age INT,
ADD INDEX idx_age (age);
```

### primary_key_type

**Category**: schema
**Severity**: Error (invalid types), Warning (signed BIGINT)

Ensures primary keys use BIGINT (preferably UNSIGNED) or BINARY/VARBINARY types.

```go
// ❌ Error - invalid type
CREATE TABLE users (
id INT PRIMARY KEY -- Should be BIGINT
);

// ⚠️ Warning - should be unsigned
CREATE TABLE users (
id BIGINT PRIMARY KEY -- Should be BIGINT UNSIGNED
);

// ✅ Correct
CREATE TABLE users (
id BIGINT UNSIGNED PRIMARY KEY
);
```

## Example Linters

The `example` package provides demonstration linters for learning purposes:

### TableNameLengthLinter

Checks that table names don't exceed MySQL's 64 character limit.

### DuplicateColumnLinter

Detects duplicate column definitions in CREATE TABLE statements.

See `pkg/lint/example/` for reference implementations.

## Contributing

When adding new linters to the `lint` package:

1. **Create a new file** with the `lint_` prefix (e.g., `lint_my_rule.go`)
2. **Implement the `Linter` interface** with all required methods
3. **Register in `init()`** function to enable automatic registration
4. **Add comprehensive tests** in a corresponding `lint_my_rule_test.go` file
5. **Document the linter** with clear comments and examples
6. **Choose appropriate severity levels**:
- `SeverityError` for violations that will cause actual problems
- `SeverityWarning` for best practice violations
- `SeverityInfo` for suggestions and style preferences
7. **Provide helpful messages** with actionable suggestions when possible
8. **Update this README** with documentation for the new linter

### File Naming Convention

- Linter implementation: `lint_<name>.go`
- Linter tests: `lint_<name>_test.go`

### Example Structure

```
pkg/lint/
├── lint.go # Core API
├── linter.go # Interface definition
├── registry.go # Registration system
├── violation.go # Violation types
├── lint_invisible_index.go # Built-in linter
├── lint_multiple_alter.go # Built-in linter
├── lint_primary_key_type.go # Built-in linter
├── lint_my_new_rule.go # Your new linter
└── lint_my_new_rule_test.go # Your tests
```
Loading
Loading