Address comments

sqlc-dev · kyleconroy · Oct 20, 2023 · Oct 19, 2023 · Oct 19, 2023 · Oct 20, 2023
commit ea2001bdc589cc3c3d4dfea7cc2ed92ab875865c
diff --git a/internal/analyzer/pb/pb.pb.go → internal/analysis/analysis.pb.go b/internal/analyzer/pb/pb.pb.go → internal/analysis/analysis.pb.go
diff --git a/internal/analyzer/pb/pb_vtproto.pb.go → internal/analysis/analysis_vtproto.pb.go b/internal/analyzer/pb/pb_vtproto.pb.go → internal/analysis/analysis_vtproto.pb.go
diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go
@@ -2,14 +2,16 @@ package analyzer
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"hash/fnv"
 	"os"
 	"path/filepath"
 
+	"golang.org/x/exp/slog"
 	"google.golang.org/protobuf/proto"
 
-	"github.com/sqlc-dev/sqlc/internal/analyzer/pb"
+	"github.com/sqlc-dev/sqlc/internal/analysis"
 	"github.com/sqlc-dev/sqlc/internal/cache"
 	"github.com/sqlc-dev/sqlc/internal/config"
 	"github.com/sqlc-dev/sqlc/internal/info"
@@ -18,43 +20,56 @@ import (
 )
 
 type CachedAnalyzer struct {
-	a          Analyzer
-	configPath string
-	config     []byte
-	db         config.Database
+	a           Analyzer
+	config      config.Config
+	configBytes []byte
+	db          config.Database
 }
 
