Fix and disallow all snake_case style warnings

[jdm]

https://github.com/servo/servo/blame/5c7a4db5f4e374dee287d403d90c7bc0e07ac9d0/components/script/lib.rs#L12 was added several years ago. We should remove it and fix the existing violations that have been added to the code in that time.

[highfive]

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

[kunalmohan]

@highfive: assign me

[highfive]

Hey @kunalmohan! Thanks for your interest in working on this issue. It's now assigned to you!

[kunalmohan]

How should I translate camelCase methods (of webidl) to snake_case? I found js_name attribute in wasm-bindgen (#[wasm_bindgen(js_name = jsOftenUsesCamelCase)]). Is there some other way, maybe specific to servo that I am missing out?

[pshaughn]

That must be why ignore_snake_case was on in the first place. Maybe we want just the autogenerated method names from the webidl to be exempt from this, somehow?

[jdm]

Yes, we can apply that attribute to specific modules or trait implementations when necessary, I suppose. We can start by making use of the ignored_warnings argument to CGImports in CodegenRust.py - try adding it to the list in https://github.com/servo/servo/blob/master/components/script/dom/bindings/codegen/CodegenRust.py#L1958, and then for any generated files still causing trouble look for uses like

servo/components/script/dom/bindings/codegen/CodegenRust.py

Line 2465 in 90b0a7f

return CGImports(CGList(unionStructs, "\n\n"),

and add it if necessary.

[kunalmohan]

Adding 'non_snake_case' to the list

servo/components/script/dom/bindings/codegen/CodegenRust.py

Lines 1958 to 1965 in 1de9290

	ignored_warnings = [
	'non_camel_case_types',
	'non_upper_case_globals',
	'unused_imports',
	'unused_variables',
	'unused_assignments',
	'unused_mut',
	]

doesn't seem to reduce any warnings.
Adding it to

servo/components/script/dom/bindings/codegen/CodegenRust.py

Line 2473 in 1de9290

ignored_warnings=[])

and

servo/components/script/dom/bindings/codegen/CodegenRust.py

Line 7564 in 1de9290

], config=config, ignored_warnings=[])

gave the respective errors-

error: an inner attribute is not permitted in this context
 --> /mnt/sda1/Documents/webpages/rust/servo/target/debug/build/script-34e34b4c91111f3b/out/RegisterBindings.rs:1:1
  |
1 | #![allow(non_snake_case)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.

error: an inner attribute is not permitted in this context
 --> /mnt/sda1/Documents/webpages/rust/servo/target/debug/build/script-34e34b4c91111f3b/out/UnionTypes.rs:3:1
  |
3 | #![allow(non_snake_case)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files. Outer attributes, like `#[test]`, annotate the item following them.

[jdm]

Can you give a few examples of some of the warnings?

[jdm]

One other possibility is changing this line to self.cgRoot = CGList(CGGeneric('#![allow(non_snake_case)]'), cgThings)

[kunalmohan]

warning: method `Constructor` should have a snake case name
  --> components/script/dom/audiobuffer.rs:93:12
   |
93 |     pub fn Constructor(
   |            ^^^^^^^^^^^ help: convert the identifier to snake case (notice the capitalization): `constructor`

warning: variable `txPower` should have a snake case name
  --> components/script/dom/bluetoothadvertisingevent.rs:59:9
   |
59 |         txPower: Option<i8>,
   |         ^^^^^^^ help: convert the identifier to snake case: `tx_power`

[pshaughn]

txPower actually should have a snake case name, and tx_power is the correct suggestion, if I understand Servo's style correctly. Constructor's a word the codegen is putting in a trait, so it should probably be exempt?

[kunalmohan]

Yes, I think so too. There are a large number of variables and functions that come under the warnings. I guess most of the variables` names need to be changed whereas the functions are put in by the codegen and should be exempt.

[jdm]

I can't figure out where the actual Constructor name comes from in the code generator, so go ahead and add #[allow(non_snake_case)] for each instance.

[kunalmohan]

One other possibility is changing this line to self.cgRoot = CGList(CGGeneric('#![allow(non_snake_case)]'), cgThings)

This isn't working: python TypeError (I guess, because of the if block which changes cgThings type)

servo/components/script/dom/bindings/codegen/CodegenRust.py

Lines 6335 to 6344 in 1de9290

	cgThings = CGWrapper(CGNamespace(toBindingNamespace(descriptor.name),
	cgThings, public=True),
	post='\n')
	if reexports:
	reexports = ', '.join(map(lambda name: reexportedName(name), reexports))
	cgThings = CGList([CGGeneric('pub use self::%s::{%s};' % (toBindingNamespace(descriptor.name), reexports)),
	cgThings], '\n')
	self.cgRoot = cgThings

So I tried

if reexports:
            reexports = ', '.join(map(lambda name: reexportedName(name), reexports))
            cgThings = CGList([CGGeneric('#![allow(non_snake_case)]'), CGGeneric('pub use self::%s::{%s};' % (toBindingNamespace(descriptor.name), reexports)),
                               cgThings], '\n')
        else:
            cgThings = CGList([CGGeneric('#![allow(non_snake_case)]'), cgThings], '\n')

        self.cgRoot = cgThings

which again gave the error error: an inner attribute is not permitted in this context

[kunalmohan]

I can't figure out where the actual Constructor name comes from in the code generator, so go ahead and add #[allow(non_snake_case)] for each instance.

Only for the constructors? (because there are hundreds of warnings, constructors alone are more than a hundred I guess 😅 )
(I'll probably have to write a script to do so)

[jdm]

For non-constructor methods of traits, you can probably add #[allow(non_snake_case)] right before the impl FooMethods for Foo to avoid lots of those warnings.