{"action":"created","issue":{"url":"https://api.github.com/repos/jshint/jshint/issues/2546","labels_url":"https://api.github.com/repos/jshint/jshint/issues/2546/labels{/name}","comments_url":"https://api.github.com/repos/jshint/jshint/issues/2546/comments","events_url":"https://api.github.com/repos/jshint/jshint/issues/2546/events","html_url":"https://github.com/jshint/jshint/pull/2546","id":94569263,"number":2546,"title":"Loopfunc fixes","user":{"login":"lukeapage","id":309321,"avatar_url":"https://avatars.githubusercontent.com/u/309321?v=3","gravatar_id":"","url":"https://api.github.com/users/lukeapage","html_url":"https://github.com/lukeapage","followers_url":"https://api.github.com/users/lukeapage/followers","following_url":"https://api.github.com/users/lukeapage/following{/other_user}","gists_url":"https://api.github.com/users/lukeapage/gists{/gist_id}","starred_url":"https://api.github.com/users/lukeapage/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lukeapage/subscriptions","organizations_url":"https://api.github.com/users/lukeapage/orgs","repos_url":"https://api.github.com/users/lukeapage/repos","events_url":"https://api.github.com/users/lukeapage/events{/privacy}","received_events_url":"https://api.github.com/users/lukeapage/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/jshint/jshint/labels/Needs%20review","name":"Needs review","color":"fef2c0"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2015-07-12T14:26:05Z","updated_at":"2015-07-27T06:55:00Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/jshint/jshint/pulls/2546","html_url":"https://github.com/jshint/jshint/pull/2546","diff_url":"https://github.com/jshint/jshint/pull/2546.diff","patch_url":"https://github.com/jshint/jshint/pull/2546.patch"},"body":"[[FIX]] use of params is not capturing loopfunc\r\nPreviously the scope manager only considered the function body for usages\r\nthat went outside scope, indicating \"capturing\" that should trigger\r\nloopfunc. This corrects that.\r\n\r\nAlso affected are function names, but in the case of declarations there is\r\na different warning and in the case of expressions the name available to\r\nthe function will be correct even within a loop.\r\n\r\nNote - see this code\r\n```\r\nvar a = function b(arg) { if (!arg) { b(true); } else console.log(\"first\"); }, c = function b(arg) { if (!arg) { b(true); } else console.log(\"second\"); }; a(); c();\r\n```\r\n\r\n[[FIX]] Report loopfunc for all function types\r\nPreviously only function expressions caused loopfunc, meaning arrow\r\nfunctions that captured variables were not warned.\r\nFixes #2153\r\n\r\nNote: if you consider this breaking code because it will start warning about loopfunc's (I think it is enough within the spirit of the option and obscure enough that its okay in a minor release) then I'll split into 2 PR's as the first commit should really go in asap."},"comment":{"url":"https://api.github.com/repos/jshint/jshint/issues/comments/125106381","html_url":"https://github.com/jshint/jshint/pull/2546#issuecomment-125106381","issue_url":"https://api.github.com/repos/jshint/jshint/issues/2546","id":125106381,"user":{"login":"lukeapage","id":309321,"avatar_url":"https://avatars.githubusercontent.com/u/309321?v=3","gravatar_id":"","url":"https://api.github.com/users/lukeapage","html_url":"https://github.com/lukeapage","followers_url":"https://api.github.com/users/lukeapage/followers","following_url":"https://api.github.com/users/lukeapage/following{/other_user}","gists_url":"https://api.github.com/users/lukeapage/gists{/gist_id}","starred_url":"https://api.github.com/users/lukeapage/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lukeapage/subscriptions","organizations_url":"https://api.github.com/users/lukeapage/orgs","repos_url":"https://api.github.com/users/lukeapage/repos","events_url":"https://api.github.com/users/lukeapage/events{/privacy}","received_events_url":"https://api.github.com/users/lukeapage/received_events","type":"User","site_admin":false},"created_at":"2015-07-27T06:55:00Z","updated_at":"2015-07-27T06:55:00Z","body":"> I think the \"proper\" way to handle this mid-review is to use a merge commit that can be squashed prior to landing. I've never done that before, though, so if it's wrong or too much of a hassle, feel free to simply rebase (but please keep non-conflict-related changes in separate commits).\r\n\r\nI think that would work as long as you squash before rebasing. In the past I've had a branch with merges and wanted to keep the commits and rebased, but then you find you have to re-do the conflicts with each commit."}}