attachScopes doesn't recognize for loop correctly

[eight04]

• Rollup Plugin Name: pluginutils
• Rollup Plugin Version: 3.0.1
• Rollup Version:
• Operating System (or Browser): Windows 7
• Node Version: 13.5.0

How Do We Reproduce?

const {parse} = require("acorn");
const {attachScopes} = require("@rollup/pluginutils");

const code = `
for (const foo of []) {
  const bar = 1;
  var baz = 2;
}
for (var bak of []);
`;

const ast = parse(code);
let scope = attachScopes(ast, "scope");
console.log(scope.contains("foo"));
console.log(scope.contains("bar"));
console.log(scope.contains("baz"));
console.log(scope.contains("bak"));

https://repl.it/@eight041/attachScopes-and-for-scope?language=nodejs

Expected Behavior

false
false
true
true

Actual Behavior

true
false
true
true

I'm a little surprised that we didn't find this bug earlier. This breaks a lot of stuff IMHO.

[shellscape]

Thanks for reporting this. We'll have a look as soon as possible.

[shellscape]

I've got a pending fix for this on the fix/attachScope-for branch. Will PR once #141 is merged.

[eight04]

Some problems:

1. ForStatement and ForInStatement are not fixed.
2. The declaration should be added to the scope of the loop. Currently, the entire declaration is ignored.

Repro:
https://repl.it/@eight041/attachScopes-and-for-scope-1

Expected:

false
false
false
true
true
true

Actual:

false
true
true
false
true
true

Another question: should the scope object be assigned to ForStatement in this case? Otherwise, users have to inspect the loop body to check the variable and the body can be missing.