-func Cached(a Analyzer, cp string, db config.Database) *CachedAnalyzer {
+func Cached(a Analyzer, c config.Config, db config.Database) *CachedAnalyzer {
 	return &CachedAnalyzer{
-		a:          a,
-		configPath: cp,
-		db:         db,
+		a:      a,
+		config: c,
+		db:     db,
 	}
 }
 
-func (c *CachedAnalyzer) Analyze(ctx context.Context, n ast.Node, q string, schema []string, np *named.ParamSet) (*pb.Analysis, error) {
+// Create a new error here
+
+func (c *CachedAnalyzer) Analyze(ctx context.Context, n ast.Node, q string, schema []string, np *named.ParamSet) (*analysis.Analysis, error) {
+	result, rerun, err := c.analyze(ctx, n, q, schema, np)
+	if rerun {
+		if err != nil {
+			slog.Warn("first analysis failed with error", "err", err)
+		}
+		return c.a.Analyze(ctx, n, q, schema, np)
+	}
+	return result, err
+}
+
+func (c *CachedAnalyzer) analyze(ctx context.Context, n ast.Node, q string, schema []string, np *named.ParamSet) (*analysis.Analysis, bool, error) {
 	// Only cache queries for managed databases. We can't be certain the the
 	// database is in an unchanged state otherwise
 	if !c.db.Managed {
-		return c.a.Analyze(ctx, n, q, schema, np)
+		return nil, true, nil
 	}
 
 	dir, err := cache.AnalysisDir()
 	if err != nil {
-		return nil, err
+		return nil, true, err
 	}
 
-	if c.config == nil {
-		c.config, err = os.ReadFile(c.configPath)
+	if c.configBytes == nil {
+		c.configBytes, err = json.Marshal(c.config)
 		if err != nil {
-			return nil, err
+			return nil, true, err
 		}
 	}
 
 	// Calculate cache key
 	h := fnv.New64()
 	h.Write([]byte(info.Version))
-	h.Write(c.config)
+	h.Write(c.configBytes)
 	for _, m := range schema {
 		h.Write([]byte(m))
 	}
@@ -65,32 +80,37 @@ func (c *CachedAnalyzer) Analyze(ctx context.Context, n ast.Node, q string, sche
 	if _, err := os.Stat(path); err == nil {
 		contents, err := os.ReadFile(path)
 		if err != nil {
-			return nil, err
+			return nil, true, err
 		}
-		var a pb.Analysis
-		return &a, proto.Unmarshal(contents, &a)
+		var a analysis.Analysis
+		if err := proto.Unmarshal(contents, &a); err != nil {
+			return nil, true, err
+		}
+		return &a, false, nil
 	}
 
 	result, err := c.a.Analyze(ctx, n, q, schema, np)
 
 	if err == nil {
 		contents, err := proto.Marshal(result)
 		if err != nil {
-			return nil, err
+			slog.Warn("unable to marshal analysis", "err", err)
+			return result, false, nil
 		}
 		if err := os.WriteFile(path, contents, 0644); err != nil {
-			return nil, err
+			slog.Warn("saving analysis to disk failed", "err", err)
+			return result, false, nil
 		}
 	}
 
-	return result, err
+	return result, false, err
 }
 
 func (c *CachedAnalyzer) Close(ctx context.Context) error {
 	return c.a.Close(ctx)
 }
 
 type Analyzer interface {
-	Analyze(context.Context, ast.Node, string, []string, *named.ParamSet) (*pb.Analysis, error)
+	Analyze(context.Context, ast.Node, string, []string, *named.ParamSet) (*analysis.Analysis, error)
 	Close(context.Context) error
 }
diff --git a/internal/cache/cache.go b/internal/cache/cache.go
@@ -6,22 +6,18 @@ import (
 	"path/filepath"
 )
 
-// The cache directory defaults to ~/.cache/sqlc. This location can be overriden
-// by the SQLCCACHE or XDG_CACHE_HOME environment variable.
+// The cache directory defaults to os.UserCacheDir(). This location can be
+// overridden by the SQLCCACHE environment variable.
 //
 // Currently the cache stores two types of data: plugins and query analysis
 func Dir() (string, error) {
 	cache := os.Getenv("SQLCCACHE")
 	if cache != "" {
 		return cache, nil
 	}
-	cacheHome := os.Getenv("XDG_CACHE_HOME")
-	if cacheHome == "" {
-		home, err := os.UserHomeDir()
-		if err != nil {
-			return "", err
-		}
-		cacheHome = filepath.Join(home, ".cache")
+	cacheHome, err := os.UserCacheDir()
+	if err != nil {
+		return "", err
 	}
 	return filepath.Join(cacheHome, "sqlc"), nil
 }

diff --git a/internal/cmd/generate.go b/internal/cmd/generate.go
@@ -199,7 +199,7 @@ func Generate(ctx context.Context, dir, filename string, o *Options) (map[string
 		errout := &stderrs[i]
 
 		grp.Go(func() error {
-			combo := config.Combine(configPath, *conf, sql.SQL)
+			combo := config.Combine(*conf, sql.SQL)
 			if sql.Plugin != nil {
 				combo.Codegen = *sql.Plugin
 			}

diff --git a/internal/cmd/vet.go b/internal/cmd/vet.go
@@ -461,7 +461,7 @@ func (c *checker) DSN(dsn string) (string, error) {
 
 func (c *checker) checkSQL(ctx context.Context, s config.SQL) error {
 	// TODO: Create a separate function for this logic so we can
-	combo := config.Combine(c.ConfigPath, *c.Conf, s)
+	combo := config.Combine(*c.Conf, s)
 
 	// TODO: This feels like a hack that will bite us later
 	joined := make([]string, 0, len(s.Schema))

diff --git a/internal/compiler/analyze.go b/internal/compiler/analyze.go
@@ -3,7 +3,7 @@ package compiler
 import (
 	"sort"
 
-	analyzer "github.com/sqlc-dev/sqlc/internal/analyzer/pb"
+	analyzer "github.com/sqlc-dev/sqlc/internal/analysis"
 	"github.com/sqlc-dev/sqlc/internal/config"
 	"github.com/sqlc-dev/sqlc/internal/source"
 	"github.com/sqlc-dev/sqlc/internal/sql/ast"

diff --git a/internal/compiler/engine.go b/internal/compiler/engine.go
@@ -53,7 +53,7 @@ func NewCompiler(conf config.SQL, combo config.CombinedSettings) (*Compiler, err
 			if conf.Analyzer.Database == nil || *conf.Analyzer.Database {
 				c.analyzer = analyzer.Cached(
 					pganalyze.New(c.client, *conf.Database),
-					combo.Path,
+					combo.Global,
 					*conf.Database,
 				)
 			}

diff --git a/internal/config/config.go b/internal/config/config.go
@@ -240,7 +240,6 @@ func ParseConfig(rd io.Reader) (Config, error) {
 }
 
 type CombinedSettings struct {
-	Path      string
 	Global    Config
 	Package   SQL
 	Go        SQLGo
@@ -252,9 +251,8 @@ type CombinedSettings struct {
 	Codegen Codegen
 }
 
-func Combine(path string, conf Config, pkg SQL) CombinedSettings {
+func Combine(conf Config, pkg SQL) CombinedSettings {
 	cs := CombinedSettings{
-		Path:    path,
 		Global:  conf,
 		Package: pkg,
 		Rename:  map[string]string{},

diff --git a/internal/engine/postgresql/analyzer/analyze.go b/internal/engine/postgresql/analyzer/analyze.go
@@ -11,7 +11,7 @@ import (
 	"github.com/jackc/pgx/v5/pgconn"
 	"github.com/jackc/pgx/v5/pgxpool"
 
-	core "github.com/sqlc-dev/sqlc/internal/analyzer/pb"
+	core "github.com/sqlc-dev/sqlc/internal/analysis"
 	"github.com/sqlc-dev/sqlc/internal/config"
 	pb "github.com/sqlc-dev/sqlc/internal/quickdb/v1"
 	"github.com/sqlc-dev/sqlc/internal/sql/ast"

diff --git a/protos/analyzer/pb/pb.proto → protos/analysis/analysis.proto b/protos/analyzer/pb/pb.proto → protos/analysis/analysis.proto
@@ -1,8 +1,8 @@
 syntax = "proto3";
 
-package analyzer.pb;
+package analysis;
 
-option go_package = "github.com/sqlc-dev/sqlc/internal/analyzer/pb";
+option go_package = "github.com/sqlc-dev/sqlc/internal/analysis";
 
 message Identifier {
 	string catalog = 1;