GitHub PullRequestReviewCommentEvent at 2014-10-03 21:06:17
Repository: jshint
Actor: valueof
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/jshint/jshint/pulls/comments/18419566","id":18419566,"diff_hunk":"@@ -3685,10 +3686,18 @@ var JSHINT = (function () {\n       advance(\"*\");\n       generator = true;\n     }\n+\n     var i = optionalidentifier();\n-    doFunction(i, undefined, generator);\n+    var fn = doFunction(i, undefined, generator);\n+\n+    function isNotLocal(name) {\n+      return name[0] !== \"(\" && fn[name] !== \"var\";","path":"src/jshint.js","position":18,"original_position":18,"commit_id":"d3bcffcb3a2dd6d8f7a3f51b0f2d7db7b4952cf9","original_commit_id":"d3bcffcb3a2dd6d8f7a3f51b0f2d7db7b4952cf9","user":{"login":"valueof","id":49106,"avatar_url":"https://avatars.githubusercontent.com/u/49106?v=2","gravatar_id":"","url":"https://api.github.com/users/valueof","html_url":"https://github.com/valueof","followers_url":"https://api.github.com/users/valueof/followers","following_url":"https://api.github.com/users/valueof/following{/other_user}","gists_url":"https://api.github.com/users/valueof/gists{/gist_id}","starred_url":"https://api.github.com/users/valueof/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/valueof/subscriptions","organizations_url":"https://api.github.com/users/valueof/orgs","repos_url":"https://api.github.com/users/valueof/repos","events_url":"https://api.github.com/users/valueof/events{/privacy}","received_events_url":"https://api.github.com/users/valueof/received_events","type":"User","site_admin":false},"body":"Are you thinking something along these lines?\r\n\r\n```javascript\r\n    function isVariable(name) { return name[0] !== \"(\"; }\r\n    function isLocal(name) { return fn[name] === \"var\"; }\r\n\r\n    if (!state.option.loopfunc && funct[\"(loopage)\"]) {\r\n      if (_.some(fn, function (val, name) { return isVariable(name) && !isLocal(name); })) {\r\n        warning(\"W083\");\r\n      }\r\n    }\r\n```","created_at":"2014-10-03T21:06:17Z","updated_at":"2014-10-03T21:06:17Z","html_url":"https://github.com/jshint/jshint/pull/1887#discussion_r18419566","pull_request_url":"https://api.github.com/repos/jshint/jshint/pulls/1887","_links":{"self":{"href":"https://api.github.com/repos/jshint/jshint/pulls/comments/18419566"},"html":{"href":"https://github.com/jshint/jshint/pull/1887#discussion_r18419566"},"pull_request":{"href":"https://api.github.com/repos/jshint/jshint/pulls/1887"}}},"pull_request":{"url":"https://api.github.com/repos/jshint/jshint/pulls/1887","id":22215453,"html_url":"https://github.com/jshint/jshint/pull/1887","diff_url":"https://github.com/jshint/jshint/pull/1887.diff","patch_url":"https://github.com/jshint/jshint/pull/1887.patch","issue_url":"https://api.github.com/repos/jshint/jshint/issues/1887","number":1887,"state":"open","locked":false,"title":"Don't raise W083 if a function never touches the outer scope","user":{"login":"valueof","id":49106,"avatar_url":"https://avatars.githubusercontent.com/u/49106?v=2","gravatar_id":"","url":"https://api.github.com/users/valueof","html_url":"https://github.com/valueof","followers_url":"https://api.github.com/users/valueof/followers","following_url":"https://api.github.com/users/valueof/following{/other_user}","gists_url":"https://api.github.com/users/valueof/gists{/gist_id}","starred_url":"https://api.github.com/users/valueof/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/valueof/subscriptions","organizations_url":"https://api.github.com/users/valueof/orgs","repos_url":"https://api.github.com/users/valueof/repos","events_url":"https://api.github.com/users/valueof/events{/privacy}","received_events_url":"https://api.github.com/users/valueof/received_events","type":"User","site_admin":false},"body":"**W083** warns you against using functions within loops, however, it generates a lot of false positives. This patch reduces the number of false positives by checking whether functions access non-local variables. If a function uses only its local variables, then it is safe to be used within loops:\r\n\r\n```javascript\r\nfor (var i = 0; i < 10; i++) {\r\n  // This is pretty safe and shouldn't generate W083.\r\n  doSomething(function () { console.log(\"Hello\"); });\r\n}\r\n```\r\n\r\nThe crux of this implementation is in and around the **isNotLocal** function. **doFunction** returns an object representing the function it just parsed. This object has two types of properties: special properties that start with `(` (e.g. `(scope)`) and all the variables used within that function, by name. Each variable has a value that constitutes its type: **global**, **outer**, **var**, etc. We're interested in **var** because it signifies local variables.","created_at":"2014-10-03T16:56:46Z","updated_at":"2014-10-03T21:06:17Z","closed_at":null,"merged_at":null,"merge_commit_sha":"b73375f37716d951662b81bed9288fa116f98daf","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/jshint/jshint/pulls/1887/commits","review_comments_url":"https://api.github.com/repos/jshint/jshint/pulls/1887/comments","review_comment_url":"https://api.github.com/repos/jshint/jshint/pulls/comments/{number}","comments_url":"https://api.github.com/repos/jshint/jshint/issues/1887/comments","statuses_url":"https://api.github.com/repos/jshint/jshint/statuses/d3bcffcb3a2dd6d8f7a3f51b0f2d7db7b4952cf9","head":{"label":"jshint:valueof-better-funcloop","ref":"valueof-better-funcloop","sha":"d3bcffcb3a2dd6d8f7a3f51b0f2d7db7b4952cf9","user":{"login":"jshint","id":1335450,"avatar_url":"https://avatars.githubusercontent.com/u/1335450?v=2","gravatar_id":"","url":"https://api.github.com/users/jshint","html_url":"https://github.com/jshint","followers_url":"https://api.github.com/users/jshint/followers","following_url":"https://api.github.com/users/jshint/following{/other_user}","gists_url":"https://api.github.com/users/jshint/gists{/gist_id}","starred_url":"https://api.github.com/users/jshint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jshint/subscriptions","organizations_url":"https://api.github.com/users/jshint/orgs","repos_url":"https://api.github.com/users/jshint/repos","events_url":"https://api.github.com/users/jshint/events{/privacy}","received_events_url":"https://api.github.com/users/jshint/received_events","type":"Organization","site_admin":false},"repo":{"id":1272666,"name":"jshint","full_name":"jshint/jshint","owner":{"login":"jshint","id":1335450,"avatar_url":"https://avatars.githubusercontent.com/u/1335450?v=2","gravatar_id":"","url":"https://api.github.com/users/jshint","html_url":"https://github.com/jshint","followers_url":"https://api.github.com/users/jshint/followers","following_url":"https://api.github.com/users/jshint/following{/other_user}","gists_url":"https://api.github.com/users/jshint/gists{/gist_id}","starred_url":"https://api.github.com/users/jshint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jshint/subscriptions","organizations_url":"https://api.github.com/users/jshint/orgs","repos_url":"https://api.github.com/users/jshint/repos","events_url":"https://api.github.com/users/jshint/events{/privacy}","received_events_url":"https://api.github.com/users/jshint/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/jshint/jshint","description":"JSHint is a tool that helps to detect errors and potential problems in your JavaScript code","fork":false,"url":"https://api.github.com/repos/jshint/jshint","forks_url":"https://api.github.com/repos/jshint/jshint/forks","keys_url":"https://api.github.com/repos/jshint/jshint/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jshint/jshint/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jshint/jshint/teams","hooks_url":"https://api.github.com/repos/jshint/jshint/hooks","issue_events_url":"https://api.github.com/repos/jshint/jshint/issues/events{/number}","events_url":"https://api.github.com/repos/jshint/jshint/events","assignees_url":"https://api.github.com/repos/jshint/jshint/assignees{/user}","branches_url":"https://api.github.com/repos/jshint/jshint/branches{/branch}","tags_url":"https://api.github.com/repos/jshint/jshint/tags","blobs_url":"https://api.github.com/repos/jshint/jshint/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jshint/jshint/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jshint/jshint/git/refs{/sha}","trees_url":"https://api.github.com/repos/jshint/jshint/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jshint/jshint/statuses/{sha}","languages_url":"https://api.github.com/repos/jshint/jshint/languages","stargazers_url":"https://api.github.com/repos/jshint/jshint/stargazers","contributors_url":"https://api.github.com/repos/jshint/jshint/contributors","subscribers_url":"https://api.github.com/repos/jshint/jshint/subscribers","subscription_url":"https://api.github.com/repos/jshint/jshint/subscription","commits_url":"https://api.github.com/repos/jshint/jshint/commits{/sha}","git_commits_url":"https://api.github.com/repos/jshint/jshint/git/commits{/sha}","comments_url":"https://api.github.com/repos/jshint/jshint/comments{/number}","issue_comment_url":"https://api.github.com/repos/jshint/jshint/issues/comments/{number}","contents_url":"https://api.github.com/repos/jshint/jshint/contents/{+path}","compare_url":"https://api.github.com/repos/jshint/jshint/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jshint/jshint/merges","archive_url":"https://api.github.com/repos/jshint/jshint/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jshint/jshint/downloads","issues_url":"https://api.github.com/repos/jshint/jshint/issues{/number}","pulls_url":"https://api.github.com/repos/jshint/jshint/pulls{/number}","milestones_url":"https://api.github.com/repos/jshint/jshint/milestones{/number}","notifications_url":"https://api.github.com/repos/jshint/jshint/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jshint/jshint/labels{/name}","releases_url":"https://api.github.com/repos/jshint/jshint/releases{/id}","created_at":"2011-01-19T22:28:53Z","updated_at":"2014-10-03T18:38:17Z","pushed_at":"2014-10-03T20:16:36Z","git_url":"git://github.com/jshint/jshint.git","ssh_url":"git@github.com:jshint/jshint.git","clone_url":"https://github.com/jshint/jshint.git","svn_url":"https://github.com/jshint/jshint","homepage":"http://jshint.com","size":16115,"stargazers_count":3873,"watchers_count":3873,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":814,"mirror_url":null,"open_issues_count":233,"forks":814,"open_issues":233,"watchers":3873,"default_branch":"master"}},"base":{"label":"jshint:master","ref":"master","sha":"47ff4ca921253c06fe039cf1f3438355d4a9addd","user":{"login":"jshint","id":1335450,"avatar_url":"https://avatars.githubusercontent.com/u/1335450?v=2","gravatar_id":"","url":"https://api.github.com/users/jshint","html_url":"https://github.com/jshint","followers_url":"https://api.github.com/users/jshint/followers","following_url":"https://api.github.com/users/jshint/following{/other_user}","gists_url":"https://api.github.com/users/jshint/gists{/gist_id}","starred_url":"https://api.github.com/users/jshint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jshint/subscriptions","organizations_url":"https://api.github.com/users/jshint/orgs","repos_url":"https://api.github.com/users/jshint/repos","events_url":"https://api.github.com/users/jshint/events{/privacy}","received_events_url":"https://api.github.com/users/jshint/received_events","type":"Organization","site_admin":false},"repo":{"id":1272666,"name":"jshint","full_name":"jshint/jshint","owner":{"login":"jshint","id":1335450,"avatar_url":"https://avatars.githubusercontent.com/u/1335450?v=2","gravatar_id":"","url":"https://api.github.com/users/jshint","html_url":"https://github.com/jshint","followers_url":"https://api.github.com/users/jshint/followers","following_url":"https://api.github.com/users/jshint/following{/other_user}","gists_url":"https://api.github.com/users/jshint/gists{/gist_id}","starred_url":"https://api.github.com/users/jshint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jshint/subscriptions","organizations_url":"https://api.github.com/users/jshint/orgs","repos_url":"https://api.github.com/users/jshint/repos","events_url":"https://api.github.com/users/jshint/events{/privacy}","received_events_url":"https://api.github.com/users/jshint/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/jshint/jshint","description":"JSHint is a tool that helps to detect errors and potential problems in your JavaScript code","fork":false,"url":"https://api.github.com/repos/jshint/jshint","forks_url":"https://api.github.com/repos/jshint/jshint/forks","keys_url":"https://api.github.com/repos/jshint/jshint/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jshint/jshint/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jshint/jshint/teams","hooks_url":"https://api.github.com/repos/jshint/jshint/hooks","issue_events_url":"https://api.github.com/repos/jshint/jshint/issues/events{/number}","events_url":"https://api.github.com/repos/jshint/jshint/events","assignees_url":"https://api.github.com/repos/jshint/jshint/assignees{/user}","branches_url":"https://api.github.com/repos/jshint/jshint/branches{/branch}","tags_url":"https://api.github.com/repos/jshint/jshint/tags","blobs_url":"https://api.github.com/repos/jshint/jshint/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jshint/jshint/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jshint/jshint/git/refs{/sha}","trees_url":"https://api.github.com/repos/jshint/jshint/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jshint/jshint/statuses/{sha}","languages_url":"https://api.github.com/repos/jshint/jshint/languages","stargazers_url":"https://api.github.com/repos/jshint/jshint/stargazers","contributors_url":"https://api.github.com/repos/jshint/jshint/contributors","subscribers_url":"https://api.github.com/repos/jshint/jshint/subscribers","subscription_url":"https://api.github.com/repos/jshint/jshint/subscription","commits_url":"https://api.github.com/repos/jshint/jshint/commits{/sha}","git_commits_url":"https://api.github.com/repos/jshint/jshint/git/commits{/sha}","comments_url":"https://api.github.com/repos/jshint/jshint/comments{/number}","issue_comment_url":"https://api.github.com/repos/jshint/jshint/issues/comments/{number}","contents_url":"https://api.github.com/repos/jshint/jshint/contents/{+path}","compare_url":"https://api.github.com/repos/jshint/jshint/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jshint/jshint/merges","archive_url":"https://api.github.com/repos/jshint/jshint/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jshint/jshint/downloads","issues_url":"https://api.github.com/repos/jshint/jshint/issues{/number}","pulls_url":"https://api.github.com/repos/jshint/jshint/pulls{/number}","milestones_url":"https://api.github.com/repos/jshint/jshint/milestones{/number}","notifications_url":"https://api.github.com/repos/jshint/jshint/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jshint/jshint/labels{/name}","releases_url":"https://api.github.com/repos/jshint/jshint/releases{/id}","created_at":"2011-01-19T22:28:53Z","updated_at":"2014-10-03T18:38:17Z","pushed_at":"2014-10-03T20:16:36Z","git_url":"git://github.com/jshint/jshint.git","ssh_url":"git@github.com:jshint/jshint.git","clone_url":"https://github.com/jshint/jshint.git","svn_url":"https://github.com/jshint/jshint","homepage":"http://jshint.com","size":16115,"stargazers_count":3873,"watchers_count":3873,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":814,"mirror_url":null,"open_issues_count":233,"forks":814,"open_issues":233,"watchers":3873,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/jshint/jshint/pulls/1887"},"html":{"href":"https://github.com/jshint/jshint/pull/1887"},"issue":{"href":"https://api.github.com/repos/jshint/jshint/issues/1887"},"comments":{"href":"https://api.github.com/repos/jshint/jshint/issues/1887/comments"},"review_comments":{"href":"https://api.github.com/repos/jshint/jshint/pulls/1887/comments"},"review_comment":{"href":"https://api.github.com/repos/jshint/jshint/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/jshint/jshint/pulls/1887/commits"},"statuses":{"href":"https://api.github.com/repos/jshint/jshint/statuses/d3bcffcb3a2dd6d8f7a3f51b0f2d7db7b4952cf9"}}}}