Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable the no-var linting rule in src/core/{crypto,function}.js #13321

Merged
merged 3 commits into from May 2, 2021

Conversation

timvandermeij
Copy link
Contributor

The commit messages contain more information about the individual changes.

This is done automatically with `gulp lint --fix` and the following
manual changes:

```diff
diff --git a/src/core/function.js b/src/core/function.js
index 878001057..b7e3e6ccf 100644
--- a/src/core/function.js
+++ b/src/core/function.js
@@ -131,7 +131,7 @@ function toNumberArray(arr) {
   return arr;
 }

-var PDFFunction = (function PDFFunctionClosure() {
+const PDFFunction = (function PDFFunctionClosure() {
   const CONSTRUCT_SAMPLED = 0;
   const CONSTRUCT_INTERPOLATED = 2;
   const CONSTRUCT_STICHED = 3;
@@ -484,7 +484,9 @@ var PDFFunction = (function PDFFunctionClosure() {
         // clip to domain
         const v = clip(src[srcOffset], domain[0], domain[1]);
         // calculate which bound the value is in
-        for (var i = 0, ii = bounds.length; i < ii; ++i) {
+        const length = bounds.length;
+        let i;
+        for (i = 0; i < length; ++i) {
           if (v < bounds[i]) {
             break;
           }
@@ -673,23 +675,21 @@ const PostScriptStack = (function PostScriptStackClosure() {
     roll(n, p) {
       const stack = this.stack;
       const l = stack.length - n;
-      let r = stack.length - 1,
-        c = l + (p - Math.floor(p / n) * n),
-        i,
-        j,
-        t;
-      for (i = l, j = r; i < j; i++, j--) {
-        t = stack[i];
+      const r = stack.length - 1;
+      const c = l + (p - Math.floor(p / n) * n);
+
+      for (let i = l, j = r; i < j; i++, j--) {
+        const t = stack[i];
         stack[i] = stack[j];
         stack[j] = t;
       }
-      for (i = l, j = c - 1; i < j; i++, j--) {
-        t = stack[i];
+      for (let i = l, j = c - 1; i < j; i++, j--) {
+        const t = stack[i];
         stack[i] = stack[j];
         stack[j] = t;
       }
-      for (i = c, j = r; i < j; i++, j--) {
-        t = stack[i];
+      for (let i = c, j = r; i < j; i++, j--) {
+        const t = stack[i];
         stack[i] = stack[j];
         stack[j] = t;
       }
@@ -939,7 +939,7 @@ class PostScriptEvaluator {
 // We can compile most of such programs, and at the same moment, we can
 // optimize some expressions using basic math properties. Keeping track of
 // min/max values will allow us to avoid extra Math.min/Math.max calls.
-var PostScriptCompiler = (function PostScriptCompilerClosure() {
+const PostScriptCompiler = (function PostScriptCompilerClosure() {
   class AstNode {
     constructor(type) {
       this.type = type;
```
This is done automatically with `gulp lint --fix`.
@timvandermeij timvandermeij changed the title Enable the no-var linting rule in src/core/{crypto,function}.js Enable the no-var linting rule in src/core/{crypto,function}.js May 1, 2021
@timvandermeij timvandermeij changed the title Enable the no-var linting rule in src/core/{crypto,function}.js Enable the no-var linting rule in src/core/{crypto,function}.js May 1, 2021
@timvandermeij
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented May 1, 2021

From: Bot.io (Linux m4)


Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/15e159cb7dae089/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 1, 2021

From: Bot.io (Windows)


Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://3.101.106.178:8877/d4a01a3b4ec3df5/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 1, 2021

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/15e159cb7dae089/output.txt

Total script time: 25.81 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED

Image differences available at: http://54.67.70.0:8877/15e159cb7dae089/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

pdfjsbot commented May 1, 2021

From: Bot.io (Windows)


Failed

Full output at http://3.101.106.178:8877/d4a01a3b4ec3df5/output.txt

Total script time: 29.83 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED

Image differences available at: http://3.101.106.178:8877/d4a01a3b4ec3df5/reftest-analyzer.html#web=eq.log

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me, looks good to me; thanks for doing this!

src/core/crypto.js Outdated Show resolved Hide resolved
@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented May 1, 2021

Just a FYI, so that we avoid duplicate work here: I've started looking at splitting the src/core/fonts.js file a bit, similar to (but not as invasive as) the recent Streams re-factoring. As part of those changes I'd obviously fix the no-var errors as well, so feel free to skip that file :-)


Also, a warning if you feel like trying to fix src/core/evaluator.js, since I looked at that one awhile back and decided to ignore it for now:
There's quite a few cases in that file which cannot be fixed by ESLint, and a fair number of them were surprisingly difficult to fix manually. Particularly around the Promises and while-loops in the getOperatorList/getTextContent methods, there's some quite tricky scope-issues which aren't totally obvious at first sight. If I recall correctly, it's easy to make manual changes which ESLint happily accepts but which however leads to run-time errors in the viewer/tests.

@timvandermeij
Copy link
Contributor Author

Ah, thanks, I won't touch those files for now then. For the evaluator it might be best to first do some refactoring then before applying ESLint to make the process less error-prone.

…ldn't be changed automatically by ESLint

This is done in a separate commit due to the required number of changes
so that reviewing is easier than in a plain-text diff in the commit
message.
@timvandermeij
Copy link
Contributor Author

/botio unittest

@pdfjsbot
Copy link

pdfjsbot commented May 2, 2021

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/6489ac18872b0f7/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 2, 2021

From: Bot.io (Windows)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://3.101.106.178:8877/0f8b3b87a3a0e29/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 2, 2021

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/6489ac18872b0f7/output.txt

Total script time: 3.79 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented May 2, 2021

From: Bot.io (Windows)


Success

Full output at http://3.101.106.178:8877/0f8b3b87a3a0e29/output.txt

Total script time: 6.26 mins

  • Unit Tests: Passed

@timvandermeij timvandermeij merged commit af9feb1 into mozilla:master May 2, 2021
@timvandermeij timvandermeij deleted the src-core-no-var branch May 2, 2021 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants