snapshot of https://github.com/mattermost/mattermost.git (master, 2026-09-16; blobs > 131072B omitted)

tools/mattermost-govet/concurrentIndex/concurrentIndex.go

@@ -0,0 +1,201 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package concurrentIndex
+
+import (
+	"go/ast"
+	"go/token"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strconv"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+const (
+	diagCreateIndex = "use CREATE INDEX CONCURRENTLY instead of CREATE INDEX to avoid blocking DML"
+	diagDropIndex   = "use DROP INDEX CONCURRENTLY instead of DROP INDEX to avoid blocking DML"
+)
+
+var (
+	createIndexRegex      = regexp.MustCompile(`(?i)\bCREATE\s+(UNIQUE\s+)?INDEX\b`)
+	concurrentlyRegex     = regexp.MustCompile(`(?i)\bCREATE\s+(UNIQUE\s+)?INDEX\s+CONCURRENTLY\b`)
+	dropIndexRegex        = regexp.MustCompile(`(?i)\bDROP\s+INDEX\b`)
+	dropConcurrentlyRegex = regexp.MustCompile(`(?i)\bDROP\s+INDEX\s+CONCURRENTLY\b`)
+)
+
+var sqlPath string
+var minMigration int
+
+var migrationNumberRegex = regexp.MustCompile(`^\d+`)
+
+var Analyzer = &analysis.Analyzer{
+	Name: "concurrentIndex",
+	Doc:  "checks that CREATE INDEX and DROP INDEX use CONCURRENTLY to avoid blocking DML",
+	Run:  run,
+}
+
+func init() {
+	Analyzer.Flags.StringVar(&sqlPath, "path", "", "Relative path to a directory of .sql files to scan recursively")
+	Analyzer.Flags.IntVar(&minMigration, "minMigration", 0, "Skip SQL files with a migration number below this value")
+}
+
+func checkStatement(stmt string) string {
+	if createIndexRegex.MatchString(stmt) && !concurrentlyRegex.MatchString(stmt) {
+		return diagCreateIndex
+	}
+	if dropIndexRegex.MatchString(stmt) && !dropConcurrentlyRegex.MatchString(stmt) {
+		return diagDropIndex
+	}
+	return ""
+}
+
+func containsIndexKeyword(s string) bool {
+	return strings.Contains(strings.ToLower(s), "index")
+}
+
+func checkLine(line string) string {
+	if !containsIndexKeyword(line) {
+		return ""
+	}
+	for _, stmt := range strings.Split(line, ";") {
+		stmt = strings.TrimSpace(stmt)
+		if stmt == "" {
+			continue
+		}
+		if msg := checkStatement(stmt); msg != "" {
+			return msg
+		}
+	}
+	return ""
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	for _, file := range pass.Files {
+		if strings.HasSuffix(pass.Fset.File(file.Pos()).Name(), "_test.go") {
+			continue
+		}
+		ast.Inspect(file, func(node ast.Node) bool {
+			lit, ok := node.(*ast.BasicLit)
+			if !ok || lit.Kind != token.STRING {
+				return true
+			}
+
+			val, err := strconv.Unquote(lit.Value)
+			if err != nil {
+				val = strings.Trim(lit.Value, "`")
+			}
+
+			if msg := checkLine(val); msg != "" {
+				pass.Reportf(lit.Pos(), "%s", msg)
+			}
+			return true
+		})
+	}
+
+	if sqlPath != "" {
+		if err := scanSQLDir(pass, sqlPath); err != nil {
+			return nil, err
+		}
+	}
+
+	return nil, nil
+}
+
+func getMigrationNumberFromFilename(filename string) int {
+	m := migrationNumberRegex.FindString(filename)
+	if m == "" {
+		return -1
+	}
+	n, _ := strconv.Atoi(m)
+	return n
+}
+
+func scanSQLDir(pass *analysis.Pass, root string) error {
+	return filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			if os.IsNotExist(err) {
+				return nil
+			}
+			return err
+		}
+		if d.IsDir() || !strings.HasSuffix(path, ".sql") {
+			return nil
+		}
+		if minMigration > 0 {
+			if n := getMigrationNumberFromFilename(d.Name()); n >= 0 && n < minMigration {
+				return nil
+			}
+		}
+		return checkSQLFile(pass, path)
+	})
+}
+
+func checkSQLFile(pass *analysis.Pass, name string) error {
+	content, err := os.ReadFile(name)
+	if err != nil {
+		return err
+	}
+
+	if !containsIndexKeyword(string(content)) {
+		return nil
+	}
+
+	tf := pass.Fset.AddFile(name, -1, len(content))
+	tf.SetLinesForContent(content)
+
+	lines := strings.Split(string(content), "\n")
+	var stmtBuf strings.Builder
+	startLine := 1
+
+	for i, line := range lines {
+		lineNum := i + 1
+		trimmed := strings.TrimSpace(line)
+
+		if trimmed == "" {
+			continue
+		}
+
+		if stmtBuf.Len() == 0 {
+			startLine = lineNum
+		}
+		if stmtBuf.Len() > 0 {
+			stmtBuf.WriteByte(' ')
+		}
+		stmtBuf.WriteString(trimmed)
+
+		for {
+			current := stmtBuf.String()
+			semi := strings.Index(current, ";")
+			if semi < 0 {
+				break
+			}
+
+			stmt := strings.TrimSpace(current[:semi])
+			if stmt != "" {
+				if msg := checkStatement(stmt); msg != "" {
+					pass.Reportf(tf.LineStart(startLine), "%s", msg)
+				}
+			}
+
+			rest := strings.TrimSpace(current[semi+1:])
+			stmtBuf.Reset()
+			if rest != "" {
+				stmtBuf.WriteString(rest)
+				startLine = lineNum
+			}
+		}
+	}
+
+	if stmtBuf.Len() > 0 {
+		if msg := checkStatement(stmtBuf.String()); msg != "" {
+			pass.Reportf(tf.LineStart(startLine), "%s", msg)
+		}
+	}
+
+	return nil
+}

