Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading variable name annotation #112

Open
wyozi opened this issue Dec 18, 2015 · 3 comments
Open

Misleading variable name annotation #112

wyozi opened this issue Dec 18, 2015 · 3 comments

Comments

@wyozi
Copy link
Contributor

wyozi commented Dec 18, 2015

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
Copy link
Contributor Author

wyozi commented Dec 18, 2015

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

@Darkyenus
Copy link
Owner

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
Copy link
Contributor Author

wyozi commented Dec 18, 2015

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 🎅. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants