Repository: ariga/atlas
Pull request #1547 (2023-03-21 12:07:22)

sql/sqlcheck: add naming policy checker


Patch:
diff --git a/doc/md/lint/analyzers.md b/doc/md/lint/analyzers.mdx
similarity index 75%
rename from doc/md/lint/analyzers.md
rename to doc/md/lint/analyzers.mdx
index c7ff23dc..efd4ae90 100644
--- a/doc/md/lint/analyzers.md
+++ b/doc/md/lint/analyzers.mdx
@@ -2,6 +2,10 @@
 title: Migration Analyzers
 slug: /lint/analyzers
 ---
+
+import Tabs from '@theme/Tabs';
+import TabItem from '@theme/TabItem';
+
 The database is often the most critical component in software architectures. Being a stateful component, it cannot be
 easily rebuilt, scaled-out or fixed by a restart. Outages that involve damage to data or simply unavailability of the database
 are notoriously hard to manage and recover from, often taking long hours of careful work by a team's most senior
@@ -106,34 +110,97 @@ lint {
 }
 ```
 
+### Naming Conventions Policy
+
+In database schema design, maintaining consistency and readability through naming conventions is a widely common
+practice. Atlas provides an analyzer that can help enforce naming conventions on a variety of schema resources,
+including tables, columns, and indexes.
+
+Users can enable this by configuring the `naming` analyzer in their
+[`atlas.hcl`](../atlas-schema/projects#configure-migration-linting) file:
+
+<Tabs>
+<TabItem value="Global policy">
+
+```hcl title="atlas.hcl"
+lint {
+  naming {
+    match   = "^[a-z]+$"
+    message = "must be lowercase"
+  }
+}
+```
+
+</TabItem>
+<TabItem value="Resource-specific policy">
+
+```hcl title="atlas.hcl"
+lint {
+  naming {
+    match   = "^[a-z]+$"
+    message = "must be lowercase"
+    // highlight-start
+    index {
+      match   = "^[a-z]+_idx$"
+      message = "must be lowercase and end with _idx"
+    }
+    // highlight-end
+    // schema, table, column, foreign_key and check are also supported.
+  }
+}
+```
+
+</TabItem>
+</Tabs>
+
+By default, detected naming violations are reported but do not cause migration linting to fail. Users can change this by
+configuring the `naming` analyzer in the [`atlas.hcl`](../atlas-schema/projects#configure-migration-linting) file:
+
+```hcl title="atlas.hcl" {2-6}
+lint {
+  naming {
+    error   = true
+    match   = "^[a-z]+$"
+    message = "must be lowercase"
+  }
+}
+```
+
 ## Checks
 
 The following schema change checks are provided by Atlas:
 
-| **Check**                                 | **Short Description**                                                        |
-|-------------------------------------------|------------------------------------------------------------------------------|
-| [**DS1**](#destructive-changes)           | Destructive changes                                                          |
-| [DS101](#DS101)                           | Schema was dropped                                                           |
-| [DS102](#DS102)                           | Table was dropped                                                            |
-| [DS103](#DS103)                           | Non-virtual column was dropped                                               |
-| [**MF1**](#data-dependent-changes)        | Changes that might fail                                                      |
-| [MF101](#MF101)                           | Add unique index to existing column                                          |
-| [MF102](#MF102)                           | Modifying non-unique index to unique                                         |
-| [MF103](#MF103)                           | Adding a non-nullable column to an existing table                            |
-| [MF104](#MF104)                           | Modifying a nullable column to non-nullable                                  |
-| [MF101](#MF101)                           | Add unique index to existing column                                          |
-| [**BC1**](#backward-incompatible-changes) | Backward incompatible changes                                                |
-| [BC101](#BC101)                           | Renaming a table                                                             |
-| [BC102](#BC102)                           | Renaming a column                                                            |
-| **CD1**                                   | Constraint deletion changes                                                  |
-| [CD101](#CD101)                           | Foreign-key constraint was dropped                                           |
-| **MY**                                    | MySQL and MariaDB specific checks                                            |
-| [MY101](#MY101)                           | Adding a non-nullable column without a `DEFAULT` value to an existing table  |
-| [MY102](#MY102)                           | Adding a column with an inline `REFERENCES` clause has no actual effect      |
-| **LT**                                    | SQLite specific checks                                                       |
-| [LT101](#LT101)                           | Modifying a nullable column to non-nullable without a `DEFAULT` value        |
-| **AR**                                    | Atlas cloud checks                                                           |
-| [AR101](#AR101)                           | Creating table with non-optimal data alignment                               |
+| **Check**                                 | **Short Description**                                                           |
+|-------------------------------------------|---------------------------------------------------------------------------------|
+| **AR**                                    | Atlas cloud checks                                                              |
+| [AR101](#AR101)                           | Creating table with non-optimal data alignment                                  |
+| **BC**                                    | **[Backward incompatible changes](#backward-incompatible-changes)**             |
+| [BC101](#BC101)                           | Renaming a table                                                                |
+| [BC102](#BC102)                           | Renaming a column                                                               |
+| **CD**                                    | Constraint deletion changes                                                     |
+| [CD101](#CD101)                           | Foreign-key constraint was dropped                                              |
+| **DS**                                    | **[Destructive changes](#destructive-changes)**                                 |
+| [DS101](#DS101)                           | Schema was dropped                                                              |
+| [DS102](#DS102)                           | Table was dropped                                                               |
+| [DS103](#DS103)                           | Non-virtual column was dropped                                                  |
+| **LT**                                    | SQLite specific checks                                                          |
+| [LT101](#LT101)                           | Modifying a nullable column to non-nullable without a `DEFAULT` value           |
+| **MF**                                    | **[Data-dependent changes](#data-dependent-changes)** (changes that might fail) |
+| [MF101](#MF101)                           | Add unique index to existing column                                             |
+| [MF102](#MF102)                           | Modifying non-unique index to unique                                            |
+| [MF103](#MF103)                           | Adding a non-nullable column to an existing table                               |
+| [MF104](#MF104)                           | Modifying a nullable column to non-nullable                                     |
+| [MF101](#MF101)                           | Add unique index to existing column                                             |
+| **MY**                                    | MySQL and MariaDB specific checks                                               |
+| [MY101](#MY101)                           | Adding a non-nullable column without a `DEFAULT` value to an existing table     |
+| [MY102](#MY102)                           | Adding a column with an inline `REFERENCES` clause has no actual effect         |
+| **NM**                                    | **[Naming Conventions](#naming-conventions-policy)**                            |
+| [NM101](#NM101)                           | Schema name violates the naming convention                                      |
+| [NM102](#NM102)                           | Table name violates the naming convention                                       |
+| [NM103](#NM103)                           | Column name violates the naming convention                                      |
+| [NM104](#NM104)                           | Index name violates the naming convention                                       |
+| [NM105](#NM105)                           | Foreign-key constraint name violates the naming convention                      |
+| [NM106](#NM106)                           | Check constraint name violates the naming convention                            |
 
 #### DS101 {#DS101}
 
@@ -341,7 +408,7 @@ PRAGMA foreign_keys = on;
 Creating a table with optimal data alignment may help minimize the amount of required disk space.
 For example consider the next Postgres table on a 64-bit system:
 
-```postgresql
+```sql
 CREATE TABLE accounts (
     id bigint PRIMARY KEY,
     premium boolean,
@@ -356,7 +423,7 @@ In total 9 bytes of padding are allocated for each row.
 
 Compared to same table with different ordering which only takes 16 bytes in memory with 1 byte of padding:
 
-```postgresql
+```sql
 CREATE TABLE accounts (
     id bigint PRIMARY KEY,
     balance integer,
@@ -364,3 +431,21 @@ CREATE TABLE accounts (
     premium boolean
 );
 ```
+
+#### NM101 {#NM101}
+A schema has been given a name that violates the naming convention.
+
+#### NM102 {#NM102}
+A table has been given a name that violates the naming convention.
+
+#### NM103 {#NM103}
+A column has been given a name that violates the naming convention.
+
+#### NM104 {#NM104}
+An index has been given a name that violates the naming convention.
+
+#### NM105 {#NM105}
+A foreign-key constraint has been given a name that violates the naming convention.
+
+#### NM106 {#NM106}
+A check constraint has been given a name that violates the naming convention.
diff --git a/doc/md/versioned/lint.mdx b/doc/md/versioned/lint.mdx
index 4c888dba..c85dafd5 100644
--- a/doc/md/versioned/lint.mdx
+++ b/doc/md/versioned/lint.mdx
@@ -96,7 +96,7 @@ ALTER TABLE `t4` DROP COLUMN `c4`, ADD COLUMN `d4` varchar(255) NOT NULL;
 
 <TabItem value="by-check">
 
-Using `--atlas:nolint [checks...]` excludes reporting specific [checks](../lint/analyzers.md#checks) for the annotated
+Using `--atlas:nolint [checks...]` excludes reporting specific [checks](../lint/analyzers.mdx#checks) for the annotated
 statement.
 
 ```sql
@@ -111,7 +111,7 @@ ALTER TABLE `t1` DROP COLUMN `c1`, ADD COLUMN `d1` varchar(255) NOT NULL;
 <TabItem value="file-level">
 
 The `--atlas:nolint` directive can be used to exclude the whole file (given at top) from analysis, or to exclude specific
-statements in the file from specific [checks](../lint/analyzers.md#checks) or analyzers.
+statements in the file from specific [checks](../lint/analyzers.mdx#checks) or analyzers.
 
 ```sql
 //highlight-next-line
diff --git a/sql/mysql/mysqlcheck/mysqlcheck.go b/sql/mysql/mysqlcheck/mysqlcheck.go
index 998c330e..c72d9a2b 100644
--- a/sql/mysql/mysqlcheck/mysqlcheck.go
+++ b/sql/mysql/mysqlcheck/mysqlcheck.go
@@ -19,6 +19,7 @@ import (
 	"ariga.io/atlas/sql/sqlcheck/datadepend"
 	"ariga.io/atlas/sql/sqlcheck/destructive"
 	"ariga.io/atlas/sql/sqlcheck/incompatible"
+	"ariga.io/atlas/sql/sqlcheck/naming"
 )
 
 var (
@@ -216,6 +217,10 @@ func init() {
 		if err != nil {
 			return nil, err
 		}
-		return []sqlcheck.Analyzer{ds, dd, cd, bc, sqlcheck.AnalyzerFunc(inlineRefs)}, nil
+		nm, err := naming.New(r)
+		if err != nil {
+			return nil, err
+		}
+		return []sqlcheck.Analyzer{ds, dd, cd, bc, nm, sqlcheck.AnalyzerFunc(inlineRefs)}, nil
 	})
 }
diff --git a/sql/postgres/postgrescheck/postgrescheck.go b/sql/postgres/postgrescheck/postgrescheck.go
index d17f9275..e4358ea1 100644
--- a/sql/postgres/postgrescheck/postgrescheck.go
+++ b/sql/postgres/postgrescheck/postgrescheck.go
@@ -14,6 +14,7 @@ import (
 	"ariga.io/atlas/sql/sqlcheck/datadepend"
 	"ariga.io/atlas/sql/sqlcheck/destructive"
 	"ariga.io/atlas/sql/sqlcheck/incompatible"
+	"ariga.io/atlas/sql/sqlcheck/naming"
 )
 
 func addNotNull(p *datadepend.ColumnPass) (diags []sqlcheck.Diagnostic, err error) {
@@ -49,6 +50,10 @@ func init() {
 		if err != nil {
 			return nil, err
 		}
-		return []sqlcheck.Analyzer{ds, dd, cd, bc}, nil
+		nm, err := naming.New(r)
+		if err != nil {
+			return nil, err
+		}
+		return []sqlcheck.Analyzer{ds, dd, cd, bc, nm}, nil
 	})
 }
