Update buf breaking checks to support extensions (#2960)

This is really two non-trivial changes
1. Make the existing field checks (other than field deletion) support extensions.
2. Add new checks to cover deletion of extension fields.

CHANGELOG.md

@@ -33,6 +33,13 @@
     2. `FIELD_SAME_UTF8_VALIDATION` catches changes to the `utf8_validation`
        feature, which controls validation of string values.
     3. `ENUM_SAME_TYPE` catches changes to an enum's type, open vs. closed.
+- Adds support for extensions to `buf breaking`. All existing rules for
+  fields are now applied to extensions, except for `FIELD_NO_DELETE` (and its
+  variants). There are also new `EXTENSION_NO_DELETE` and
+  `PACKAGE_EXTENSION_NO_DELETE` rules for catching deletions of an extension.
+  The new rules are not active by default in existing v1 and v1beta1
+  configurations, for backwards-compatibility reasons. Migrate your config to
+  v2 to use them.
 - Adds support for top-level extensions to `buf lint`. It previously only
   checked extensions that were defined inside of messages.
 - Adds a new `FIELD_NOT_REQUIRED` lint rule that prevents use of required

private/buf/cmd/buf/buf_test.go

@@ -438,7 +438,7 @@ func TestFailCheckBreaking2(t *testing.T) {
 		t,
 		nil,
 		bufctl.ExitCodeFileAnnotation,
-		filepath.FromSlash(`testdata/protofileref/breaking/a/foo.proto:7:3:Field "2" on message "Foo" changed type from "int32" to "string".`),
+		filepath.FromSlash(`testdata/protofileref/breaking/a/foo.proto:7:3:Field "2" with name "world" on message "Foo" changed type from "int32" to "string".`),
 		"breaking",
 		filepath.Join("testdata", "protofileref", "breaking", "a", "foo.proto"),
 		"--against",
@@ -454,7 +454,7 @@ func TestFailCheckBreaking3(t *testing.T) {
 		bufctl.ExitCodeFileAnnotation,
 		filepath.FromSlash(`
 		<input>:1:1:Previously present file "bar.proto" was deleted.
-		testdata/protofileref/breaking/a/foo.proto:7:3:Field "2" on message "Foo" changed type from "int32" to "string".
+		testdata/protofileref/breaking/a/foo.proto:7:3:Field "2" with name "world" on message "Foo" changed type from "int32" to "string".
 		`),
 		"breaking",
 		filepath.Join("testdata", "protofileref", "breaking", "a", "foo.proto"),
@@ -471,7 +471,7 @@ func TestFailCheckBreaking4(t *testing.T) {
 		bufctl.ExitCodeFileAnnotation,
 		filepath.FromSlash(`
 		testdata/protofileref/breaking/a/bar.proto:5:1:Previously present field "2" with name "value" on message "Bar" was deleted.
-		testdata/protofileref/breaking/a/foo.proto:7:3:Field "2" on message "Foo" changed type from "int32" to "string".
+		testdata/protofileref/breaking/a/foo.proto:7:3:Field "2" with name "world" on message "Foo" changed type from "int32" to "string".
 		`),
 		"breaking",
 		fmt.Sprintf("%s#include_package_files=true", filepath.Join("testdata", "protofileref", "breaking", "a", "foo.proto")),
@@ -488,7 +488,7 @@ func TestFailCheckBreaking5(t *testing.T) {
 		bufctl.ExitCodeFileAnnotation,
 		filepath.FromSlash(`
     <input>:1:1:Previously present file "bar.proto" was deleted.
-		testdata/protofileref/breaking/a/foo.proto:7:3:Field "2" on message "Foo" changed type from "int32" to "string".
+		testdata/protofileref/breaking/a/foo.proto:7:3:Field "2" with name "world" on message "Foo" changed type from "int32" to "string".
 		`),
 		"breaking",
 		filepath.Join("testdata", "protofileref", "breaking", "a", "foo.proto"),
@@ -854,6 +854,7 @@ func TestCheckLsBreakingRulesV2(t *testing.T) {
 	expectedStdout := `
 ID                                              CATEGORIES                      PURPOSE
 ENUM_NO_DELETE                                  FILE                            Checks that enums are not deleted from a given file.
+EXTENSION_NO_DELETE                             FILE                            Checks that extensions are not deleted from a given file.
 FILE_NO_DELETE                                  FILE                            Checks that files are not deleted.
 MESSAGE_NO_DELETE                               FILE                            Checks that messages are not deleted from a given file.
 SERVICE_NO_DELETE                               FILE                            Checks that services are not deleted from a given file.
@@ -905,6 +906,7 @@ RPC_SAME_REQUEST_TYPE                           FILE, PACKAGE, WIRE_JSON, WIRE
 RPC_SAME_RESPONSE_TYPE                          FILE, PACKAGE, WIRE_JSON, WIRE  Checks that rpcs are have the same response type.
 RPC_SAME_SERVER_STREAMING                       FILE, PACKAGE, WIRE_JSON, WIRE  Checks that rpcs have the same server streaming value.
 PACKAGE_ENUM_NO_DELETE                          PACKAGE                         Checks that enums are not deleted from a given package.
+PACKAGE_EXTENSION_NO_DELETE                     PACKAGE                         Checks that extensions are not deleted from a given package.
 PACKAGE_MESSAGE_NO_DELETE                       PACKAGE                         Checks that messages are not deleted from a given package.
 PACKAGE_NO_DELETE                               PACKAGE                         Checks that packages are not deleted.
 PACKAGE_SERVICE_NO_DELETE                       PACKAGE                         Checks that services are not deleted from a given package.
@@ -1905,7 +1907,7 @@ func TestBreakingWithPaths(t *testing.T) {
 		t,
 		nil,
 		bufctl.ExitCodeFileAnnotation,
-		`a/v3/a.proto:6:3:Field "1" on message "Foo" changed type from "string" to "int32".
+		`a/v3/a.proto:6:3:Field "1" with name "key" on message "Foo" changed type from "string" to "int32".
 a/v3/a.proto:7:3:Field "2" with name "Value" on message "Foo" changed option "json_name" from "value" to "Value".
 a/v3/a.proto:7:10:Field "2" on message "Foo" changed name from "value" to "Value".`,
 		"",

private/bufpkg/bufanalysis/bufanalysistesting/bufanalysistesting.go

@@ -110,19 +110,33 @@ func AssertFileAnnotationsEqual(
 	actual = normalizeFileAnnotations(t, actual)
 	if !assert.Equal(
 		t,
-		slicesext.Map(expected, func(annotation bufanalysis.FileAnnotation) string { return annotation.String() }),
-		slicesext.Map(actual, func(annotation bufanalysis.FileAnnotation) string { return annotation.String() }),
+		slicesext.Map(expected, bufanalysis.FileAnnotation.String),
+		slicesext.Map(actual, bufanalysis.FileAnnotation.String),
 	) {
-		t.Log("Expecting:")
+		t.Log("If actuals are correct, change expectations to the following:")
 		for _, annotation := range actual {
-			t.Logf("    bufanalysistesting.NewFileAnnotation(t, %q, %d, %d, %d, %d, %q),",
-				annotation.FileInfo().Path(),
-				annotation.StartLine(),
-				annotation.StartColumn(),
-				annotation.EndLine(),
-				annotation.EndColumn(),
-				annotation.Type(),
-			)
+			if annotation.StartLine() == 0 && annotation.StartColumn() == 0 &&
+				annotation.EndLine() == 0 && annotation.EndColumn() == 0 {
+				if annotation.FileInfo().Path() == "" {
+					t.Logf("    bufanalysistesting.NewFileAnnotationNoLocationOrPath(t, %q),",
+						annotation.Type(),
+					)
+				} else {
+					t.Logf("    bufanalysistesting.NewFileAnnotationNoLocation(t, %q, %q),",
+						annotation.FileInfo().Path(),
+						annotation.Type(),
+					)
+				}
+			} else {
+				t.Logf("    bufanalysistesting.NewFileAnnotation(t, %q, %d, %d, %d, %d, %q),",
+					annotation.FileInfo().Path(),
+					annotation.StartLine(),
+					annotation.StartColumn(),
+					annotation.EndLine(),
+					annotation.EndColumn(),
+					annotation.Type(),
+				)
+			}
 		}
 	}
 }