uber / prototool

Your Swiss Army Knife for Protocol Buffers
MIT License
5.04k stars 345 forks source link

Inconsistent linter naming restrictions #511

Closed aquartulli closed 5 years ago

aquartulli commented 5 years ago

Caught this inconsistency when trying to figure out why my C# namespace was not allowed to be properly Pascal cased:

https://github.com/uber/prototool/blob/dev/internal/lint/check_file_options_equal.go checks that the C# namespace is the capitalized version of the package name.

https://github.com/uber/prototool/blob/dev/internal/lint/check_package_lower_snake_case.go checks if the package name is lower snake case.

This would suggest that if you want your C# namespace to be FooBar then the package name needs to be foo_bar. However https://github.com/uber/prototool/blob/dev/internal/lint/check_package_lower_case.go checks that the package name is lower case and disallows underscores. This seems to conflict with the other linter's intentions.

smaye81 commented 5 years ago

If you want your C# namespace to be Foobar then you would set your package as:

package foobar.v1;
option csharp_namespace = "Foobar.V1";

Likewise if you want your namespace to be Foo_bar, you would set your package as:

package foo_bar.v1;
option csharp_namespace = "Foo_bar.V1";

Following these should hopefully pass the linter.

smaye81 commented 5 years ago

Closing this. Please reopen if there are any additional problems.

Lette commented 3 years ago

@smaye81, this is not resolved. In your response, you did not consider the case where the desired namespace is FooBar.V1.

How do you make the linter happy when you want to have a PascalCased identifier in your C# namespace?

The code below will make the linter fail with Expected "Product.Foobar.V1" for option [...] (note the lower-case 'b' in 'bar'):

package product.foobar.v1;
option csharp_namespace = "Product.FooBar.V1";

package product.fooBar.v1; and package product.foo_bar.v1; fails with Packages should only contain characters in the range a-z0-9 and periods [...]