Remove variable shadowing from the JavaScript files in the src/core/ folder
#11738
Conversation
…` folder *This is part of a series of patches that will try to split PR 11566 into smaller chunks, to make reviewing more feasible.* Once all the code has been fixed, we'll be able to eventually enable the ESLint no-shadow rule; see https://eslint.org/docs/rules/no-shadow
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/8ba08843d9ae3cd/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/2961a36c685237f/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/2961a36c685237f/output.txt Total script time: 19.52 mins
Image differences available at: http://54.67.70.0:8877/2961a36c685237f/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/8ba08843d9ae3cd/output.txt Total script time: 25.15 mins
Image differences available at: http://54.215.176.217:8877/8ba08843d9ae3cd/reftest-analyzer.html#web=eq.log |
| @@ -1409,7 +1408,7 @@ var Font = (function FontClosure() { | |||
| tables["post"] = null; | |||
|
|
|||
| for (let i = 0; i < numTables; i++) { | |||
| const table = readTableEntry(font); | |||
| const table = readTableEntry(file); | |||
There was a problem hiding this comment.
This one actually looks like it caused a bug since we always read the entry for the font variable of the upper scope here and not for the given file parameter, and I found a call-site that actually passes a different value than font here. If I'm not missing anything here, I'm surprised we haven't seen any problems because of it, but it would show that de-shadowing is a good thing!
There was a problem hiding this comment.
Yes, this case is absolutely confusing!
The only reason that this hasn't caused any bugs is that these properties are Objects (in this case instanceof Stream), and those are essentially passed by reference rather than value in JavaScript (the names font/file/... thus point to the same data).
|
Thanks a lot! |
This is part of a series of patches that will try to split PR #11566 into smaller chunks, to make reviewing more feasible.
Once all the code has been fixed, we'll be able to eventually enable the ESLint no-shadow rule; see https://eslint.org/docs/rules/no-shadow