diff --git a/sql/sqlcheck/naming/naming.go b/sql/sqlcheck/naming/naming.go
new file mode 100644
index 00000000..161c1327
--- /dev/null
+++ b/sql/sqlcheck/naming/naming.go
@@ -0,0 +1,152 @@
+// 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 naming
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"regexp"
+
+	"ariga.io/atlas/schemahcl"
+	"ariga.io/atlas/sql/internal/sqlx"
+	"ariga.io/atlas/sql/schema"
+	"ariga.io/atlas/sql/sqlcheck"
+)
+
+type (
+	// Analyzer checks for resource naming.
+	Analyzer struct {
+		sqlcheck.Options
+		// Global or resource-specific options.
+		All, Schema, Table, Column,
+		Index, ForeignKey, Check Options
+	}
+	// Options for resource naming.
+	Options struct {
+		re      *regexp.Regexp
+		Match   string `spec:"match"`
+		Message string `spec:"message"`
+	}
+)
+
+// New creates a new destructive changes Analyzer with the given options.
+func New(r *schemahcl.Resource) (*Analyzer, error) {
+	az := &Analyzer{}
+	if r, ok := r.Resource(az.Name()); ok {
+		if err := r.As(&az.Options); err != nil {
+			return nil, fmt.Errorf("sql/sqlcheck: parsing naming default options: %w", err)
+		}
+		if err := readOptions(r, &az.All); err != nil {
+			return nil, err
+		}
+		for _, f := range []struct {
+			string
+			*Options
+		}{
+			{"schema", &az.Schema},
+			{"table", &az.Table},
+			{"column", &az.Column},
+			{"index", &az.Index},
+			{"foreign_key", &az.ForeignKey},
+			{"check", &az.Check},
+		} {
+			if fr, ok := r.Resource(f.string); ok {
+				if err := readOptions(fr, f.Options); err != nil {
+					return nil, err
+				}
+			}
+		}
+	}
+	return az, nil
+}
+
+func readOptions(r *schemahcl.Resource, opts *Options) error {
+	if err := r.As(opts); err != nil {
+		return fmt.Errorf("sql/sqlcheck: parsing naming options: %w", err)
+	}
+	re, err := regexp.Compile(opts.Match)
+	if err != nil {
+		return fmt.Errorf("sql/sqlcheck: parsing naming regexp: %w", err)
+	}
+	opts.re = re
+	return nil
+}
+
+// List of codes.
+var (
+	codeNameS = sqlcheck.Code("NM101")
+	codeNameT = sqlcheck.Code("NM102")
+	codeNameC = sqlcheck.Code("NM103")
+	codeNameI = sqlcheck.Code("NM104")
+	codeNameF = sqlcheck.Code("NM105")
+	codeNameK = sqlcheck.Code("NM106")
+)
+
+// Name of the analyzer. Implements the sqlcheck.NamedAnalyzer interface.
+func (*Analyzer) Name() string {
+	return "naming"
+}
+
+// Analyze implements sqlcheck.Analyzer.
+func (a *Analyzer) Analyze(_ context.Context, p *sqlcheck.Pass) error {
+	var diags []sqlcheck.Diagnostic
+	for _, sc := range p.File.Changes {
+		for _, c := range sc.Changes {
+			switch c := c.(type) {
+			case *schema.AddSchema:
+				diags = append(diags, a.match(sc, codeNameS, c.S.Name, "Schema", a.Schema)...)
+			case *schema.AddTable:
+				diags = append(diags, a.match(sc, codeNameT, c.T.Name, "Table", a.Table)...)
+			case *schema.RenameTable:
+				diags = append(diags, a.match(sc, codeNameT, c.To.Name, "Table", a.Table)...)
+			case *schema.ModifyTable:
+				for i := range c.Changes {
+					switch mc := c.Changes[i].(type) {
+					case *schema.AddColumn:
+						diags = append(diags, a.match(sc, codeNameC, mc.C.Name, "Column", a.Column)...)
+					case *schema.RenameColumn:
+						diags = append(diags, a.match(sc, codeNameC, mc.To.Name, "Column", a.Column)...)
+					case *schema.AddIndex:
+						diags = append(diags, a.match(sc, codeNameI, mc.I.Name, "Index", a.Index)...)
+					case *schema.RenameIndex:
+						diags = append(diags, a.match(sc, codeNameI, mc.To.Name, "Index", a.Index)...)
+					case *schema.AddForeignKey:
+						diags = append(diags, a.match(sc, codeNameF, mc.F.Symbol, "Foreign-key constraint", a.ForeignKey)...)
+					case *schema.AddCheck:
+						diags = append(diags, a.match(sc, codeNameK, mc.C.Name, "Check constraint", a.Check)...)
+					}
+				}
+			}
+		}
+	}
+	if len(diags) > 0 {
+		const reportText = "naming violations detected"
+		p.Reporter.WriteReport(sqlcheck.Report{Text: reportText, Diagnostics: diags})
+		if sqlx.V(a.Error) {
+			return errors.New(reportText)
+		}
+	}
+	return nil
+}
+
+func (a *Analyzer) match(c *sqlcheck.Change, code, name, resource string, opts Options) []sqlcheck.Diagnostic {
+	re, msg := opts.re, opts.Message
+	if re == nil {
+		re, msg = a.All.re, a.All.Message
+	}
+	if re == nil || re.MatchString(name) {
+		return nil
+	}
+	d := sqlcheck.Diagnostic{
+		Pos:  c.Stmt.Pos,
+		Text: fmt.Sprintf("%s named %q violates the naming policy", resource, name),
+		Code: code,
+	}
+	if msg != "" {
+		d.Text += ": " + msg
+	}
+	return []sqlcheck.Diagnostic{d}
+}
diff --git a/sql/sqlite/sqlitecheck/sqlitecheck.go b/sql/sqlite/sqlitecheck/sqlitecheck.go
index c9266604..5567a332 100644
--- a/sql/sqlite/sqlitecheck/sqlitecheck.go
+++ b/sql/sqlite/sqlitecheck/sqlitecheck.go
@@ -17,6 +17,7 @@ import (
 	"ariga.io/atlas/sql/sqlcheck/datadepend"
 	"ariga.io/atlas/sql/sqlcheck/destructive"
 	"ariga.io/atlas/sql/sqlcheck/incompatible"
+	"ariga.io/atlas/sql/sqlcheck/naming"
 	"ariga.io/atlas/sql/sqlite"
 )
 
