Repository: ariga/atlas
Pull request #853 (2022-06-09 11:07:44)

sql/sqlcheck: add first basic analyzer
This is an example implementation (that I'm going to stabilize) to demonstrate how to write a driver-aware analyzer.   


Patch:
diff --git a/cmd/atlasci/internal/ci/run.go b/cmd/atlasci/internal/ci/run.go
index 5fe372bf..ef9b9b28 100644
--- a/cmd/atlasci/internal/ci/run.go
+++ b/cmd/atlasci/internal/ci/run.go
@@ -29,7 +29,7 @@ type Runner struct {
 	Analyzer sqlcheck.Analyzer
 
 	// Reporter is used to report diagnostics in the CI.
-	Reporter sqlcheck.Reporter
+	Reporter sqlcheck.ReportWriter
 }
 
 // Run executes the CI job.
@@ -56,7 +56,7 @@ func (r *Runner) Run(ctx context.Context) error {
 		return err
 	}
 	for _, f := range files {
-		if err := r.Analyzer.Analyze(ctx, &sqlcheck.Pass{File: f, Dev: r.Dev, Report: r.Reporter}); err != nil {
+		if err := r.Analyzer.Analyze(ctx, &sqlcheck.Pass{File: f, Dev: r.Dev, Reporter: r.Reporter}); err != nil {
 			return err
 		}
 	}
diff --git a/sql/sqlcheck/sqlcheck.go b/sql/sqlcheck/sqlcheck.go
index c6a1d573..06da8225 100644
--- a/sql/sqlcheck/sqlcheck.go
+++ b/sql/sqlcheck/sqlcheck.go
@@ -6,7 +6,10 @@ package sqlcheck
 
 import (
 	"context"
+	"fmt"
+	"sync"
 
+	"ariga.io/atlas/sql/internal/sqlx"
 	"ariga.io/atlas/sql/migrate"
 	"ariga.io/atlas/sql/schema"
 	"ariga.io/atlas/sql/sqlclient"
@@ -28,8 +31,8 @@ type (
 		// Dev is a driver-specific environment used to execute analysis work.
 		Dev *sqlclient.Client
 
-		// Report reports a diagnostic
-		Report Reporter
+		// Report reports a analysis reports.
+		Reporter ReportWriter
 	}
 
 	// File represents a parsed version of a migration file.
@@ -46,15 +49,21 @@ type (
 		Pos            int    // The position of the statement in the file.
 	}
 
-	// A Diagnostic is a message associated with a source location or range.
+	// A Report describes an analysis report with an optional specific diagnostic.
+	Report struct {
+		Text        string       // Report text.
+		Diagnostics []Diagnostic // Report diagnostics.
+	}
+
+	// A Diagnostic is a text associated with a specific position of a statement in a file.
 	Diagnostic struct {
-		Pos     int    // Diagnostic position.
-		Message string // Diagnostic message.
+		Pos  int    // Diagnostic position.
+		Text string // Diagnostic text.
 	}
 
-	// Reporter represents a diagnostic reporter.
-	Reporter interface {
-		Report(Diagnostic)
+	// ReportWriter represents a writer for analysis reports.
+	ReportWriter interface {
+		WriteReport(Report)
 	}
 )
 
@@ -71,13 +80,85 @@ func (a Analyzers) Analyze(ctx context.Context, p *Pass) error {
 	return nil
 }
 
-// ReporterFunc is a function that implements Reporter.
-type ReporterFunc func(Diagnostic)
+// ReportWriterFunc is a function that implements Reporter.
+type ReportWriterFunc func(Report)
+
+// WriteReport calls f(r).
+func (f ReportWriterFunc) WriteReport(r Report) {
+	f(r)
+}
+
+// NopReportWriter is a Reporter that does nothing.
+var NopReportWriter ReportWriter = ReportWriterFunc(func(Report) {})
+
+// Destructive checks destructive changes.
+var Destructive = &driverAware{
+	run: func(ctx context.Context, diags []Diagnostic, p *Pass) error {
+		for _, sc := range p.File.Changes {
+			for _, c := range sc.Changes {
+				switch c := c.(type) {
+				case *schema.DropSchema:
+					diags = append(diags, Diagnostic{
+						Pos:  sc.Pos,
+						Text: fmt.Sprintf("Dropping schema %q", c.S.Name),
+					})
+				case *schema.DropTable:
+					diags = append(diags, Diagnostic{
+						Pos:  sc.Pos,
+						Text: fmt.Sprintf("Dropping table %q", c.T.Name),
+					})
+				case *schema.ModifyTable:
+					for i := range c.Changes {
+						d, ok := c.Changes[i].(*schema.DropColumn)
+						if !ok {
+							continue
+						}
+						if g := (schema.GeneratedExpr{}); !sqlx.Has(d.C.Attrs, &g) || g.Type != "VIRTUAL" {
+							diags = append(diags, Diagnostic{
+								Pos:  sc.Pos,
+								Text: fmt.Sprintf("Dropping non-virtual column %q", d.C.Name),
+							})
+						}
+					}
+				}
+			}
+		}
+		if len(diags) > 0 {
+			p.Reporter.WriteReport(Report{
+				Text:        fmt.Sprintf("Destructive changes detected in file %q", p.File.Name()),
+				Diagnostics: diags,
+			})
+		}
+		return nil
+	},
+}
 
-// Report calls f(d).
-func (f ReporterFunc) Report(d Diagnostic) {
-	f(d)
+// driverAware is a type of analyzer that allows registering driver-level diagnostic functions.
+type driverAware struct {
+	run     func(context.Context, []Diagnostic, *Pass) error
+	mu      sync.RWMutex
+	drivers map[string]func(context.Context, *Pass) ([]Diagnostic, error)
 }
 
-// NopReporter is a Reporter that does nothing.
-var NopReporter Reporter = ReporterFunc(func(Diagnostic) {})
+// Register registers driver-level run function to extend the analyzer.
+func (a *driverAware) Register(name string, run func(context.Context, *Pass) ([]Diagnostic, error)) {
+	a.mu.Lock()
+	if a.drivers == nil {
+		a.drivers = make(map[string]func(context.Context, *Pass) ([]Diagnostic, error))
+	}
+	a.drivers[name] = run
+	a.mu.Unlock()
+}
+
+// Analyze implements the Analyzer interface.
+func (a *driverAware) Analyze(ctx context.Context, p *Pass) error {
+	var diags []Diagnostic
+	if run, ok := a.drivers[p.Dev.Name]; ok {
+		d, err := run(ctx, p)
+		if err != nil {
+			return err
+		}
+		diags = d
+	}
+	return a.run(ctx, diags, p)
+}
diff --git a/sql/sqlite/migrate.go b/sql/sqlite/migrate.go
index f5289dae..6b9b8fa8 100644
--- a/sql/sqlite/migrate.go
+++ b/sql/sqlite/migrate.go
@@ -456,7 +456,7 @@ func (s *state) tableSeq(ctx context.Context, add *schema.AddTable) error {
 	// SQLite tracks the AUTOINCREMENT in the "sqlite_sequence" table that is created and initialized automatically
 	// whenever the first "PRIMARY KEY AUTOINCREMENT" is created. However, rows in this table are populated after the
 	// first insertion to the associated table (name, seq). Therefore, we check if the sequence table and the row exist,
-	// and in case they are not, we insert a new non zero sequence to it.
+	// and in case they are not, we insert a new non-zero sequence to it.
 	rows, err := s.QueryContext(ctx, "SELECT seq FROM sqlite_sequence WHERE name = ?", add.T.Name)
 	if err != nil || !rows.Next() {
 		s.append(&migrate.Change{
@@ -467,9 +467,9 @@ func (s *state) tableSeq(ctx context.Context, add *schema.AddTable) error {
 		})
 	}
 	if rows != nil {
-		err = rows.Close()
+		return rows.Close()
 	}
-	return err
+	return nil
 }
 
 func (s *state) append(c *migrate.Change) {


Test patch:
diff --git a/cmd/atlasci/internal/ci/run_test.go b/cmd/atlasci/internal/ci/run_test.go
index 158dad3f..191e900a 100644
--- a/cmd/atlasci/internal/ci/run_test.go
+++ b/cmd/atlasci/internal/ci/run_test.go
@@ -32,7 +32,7 @@ func TestRunner_Run(t *testing.T) {
 			},
 		},
 		Analyzer: &testAnalyzer{},
-		Reporter: sqlcheck.NopReporter,
+		Reporter: sqlcheck.NopReportWriter,
 	}
 	require.NoError(t, r.Run(ctx))
 
diff --git a/internal/integration/sqlite_test.go b/internal/integration/sqlite_test.go
index 3e04ea45..670d99d0 100644
--- a/internal/integration/sqlite_test.go
+++ b/internal/integration/sqlite_test.go
@@ -207,6 +207,30 @@ func TestSQLite_AutoIncrement(t *testing.T) {
 	})
 }
 
+func TestSQLite_AutoIncrementSequence(t *testing.T) {
+	// This test shows a bug detected in Ent when working with pre-defined auto-increment start values.
+	// If there is a change somewhere to create an auto-increment with a start value, Atlas must make sure to create
+	// an entry in the 'sqlite_sequence' table (and also ensure the table exists before attempting to create the entry).
+	liteRun(t, func(t *liteTest) {
+		t1 := &schema.Table{
+			Name: "users",
+			Columns: []*schema.Column{
+				{
+					Name:  "id",
+					Type:  &schema.ColumnType{Type: &schema.IntegerType{T: "integer"}},
+					Attrs: []schema.Attr{&sqlite.AutoIncrement{Seq: 10}},
+				},
+			},
+			Attrs: []schema.Attr{&sqlite.AutoIncrement{}},
+		}
+		t1.PrimaryKey = &schema.Index{Table: t1, Parts: []*schema.IndexPart{{C: t1.Columns[0]}}}
+		t1.Columns[0].Indexes = append(t1.Columns[0].Indexes, t1.PrimaryKey)
+
+		// Planning the changes should not result in an error.
+		_ = plan(t, "col_seq", &schema.AddTable{T: t1})
+	})
+}
+
 func TestSQLite_AddColumns(t *testing.T) {
 	liteRun(t, func(t *liteTest) {
 		usersT := &schema.Table{
diff --git a/sql/sqlcheck/sqlcheck_test.go b/sql/sqlcheck/sqlcheck_test.go
new file mode 100644
index 00000000..8b5f2258
--- /dev/null
+++ b/sql/sqlcheck/sqlcheck_test.go
@@ -0,0 +1,91 @@
+// Copyright 2021-present The Atlas Authors. All rights reserved.
+// This source code is licensed under the Apache 2.0 license found
+// in the LICENSE file in the root directory of this source tree.
+
+package sqlcheck_test
+
+import (
+	"context"
+	"testing"
+
+	"ariga.io/atlas/sql/migrate"
+	"ariga.io/atlas/sql/schema"
+	"ariga.io/atlas/sql/sqlcheck"
+	"ariga.io/atlas/sql/sqlclient"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestDestructive(t *testing.T) {
+	sqlcheck.Destructive.Register("mysql", func(ctx context.Context, p *sqlcheck.Pass) (diags []sqlcheck.Diagnostic, _ error) {
+		for _, cs := range p.File.Changes {
+			for _, c := range cs.Changes {
+				_, ok := c.(*schema.ModifyTable)
+				if !ok {
+					continue
+				}
+				// A fake driver-level diagnostic.
+				diags = append(diags, sqlcheck.Diagnostic{Text: "modify table", Pos: cs.Pos})
+			}
+		}
+		return
+	})
+	var (
+		report sqlcheck.Report
+		pass   = &sqlcheck.Pass{
+			Dev: &sqlclient.Client{Name: "mysql"},
+			File: &sqlcheck.File{
+				File: testFile{name: "1.sql"},
+				Changes: []*sqlcheck.Change{
+					{
+						Stmt: "DROP DATABASE `test`",
+						Changes: schema.Changes{
+							&schema.DropSchema{S: schema.New("test")},
+						},
+					},
+					{
+						Stmt: "DROP TABLE `users`",
+						Changes: schema.Changes{
+							&schema.DropTable{T: schema.NewTable("users")},
+						},
+					},
+				},
+			},
+			Reporter: sqlcheck.ReportWriterFunc(func(r sqlcheck.Report) {
+				report = r
+			}),
+		}
+	)
+	err := sqlcheck.Destructive.Analyze(context.Background(), pass)
+	require.NoError(t, err)
+	require.Equal(t, `Destructive changes detected in file "1.sql"`, report.Text)
+	require.Len(t, report.Diagnostics, 2)
+
+	pass.File.Changes = append(pass.File.Changes, &sqlcheck.Change{
+		Stmt: "MODIFY TABLE `pets`",
+		Changes: schema.Changes{
+			&schema.ModifyTable{
+				T: schema.NewTable("pets"),
+				Changes: schema.Changes{
+					&schema.ModifyColumn{
+						Change: schema.ChangeType,
+						From:   schema.NewIntColumn("c", "int"),
+						To:     schema.NewDecimalColumn("c", "decimal"),
+					},
+				},
+			},
+		},
+	})
+	err = sqlcheck.Destructive.Analyze(context.Background(), pass)
+	require.NoError(t, err)
+	require.Len(t, report.Diagnostics, 3)
+}
+
+type testFile struct {
+	name string
+	migrate.File
+}
+
+func (t testFile) Name() string {
+	return t.name
+}