Fix config migration for removed rules (#3810)

Author: emcfarlane

.golangci.yml

@@ -175,6 +175,11 @@ linters:
         # because they all call createZipFromDir on the same path, writing to the same file.
         path: private/buf/cmd/buf/workspace_test.go
         text: missing the call to method parallel
+      - linters:
+          - paralleltest
+        # This test shouldn't run in parallel as it needs osext.Getwd.
+        path: private/buf/cmd/buf/command/config/configmigrate/configmigrate_test.go
+        text: missing the call to method parallel
       - linters:
           - forbidigo
         # This is a legacy usage of os.Getwd we're not bothering to port yet.

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Add RISC-V (64-bit) binaries for Linux to releases.
 - Fix type filtering on `buf generate` for empty files, files with no declared types.
 - Fix CEL check on `buf lint` for predefined `rules` variables.
+- Fix `buf config migrate` to filter out removed rules. 
 
 ## [v1.53.0] - 2025-04-21
 

private/buf/bufmigrate/migrator.go

@@ -27,6 +27,7 @@ import (
 
 	"buf.build/go/bufplugin/check"
 	"github.com/bufbuild/buf/private/bufpkg/bufcheck"
+	"github.com/bufbuild/buf/private/bufpkg/bufcheck/bufcheckserver"
 	"github.com/bufbuild/buf/private/bufpkg/bufconfig"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule"
 	"github.com/bufbuild/buf/private/bufpkg/bufparse"
@@ -761,10 +762,35 @@ func equivalentCheckConfigInV2(
 			return !ok
 		},
 	)
-	return bufconfig.NewEnabledCheckConfig(
-		bufconfig.FileVersionV2,
+	// Filter remaining rules to match the V2 rule set. Any other rules are ignored.
+	// Theres no additional rules from plugins as plugins didn't exist before v2.
+	validV2IDsMap := make(map[string]struct{})
+	for _, ruleSpec := range bufcheckserver.V2Spec.Rules {
+		if ruleSpec.Type == ruleType {
+			validV2IDsMap[ruleSpec.ID] = struct{}{}
+		}
+	}
+	for _, categorySpec := range bufcheckserver.V2Spec.Categories {
+		validV2IDsMap[categorySpec.ID] = struct{}{}
+	}
+	useIDsAndCategories := slicesext.Filter(
 		append(simplyTranslatedCheckConfig.UseIDsAndCategories(), missingIDs...),
+		func(ruleID string) bool {
+			_, ok := validV2IDsMap[ruleID]
+			return ok
+		},
+	)
+	exceptIDsAndCategories := slicesext.Filter(
 		append(simplyTranslatedCheckConfig.ExceptIDsAndCategories(), extraIDs...),
+		func(ruleID string) bool {
+			_, ok := validV2IDsMap[ruleID]
+			return ok
+		},
+	)
+	return bufconfig.NewEnabledCheckConfig(
+		bufconfig.FileVersionV2,
+		useIDsAndCategories,
+		exceptIDsAndCategories,
 		simplyTranslatedCheckConfig.IgnorePaths(),
 		simplyTranslatedCheckConfig.IgnoreIDOrCategoryToPaths(),
 		simplyTranslatedCheckConfig.DisableBuiltin(),

private/buf/cmd/buf/command/config/configmigrate/configmigrate_test.go

@@ -0,0 +1,93 @@
+// Copyright 2020-2025 Buf Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package configmigrate
+
+import (
+	"bytes"
+	"context"
+	"path/filepath"
+	"testing"
+
+	"github.com/bufbuild/buf/private/buf/cmd/buf/internal/internaltesting"
+	"github.com/bufbuild/buf/private/pkg/app/appcmd"
+	"github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting"
+	"github.com/bufbuild/buf/private/pkg/app/appext"
+	"github.com/bufbuild/buf/private/pkg/osext"
+	"github.com/bufbuild/buf/private/pkg/storage"
+	"github.com/bufbuild/buf/private/pkg/storage/storageos"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestConfigMigrateV1DefaultConfig(t *testing.T) {
+	// Cannot be parallel since we chdir.
+	testCompareConfigMigrate(t, "testdata/defaultv1", 0, "")
+}
+
+func TestConfigMigrateV1BetaV1DefaultConfig(t *testing.T) {
+	// Cannot be parallel since we chdir.
+	testCompareConfigMigrate(t, "testdata/defaultv1beta1", 0, "")
+}
+
+func TestConfigMigrateUnknownVersion(t *testing.T) {
+	// Cannot be parallel since we chdir.
+	testCompareConfigMigrate(t, "testdata/unknown", 1, "decode buf.yaml: \"version\" is not set. Please add \"version: v2\"")
+}
+
+func testCompareConfigMigrate(t *testing.T, dir string, expectCode int, expectStderr string) {
+	// Setup temporary bucket with input, then compare it to the output.
+	storageosProvider := storageos.NewProvider()
+	inputBucket, err := storageosProvider.NewReadWriteBucket(filepath.Join(dir, "input"))
+	require.NoError(t, err)
+	tempDir := t.TempDir()
+	tempBucket, err := storageosProvider.NewReadWriteBucket(tempDir)
+	require.NoError(t, err)
+	ctx := context.Background()
+	_, err = storage.Copy(ctx, inputBucket, tempBucket)
+	require.NoError(t, err)
+	var outputBucket storage.ReadWriteBucket
+	if expectCode == 0 {
+		outputBucket, err = storageosProvider.NewReadWriteBucket(filepath.Join(dir, "output"))
+		require.NoError(t, err)
+	}
+	// Run in the temp directory.
+	func() {
+		pwd, err := osext.Getwd()
+		require.NoError(t, err)
+		require.NoError(t, osext.Chdir(tempDir))
+		defer func() {
+			r := recover()
+			assert.NoError(t, osext.Chdir(pwd))
+			if r != nil {
+				panic(r)
+			}
+		}()
+		appcmdtesting.Run(
+			t,
+			func(use string) *appcmd.Command {
+				return NewCommand(use, appext.NewBuilder(use))
+			},
+			appcmdtesting.WithExpectedExitCode(expectCode),
+			appcmdtesting.WithExpectedStderr(expectStderr),
+			appcmdtesting.WithEnv(internaltesting.NewEnvFunc(t)),
+		)
+	}()
+	if expectCode != 0 {
+		return // Nothing to compare.
+	}
+	var diff bytes.Buffer
+	require.NoError(t, storage.Diff(ctx, &diff, outputBucket, tempBucket))
+	assert.Empty(t, diff.String())
+}

private/buf/cmd/buf/command/config/configmigrate/testdata/defaultv1/input/buf.yaml

@@ -0,0 +1,24 @@
+version: v1
+name: ""
+deps: []
+build:
+  excludes: []
+lint:
+  use:
+    - STANDARD
+  except: []
+  ignore: []
+  ignore_only: {}
+  allow_comment_ignores: false
+  enum_zero_value_suffix: _UNSPECIFIED
+  rpc_allow_same_request_response: false
+  rpc_allow_google_protobuf_empty_requests: false
+  rpc_allow_google_protobuf_empty_responses: false
+  service_suffix: Service
+breaking:
+  use:
+    - FILE
+  except: []
+  ignore: []
+  ignore_only: {}
+  ignore_unstable_packages: false

private/buf/cmd/buf/command/config/configmigrate/testdata/defaultv1/output/buf.yaml

@@ -0,0 +1,16 @@
+version: v2
+lint:
+  use:
+    - STANDARD
+  except:
+    - FIELD_NOT_REQUIRED
+    - PACKAGE_NO_IMPORT_CYCLE
+  enum_zero_value_suffix: _UNSPECIFIED
+  service_suffix: Service
+  disallow_comment_ignores: true
+breaking:
+  use:
+    - FILE
+  except:
+    - EXTENSION_NO_DELETE
+    - FIELD_SAME_DEFAULT

private/buf/cmd/buf/command/config/configmigrate/testdata/defaultv1beta1/input/buf.yaml

@@ -0,0 +1,18 @@
+version: v1beta1
+name: ""
+deps: []
+build:
+  roots:
+    - .
+  excludes: []
+lint:
+  use:
+    - STANDARD
+  enum_zero_value_suffix: _UNSPECIFIED
+  rpc_allow_same_request_response: false
+  rpc_allow_google_protobuf_empty_requests: false
+  rpc_allow_google_protobuf_empty_responses: false
+  service_suffix: Service
+breaking:
+  use:
+    - FILE

private/buf/cmd/buf/command/config/configmigrate/testdata/defaultv1beta1/output/buf.yaml

@@ -0,0 +1,20 @@
+version: v2
+lint:
+  use:
+    - STANDARD
+  except:
+    - ENUM_FIRST_VALUE_ZERO
+    - FIELD_NOT_REQUIRED
+    - IMPORT_USED
+    - PACKAGE_NO_IMPORT_CYCLE
+    - PROTOVALIDATE
+    - SYNTAX_SPECIFIED
+  enum_zero_value_suffix: _UNSPECIFIED
+  service_suffix: Service
+  disallow_comment_ignores: true
+breaking:
+  use:
+    - FILE
+  except:
+    - EXTENSION_NO_DELETE
+    - FIELD_SAME_DEFAULT

private/buf/cmd/buf/command/config/configmigrate/testdata/unknown/input/buf.yaml

@@ -0,0 +1 @@
+name: "empty"