tools/mattermost-govet/concurrentIndex/concurrentIndex_test.go

@@ -0,0 +1,208 @@
+package concurrentIndex
+
+import (
+	"go/token"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestGoFiles(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, Analyzer, "a")
+}
+
+func TestSQLDir(t *testing.T) {
+	testdata := analysistest.TestData()
+	migrationsDir := filepath.Join(testdata, "migrations")
+
+	fset := token.NewFileSet()
+	var diags []analysis.Diagnostic
+	pass := &analysis.Pass{
+		Analyzer: Analyzer,
+		Fset:     fset,
+		Report: func(d analysis.Diagnostic) {
+			diags = append(diags, d)
+		},
+	}
+
+	err := scanSQLDir(pass, migrationsDir)
+	require.NoError(t, err)
+
+	var files []string
+	for _, d := range diags {
+		pos := fset.Position(d.Pos)
+		files = append(files, filepath.Base(pos.Filename)+":"+d.Message)
+	}
+
+	createCount := 0
+	dropCount := 0
+	for _, f := range files {
+		switch {
+		case strings.Contains(f, diagCreateIndex):
+			createCount++
+		case strings.Contains(f, diagDropIndex):
+			dropCount++
+		}
+	}
+	assert.Equal(t, 4, createCount, "expected 4 CREATE INDEX diagnostics")
+	assert.Equal(t, 2, dropCount, "expected 2 DROP INDEX diagnostics")
+
+	badCount := 0
+	nestedCount := 0
+	multilineCount := 0
+	multistmtCount := 0
+	for _, f := range files {
+		switch {
+		case strings.HasPrefix(f, "000002_bad"):
+			badCount++
+		case strings.HasPrefix(f, "000003_nested"):
+			nestedCount++
+		case strings.HasPrefix(f, "000004_multiline"):
+			multilineCount++
+		case strings.HasPrefix(f, "000005_multistmt"):
+			multistmtCount++
+		}
+	}
+	assert.Equal(t, 3, badCount, "expected 3 diagnostics from 000002_bad.up.sql")
+	assert.Equal(t, 1, nestedCount, "expected 1 diagnostic from sub/000003_nested.up.sql")
+	assert.Equal(t, 1, multilineCount, "expected 1 diagnostic from 000004_multiline.up.sql")
+	assert.Equal(t, 1, multistmtCount, "expected 1 diagnostic from 000005_multistmt.up.sql")
+
+	for _, f := range files {
+		assert.NotContains(t, f, "000001_good",
+			"good migration file should not have diagnostics")
+	}
+}
+
+func TestSQLDirBadPath(t *testing.T) {
+	fset := token.NewFileSet()
+	pass := &analysis.Pass{
+		Analyzer: Analyzer,
+		Fset:     fset,
+		Report:   func(d analysis.Diagnostic) {},
+	}
+
+	err := scanSQLDir(pass, "/nonexistent/path")
+	assert.NoError(t, err)
+}
+
+func TestSQLDirMinMigration(t *testing.T) {
+	testdata := analysistest.TestData()
+	migrationsDir := filepath.Join(testdata, "migrations")
+
+	old := minMigration
+	t.Cleanup(func() { minMigration = old })
+	minMigration = 5
+
+	fset := token.NewFileSet()
+	var diags []analysis.Diagnostic
+	pass := &analysis.Pass{
+		Analyzer: Analyzer,
+		Fset:     fset,
+		Report: func(d analysis.Diagnostic) {
+			diags = append(diags, d)
+		},
+	}
+
+	err := scanSQLDir(pass, migrationsDir)
+	require.NoError(t, err)
+
+	for _, d := range diags {
+		pos := fset.Position(d.Pos)
+		base := filepath.Base(pos.Filename)
+		n := getMigrationNumberFromFilename(base)
+		require.GreaterOrEqual(t, n, 0, "expected numeric prefix in %s", base)
+		assert.GreaterOrEqual(t, n, 5,
+			"expected only migrations >= 5, got diagnostic from %s", base)
+	}
+	assert.Equal(t, 1, len(diags), "expected 1 diagnostic from migration 5+")
+}
+
+func TestCheckLine(t *testing.T) {
+	tests := []struct {
+		name    string
+		line    string
+		wantMsg string
+	}{
+		{
+			name:    "CREATE INDEX without CONCURRENTLY",
+			line:    "CREATE INDEX IF NOT EXISTS idx_foo_bar ON foo (bar);",
+			wantMsg: diagCreateIndex,
+		},
+		{
+			name:    "CREATE UNIQUE INDEX without CONCURRENTLY",
+			line:    "CREATE UNIQUE INDEX IF NOT EXISTS idx_foo_bar ON foo (bar);",
+			wantMsg: diagCreateIndex,
+		},
+		{
+			name:    "DROP INDEX without CONCURRENTLY",
+			line:    "DROP INDEX IF EXISTS idx_foo_bar;",
+			wantMsg: diagDropIndex,
+		},
+		{
+			name:    "lowercase create index",
+			line:    "create index if not exists idx_foo on foo (bar);",
+			wantMsg: diagCreateIndex,
+		},
+		{
+			name:    "lowercase drop index",
+			line:    "drop index if exists idx_foo;",
+			wantMsg: diagDropIndex,
+		},
+		{
+			name: "CREATE INDEX CONCURRENTLY is fine",
+			line: "CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_foo_bar ON foo (bar);",
+		},
+		{
+			name: "CREATE UNIQUE INDEX CONCURRENTLY is fine",
+			line: "CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS idx_foo_bar ON foo (bar);",
+		},
+		{
+			name: "DROP INDEX CONCURRENTLY is fine",
+			line: "DROP INDEX CONCURRENTLY IF EXISTS idx_foo_bar;",
+		},
+		{
+			name: "unrelated SQL",
+			line: "CREATE TABLE foo (id int);",
+		},
+		{
+			name: "SELECT statement",
+			line: "SELECT * FROM indexes;",
+		},
+		{
+			name: "plain string",
+			line: "just a regular string",
+		},
+		{
+			name:    "multi-statement with unsafe and safe",
+			line:    "CREATE INDEX idx_a ON t (c); CREATE INDEX CONCURRENTLY idx_b ON t (c);",
+			wantMsg: diagCreateIndex,
+		},
+		{
+			name:    "multi-statement with safe then unsafe drop",
+			line:    "DROP INDEX CONCURRENTLY IF EXISTS idx_a; DROP INDEX IF EXISTS idx_b;",
+			wantMsg: diagDropIndex,
+		},
+		{
+			name: "multi-statement all safe",
+			line: "CREATE INDEX CONCURRENTLY idx_a ON t (c); DROP INDEX CONCURRENTLY IF EXISTS idx_b;",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			msg := checkLine(tt.line)
+			if tt.wantMsg == "" {
+				assert.Empty(t, msg)
+			} else {
+				assert.Equal(t, tt.wantMsg, msg)
+			}
+		})
+	}
+}

