Fix some static analyzer warnings (issue 11965)

[Snuffleupagus]

This fixes only those warnings, as reported by https://lgtm.com/projects/g/mozilla/pdf.js?mode=list, that make sense (as far as I'm concerned).

Hence this patch leaves the following things unaddressed:

• The "recommendation"-category, since it only complains about unused variables. However, note that all of those cases are purposely included and that there's thus ESLint-disable comments added to explictly allow them.
• The "warning"-category, which still contains two complaints. However, as far as I can tell, they are both false positives.

Given first of all the false positives of the LGTM static analyzer, and secondly that we'd need to add (essentially duplicated) disable-comments for the unused variable cases, it's not entirely clear to me if we actually want to work towards including LGTM in the PDF.js project (e.g. running alongside Travis) or if we should just close issue #11965.

[Snuffleupagus]

/botio-linux preview

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/89dae7c4bbc7aa5/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.67.70.0:8877/89dae7c4bbc7aa5/output.txt

Total script time: 4.00 mins

Published

• Viewer: http://54.67.70.0:8877/89dae7c4bbc7aa5/web/viewer.html
• Viewer (ES5): http://54.67.70.0:8877/89dae7c4bbc7aa5/es5/web/viewer.html

[timvandermeij]

Thank you! I'd say this is enough to close the issue. If we ever want to do more with this, we can always do so.

[automatedbugreportingfacility]

However, as far as I can tell, they are both false positives.

I don't think the useless comparison test in /src/display/pattern_helper.js is a false positive. At line 121, y1 === y2 is evaluated only when both these conditions are met:

1. At line 117, y < y2 evaluates to true
2. At line 119, y < y1 evaluates to false

If y is lower than y2 and not lower than y1, then y1 cannot be equal to y2 (unless any of those variables can be modified between the comparisons, which is not the case here).

[Snuffleupagus]

However, as far as I can tell, they are both false positives.

I don't think the useless comparison test in /src/display/pattern_helper.js is a false positive. At line 121, y1 === y2 is evaluated only when both these conditions are met:

1. At line 117, `y < y2` evaluates to true

2. At line 119, `y < y1` evaluates to false

If y is lower than y2 and not lower than y1, then y1 cannot be equal to y2 (unless any of those variables can be modified between the comparisons, which is not the case here).

That's certainly an interesting find; thank you!

However, since that we're currently not intending on integrating LGTM officially for this repo, we can probably just leave this as-is for now. Given that there's similar code in the else-branch below (with a valid y2 === y3 check), I do wonder if there's perhaps a bug in the code as-is, however it seems better to just leave it alone for now since we're (currently) not aware of any PDF documents that'd benefit from (general) changes to this code.