-
Notifications
You must be signed in to change notification settings - Fork 16.1k
Closed as not planned
Closed as not planned
Copy link
Labels
inactiveDenotes the issue/PR has not seen activity in the last 90 days.Denotes the issue/PR has not seen activity in the last 90 days.
Description
Most field options look like any other options that have a "compact" representation (such as enum values and extension ranges):
- The
optionsfield (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 (withoptionskeyword, 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
defaultoption is associated with a span that represents only the value and does not include thedefault =part of the declaration. - The
json_nameoption is associated with two spans: one that represents the entire declaration (like normal options), and also a second that represents only the value (similar todefault).
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]
}Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
inactiveDenotes the issue/PR has not seen activity in the last 90 days.Denotes the issue/PR has not seen activity in the last 90 days.