Warn on breaking function signatures (#49)

* Warn on a function signature change

* Download schemas in parallel

* Allow setting -1 to accept all items

* Fix nonZeroArgs order

Author: iwahbe

internal/cmd/compare.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -21,19 +22,21 @@ import (
 
 func compareCmd() *cobra.Command {
 	var provider, repository, oldCommit, newCommit string
+	var maxChanges int
 
 	command := &cobra.Command{
 		Use:   "compare",
 		Short: "Compare two versions of a Pulumi schema",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return compare(provider, repository, oldCommit, newCommit)
+			return compare(provider, repository, oldCommit, newCommit, maxChanges)
 		},
 	}
 
 	command.Flags().StringVarP(&provider, "provider", "p", "", "the provider whose schema we are comparing")
 	_ = command.MarkFlagRequired("provider")
 
-	command.Flags().StringVarP(&repository, "repository", "r", "github://api.github.com/pulumi", "the Git repository to download the schema file from")
+	command.Flags().StringVarP(&repository, "repository", "r",
+		"github://api.github.com/pulumi", "the Git repository to download the schema file from")
 	_ = command.MarkFlagRequired("provider")
 
 	command.Flags().StringVarP(&oldCommit, "old-commit", "o", "master",
@@ -43,22 +46,33 @@ func compareCmd() *cobra.Command {
 		"the new commit to compare against the old commit")
 	_ = command.MarkFlagRequired("new-commit")
 
+	command.Flags().IntVarP(&maxChanges, "max-changes", "m", 500,
+		"the maximum number of breaking changes to display. Pass -1 to display all changes")
+
 	return command
 }
 
-func compare(provider string, repository string, oldCommit string, newCommit string) error {
-	schOld, err := pkg.DownloadSchema(repository, provider, oldCommit)
-	if err != nil {
-		return err
-	}
+func compare(provider string, repository string, oldCommit string, newCommit string, maxChanges int) error {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	var schOld schema.PackageSpec
+	schOldDone := make(chan error)
+	go func() {
+		var err error
+		schOld, err = pkg.DownloadSchema(ctx, repository, provider, oldCommit)
+		if err != nil {
+			cancel()
+		}
+		schOldDone <- err
+	}()
 
 	var schNew schema.PackageSpec
-
 	if newCommit == "--local" {
 		usr, _ := user.Current()
 		basePath := fmt.Sprintf("%s/go/src/github.com/pulumi/%s", usr.HomeDir, provider)
 		schemaFile := pkg.StandardSchemaPath(provider)
 		schemaPath := filepath.Join(basePath, schemaFile)
+		var err error
 		schNew, err = pkg.LoadLocalPackageSpec(schemaPath)
 		if err != nil {
 			return err
@@ -74,12 +88,18 @@ func compare(provider string, repository string, oldCommit string, newCommit str
 			return err
 		}
 	} else {
-		schNew, err = pkg.DownloadSchema(repository, provider, newCommit)
+		var err error
+		schNew, err = pkg.DownloadSchema(ctx, repository, provider, newCommit)
 		if err != nil {
 			return err
 		}
 	}
-	compareSchemas(os.Stdout, provider, schOld, schNew)
+
+	if err := <-schOldDone; err != nil {
+		return err
+	}
+
+	compareSchemas(os.Stdout, provider, schOld, schNew, maxChanges)
 	return nil
 }
 
@@ -184,6 +204,24 @@ func breakingChanges(oldSchema, newSchema schema.PackageSpec) *diagtree.Node {
 			}
 		}
 
+		// The upstream issue is tracked at
+		// https://github.com/pulumi/pulumi/issues/13563.
+		isNonZeroArgs := func(ts *schema.ObjectTypeSpec) bool {
+			if ts == nil {
+				return false
+			}
+			return len(ts.Properties) > 0
+		}
+		type nonZeroArgs struct{ old, new bool }
+		switch (nonZeroArgs{old: isNonZeroArgs(f.Inputs), new: isNonZeroArgs(newFunc.Inputs)}) {
+		case nonZeroArgs{false, true}:
+			msg.SetDescription(diagtree.Danger,
+				"signature change (pulumi.InvokeOptions)->T => (Args, pulumi.InvokeOptions)->T")
+		case nonZeroArgs{true, false}:
+			msg.SetDescription(diagtree.Danger,
+				"signature change (Args, pulumi.InvokeOptions)->T => (pulumi.InvokeOptions)->T")
+		}
+
 		if f.Outputs != nil {
 			msg := msg.Label("outputs")
 			for propName, prop := range f.Outputs.Properties {
@@ -260,11 +298,11 @@ func breakingChanges(oldSchema, newSchema schema.PackageSpec) *diagtree.Node {
 	return msg
 }
 
-func compareSchemas(out io.Writer, provider string, oldSchema, newSchema schema.PackageSpec) {
+func compareSchemas(out io.Writer, provider string, oldSchema, newSchema schema.PackageSpec, maxChanges int) {
 	fmt.Fprintf(out, "### Does the PR have any schema changes?\n\n")
 	violations := breakingChanges(oldSchema, newSchema)
 	displayedViolations := new(bytes.Buffer)
-	lenViolations := violations.Display(displayedViolations, 500)
+	lenViolations := violations.Display(displayedViolations, maxChanges)
 	switch lenViolations {
 	case 0:
 		fmt.Fprintln(out, "Looking good! No breaking changes found.")

internal/cmd/stats.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -37,7 +38,8 @@ func statsCmd() *cobra.Command {
 }
 
 func stats(provider string, repositoryUrl string, details bool) error {
-	sch, err := pkg.DownloadSchema(repositoryUrl, provider, "master")
+	ctx := context.Background()
+	sch, err := pkg.DownloadSchema(ctx, repositoryUrl, provider, "master")
 	if err != nil {
 		return err
 	}

internal/pkg/git.go

@@ -1,6 +1,7 @@
 package pkg
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -21,7 +22,7 @@ import (
 type GitSource interface {
 	// Download fetches an io.ReadCloser for the schema file to download and also returns the size of the response (if known).
 	Download(
-		commit string,
+		ctx context.Context, commit string,
 		getHTTPResponse func(*http.Request) (io.ReadCloser, int64, error)) (io.ReadCloser, int64, error)
 }
 
@@ -75,13 +76,13 @@ func newGitlabSource(url *url.URL, name string) (*gitlabSource, error) {
 	}, nil
 }
 
-func (source *gitlabSource) newHTTPRequest(url, accept string) (*http.Request, error) {
+func (source *gitlabSource) newHTTPRequest(ctx context.Context, url, accept string) (*http.Request, error) {
 	var authorization string
 	if source.token != "" {
 		authorization = fmt.Sprintf("Bearer %s", source.token)
 	}
 
-	req, err := buildHTTPRequest(url, authorization)
+	req, err := buildHTTPRequest(ctx, url, authorization)
 	if err != nil {
 		return nil, err
 	}
@@ -90,7 +91,7 @@ func (source *gitlabSource) newHTTPRequest(url, accept string) (*http.Request, e
 }
 
 func (source *gitlabSource) Download(
-	commit string,
+	ctx context.Context, commit string,
 	getHTTPResponse func(*http.Request) (io.ReadCloser, int64, error),
 ) (io.ReadCloser, int64, error) {
 	assetName := url.QueryEscape(StandardSchemaPath(source.name))
@@ -102,7 +103,7 @@ func (source *gitlabSource) Download(
 		source.host, project, assetName, commit)
 	logging.V(1).Infof("%s downloading from %s", source.name, assetURL)
 
-	req, err := source.newHTTPRequest(assetURL, "application/octet-stream")
+	req, err := source.newHTTPRequest(ctx, assetURL, "application/octet-stream")
 	if err != nil {
 		return nil, -1, err
 	}
@@ -158,13 +159,13 @@ func newGithubSource(url *url.URL, name string) (*githubSource, error) {
 	}, nil
 }
 
-func (source *githubSource) newHTTPRequest(url, accept string) (*http.Request, error) {
+func (source *githubSource) newHTTPRequest(ctx context.Context, url, accept string) (*http.Request, error) {
 	var authorization string
 	if source.token != "" {
 		authorization = fmt.Sprintf("token %s", source.token)
 	}
 
-	req, err := buildHTTPRequest(url, authorization)
+	req, err := buildHTTPRequest(ctx, url, authorization)
 	if err != nil {
 		return nil, err
 	}
@@ -209,23 +210,23 @@ func (source *githubSource) getHTTPResponse(
 }
 
 func (source *githubSource) Download(
-	commit string,
+	ctx context.Context, commit string,
 	getHTTPResponse func(*http.Request) (io.ReadCloser, int64, error),
 ) (io.ReadCloser, int64, error) {
 	schemaURL := fmt.Sprintf(
 		"https://%s/repos/%s/%s/contents/%s?ref=%s",
 		source.host, source.organization, source.repository, StandardSchemaPath(source.name), commit)
 	logging.V(9).Infof("plugin GitHub schema url: %s", schemaURL)
 
-	req, err := source.newHTTPRequest(schemaURL, "application/vnd.github.v4.raw")
+	req, err := source.newHTTPRequest(ctx, schemaURL, "application/vnd.github.v4.raw")
 	if err != nil {
 		return nil, -1, err
 	}
 	return source.getHTTPResponse(getHTTPResponse, req)
 }
 
-func buildHTTPRequest(pluginEndpoint string, authorization string) (*http.Request, error) {
-	req, err := http.NewRequest("GET", pluginEndpoint, nil)
+func buildHTTPRequest(ctx context.Context, pluginEndpoint string, authorization string) (*http.Request, error) {
+	req, err := http.NewRequestWithContext(ctx, "GET", pluginEndpoint, nil)
 	if err != nil {
 		return nil, err
 	}

internal/pkg/util.go

@@ -1,6 +1,7 @@
 package pkg
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -10,7 +11,8 @@ import (
 	"github.com/pulumi/pulumi/pkg/v3/codegen/schema"
 )
 
-func DownloadSchema(repositoryUrl string, provider string, commit string) (schema.PackageSpec, error) {
+func DownloadSchema(ctx context.Context, repositoryUrl string,
+	provider string, commit string) (schema.PackageSpec, error) {
 	baseSource, err := func() (GitSource, error) {
 		// Support schematised URLS if the URL has a "schema" part we recognize
 		url, err := url.Parse(repositoryUrl)
@@ -31,7 +33,7 @@ func DownloadSchema(repositoryUrl string, provider string, commit string) (schem
 		return schema.PackageSpec{}, err
 	}
 
-	resp, _, err := baseSource.Download(commit, getHTTPResponse)
+	resp, _, err := baseSource.Download(ctx, commit, getHTTPResponse)
 	if err != nil {
 		return schema.PackageSpec{}, err
 	}

internal/pkg/util_test.go

@@ -1,6 +1,7 @@
 package pkg
 
 import (
+	"context"
 	"testing"
 
 	"github.com/h2non/gock"
@@ -16,7 +17,8 @@ func TestDownloadValidGithubOrg(t *testing.T) {
 		Reply(200).
 		File("schema.json")
 
-	spec, err := DownloadSchema("github://api.github.com/pulumiverse", "unifi", "main")
+	spec, err := DownloadSchema(context.Background(),
+		"github://api.github.com/pulumiverse", "unifi", "main")
 
 	assert.Nil(t, err)
 	assert.NotNil(t, spec)
@@ -33,7 +35,8 @@ func TestDownloadValidGithubRepo(t *testing.T) {
 		Reply(200).
 		File("schema.json")
 
-	spec, err := DownloadSchema("github://api.github.com/pulumiverse/pulumi-unifi", "unifi", "main")
+	spec, err := DownloadSchema(context.Background(),
+		"github://api.github.com/pulumiverse/pulumi-unifi", "unifi", "main")
 
 	assert.Nil(t, err)
 	assert.NotNil(t, spec)
@@ -49,7 +52,8 @@ func TestDownloadUnknownGithubRef(t *testing.T) {
 		MatchParam("ref", "unknown").
 		Reply(404)
 
-	_, err := DownloadSchema("github://api.github.com/pulumiverse/pulumi-unifi", "unifi", "unknown")
+	_, err := DownloadSchema(context.Background(),
+		"github://api.github.com/pulumiverse/pulumi-unifi", "unifi", "unknown")
 
 	assert.NotNil(t, err)
 	assert.Equal(t, "404 HTTP error fetching schema from https://api.github.com/repos/pulumiverse/pulumi-unifi/contents/provider/cmd/pulumi-resource-unifi/schema.json?ref=unknown. If this is a private GitHub repository, try providing a token via the GITHUB_TOKEN environment variable. See: https://github.com/settings/tokens", err.Error())
@@ -64,7 +68,7 @@ func TestDownloadValidGitlabOwner(t *testing.T) {
 		Reply(200).
 		File("schema.json")
 
-	spec, err := DownloadSchema("gitlab://gitlab.com/pulumiverse", "unifi", "main")
+	spec, err := DownloadSchema(context.Background(), "gitlab://gitlab.com/pulumiverse", "unifi", "main")
 
 	assert.Nil(t, err)
 	assert.NotNil(t, spec)
@@ -81,7 +85,7 @@ func TestDownloadValidGitlabRepo(t *testing.T) {
 		Reply(200).
 		File("schema.json")
 
-	spec, err := DownloadSchema("gitlab://gitlab.com/pulumiverse/pulumi-unifi", "unifi", "main")
+	spec, err := DownloadSchema(context.Background(), "gitlab://gitlab.com/pulumiverse/pulumi-unifi", "unifi", "main")
 
 	assert.Nil(t, err)
 	assert.NotNil(t, spec)
@@ -97,7 +101,7 @@ func TestDownloadUnknownGitlabRef(t *testing.T) {
 		MatchParam("ref", "unknown").
 		Reply(404)
 
-	_, err := DownloadSchema("gitlab://gitlab.com/pulumiverse/pulumi-unifi", "unifi", "unknown")
+	_, err := DownloadSchema(context.Background(), "gitlab://gitlab.com/pulumiverse/pulumi-unifi", "unifi", "unknown")
 
 	assert.NotNil(t, err)
 	assert.Equal(t, "404 HTTP error fetching schema from https://gitlab.com/api/v4/projects/pulumiverse%2Fpulumi-unifi/repository/files/provider%2Fcmd%2Fpulumi-resource-unifi%2Fschema.json/raw?ref=unknown", err.Error())

internal/util/diagtree/diagtree.go

@@ -78,12 +78,13 @@ type cappedWriter struct {
 
 func (c *cappedWriter) incr() {
 	if c.remaining > 0 {
+		// We never step past 0, because -1 indicates that we should always print
 		c.remaining--
 	}
 }
 
 func (c *cappedWriter) Write(p []byte) (n int, err error) {
-	if c.remaining > 0 {
+	if c.remaining > 0 || c.remaining == -1 {
 		return c.out.Write(p)
 	}
 	// We pretend we finished the write, but we do nothing.