judofyr / temple

Template compilation framework in Ruby
http://judofyr.net/posts/temple.html
MIT License
491 stars 53 forks source link

AttributeRemover with remove_empty_attrs generates code that causes a "useless use of a variable" warning #125

Closed wilkie closed 1 year ago

wilkie commented 5 years ago

Description

Originally reported in slim-template/slim#829.

When slim uses :remove_empty_attrs for its "class" attribute, any template that assigns a variable to the attribute, e.g.: ul class=my_variable will generate a warning at that line in the template as such:

page.slim:2: warning: possibly useless use of a variable in void context

The problem lies in the generated code that temple produces, specifically an instance where :capture yields a string variable as a expression not as the last statement of a block but rather followed immediately by an if statement.

For more information specific to the slim case, refer to the linked issue.

Expected

Either fixing the problem causing the warning or documenting a workaround to avoid the warning, particularly of use to developers of slim.

The postamble for :capture and the generators seems over-relying on the idea that :capture will be contained within a block and be at the end of such a block. Not sure the best way to avoid the warning and not hurt dependent behavior.

More Information

Looking at AttributeRemover#on_html_attr, when an attribute within the remove_empty_attrs list is being compiled, and that value is dynamic, it will compile to a :multi containing a :capture that wraps and eventually evals the dynamic value followed by an :if that will omit the attribute if that captured value is empty. A.K.A. it removes an attribute that would resolve to an empty string or list.

Since :capture, using the provided generators at least, will resolve to a [preamble, ..., postamble] where the preamble is the initialization of the variable and the postable is, by virtue of a complex relationship between Generator#call, Generator#postamble and the Generators::StringBuffer#return_buffer methods, is just the variable name by itself.

The variable name by itself followed by an if statement mean the code looks something like:

...
tmp = '';
...
tmp;
if !tmp.empty?
...

Causing a warning on the tmp; since it is, in fact, useless.

Environment

ruby: 2.6.3p62 slim: 4.0.1 (from rubygems) temple: 0.8.1 tilt: 2.0.9 sinatra: 2.0.5

kolen commented 4 years ago

I no longer see this warning in our codebase after upgrading to temple 0.8.2. Not 100% sure that it isn't caused by other changed things.

kbrock commented 1 year ago

Is this still a problem that we are seeing?

minad commented 1 year ago

@kolen This issue has been fixed, right? Please reopen if that's not the case.