search

russplaysguitar / UnderscoreCF

An UnderscoreJS port for Coldfusion. Functional programming library.
http://russplaysguitar.github.com/UnderscoreCF/
MIT License
89 stars 38 forks source link

Fix ambiguous scoping issue #53

Closed redler closed 2 years ago

redler commented 8 years ago

In cases where the optional to argument is not provided to slice(), the scoping of arguments.to on line 881 fails because arguments.to is scoped too specifically. The unscoped to assignment on line 873 is thus not an overwriting of arguments.to, but instead the creation of a non-locally-scoped variable (in effect, variables.to, depending on the cfml engine being used and settings in effect).

This change removes the ambiguity by locally var-scoping calculatedTo and calculatedFrom. The latter is technically unnecessary because the from argument includes a default value and therefore always exists, but I added it for symmetry and clarity.

russplaysguitar commented 8 years ago

Looks like this breaks a Railo 4.0 test? 

[mxunitoutputtask]   ✕ testSlice: Expected arrays to match but did not. See debug output for a visual display of the differences. Data mismatch. . :: Expected [row 1: 2 row 2: 3] BUT RECEIVED [row 1: 1 row 2: 2]. These values should be the same. 
[mxunitoutputtask]     Expected arrays to match but did not. See debug output for a visual display of the differences. Data mismatch. . :: Expected [row 1: 2
[mxunitoutputtask] row 2: 3] BUT RECEIVED [row 1: 1
[mxunitoutputtask] row 2: 2]. These values should be the same.  at /home/travis/work/railo/webapps/www/mxunit/framework/Assert.cfc:176 at /home/travis/work/railo/webapps/www/mxunit/framework/Assert.cfc:267 at /home/travis/work/railo/webapps/www/mxunit/framework/Assert.cfc:361 at /home/travis/work/railo/webapps/www/mxunit/framework/Assert.cfc:254 at /home/travis/build/russplaysguitar/UnderscoreCF/mxunit_tests/examplesTest.cfc:253 at /home/travis/work/railo/webapps/www/mxunit/framework/TestCase.cfc:141 at /home/travis/work/railo/webapps/www/mxunit/framework/decorators/DataProviderDecorator.cfc:31 at /home/travis/work/railo/webapps/www/mxunit/framework/TestSuiteRunner.cfc:105 at /home/travis/work/railo/webapps/www/mxunit/framework/TestSuiteRunner.cfc:55 at /home/travis/work/railo/webapps/www/mxunit/framework/TestSuite.cfc:131 at /home/travis/work/railo/webapps/www/mxunit/runner/DirectoryTestSuite.cfc:37 at /home/travis/build/russplaysguitar/UnderscoreCF/mxunit_tests/ci/HttpAntRunner.cfc:28

See: https://travis-ci.org/russplaysguitar/UnderscoreCF/jobs/123993333

russplaysguitar commented 8 years ago

Let me know if you think this is unrelated. Thanks for the contribution!

redler commented 8 years ago

I don't know enough about mxUnit to judge, but those test failures look like something incidental going on at the framework level, and possibly not related to this change.

In the first commit I had failed to change the final usage of from to calculatedFrom. The revised function now seems to work fine, passing its actual unit tests in isolation, as per this ad hoc test gist. This can be loaded and run on trycf.com -- so if it runs there, perhaps it is a problem with the testing machinery here.

redler commented 8 years ago

Ah, I see from the other pull request that the CI tests fail generally.