@@ -73,6 +74,10 @@ func init() {
 		if err != nil {
 			return nil, err
 		}
+		nm, err := naming.New(r)
+		if err != nil {
+			return nil, err
+		}
 		return []sqlcheck.Analyzer{
 			sqlcheck.AnalyzerFunc(func(ctx context.Context, p *sqlcheck.Pass) error {
 				var changes []*sqlcheck.Change
@@ -115,7 +120,7 @@ func init() {
 				p.File.Changes = changes
 				return nil
 			}),
-			ds, dd, cd, bc,
+			ds, dd, cd, bc, nm,
 		}, nil
 	})
 }


Test patch:
diff --git a/internal/integration/testdata/sqlite/cli-migrate-lint-naming.txt b/internal/integration/testdata/sqlite/cli-migrate-lint-naming.txt
new file mode 100644
index 00000000..f52d6e0d
--- /dev/null
+++ b/internal/integration/testdata/sqlite/cli-migrate-lint-naming.txt
@@ -0,0 +1,27 @@
+atlas migrate lint --env local --latest=1 > got.txt
+cmp got.txt expected.txt
+
+-- atlas.hcl --
+env "local" {
+  dev = "URL"
+}
+lint {
+  naming {
+    match   = "^[a-z]+$"
+    message = "must be lowercase"
+    index {
+      match   = "^[a-z]+_idx$"
+      message = "must be lowercase and end with _idx"
+    }
+  }
+}
+-- migrations/1.sql --
+CREATE TABLE Users (id int, name varchar(255));
+CREATE INDEX users_name ON Users (name);
+
+-- expected.txt --
+1.sql: naming violations detected:
+
+	L1: Table named "Users" violates the naming policy: must be lowercase
+	L2: Index named "users_name" violates the naming policy: must be lowercase and end with _idx
+
diff --git a/sql/sqlcheck/naming/naming_test.go b/sql/sqlcheck/naming/naming_test.go
new file mode 100644
index 00000000..43cd790a
--- /dev/null
+++ b/sql/sqlcheck/naming/naming_test.go
@@ -0,0 +1,78 @@
+// 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 naming_test
+
+import (
+	"context"
+	"testing"
+
+	"ariga.io/atlas/schemahcl"
+	"ariga.io/atlas/sql/migrate"
+	"ariga.io/atlas/sql/schema"
+	"ariga.io/atlas/sql/sqlcheck"
+	"ariga.io/atlas/sql/sqlcheck/naming"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestOptions(t *testing.T) {
+	var cfg struct {
+		schemahcl.DefaultExtension
+	}
+	// language=hcl
+	err := schemahcl.New().EvalBytes([]byte(`
+naming {
+  match   = "^[a-z]+$"
+  message = "must be lowercase"
+  index {
+    match   = "^[a-z]+_idx$"
+    message = "must be lowercase and end with _idx"
+  } 
+}
+`), &cfg, nil)
+	require.NoError(t, err)
+	az, err := naming.New(cfg.Remain())
+	require.NoError(t, err)
+	var report *sqlcheck.Report
+	err = az.Analyze(context.Background(), &sqlcheck.Pass{
+		File: &sqlcheck.File{
+			Changes: []*sqlcheck.Change{
+				{
+					Changes: schema.Changes{
+						&schema.AddTable{T: schema.NewTable("Users")},
+					},
+					Stmt: &migrate.Stmt{
+						Pos:  0,
+						Text: "CREATE TABLE `Users`",
+					},
+				},
+				{
+					Changes: schema.Changes{
+						&schema.ModifyTable{
+							T: schema.NewTable("pets"),
+							Changes: schema.Changes{
+								&schema.AddIndex{
+									I: schema.NewIndex("pet_name"),
+								},
+							},
+						},
+					},
+					Stmt: &migrate.Stmt{
+						Pos:  20,
+						Text: "ALTER TABLE `pets` ADD INDEX `pet_name`",
+					},
+				},
+			},
+		},
+		Reporter: sqlcheck.ReportWriterFunc(func(r sqlcheck.Report) {
+			report = &r
+		}),
+	})
+	require.NoError(t, err)
+	require.NotNil(t, report)
+	require.Len(t, report.Diagnostics, 2)
+	require.Equal(t, `Table named "Users" violates the naming policy: must be lowercase`, report.Diagnostics[0].Text)
+	require.Equal(t, `Index named "pet_name" violates the naming policy: must be lowercase and end with _idx`, report.Diagnostics[1].Text)
+}
diff --git a/sql/sqlite/sqlitecheck/sqlitecheck_test.go b/sql/sqlite/sqlitecheck/sqlitecheck_test.go
index 5899577c..c966d4be 100644
--- a/sql/sqlite/sqlitecheck/sqlitecheck_test.go
+++ b/sql/sqlite/sqlitecheck/sqlitecheck_test.go
@@ -125,7 +125,7 @@ func TestDetectModifyTable(t *testing.T) {
 	)
 	azs, err := sqlcheck.AnalyzerFor(sqlite.DriverName, nil)
 	require.NoError(t, err)
-	require.Len(t, azs, 5)
+	require.Len(t, azs, 6)
 	require.NoError(t, azs[0].Analyze(context.Background(), pass))
 	err = azs[1].Analyze(context.Background(), pass)
 	require.EqualError(t, err, "destructive changes detected")