Skip to content

protoc: source code info for field options is inconsistent #10478

@jhump

Description

@jhump

Most field options look like any other options that have a "compact" representation (such as enum values and extension ranges):

  • The options field (number 8) is associated with a span that represents the entire options section, including enclosing [ and ].
  • Options therein are associated with a span that represents the option declaration, name = value. This is consistent with the longer options form (with options keyword, ending in ;), since they need to be able to include leading and trailing comments around the full declaration.

But the default and json_name pseudo-options do not behave this way.

  • The default option is associated with a span that represents only the value and does not include the default = part of the declaration.
  • The json_name option is associated with two spans: one that represents the entire declaration (like normal options), and also a second that represents only the value (similar to default).

Take this test file for example:

syntax = "proto2";

package foo.bar;

import "google/protobuf/descriptor.proto";

extend google.protobuf.FieldOptions {
  optional int64 abc = 10000;
}

message M {
  optional string name = 1 [
    default = "blahblah",
    json_name = "_NAME_",
    deprecated = false,
    (foo.bar.abc) = -99
  ];
}

It produces source code info for options and pseudo-options with locations like so:

    // entire options block, from "[" to "]"
    location {
      path: [4, 0, 2, 0, 8]
      span: [11, 27, 16, 3]
    }
    // the field's default_value, span includes only the value
    location {
      path: [4, 0, 2, 0, 7]
      span: [12, 14, 24]
    }
    // the field's json_name, span includes entire declaration
    location {
      path: [4, 0, 2, 0, 10]
      span: [13, 4, 24]
    }
    // the field's json_name (again), span includes only the value
    location {
      path: [4, 0, 2, 0, 10]
      span: [13, 16, 24]
    }
    // the deprecated option, span includes entire declaration
    location {
      path: [4, 0, 2, 0, 8, 3]
      span: [14, 4, 22]
    }
    // the (foo.bar.abc) custom option, span includes entire declaration
    location {
      path: [4, 0, 2, 0, 8, 10000]
      span: [15, 4, 23]
    }

But, for consistency, I assert it should instead produce locations that look like so:

    location {
      path: [4, 0, 2, 0, 8]
      span: [11, 27, 16, 3]
    }
    location {
      path: [4, 0, 2, 0, 7]
      span: [12, 4, 24]
    }
    location {
      path: [4, 0, 2, 0, 10]
      span: [13, 4, 24]
    }
    location {
      path: [4, 0, 2, 0, 8, 3]
      span: [14, 4, 22]
    }
    location {
      path: [4, 0, 2, 0, 8, 10000]
      span: [15, 4, 23]
    }

Metadata

Metadata

Labels

inactiveDenotes the issue/PR has not seen activity in the last 90 days.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions