File-level go.lint option

[ydnar]

This PR borrows code from https://golang.org/x/lint to automatically fix initialisms and stuttering in generated names.

Thanks to @Green7 for the initial implementation in #22!

[ydnar]

I’m not sure about this. It’s kind of gross, kind of magic.

[ydnar]

Feeling better about this now. @Green7 want to take a look?

[Green7]

Feeling better about this now. @Green7 want to take a look?

I tried and I have such observations:
If option go.lint is set, common initialisms are always used. Even if we set option (go.initialisms) = "MYOWN".
This can be problematic because we cannot turn off standard initialisms and they change type names too.
Also if we want to change only the enums all common initlialism will be changed.

Regarding the enums:
In this version I was unable to truncate the enum prefix. For example I have value OriginalEnum_A, and it has been changed to OriginalenumA but how to get A ?

[ydnar]

If option go.lint is set, common initialisms are always used. Even if we set option (go.initialisms) = "MYOWN".

This can be problematic because we cannot turn off standard initialisms and they change type names too.

Do you have an example of this? What do you want or expect to happen?

Also if we want to change only the enums all common initlialism will be changed.

What do you mean by this?

Regarding the enums:

In this version I was unable to truncate the enum prefix. For example I have value OriginalEnum_A, and it has been changed to OriginalenumA but how to get A ?

Idiomatic Go specifies that global vars or const values should be prefixed with the type name.

https://talks.golang.org/2014/names.slide#14

[Green7]

Do you have an example of this? What do you want or expect to happen?

Here is an example proto file:

syntax = "proto3";
package tests.enum;
import "patch/go.proto";

option go_package = "test/test";
option (go.lint) = true;
option (go.initialisms) = "ID";

enum OriginalEnum {
  INVALID = 0;
  A = 1;
  B = 2;
  C = 3;
}
message IdFieldsTest {
  string id = 1;
  string customer_id = 2;
  string api_path = 3;
}

I have: option (go.initialisms) = "ID";
So I expect that only fields named id will be changed.
But there are more changes:
ApiPath -> APIPath
IdFieldsTest -> IDFieldsTest
OriginalEnum_A -> OriginalenumA etc.

Idiomatic Go specifies that global vars or const values should be prefixed with the type name.

Yes, but i have codebase without prefixes because I used gogoprotobuf with option goproto_enum_prefix_all to generate my files. Now I want replace gogoprotobuf with protopatch and get the same result.

[ydnar]

@Green7 I added your examples as test cases and fixed an issue with enum value name generation.

Fixed: OriginalEnum_A → OriginalEnumA

Are you saying you don’t want all names like ApiPath to be linted?

[Green7]

Are you saying you don’t want all names like ApiPath to be linted?

I think it should be fully configurable. It should be possible to disable common initlialism if we don't want them.
Why? I have many proto files in my current project. Until now, I only changed the ID fields (through gogoprotobuf). If I use common initlialism now, I think many other fields will be fixed, which will result in a lot of code changes.
What's worse, some messages are saved to the database, which may also require conversion in the database.
Generally, this may be relevant for existing projects.

[ydnar]

Makes sense. In your case I'd recommend using an explicit (go.value).name option on the enum values with ID.

[Green7]

Makes sense. In your case I'd recommend using an explicit (go.value).name option on the enum values with ID.

I do not understand. It's not just about the enum values. I would like to change only the ID fields in all files without touching other names. How can this be achieved?

[ydnar]

Makes sense. In your case I'd recommend using an explicit (go.value).name option on the enum values with ID.

I do not understand. It's not just about the enum values. I would like to change only the ID fields in all files without touching other names. How can this be achieved?

For messages:

message ExampleId {
    option (go.message).name = "ExampleID";
    // ...
}

For fields:

message Account {
    string id = 1; [(go.field).name = "ID"];
}

etc.

[ydnar]

To be clear, it doesn’t make sense to use a hammer (linting/renaming) when you only want to selectively rename certain things. It’s better to be explicit at the declaration of the specific field or enum value you wish to rename, particularly if you only care about renames with ID.

[Green7]

For fields:

message Account {
    string id = 1; [(go.field).name = "ID"];
}

I have about 50 proto files. There are many fields in each with the name id or _id. Tagging each one separately is a lot of work. And I will have to remember to tag each newly added field as well.
Don't you think it would be simpler to be able to enable this option once for the entire file?

[ydnar]

For fields:

message Account {
    string id = 1; [(go.field).name = "ID"];
}

I have about 50 proto files. There are many fields in each with the name id or _id. Tagging each one separately is a lot of work. And I will have to remember to tag each newly added field as well.
Don't you think it would be simpler to be able to enable this option once for the entire file?

I actually think it’s simpler for you to add this to your fields. It’s one change, ideally a straightforward regexp find and replace. Assuming you have tests, it’s a single commit, and you’re done.

I prefer that to a specialized code path in this package that will have to be maintained forever just to rename some things with the word ID in them.

To explain further: a goal of this project is to have a minimal interface, with fewer options, and in general, one way to do something. Second, if something can be accomplished with a bit more typing, but is simpler and more understandable by folks reading the code, that’s preferable to a global option.

[Green7]

To explain further: a goal of this project is to have a minimal interface, with fewer options, and in general, one way to do something. Second, if something can be accomplished with a bit more typing, but is simpler and more understandable by folks reading the code, that’s preferable to a global option.

I understand your point of view: but I have a different. For me, a tool is good when it allows me to achieve the effect I want. gogoprotobuf was such a tool, it was easy to add any code modifying field names.

Summarizing my attempt to make changes in protopatch:

• I can set the id fields manually (through (go.value).name) exactly as before
• I can't disable enum prefixes
  Basically nothing has changed and after 25 days I am at the starting point. I am honestly disappointed :(

[ydnar]

• I can't disable enum prefixes

This is a separate feature, and I agree it’s worth implementing.

[Green7]

• I can't disable enum prefixes

This is a separate feature, and I agree it’s worth implementing.

that's good 👍 when it may be available ?
After thinking: I will try to enable the liner option in my project - it will correct the ID fields and others. Then for the other fields I will set old name (through go.field), or change the code.
It will probably be a lot less work. This could be the solution.