Disable format on files with errors

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## [Unreleased]
 
-- No changes yet.
+- Disable format on unknown or invalid syntax.
 
 ## [v1.60.0] - 2025-11-14
 

private/buf/bufformat/bufformat.go

@@ -82,6 +82,12 @@ func FormatBucket(ctx context.Context, bucket storage.ReadBucket) (_ storage.Rea
 
 // FormatFileNode formats the given file node and writ the result to dest.
 func FormatFileNode(dest io.Writer, fileNode *ast.FileNode) error {
+	// Construct the file descriptor to ensure the AST is valid. This will
+	// capture unknown syntax like edition "2024" which at the current time is
+	// not supported.
+	if _, err := parser.ResultFromAST(fileNode, true, reporter.NewHandler(nil)); err != nil {
+		return err
+	}
 	formatter := newFormatter(dest, fileNode)
 	return formatter.Run()
 }

private/buf/bufformat/formatter_test.go

@@ -42,7 +42,8 @@ func testFormatCustomOptions(t *testing.T) {
 }
 
 func testFormatEditions(t *testing.T) {
-	testFormatNoDiff(t, "testdata/editions")
+	testFormatNoDiff(t, "testdata/editions/2023")
+	testFormatError(t, "testdata/editions/2024", `edition "2024" not yet fully supported; latest supported edition "2023"`)
 }
 
 func testFormatProto2(t *testing.T) {
@@ -78,40 +79,54 @@ func testFormatNoDiff(t *testing.T, path string) {
 		ctx := context.Background()
 		bucket, err := storageos.NewProvider().NewReadWriteBucket(path)
 		require.NoError(t, err)
+
 		moduleSetBuilder := bufmodule.NewModuleSetBuilder(ctx, slogtestext.NewLogger(t), bufmodule.NopModuleDataProvider, bufmodule.NopCommitProvider)
 		moduleSetBuilder.AddLocalModule(bucket, path, true)
 		moduleSet, err := moduleSetBuilder.Build()
 		require.NoError(t, err)
-		readBucket, err := FormatModuleSet(ctx, moduleSet)
+		formatBucket, err := FormatModuleSet(ctx, moduleSet)
 		require.NoError(t, err)
-		require.NoError(
-			t,
-			storage.WalkReadObjects(
+		assertGolden := func(formatBucket storage.ReadBucket) {
+			err := storage.WalkReadObjects(
 				ctx,
-				readBucket,
+				formatBucket,
 				"",
 				func(formattedFile storage.ReadObject) error {
-					expectedPath := formattedFile.Path()
-					t.Run(expectedPath, func(t *testing.T) {
-						// The expected format result is the golden file. If
-						// this file IS a golden file, it is expected to not
-						// change.
-						if !strings.HasSuffix(expectedPath, ".golden.proto") {
-							expectedPath = strings.Replace(expectedPath, ".proto", ".golden.proto", 1)
-						}
-						formattedData, err := io.ReadAll(formattedFile)
-						require.NoError(t, err)
-						expectedFile, err := bucket.Get(ctx, expectedPath)
-						require.NoError(t, err)
-						expectedData, err := io.ReadAll(expectedFile)
-						require.NoError(t, err)
-						fileDiff, err := diff.Diff(ctx, expectedData, formattedData, expectedPath, formattedFile.Path()+" (formatted)")
-						require.NoError(t, err)
-						require.Empty(t, string(fileDiff))
-					})
+					formattedData, err := io.ReadAll(formattedFile)
+					require.NoError(t, err)
+					expectedPath := strings.Replace(formattedFile.Path(), ".proto", ".golden", 1)
+					expectedData, err := storage.ReadPath(ctx, bucket, expectedPath)
+					require.NoError(t, err)
+					fileDiff, err := diff.Diff(ctx, expectedData, formattedData, expectedPath, formattedFile.Path()+" (formatted)")
+					require.NoError(t, err)
+					require.Empty(t, string(fileDiff))
 					return nil
 				},
-			),
-		)
+			)
+			require.NoError(t, err)
+		}
+		assertGolden(formatBucket)
+
+		moduleSetBuilder = bufmodule.NewModuleSetBuilder(ctx, slogtestext.NewLogger(t), bufmodule.NopModuleDataProvider, bufmodule.NopCommitProvider)
+		moduleSetBuilder.AddLocalModule(formatBucket, path, true)
+		moduleSet, err = moduleSetBuilder.Build()
+		require.NoError(t, err)
+		reformattedBucket, err := FormatModuleSet(ctx, moduleSet)
+		require.NoError(t, err)
+		assertGolden(reformattedBucket)
+	})
+}
+
+func testFormatError(t *testing.T, path string, errContains string) {
+	t.Run(path, func(t *testing.T) {
+		ctx := context.Background()
+		bucket, err := storageos.NewProvider().NewReadWriteBucket(path)
+		require.NoError(t, err)
+		moduleSetBuilder := bufmodule.NewModuleSetBuilder(ctx, slogtestext.NewLogger(t), bufmodule.NopModuleDataProvider, bufmodule.NopCommitProvider)
+		moduleSetBuilder.AddLocalModule(bucket, path, true)
+		moduleSet, err := moduleSetBuilder.Build()
+		require.NoError(t, err)
+		_, err = FormatModuleSet(ctx, moduleSet)
+		require.ErrorContains(t, err, errContains)
 	})
 }

