From 44b1b9d4a4d8fa916fc033f6bf792e78c2d7a2d7 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Fri, 21 Nov 2025 11:19:55 -0800 Subject: [PATCH 1/3] fix(lint): allow deprecation rule on deprecated descriptor --- lint/rule_enabled.go | 15 +++++++++++---- lint/rule_enabled_test.go | 22 +++++++++++++++------- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/lint/rule_enabled.go b/lint/rule_enabled.go index d2c4e02ac..d824f600d 100644 --- a/lint/rule_enabled.go +++ b/lint/rule_enabled.go @@ -25,8 +25,15 @@ import ( // disabled, because they are scoped to a very specific set of AIPs. var defaultDisabledRules = []string{"cloud"} -// Disable all rules for deprecated descriptors. -func disableDeprecated(d protoreflect.Descriptor) bool { +// Disable rules for deprecated descriptors, except for those designed to lint +// deprecated descriptors. +func disableDeprecated(d protoreflect.Descriptor, rule string) bool { + // Exception to rule, because it is designed to be applied to deprecated + // elements. + if matchRule(rule, "core::0192::deprecated-comment") { + return false + } + switch v := d.(type) { case protoreflect.EnumDescriptor: return v.Options().(*dpb.EnumOptions).GetDeprecated() @@ -56,7 +63,7 @@ func disableDeprecated(d protoreflect.Descriptor) bool { // func init() { // descriptorDisableChecks = append(descriptorDisableChecks, disableForInternalReason) // } -var descriptorDisableChecks = []func(d protoreflect.Descriptor) bool{ +var descriptorDisableChecks = []func(d protoreflect.Descriptor, rule string) bool{ disableDeprecated, } @@ -72,7 +79,7 @@ func ruleIsEnabled(rule ProtoRule, d protoreflect.Descriptor, l *dpb.SourceCodeI for _, mustDisable := range descriptorDisableChecks { // The only thing the disable functions can do is force a rule to // be disabled. (They can not force a rule to be enabled.) - if mustDisable(d) { + if mustDisable(d, string(rule.GetName())) { return false } } diff --git a/lint/rule_enabled_test.go b/lint/rule_enabled_test.go index b20b9d089..37db6bd1d 100644 --- a/lint/rule_enabled_test.go +++ b/lint/rule_enabled_test.go @@ -197,24 +197,32 @@ func TestRuleIsEnabledParent(t *testing.T) { } func TestRuleIsEnabledDeprecated(t *testing.T) { - // Create a rule that we can check enabled status on. - rule := &FieldRule{ + // Create a generalRule that we can check enabled status on. + generalRule := &FieldRule{ Name: RuleName("test"), LintField: func(f protoreflect.FieldDescriptor) []Problem { return nil }, } + deprecationRule := &FieldRule{ + Name: RuleName("core::0192::deprecated-comment"), + LintField: func(f protoreflect.FieldDescriptor) []Problem { + return nil + }, + } for _, test := range []struct { name string + rule ProtoRule msgDeprecated bool fieldDeprecated bool enabled bool }{ - {"Both", true, true, false}, - {"Message", true, false, false}, - {"Field", false, true, false}, - {"Neither", false, false, true}, + {"Both", generalRule, true, true, false}, + {"Message", generalRule, true, false, false}, + {"Field", generalRule, false, true, false}, + {"Neither", generalRule, false, false, true}, + {"DeprecationRule", deprecationRule, false, true, true}, } { t.Run(test.name, func(t *testing.T) { // Build a proto with a message and field, possibly deprecated. @@ -242,7 +250,7 @@ func TestRuleIsEnabledDeprecated(t *testing.T) { if err != nil { t.Fatalf("Error building test file: %q", err) } - if got, want := ruleIsEnabled(rule, f.Messages().Get(0).Fields().Get(0), nil, nil, false), test.enabled; got != want { + if got, want := ruleIsEnabled(test.rule, f.Messages().Get(0).Fields().Get(0), nil, nil, false), test.enabled; got != want { t.Errorf("Expected the foo field to return %v from ruleIsEnabled; got %v", want, got) } }) From 8d3c85ac84415ee694e068232a10bf399f501ad0 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Fri, 21 Nov 2025 11:28:34 -0800 Subject: [PATCH 2/3] make deprecation rules a list --- go.sum | 22 ---------------------- lint/rule_enabled.go | 5 ++++- 2 files changed, 4 insertions(+), 23 deletions(-) diff --git a/go.sum b/go.sum index 8771431e5..e8320247b 100644 --- a/go.sum +++ b/go.sum @@ -31,8 +31,6 @@ github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/spf13/pflag v1.0.7 h1:vN6T9TfwStFPFM5XzjsvmzZkLuaLX+HS+0SeFLRgU6M= -github.com/spf13/pflag v1.0.7/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stoewer/go-strcase v1.3.1 h1:iS0MdW+kVTxgMoE1LAZyMiYJFKlOzLooE4MxjirtkAs= @@ -59,38 +57,18 @@ go.opentelemetry.io/otel/trace v1.37.0 h1:HLdcFNbRQBE2imdSEgm/kwqmQj1Or1l/7bW6mx go.opentelemetry.io/otel/trace v1.37.0/go.mod h1:TlgrlQ+PtQO5XFerSPUYG0JSgGyryXewPGyayAWSBS0= golang.org/x/net v0.44.0 h1:evd8IRDyfNBMBTTY5XRF1vaZlD+EmWx6x8PkhR04H/I= golang.org/x/net v0.44.0/go.mod h1:ECOoLqd5U3Lhyeyo/QDCEVQ4sNgYsqvCZ722XogGieY= -golang.org/x/sync v0.17.0 h1:l60nONMj9l5drqw6jlhIELNv9I0A4OFgRsG9k2oT9Ug= -golang.org/x/sync v0.17.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I= golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k= golang.org/x/sys v0.36.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk= -golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4= -golang.org/x/text v0.30.0 h1:yznKA/E9zq54KzlzBEAWn1NXSQ8DIp/NYMy88xJjl4k= -golang.org/x/text v0.30.0/go.mod h1:yDdHFIX9t+tORqspjENWgzaCVXgk0yYnYuSZ8UzzBVM= golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM= golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM= gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk= gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E= -google.golang.org/genproto v0.0.0-20251022142026-3a174f9686a8 h1:a12a2/BiVRxRWIqBbfqoSK6tgq8cyUgMnEI81QlPge0= -google.golang.org/genproto v0.0.0-20251022142026-3a174f9686a8/go.mod h1:1Ic78BnpzY8OaTCmzxJDP4qC9INZPbGZl+54RKjtyeI= -google.golang.org/genproto v0.0.0-20251103181224-f26f9409b101 h1:MgBTzgUJFAmp2PlyqKJecSpZpjFxkYL3nDUIeH/6Q30= -google.golang.org/genproto v0.0.0-20251103181224-f26f9409b101/go.mod h1:bbWg36d7wp3knc0hIlmJAnW5R/CQ2rzpEVb72eH4ex4= google.golang.org/genproto v0.0.0-20251111163417-95abcf5c77ba h1:Ze6qXW0j37YCqZdCD2LkzVSxgEWez0cO4NUyd44DiDY= google.golang.org/genproto v0.0.0-20251111163417-95abcf5c77ba/go.mod h1:4FLPzLA8eGAktPOTemJGDgDYRpLYwrNu4u2JtWINhnI= -google.golang.org/genproto/googleapis/api v0.0.0-20251014184007-4626949a642f h1:OiFuztEyBivVKDvguQJYWq1yDcfAHIID/FVrPR4oiI0= -google.golang.org/genproto/googleapis/api v0.0.0-20251014184007-4626949a642f/go.mod h1:kprOiu9Tr0JYyD6DORrc4Hfyk3RFXqkQ3ctHEum3ZbM= -google.golang.org/genproto/googleapis/api v0.0.0-20251022142026-3a174f9686a8 h1:mepRgnBZa07I4TRuomDE4sTIYieg/osKmzIf4USdWS4= -google.golang.org/genproto/googleapis/api v0.0.0-20251022142026-3a174f9686a8/go.mod h1:fDMmzKV90WSg1NbozdqrE64fkuTv6mlq2zxo9ad+3yo= -google.golang.org/genproto/googleapis/api v0.0.0-20251103181224-f26f9409b101 h1:vk5TfqZHNn0obhPIYeS+cxIFKFQgser/M2jnI+9c6MM= -google.golang.org/genproto/googleapis/api v0.0.0-20251103181224-f26f9409b101/go.mod h1:E17fc4PDhkr22dE3RgnH2hEubUaky6ZwW4VhANxyspg= google.golang.org/genproto/googleapis/api v0.0.0-20251111163417-95abcf5c77ba h1:B14OtaXuMaCQsl2deSvNkyPKIzq3BjfxQp8d00QyWx4= google.golang.org/genproto/googleapis/api v0.0.0-20251111163417-95abcf5c77ba/go.mod h1:G5IanEx8/PgI9w6CFcYQf7jMtHQhZruvfM1i3qOqk5U= -google.golang.org/genproto/googleapis/rpc v0.0.0-20251014184007-4626949a642f h1:1FTH6cpXFsENbPR5Bu8NQddPSaUUE6NA2XdZdDSAJK4= -google.golang.org/genproto/googleapis/rpc v0.0.0-20251014184007-4626949a642f/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk= -google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda h1:i/Q+bfisr7gq6feoJnS/DlpdwEL4ihp41fvRiM3Ork0= -google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk= google.golang.org/genproto/googleapis/rpc v0.0.0-20251103181224-f26f9409b101 h1:tRPGkdGHuewF4UisLzzHHr1spKw92qLM98nIzxbC0wY= google.golang.org/genproto/googleapis/rpc v0.0.0-20251103181224-f26f9409b101/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk= google.golang.org/grpc v1.75.1 h1:/ODCNEuf9VghjgO3rqLcfg8fiOP0nSluljWFlDxELLI= diff --git a/lint/rule_enabled.go b/lint/rule_enabled.go index d824f600d..13bab0da2 100644 --- a/lint/rule_enabled.go +++ b/lint/rule_enabled.go @@ -25,12 +25,15 @@ import ( // disabled, because they are scoped to a very specific set of AIPs. var defaultDisabledRules = []string{"cloud"} +// deprecationRules are rules specifically designed to lint deprecated elements. +var deprecationRules = []string{"core::0192::deprecated-comment"} + // Disable rules for deprecated descriptors, except for those designed to lint // deprecated descriptors. func disableDeprecated(d protoreflect.Descriptor, rule string) bool { // Exception to rule, because it is designed to be applied to deprecated // elements. - if matchRule(rule, "core::0192::deprecated-comment") { + if matchRule(rule, deprecationRules...) { return false } From 91e9a15b026e4be2b755eee108b7e0d071609bea Mon Sep 17 00:00:00 2001 From: noahdietz Date: Fri, 21 Nov 2025 11:29:50 -0800 Subject: [PATCH 3/3] fix test error msg --- lint/rule_enabled_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lint/rule_enabled_test.go b/lint/rule_enabled_test.go index 37db6bd1d..d8159e499 100644 --- a/lint/rule_enabled_test.go +++ b/lint/rule_enabled_test.go @@ -251,7 +251,7 @@ func TestRuleIsEnabledDeprecated(t *testing.T) { t.Fatalf("Error building test file: %q", err) } if got, want := ruleIsEnabled(test.rule, f.Messages().Get(0).Fields().Get(0), nil, nil, false), test.enabled; got != want { - t.Errorf("Expected the foo field to return %v from ruleIsEnabled; got %v", want, got) + t.Errorf("Expected the bar field to return %v from ruleIsEnabled; got %v", want, got) } }) }