search

nikgoodley-ibboost / jslint4java

Automatically exported from code.google.com/p/jslint4java
Other
0 stars 0 forks source link

Unused variables not being reported #78

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1.Run this code through jslint4java
        // Filter level 1
        $('#level1 div.nav-con div.ArrayCollection').each(function(i, o) {
            var iteratedLevel1Id = jQuery.data(o, 'level1Id');
            if (iteratedLevel1Id === level1IdToShow) {
                $(o).show();
            } else {
                $(o).hide();
            }
        });

2.Run this code on jslint at jslint.com or using other jslint text editor 
tools. 

What is the expected output? What do you see instead?

 - the unused variable i should be reported but isnt in jslint4java.

What version of the product are you using? On what operating system?

 - 2.0.2

Please provide any additional information below.

 - running via powershell. Other errors being reported as expected but not any unused vars. Config: --report "junit" --sloppy --plusplus --sub --vars  --predef "$, jQuery, Sys, Window, SWFAddress, SWFAddressEvent, document" --maxerr 1000 --evil

Original issue reported on code.google.com by dontfoll...@gmail.com on 6 Mar 2012 at 3:51

GoogleCodeExporter commented 9 years ago 
I see the same thing you do:

% java -jar /opt/misc/jslint4java/jslint4java-2.0.2.jar --report "junit" 
--sloppy --plusplus --sub --vars  --predef "$, jQuery, Sys, Window, SWFAddress, 
SWFAddressEvent, document" --maxerr 1000 --evil ~/Desktop/issue78.js 
<testsuites>
<testsuite failures='1' time='0.076' skipped='0' errors='1' tests='1' 
name='/Users/dom/Desktop/issue78.js'><testcase time='0.076' 
classname='com.googlecode.jslint4java' 
name='/Users/dom/Desktop/issue78.js'><failure message='Found 2 problems' 
type='java.lang.AssertionError'>/Users/dom/Desktop/issue78.js:2:59:Expected 
exactly one space between 'function' and '('.
/Users/dom/Desktop/issue78.js:4:30:'level1IdToShow' was used before it was 
defined.
</failure></testcase></testsuite>
</testsuites>

However, if I switch the report type to the HTML report:

% java -jar /opt/misc/jslint4java/jslint4java-2.0.2.jar --report "report" 
--sloppy --plusplus --sub --vars  --predef "$, jQuery, Sys, Window, SWFAddress, 
SWFAddressEvent, document" --maxerr 1000 --evil ~/Desktop/issue78.js
<html><head></head><body>
<div class='file'><h1 
id='/Users/dom/Desktop/issue78.js'>/Users/dom/Desktop/issue78.js</h1><div 
id=errors><i>Error:</i><p>Problem at line 2 character 59: Expected exactly one 
space between 'function' and '('.</p><p class=evidence>$('#level1 div.nav-con 
div.ArrayCollection').each(function(i, o) {</p><p>Problem at line 4 character 
30: 'level1IdToShow' was used before it was defined.</p><p class=evidence>    
if (iteratedLevel1Id === level1IdToShow) {</p><p><i>Undefined variable:</i> 
<code><u>level1IdToShow</u></code> <i>2 </i> 
<small>'each'</small></p><p><i>Unused variable:</i> <code><u>i</u></code> <i>2 
</i> <small>'each'</small></p></div><br><div id=functions><div><i>Global</i> $, 
jQuery</div><br><div class=function><i>2</i> 'each'(i, 
o)</div><div><i><big>Unused</big></i> i</div><div><i>Variable</i> 
iteratedLevel1Id</div><div><i>Global</i> $, jQuery</div><br><pre 
id=properties>/*properties<br>    <i>data</i>, <i>each</i>, <i>hide</i>, 
<i>show</i><br>*/</pre></div></div></div>
</body></html>

You can see the “unused variable” mentioned in there.  Unfortunately, this 
is not reported back to jslint4java via the usual mechanisms, only in the HTML 
report. 

Just to check I've downloaded the latest JSLint and verified that the same 
behaviour occurs.

Original comment by d...@happygiraffe.net on 6 Mar 2012 at 6:33

GoogleCodeExporter commented 9 years ago 
Thanks for confirming the bug.

Do you think this might be looked at for a later release? 

We're hoping to use the tool as part of our server build to run a JsLint after 
check-in. 

On the developers' machines we're looking at having a plugin like JsLint.VS2010 
to validate as part of the local build. This would catch unused variables 
before the dev could check in for the situation above. This isnt failsafe 
though of course because the dev could always alter their local config then our 
server build with this version of JsLint4Java would miss it.

thanks again

Original comment by dontfoll...@gmail.com on 7 Mar 2012 at 10:29

GoogleCodeExporter commented 9 years ago 
One more thing - Just noticed the same for "Line too long" errors in the above 
example. They show when using JsLint.VS2010 but not with jslint4java as 
described above.

Original comment by dontfoll...@gmail.com on 7 Mar 2012 at 2:33

GoogleCodeExporter commented 9 years ago 
Given what the JSLINT() function reports back, I'm unlikely to be able to 
support this any time soon.  Have a look at the supported APIs:

https://github.com/douglascrockford/JSLint/blob/master/jslint.js

The only API that returns the necessary information is the HTML report.  In 
order to usefully interpret that, I'd have to end up parsing it, which is quite 
fragile.  Although, looking at the source code for the report function, it 
appears that most of the information is exposed via the .data() function, which 
we already extract.  Let me have a think about this.

Original comment by d...@happygiraffe.net on 7 Mar 2012 at 8:03

GoogleCodeExporter commented 9 years ago 
Fixed in: 9ffed2ce

https://github.com/happygiraffe/jslint4java/commit/9ffed2ceb7cf2201938ef9022f061
4024a5d1496

Original comment by d...@happygiraffe.net on 13 Mar 2012 at 8:26

GoogleCodeExporter commented 9 years ago

Original comment by d...@happygiraffe.net on 14 Mar 2012 at 7:59

GoogleCodeExporter commented 9 years ago 
This is marked as fixed, but it still isn't working.  If it isn't supportable, 
then okay, but why is it marked as fixed?

Original comment by earth2m...@gmail.com on 28 Mar 2013 at 11:04

GoogleCodeExporter commented 9 years ago 
It is fixed, but you need to add a --warnings flag.  This is less than ideal, 
so it should be made the default.

Original comment by d...@happygiraffe.net on 10 Apr 2013 at 7:44