Skip to content

Commit

Permalink
Migrator: properly handle ignore generation on fields (#209)
Browse files Browse the repository at this point in the history
`ignore` field migration (from `ignore_empty` and `skipped`) was
preserving the field type (e.g., `string` or `repeated`) in the new
format, which is erroneous. `required` was already handling this
correctly, `ignore` just slipped through the cracks here.

Fixes #202

Thanks again @pquerna & @jschaf for the reports!
  • Loading branch information
rodaine authored May 7, 2024
1 parent d52c9e3 commit df918ea
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2023 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.

syntax = "proto3";

package tests.migrate;

import "validate/validate.proto";

import "buf/validate/validate.proto";

message IgnoreAndSkips {
string x = 1 [
(validate.rules).string = {
ignore_empty: true,
pattern: '^a+$',
},
(buf.validate.field).string = {
pattern: '^a+$',
}
, (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED
];
repeated string y = 2 [
(validate.rules).repeated = {
ignore_empty: true,
unique: true,
items: { string: { in: ['a', 'b'] } }
},
(buf.validate.field).repeated = {
unique: true,
items: { string: { in: ['a', 'b'] } }
}
, (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED
];
IgnoreAndSkips z = 3 [
(validate.rules).message = { skip: true },
(buf.validate.field) = { }
, (buf.validate.field).ignore = IGNORE_ALWAYS
];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2023 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.

syntax = "proto3";

package tests.migrate;

import "validate/validate.proto";

message IgnoreAndSkips {
string x = 1 [
(validate.rules).string = {
ignore_empty: true,
pattern: '^a+$',
}
];
repeated string y = 2 [
(validate.rules).repeated = {
ignore_empty: true,
unique: true,
items: { string: { in: ['a', 'b'] } }
}
];
IgnoreAndSkips z = 3 [
(validate.rules).message = { skip: true }
];
}
11 changes: 3 additions & 8 deletions tools/protovalidate-migrate/internal/migrator/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ package migrator

import (
"bytes"
"fmt"
"strings"

"github.com/bufbuild/protocompile/ast"
"github.com/bufbuild/protovalidate/tools/internal/gen/buf/validate"
Expand Down Expand Up @@ -214,14 +212,11 @@ func HandleMessageLiteral(
}

if msgLitVisitor.ignoreNeeded != nil {
newName := ", ignore"
if prefixed {
newName := printer.file.NodeInfo(name).RawText()
newName = strings.Replace(newName, "(validate.rules)", "(buf.validate.field)", 1)
newName = fmt.Sprintf(", %s.ignore", newName)
name = printer.replaceNode(name, newName)
} else {
name = printer.replaceNode(name, ", ignore")
newName = ", (buf.validate.field).ignore"
}
name = printer.replaceNode(name, newName)

if err := printer.PrintNodes(false,
name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ func TestMigrator(t *testing.T) {
srcPath: "field_ignore_empty.source.proto",
dstPath: "field_ignore_empty.migrated.proto",
},
{
srcPath: "field_ignore_skip.source.proto",
dstPath: "field_ignore_skip.migrated.proto",
},
{
srcPath: "no_separator.source.proto",
dstPath: "no_separator.migrated.proto",
Expand Down

0 comments on commit df918ea

Please sign in to comment.