tools/mattermost-govet/concurrentIndex/testdata/migrations/000001_good.up.sql

@@ -0,0 +1,3 @@
+CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_foo_bar ON foo (bar);
+CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS idx_foo_baz ON foo (baz);
+DROP INDEX CONCURRENTLY IF EXISTS idx_foo_old;

tools/mattermost-govet/concurrentIndex/testdata/migrations/000002_bad.up.sql

@@ -0,0 +1,3 @@
+CREATE INDEX IF NOT EXISTS idx_foo_bar ON foo (bar);
+CREATE UNIQUE INDEX IF NOT EXISTS idx_foo_baz ON foo (baz);
+DROP INDEX IF EXISTS idx_foo_old;

tools/mattermost-govet/concurrentIndex/testdata/migrations/000004_multiline.up.sql

@@ -0,0 +1,4 @@
+CREATE INDEX
+    IF NOT EXISTS idx_multiline
+    ON foo (bar);
+DROP INDEX CONCURRENTLY IF EXISTS idx_multiline_ok;

tools/mattermost-govet/concurrentIndex/testdata/migrations/000005_multistmt.up.sql

@@ -0,0 +1 @@
+CREATE INDEX CONCURRENTLY idx_safe ON t (c); CREATE INDEX idx_unsafe ON t (d);

