From f44a4418e63871c130c642a026f18bc136e3d32a Mon Sep 17 00:00:00 2001 From: Thomas Manninger Date: Fri, 31 Jul 2026 10:46:53 +0200 Subject: [PATCH 1/2] fix nested syntax in custom options --- .../schema/internal/parser/SyntaxReader.kt | 9 +++- .../schema/internal/parser/ProtoParserTest.kt | 48 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt b/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt index db6da39849..8a3e636640 100644 --- a/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt +++ b/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt @@ -199,7 +199,14 @@ class SyntaxReader( val start = pos loop@ while (pos < data.size) { when (data[pos]) { - in 'a'..'z', in 'A'..'Z', in '0'..'9', '_', '-', '.' -> pos++ + in 'a'..'z', in 'A'..'Z', in '0'..'9', '_', '-' -> pos++ + // A dot immediately followed by '(' or '[' is a separator before a parenthesized or + // bracketed extension (e.g. the second dot in "(foo.field).string.(foo.datetime)"), not + // part of this word. + '.' -> { + if (pos + 1 < data.size && (data[pos + 1] == '(' || data[pos + 1] == '[')) break@loop + pos++ + } else -> break@loop } } diff --git a/wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/ProtoParserTest.kt b/wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/ProtoParserTest.kt index f40835bb39..85dd895579 100644 --- a/wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/ProtoParserTest.kt +++ b/wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/ProtoParserTest.kt @@ -2549,6 +2549,54 @@ class ProtoParserTest { assertThat(ProtoParser.parse(location, proto)).isEqualTo(expected) } + // https://github.com/square/wire/issues/3672 + @Test + fun deepOptionAssignmentWithParenthesizedExtensionAfterFieldPathComponent() { + val proto = """ + |message Foo { + | optional string a = 1 [(foo.field).string.(foo.datetime) = true]; + |} + | + """.trimMargin() + val expected = ProtoFileElement( + location = location, + types = listOf( + MessageElement( + location = location.at(1, 1), + name = "Foo", + fields = listOf( + FieldElement( + location = location.at(2, 3), + label = OPTIONAL, + type = "string", + name = "a", + tag = 1, + options = listOf( + OptionElement( + name = "foo.field", + kind = Kind.OPTION, + isParenthesized = true, + value = OptionElement( + name = "string", + kind = Kind.OPTION, + isParenthesized = false, + value = OptionElement( + name = "foo.datetime", + kind = Kind.BOOLEAN, + isParenthesized = true, + value = "true", + ), + ), + ), + ), + ), + ), + ), + ), + ) + assertThat(ProtoParser.parse(location, proto)).isEqualTo(expected) + } + @Test fun protoKeywordAsEnumConstants() { // Note: this is consistent with protoc. val proto = """ From 1d460d49d71654ab2d02efa19afde58405565935 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Quenaudon?= Date: Fri, 14 Aug 2026 14:42:40 +0100 Subject: [PATCH 2/2] Read the dots in an option name as separators An option name component followed by a parenthesized extension failed to parse, as in protovalidate's: [(buf.validate.field).string.(buf.validate.predefined) = true] readWord() absorbed the dots in a name, so the component before "(buf.validate.predefined)" was read as "string." and the parser then failed with "expected '=' in option". The dots in an option name are separators between its components, so consume them in OptionReader.readOption(), which already loops over them, rather than letting the word absorb them. Because the reader skips whitespace and comments between any two tokens, this also accepts the equivalent spellings that protoc accepts and that a lookahead on the character after the dot would not: [(foo.field).string . (foo.datetime) = true] [(foo.field).string./* comment */(foo.datetime) = true] A component is now also kept whole when it is bracketed, instead of being split on the dots inside its brackets, which produced the unresolvable names "[foo" and "datetime]". See https://github.com/square/wire/issues/3672 --- .../schema/internal/parser/OptionReader.kt | 12 +++--- .../schema/internal/parser/SyntaxReader.kt | 29 ++++++++------ .../schema/internal/parser/ProtoParserTest.kt | 25 ++++++++++++ .../com/squareup/wire/schema/OptionsTest.kt | 39 +++++++++++++++++++ 4 files changed, 88 insertions(+), 17 deletions(-) diff --git a/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/OptionReader.kt b/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/OptionReader.kt index 34b93bc91a..9c8aef8a0d 100644 --- a/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/OptionReader.kt +++ b/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/OptionReader.kt @@ -61,12 +61,12 @@ class OptionReader(internal val reader: SyntaxReader) { break } // Read nested field name. For example "baz" in "(foo.bar).baz = 12". - val subName = reader.readName(retainWrap = true) - if (subName.startsWith("(")) { - subNames.add(subName) - } else { - subNames.addAll(subName.split(".")) - } + // + // The dots in an option name are separators, so a component must not absorb the ones that + // follow it: in "(foo.field).string.(foo.datetime)" this reads "string" and leaves the next + // dot for the loop above. Consuming dots here rather than inside the word is also what makes + // whitespace and comments around them insignificant, as they are everywhere else. + subNames += reader.readName(retainWrap = true, allowDots = false) } if (keyValueSeparator == ':' && c == '{') { // In text format, values which are maps can omit a separator. Backtrack so it can be re-read. diff --git a/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt b/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt index 8a3e636640..5d2b4c2b7b 100644 --- a/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt +++ b/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/internal/parser/SyntaxReader.kt @@ -142,8 +142,15 @@ class SyntaxReader( * If {@code retainWrap} is true and the symbol was wrapped in parens * or square brackets, the returned string retains the wrapping * punctuation. Otherwise, just the symbol is returned. + * + * @param allowDots whether a naked name may span '.' characters. Wrapped names always may; they + * are delimited by their wrapping punctuation rather than by dots. */ - fun readName(allowLeadingDigit: Boolean = true, retainWrap: Boolean = false): String = when (peekChar()) { + fun readName( + allowLeadingDigit: Boolean = true, + retainWrap: Boolean = false, + allowDots: Boolean = true, + ): String = when (peekChar()) { '(' -> { pos++ val word = readWord(allowLeadingDigit).also { @@ -160,7 +167,7 @@ class SyntaxReader( if (retainWrap) "[$word]" else word } - else -> readWord(allowLeadingDigit) + else -> readWord(allowLeadingDigit, allowDots) } /** Reads a scalar, map, or type name. */ @@ -193,20 +200,20 @@ class SyntaxReader( } } - /** Reads a non-empty word and returns it. */ - fun readWord(allowLeadingDigit: Boolean = true): String { + /** + * Reads a non-empty word and returns it. + * + * @param allowDots true for the word to span '.' characters, as in the qualified name `foo.Bar`. + * When false the word ends at the first '.', leaving it for the caller to consume as a + * separator between the components of a path. + */ + fun readWord(allowLeadingDigit: Boolean = true, allowDots: Boolean = true): String { skipWhitespace(skipComments = true) val start = pos loop@ while (pos < data.size) { when (data[pos]) { in 'a'..'z', in 'A'..'Z', in '0'..'9', '_', '-' -> pos++ - // A dot immediately followed by '(' or '[' is a separator before a parenthesized or - // bracketed extension (e.g. the second dot in "(foo.field).string.(foo.datetime)"), not - // part of this word. - '.' -> { - if (pos + 1 < data.size && (data[pos + 1] == '(' || data[pos + 1] == '[')) break@loop - pos++ - } + '.' -> if (allowDots) pos++ else break@loop else -> break@loop } } diff --git a/wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/ProtoParserTest.kt b/wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/ProtoParserTest.kt index 85dd895579..3c0dcc7697 100644 --- a/wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/ProtoParserTest.kt +++ b/wire-schema/src/commonTest/kotlin/com/squareup/wire/schema/internal/parser/ProtoParserTest.kt @@ -2597,6 +2597,31 @@ class ProtoParserTest { assertThat(ProtoParser.parse(location, proto)).isEqualTo(expected) } + // https://github.com/square/wire/issues/3672 + @Test + fun deepOptionAssignmentDotsAreSeparators() { + // The dots between the components of an option name are separators, so whitespace and comments + // around them are insignificant, as they are between any other two tokens. + val optionNames = listOf( + "(foo.field).string.(foo.datetime)", + "(foo.field).string. (foo.datetime)", + "(foo.field).string . (foo.datetime)", + "(foo.field) . string . (foo.datetime)", + "(foo.field).string./* comment */(foo.datetime)", + "(foo.field).string.\n (foo.datetime)", + ) + val canonical = parseOptionOnFieldA(optionNames.first()) + for (optionName in optionNames) { + assertThat(parseOptionOnFieldA(optionName), name = optionName).isEqualTo(canonical) + } + } + + /** Parses a message whose only field carries `optionName` set to `true`. */ + private fun parseOptionOnFieldA(optionName: String): ProtoFileElement = ProtoParser.parse( + location, + "message Foo {\n optional string a = 1 [$optionName = true];\n}\n", + ) + @Test fun protoKeywordAsEnumConstants() { // Note: this is consistent with protoc. val proto = """ diff --git a/wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/OptionsTest.kt b/wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/OptionsTest.kt index ff9ef4aea3..6f6255be9b 100644 --- a/wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/OptionsTest.kt +++ b/wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/OptionsTest.kt @@ -61,6 +61,45 @@ class OptionsTest { .isEqualTo(mapOf(fooOptions to mapOf(opt1 to "456", opt2 to "quux"))) } + // https://github.com/square/wire/issues/3672 + @Test + fun parenthesizedExtensionAfterFieldPathComponent() { + // Shaped after protovalidate's "(buf.validate.field).string.(buf.validate.predefined)". + val schema = buildSchema { + add( + "foo.proto".toPath(), + """ + |import "google/protobuf/descriptor.proto"; + |message FieldConstraints { + | optional StringRules string = 1; + |} + |message StringRules { + | extensions 1000 to max; + |} + | + |extend StringRules { + | optional bool datetime = 1000; + |} + |extend google.protobuf.FieldOptions { + | optional FieldConstraints field = 1234; + |} + | + |message Bar { + | optional string a = 1 [(field).string.(datetime) = true]; + |} + """.trimMargin(), + ) + } + + val field = ProtoMember.get(Options.FIELD_OPTIONS, "field") + val string = ProtoMember.get(ProtoType.get("FieldConstraints"), "string") + val datetime = ProtoMember.get(ProtoType.get("StringRules"), "datetime") + + val bar = schema.getType("Bar") as MessageType + assertThat(bar.field("a")!!.options.map) + .isEqualTo(mapOf(field to mapOf(string to mapOf(datetime to "true")))) + } + @Test fun textFormatCanOmitMapValueSeparator() { val schema = buildSchema {