search

Darkyenus / glsl4idea

A GLSL language plugin for IntelliJ IDEA
GNU Lesser General Public License v3.0
100 stars 30 forks source link

Misleading variable name annotation #112

Open wyozi opened 8 years ago

wyozi commented 8 years ago

I would be happy to do this myself but posting here in case I forget or lose motivation.

For some variables you can deduce their usage. For example if the variable initializer is fragPos - lightPos you can pretty safely guess that it is the vector from light to frag or lightToFrag. Now if the variable name is fragToLight the plugin could add a warning annotation about it and maybe save some tears or at least wasted time.

This kind of analyzing doesn't work for all languages, but I think it'd be great for GLSL because of its limited use cases and somewhat consistent naming schemes.

Also the same code could later be used for naming suggestions if such thing is needed.

wyozi commented 8 years ago

This should actually be done on all expressions that have a "receiver" such as function call arguments.

Darkyenus commented 8 years ago

I would be very careful about assuming something about people's code. Fact that fragPos - lightPos is lightToFrag might seem obvious to you, in your codebase, but I might want to call it lightDelta. Now the warning is useless, because it can't recognize it. Would the annotation warn only if the naming scheme is strictly <something>To<something else>? That seems very arbitrary. And besides, I might want to have it "flipped", then the annotation is just confusing and annoying.

IntelliJ does this (for Java at least) in a very limited scope, only for some specific parameters, for example it warns when you assign something called x to parameter or variable named y. In this very limited scope it is useful, but usually it's just a false positive, at least in my experience, so I am not a big fan of it.

So, good idea, but unless you/someone comes up with a robust specification, I doubt it is worth implementing. Something like Parameter info (which works, but is kinda wonky) is much more useful for this kind of mistakes IMHO.

wyozi commented 8 years ago

xToY is a pattern I keep seeing in many GLSL snippets I find online and is a specific pattern I keep making simple logical errors with, so I have some extra motivation to create something that helps me avoid it :santa:. I think it will also be worth it if I made some kind of ExpressionAnalyzer component that could not only deduce expression 'goal' but also suggest a name. Then the same code could be used for variable introductions and it would at least sometimes seem more clever than camelCaseTypeName().

If you can think of a case where xToY would be correct name for x - y I'd like to hear that. Obviously if there's a valid usecase I should not implement the feature. As far as I'm concerned that kind of misnaming is almost a x <> y version of GLSL world.