Turn namespace assignment error into a warning

[guybedford]

Similarly to #3475 this turns the static error into a warning that will become a runtime error only on the execution paths that actually attempt it.

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:

Description

[rollup-bot]

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#warn-namespace-reassignment

or load it into the REPL:
https://rollupjs.org/repl/?circleci=11934

[codecov]

Codecov Report

Merging #3633 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3633   +/-   ##
=======================================
  Coverage   96.53%   96.54%           
=======================================
  Files         183      183           
  Lines        6242     6244    +2     
  Branches     1830     1830           
=======================================
+ Hits         6026     6028    +2     
  Misses        107      107           
  Partials      109      109           

Impacted Files	Coverage Δ
src/ast/nodes/MemberExpression.ts	98.30% <100.00%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d613137...d433eda. Read the comment docs.

[lukastaegert]

Looks good, just one minor comment.

[lukastaegert]

I know this is already merged but what was the reason to .inlcude() the namespace on reassignment? I mean, this leads to the creation of a namespace object, but this is not what is reassigned. The original variable is what is reassigned, wouldn't it be enough to include that?

https://rollupjs.org/repl/?version=2.16.1&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMColMjBhcyUyMGxpYiUyMGZyb20lMjAnLiUyRmxpYiclM0IlNUNubGliLmZvbyUyMCUzRCUyMCd0aGlzJTIwd2lsbCUyMGJlJTIwYXNzaWduZWQlMjB0byUyMGZvbyclM0IlNUNubGliLmJhciUyMCUzRCUyMCd0aGlzJTIwd2lsbCUyMGJlJTIwYXNzaWduZWQlMjB0byUyMHVuZGVmaW5lZCclM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJsaWIuanMlMjIlMkMlMjJjb2RlJTIyJTNBJTIyZXhwb3J0JTIwY29uc3QlMjBmb28lMjAlM0QlMjAnZm9vJyUyMiUyQyUyMmlzRW50cnklMjIlM0FmYWxzZSU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJlcyUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE

[guybedford]

Without the include the entire statement was being removed as it wasn’t being detected as having a side effect of throwing. I don’t know what mechanism is best to handle possible throw side effects currently.

…

On Sat, Jun 13, 2020 at 22:06 Lukas Taegert-Atkinson < ***@***.***> wrote: I know this is already merged but what was the reason to .inlcude() the namespace on reassignment? I mean, this leads to the creation of a namespace object, but this is not what is reassigned. The original variable is what is reassigned, wouldn't it be enough to include that? https://rollupjs.org/repl/?version=2.16.1&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMColMjBhcyUyMGxpYiUyMGZyb20lMjAnLiUyRmxpYiclM0IlNUNubGliLmZvbyUyMCUzRCUyMCd0aGlzJTIwd2lsbCUyMGJlJTIwYXNzaWduZWQlMjB0byUyMGZvbyclM0IlNUNubGliLmJhciUyMCUzRCUyMCd0aGlzJTIwd2lsbCUyMGJlJTIwYXNzaWduZWQlMjB0byUyMHVuZGVmaW5lZCclM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJsaWIuanMlMjIlMkMlMjJjb2RlJTIyJTNBJTIyZXhwb3J0JTIwY29uc3QlMjBmb28lMjAlM0QlMjAnZm9vJyUyMiUyQyUyMmlzRW50cnklMjIlM0FmYWxzZSU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJlcyUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3633 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAESFSW2TCMKZGXF4XCC6X3RWRLEHANCNFSM4N4YVL4A> .

[lukastaegert]

In that case I would suggest #3637