Accept whitespace and comments inside a qualified name (#255)

[MavenRain]

Summary

Fixes #255. A qualified type name split across lines or spaces, the shape that appears throughout
the googleapis .proto files, failed to parse. protoc treats whitespace and comments as
insignificant between the segments of a dotted name; ocaml-protoc did not.

The example from #255 (google/ads/googleads/v18/services/google_ads_service.proto) now compiles:

repeated google.ads.googleads.v18.resources
    .OfflineConversionUploadConversionActionSummary
        offline_conversion_upload_conversion_action_summary = 228;

Problem

pb_parsing_lexer.mll lexed an entire dotted name as one token:

let full_ident = '.' ? ident ("." * ident) *

and there was no rule for . at all, so every qualified-name position in the grammar was spelled as
a single T_ident. A name broken by whitespace therefore arrived as several tokens and no production
matched. Measured against protoc 23.2, every one of these is accepted by protoc and rejected
before this change:

input	before
a.b.c NL .Inner f = 1;	Parsing error (the reported case)
a.b.c .Inner f = 1;	Parsing error
a.b.c. Inner f = 1;	Failure("float_of_string")
a . b . c . Inner f = 1;	Failure("float_of_string")
. a.b.c.Inner f = 1;	Failure("float_of_string")
a.b.c //c NL .Inner f = 1;	Parsing error
a.b.c /*c*/ .Inner f = 1;	Parsing error

The same breakage applied to every other position that names a type: package a . b . c;, rpc
request/response types, map value types, extend targets, and oneof field types.

Two things fall out of the same root cause and are fixed here too:

• A lone . raised an uncaught exception rather than a parse error. float_literal has every
  part optional, so it matched a bare . and float_of_string "." raised. message M { int32 . = 1; }
  reported Failure("float_of_string") with a location pointing at unrelated earlier text. It now
  reports a normal Parsing error at the right place.
• Five dot-leading spellings were silently accepted where protoc rejects them, because a
  leading dot was absorbed into an ordinary identifier: a declared message name .Foo, a field name
  .y, an enum value .A, a constant .foo, and an option message-literal key .k. Lexing a
  dot-leading name as its own token makes all five parse errors, matching protoc.
• An empty path segment was silently accepted. a..b.C parsed, and the empty segment leaked
  downstream as an empty type: the old error was unresolved type for field name : f (type:, ...)
  with a blank type. protoc rejects a..b.C; so does this change, as a parse error.

Fix

Make . a real token and rejoin the segments in the grammar, rather than widening the lexer regexp.
A regexp cannot span case a.b.c /*c*/ .Inner, because the comment is consumed by a separate lexer
rule.

• pb_parsing_lexer.mll: split full_ident into ident_path (no leading dot) and dot_ident_path
  (leading dot, emitted as the new T_dot_ident), and add a rule for a bare . emitting the new
  T_dot. The "." rule sits with the other punctuation, ahead of float_literal, since
  float_literal also matches a lone . and ocamllex resolves an equal-length tie in favour of the
  earlier rule. Tightening ("." * ident) * to ("." ident) * is what rejects the empty segment.
• pb_parsing_parser.mly: add qualified_ident / qualified_ident_tail, which rebuild the dotted
  string by concatenating the pieces' lexemes. Only the first piece may be a bare identifier; every
  continuation must begin with a dot. That asymmetry is what keeps the TYPE fieldname adjacency
  unambiguous, and ocamlyacc reports zero shift/reduce and zero reduce/reduce conflicts.
  The post-dot segment is field_name, so a keyword remains legal as a path segment.
• qualified_ident is then used at the positions that name a type: normal_field, oneof_field,
  map key and value, message_type (rpc), package_declaration, extend, and the parenthesised
  option extension name. Declared names (message, enum, service, rpc, field, oneof, enum value) are
  deliberately left as plain T_ident, since protobuf allows only a simple name there.
• option_identifier_item also accepts T_dot_ident, so option (ext).sub = v keeps working, since it
  relied on .sub lexing as a T_ident.
• field_name gains a T_returns alternative. It already listed every other keyword that
  resolve_identifier produces, and returns was the only one missing, so without it a . returns . C
  would be the one split path that still failed. As a side effect this also lets a field be named
  returns, which protoc allows and ocaml-protoc previously rejected.
• pb_parsing.ml: render the two new tokens in string_of_token (used for the error context).

Keeping an unbroken dotted run as a single token is deliberate: it is what preserves a keyword as
an inner segment (a.map.C, a.to.C) and leaves a._priv.C unmangled, both of which work today
and would regress under a scheme that lexed every segment separately.

Testing

• New unit test src/tests/unit-tests/parse_qualified_ident.ml asserts that seven split spellings
  parse to the same field_type structure as the canonical a.b.c.Inner, that the path segments
  and the from_root leading-dot marker survive, that a keyword segment works split or not, that
  builtins still resolve to builtins, that a split name works in a package declaration and a oneof
  field, and that a..b.C, a trailing dot and a lone dot are all rejected as parse errors rather
  than as Failure.
• Every hunk of the change is mutation-confirmed: a battery of 16 mutations, each reverting one
  hunk on its own (each qualified-name position, each normal_field and oneof_field label/option
  variant, the T_returns alternative, dropping the leading dot, making the continuation
  non-recursive, and dropping the dot between segments), is caught by the new test. No mutation
  survives. An earlier round of this battery is what caught that the map, rpc, extend and
  option-name positions were initially asserted by nothing.
• dune build @runtest --force: all existing tests pass unchanged, including Google unittest
  (which parses Google's real descriptor.proto and unittest.proto).
• Codegen is unchanged: running the before and after binaries over every .proto under src/
  (49 files) gives byte-identical generated .ml/.mli for the 37 that compile (76 files), and
  the identical error message for the 12 that do not.
• Differentially checked against protoc 23.2 over 33 inputs covering whitespace and comments in
  every qualified-name position, keyword and underscore segments, the dotted option-name forms, and
  float/int/hex literals: agreement went from 16/33 to 29/33, and the four remaining rows are cases
  where protoc errors for a semantic reason (an undefined option extension) while ocaml-protoc
  only parses, i.e. they confirm nothing regressed.

Known limitations

Three things I deliberately left alone, all pre-existing and orthogonal to #255. Happy to follow up
on any of them separately.

Whitespace around a dot inside an option name is still a parse error, so option a . b = 1; and
option (ext) . sub = 1; still fail while protoc parses both (its complaint about them is the
semantic Option "a" unknown). Option names are assembled by a different mechanism,
option_identifier, which concatenates adjacent items rather than going through qualified_ident.
Only the unbroken and dot-leading spellings are supported, as before.

A whitespace-split segment spelled e1 or E1 still escapes as Failure("float_of_string"), and
one spelled inf lexes as a float, so a . e1 . C and a . inf . C are rejected where protoc
accepts them. That is the pre-existing float-literal-versus-identifier ambiguity the lexer already
flags in its own TODO fix: somehow E1 for field identified get lexed into a float comment; the same
inputs failed before this change, and fixing it means tightening float_literal, which felt like a
separate change.

A non-first path segment beginning with _ is treated differently when the name is split, because
resolve_identifier mangles a standalone lexeme starting with _ to p + lexeme: a._priv.C gives
segment _priv, while a . _priv . C gives p_priv. Before this change the split spelling was a
hard crash, and the remaining failure mode is a loud "unresolved type" error rather than silent
corruption, so I left that pre-existing mangling rule alone rather than widen the diff.

I did not touch CHANGES.md, since it looks like entries are added there in the prepare for <version>
release commits rather than per PR, but glad to add one if you prefer.

---

AI assistance disclosure: this change was developed with AI assistance (Claude). The design was
chosen after differentially testing candidate approaches against protoc as an oracle, and all
results reported above were produced by running the builds and the test suite locally.