tools/mattermost-govet/concurrentIndex/testdata/migrations/sub/000003_nested.up.sql

@@ -0,0 +1,2 @@
+DROP INDEX idx_nested;
+CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_nested_ok ON t (c);

tools/mattermost-govet/concurrentIndex/testdata/src/a/a.go

@@ -0,0 +1,39 @@
+package a
+
+func example() {
+	// CREATE INDEX without CONCURRENTLY should trigger warnings
+	_ = "CREATE INDEX IF NOT EXISTS idx_foo_bar ON foo (bar)"         // want `use CREATE INDEX CONCURRENTLY instead of CREATE INDEX to avoid blocking DML`
+	_ = "CREATE INDEX idx_foo_bar ON foo (bar)"                       // want `use CREATE INDEX CONCURRENTLY instead of CREATE INDEX to avoid blocking DML`
+	_ = "create index if not exists idx_foo_bar on foo (bar)"         // want `use CREATE INDEX CONCURRENTLY instead of CREATE INDEX to avoid blocking DML`
+	_ = "CREATE UNIQUE INDEX IF NOT EXISTS idx_foo_bar ON foo (bar)"  // want `use CREATE INDEX CONCURRENTLY instead of CREATE INDEX to avoid blocking DML`
+	_ = "create unique index idx_foo_bar on foo (bar)"                // want `use CREATE INDEX CONCURRENTLY instead of CREATE INDEX to avoid blocking DML`
+
+	// DROP INDEX without CONCURRENTLY should trigger warnings
+	_ = "DROP INDEX IF EXISTS idx_foo_bar"  // want `use DROP INDEX CONCURRENTLY instead of DROP INDEX to avoid blocking DML`
+	_ = "DROP INDEX idx_foo_bar"            // want `use DROP INDEX CONCURRENTLY instead of DROP INDEX to avoid blocking DML`
+	_ = "drop index if exists idx_foo_bar"  // want `use DROP INDEX CONCURRENTLY instead of DROP INDEX to avoid blocking DML`
+
+	// CREATE INDEX CONCURRENTLY should not trigger warnings
+	_ = "CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_foo_bar ON foo (bar)"
+	_ = "CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS idx_foo_bar ON foo (bar)"
+	_ = "create index concurrently if not exists idx_foo_bar on foo (bar)"
+	_ = "create unique index concurrently idx_foo_bar on foo (bar)"
+
+	// DROP INDEX CONCURRENTLY should not trigger warnings
+	_ = "DROP INDEX CONCURRENTLY IF EXISTS idx_foo_bar"
+	_ = "drop index concurrently if exists idx_foo_bar"
+
+	// Unrelated strings should not trigger warnings
+	_ = "SELECT * FROM indexes"
+	_ = "just a regular string"
+	_ = "CREATE TABLE foo (id int)"
+}
+
+func exampleBacktick() {
+	// Backtick strings should also be checked
+	_ = `CREATE INDEX IF NOT EXISTS idx_foo_bar ON foo (bar)`        // want `use CREATE INDEX CONCURRENTLY instead of CREATE INDEX to avoid blocking DML`
+	_ = `DROP INDEX IF EXISTS idx_foo_bar`                           // want `use DROP INDEX CONCURRENTLY instead of DROP INDEX to avoid blocking DML`
+	_ = `CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_foo_bar ON foo (bar)`
+	_ = `DROP INDEX CONCURRENTLY IF EXISTS idx_foo_bar`
+
+}