private/buf/bufformat/testdata/editions/2024/editions.proto

@@ -0,0 +1,5 @@
+edition = "2024";
+
+package a.b.c;
+
+import option "google/protobuf/descriptor.proto";

private/buf/bufformat/testdata/proto2/enum/v1/empty.golden.proto → private/buf/bufformat/testdata/proto2/enum/v1/empty.golden

@@ -1,11 +1,14 @@
 enum Foo {
+  FOO_UNKNOWN = 1;
   /* C-style comment in the middle */
 }
 
 enum Bar {
+  BAR_UNKNOWN = 1;
   // Normal comment in the middle
 }
 
 enum Baz {
+  BAZ_UNKNOWN = 1;
   // Body comment.
 }

private/buf/bufformat/testdata/proto2/enum/v1/empty.proto

@@ -1,10 +1,11 @@
-enum Foo { /* C-style comment in the middle */ }
+enum Foo { FOO_UNKNOWN = 1; /* C-style comment in the middle */ }
 
 enum Bar {
+  BAR_UNKNOWN = 1;
   // Normal comment in the middle
 }
 
 enum Baz {
-
+  BAZ_UNKNOWN = 1;
 // Body comment.
 }

private/buf/bufformat/testdata/proto2/field/v1/option.golden.proto → private/buf/bufformat/testdata/proto2/field/v1/option.golden

@@ -21,7 +21,7 @@ message Foo {
   */
 
   // Leading comment on name_with_options.
-  string name_with_options = 2 [
+  optional string name_with_options = 2 [
     (custom.float_field_option) = "nan",
     (custom.double_field_option) = "inf",
     (custom.int32_field_option) = -3,

private/buf/bufformat/testdata/proto2/field/v1/option.proto

@@ -18,7 +18,7 @@ message Foo {
            */
 
          // Leading comment on name_with_options.
-   string name_with_options = 2 [
+   optional string name_with_options = 2 [
    (custom.float_field_option) = "nan",
       (custom.double_field_option) = "inf",   
     (custom.int32_field_option) = -3,

private/buf/bufformat/testdata/proto2/license/v1/enum.golden.proto → private/buf/bufformat/testdata/proto2/license/v1/enum.golden

@@ -4,5 +4,9 @@
    before the enums.
 */
 
-enum Foo {}
-enum Bar {}
+enum Foo {
+  FOO_UNKNOWN = 1;
+}
+enum Bar {
+  BAR_UNKNOWN = 1;
+}

private/buf/bufformat/testdata/proto2/license/v1/enum.proto

@@ -5,5 +5,5 @@
 */
 
 
-enum Foo {}
-enum Bar {}
+enum Foo { FOO_UNKNOWN = 1;}
+enum Bar {BAR_UNKNOWN=1;}

private/buf/bufformat/testdata/proto2/message/v1/message_import.golden.proto → private/buf/bufformat/testdata/proto2/message/v1/message_import.golden

@@ -4,7 +4,7 @@ import "custom.proto";
 
 message Thing { // Trailing comment on '}'.
   // Leading comment on thing field.
-  custom.Thing thing = 1; // Trailing comment on thing field.
+  optional custom.Thing thing = 1; // Trailing comment on thing field.
 
   // Leading comment on '}',
 }

private/buf/bufformat/testdata/proto2/message/v1/message_import.proto

@@ -5,7 +5,7 @@ import "custom.proto";
 message Thing { // Trailing comment on '}'.
 
   // Leading comment on thing field.
-  custom.Thing thing = 1; // Trailing comment on thing field.
+  optional custom.Thing thing = 1; // Trailing comment on thing field.
 
 // Leading comment on '}',
 }

private/buf/bufformat/testdata/proto2/message/v1/multiple_nested.golden.proto → private/buf/bufformat/testdata/proto2/message/v1/multiple_nested.golden

@@ -14,15 +14,15 @@ message Foo {
   }
   // Comment on the nested Baz message.
   message Baz {
-    string something = 5;
+    optional string something = 5;
 
     // Comment on the nested Qux message.
     message Qux {
       option deprecated = true;
 
       option no_standard_descriptor_accessor = false;
 
-      string another = 6;
+      optional string another = 6;
     }
   }
 }

private/buf/bufformat/testdata/proto2/message/v1/multiple_nested.proto

@@ -14,17 +14,17 @@ syntax = "proto2";
                              /* This one is required */ required bool /* Between 'bool' and 'truth' */ truth = 3;
                     }         
         // Comment on the nested Baz message.
-     message Baz     { 
-         
-          string something = 5;
+     message Baz     {
+
+          optional string something = 5;
 
                  // Comment on the nested Qux message.
          message Qux { 
               option deprecated = true;
 
              option no_standard_descriptor_accessor = false;
 
-               string another = 6;
+               optional string another = 6;
          }
      }
 

private/buf/bufformat/testdata/proto2/message/v1/sparse.golden

@@ -0,0 +1,9 @@
+syntax = "proto2";
+
+message Foo {
+  // This is a trailing comment on the '{'.
+
+  optional string name = 1;
+
+  optional string value = 2; // In-line on value.
+}

private/buf/bufformat/testdata/proto2/message/v1/sparse.proto

@@ -1,4 +1,6 @@
-message  
+syntax = "proto2";
+
+message
 
 
   Foo
@@ -8,9 +10,9 @@ message
   // This is a trailing comment on the '{'.
 
 
-                                string name = 1;
+                                optional string name = 1;
 
-    string value = 2; // In-line on value.
+    optional string value = 2; // In-line on value.
 
 
 }

private/buf/bufformat/testdata/proto2/option/v1/option_array.golden.proto → private/buf/bufformat/testdata/proto2/option/v1/option_array.golden

@@ -4,7 +4,7 @@ import "custom.proto";
 
 message Foo {
   message Bar {
-    string name = 1 [(custom.field_thing_option) = {
+    optional string name = 1 [(custom.field_thing_option) = {
       // Trailing comment on '{'.
 
       // Leading comment on repeated_foo field.

private/buf/bufformat/testdata/proto2/option/v1/option_array.proto

@@ -4,7 +4,7 @@ import "custom.proto";
 
 message Foo {
   message Bar {
-    string name = 1 [
+    optional string name = 1 [
       (custom.field_thing_option) = {
         // Trailing comment on '{'.
 

private/buf/bufformat/testdata/proto2/option/v1/option_name.golden.proto → private/buf/bufformat/testdata/proto2/option/v1/option_name.golden

@@ -13,7 +13,7 @@ extend google.protobuf.FieldOptions {
 message Foo {
   option (/* One */ something /* Two */) = 1;
 
-  string name = 1 [
+  optional string name = 1 [
     // Leading comment on deprecated.
     deprecated /* After deprecated */ = true,
     // Leading comment on another.

private/buf/bufformat/testdata/proto2/option/v1/option_name.proto

@@ -13,7 +13,7 @@ extend google.protobuf.FieldOptions {
 message Foo {
   option (/* One */  something  /* Two */) = 1;
 
-  string name = 1 [
+  optional string name = 1 [
     // Leading comment on deprecated.
     deprecated /* After deprecated */ = true,
     // Leading comment on another.

private/buf/bufformat/testdata/proto2/quotes/v1/quotes.golden

@@ -0,0 +1,24 @@
+syntax = "proto2";
+
+import "google/protobuf/descriptor.proto";
+
+message Foo {
+  optional string something = 1;
+}
+
+extend google.protobuf.FieldOptions {
+  optional string name = 10001;
+  optional Foo foo = 10002;
+}
+
+message Foo {
+  optional string one = 1 [(name) = "f\"o\"'o'"];
+  optional string two = 2 [(name) = 'f"oo"'];
+  optional string three = 3 [(name) = "f'oo'"];
+  optional string four = 4 [(name) = "f\"o\"\'o\'"];
+  optional string five = 5 [(name) = "f\"o\"o"];
+  optional string six = 6 [(name) = 'f\'o\'o'];
+  optional string seven = 7 [(name) = "foo"];
+  optional string eight = 8 [(foo) = {something: 'something:"foo"'}];
+  optional string nine = 9 [(foo) = {something: "something:\"foo\"\nanother:\"bar\""}];
+}