Khan / structuredjs

Test JavaScript code, look for functionality.
http://khan.github.io/structuredjs/
196 stars 19 forks source link

Unexpected match of structures #20

Closed pamelafox closed 8 years ago

pamelafox commented 8 years ago

Replicate in demo: http://khan.github.io/structuredjs/

Put this pattern on left: sunSize = _; draw = function() { var sunSize = _; };

Put this pattern on right:

noStroke();
// the beautiful blue sky
background(82, 222, 240);

// the starting size for the sun
var sunSize = 30;
draw = function() {
    // The sun, a little circle on the horizon

    fill(255, 204, 0);
    ellipse(200, 298, sunSize, sunSize);

    // The land, blocking half of the sun
    fill(76, 168, 67);
    rect(0, 300, 400, 100);
    sunSize = sunSize + 1;
    sunSize = sunSize + 1;

};

Notice it matches.

If you add a var in front of the draw on both sides, it does not match. So something about the variable definition vs declaration seems to be resulting in a hoisting or mismatch.

mr-martian commented 8 years ago

var x = 3; expands to var x; x = 3;, which may be connected, though it doesn't explain var sunSize = _; matching sunSize = sunSize + 1;.

Also, if you put in

sunSize = _;
draw = function() {
var sunSize = _;
};

and

var sunSize = 30;
draw = function() {
    sunSize = sunSize + 1;
};

it matches, but if you remove any of the lines the match fails.

mr-martian commented 8 years ago

I really have no idea why it's doing that, but I cloned the repo and ran it locally and it gave the correct answer, so it would seem that this was fixed some time in the last year and the gh-pages branch just hasn't been updated.

Interestingly, if you add a line at the end (it doesn't seem to matter what it is), the match will fail.

Also, if you switch it around and have var ouside on the left and inside on the right it will fail.

pamelafox commented 8 years ago

Okay, I re-pushed gh-pages branch and it is showing false now. So I'll put an issue in the live-editor repo to update structuredjs. It's scary to do that since we don't have a way to re-run all our regression tests, but it might be worth it.