Skip to content

Commit

Permalink
Enable the ESLint no-shadow rule
Browse files Browse the repository at this point in the history 
This rule is *not* currently enabled in mozilla-central, but it appears commented out[1] in the ESLint definition file; see https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js#238-239

Unfortunately this rule is, for fairly obvious reasons, impossible to `--fix` automatically (even partially) and each case thus require careful manual tweaks.
Hence this ESLint rule is, by some margin, probably the most difficult one that we've enabled thus far. However, using this rule does seem like a good idea in general since allowing variable shadowing could lead to subtle (and difficult to find) bugs or at the very least confusing code.

*Please note:* Possibly these changes ought to have been split into *multiple* commits, e.g. one per directory, so if this is considered "un-reviewable" as-is I can try to do that.

Please find additional details about the ESLint rule at https://eslint.org/docs/rules/no-shadow

---
[1] This suggests, at least to me, a desire to possibly enable it at some future point. Most likely, a *very* large number of failures currently prevent doing so.
  • Loading branch information
@Snuffleupagus
Snuffleupagus committed Feb 8, 2020
1 parent cc85da2 commit 9207c17
Show file tree
Hide file tree
Showing 38 changed files with 379 additions and 380 deletions.
2 changes: 1 addition & 1 deletion .eslintrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@
"no-catch-shadow": "error",
"no-delete-var": "error",
"no-label-var": "error",
"no-shadow": "error",
"no-shadow-restricted-names": "error",
"no-shadow": "off",
"no-undef-init": "error",
"no-undef": ["error", { "typeof": true, }],
"no-unused-vars": ["error", {
Expand Down
4 changes: 2 additions & 2 deletions external/builder/preprocessor2.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ function traverseTree(ctx, node) {
for (var i in node) {
var child = node[i];
if (typeof child === "object" && child !== null && child.type) {
var result = traverseTree(ctx, child);
const result = traverseTree(ctx, child);
if (result !== child) {
node[i] = result;
}
Expand All @@ -300,7 +300,7 @@ function traverseTree(ctx, node) {
childItem !== null &&
childItem.type
) {
var result = traverseTree(ctx, childItem);
const result = traverseTree(ctx, childItem);
if (result !== childItem) {
child[index] = result;
}
Expand Down
52 changes: 25 additions & 27 deletions gulpfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,6 @@ function createStringSource(filename, content) {
}

function createWebpackConfig(defines, output) {
var path = require("path");

var versionInfo = getVersionJSON();
var bundleDefines = builder.merge(defines, {
BUNDLE_VERSION: versionInfo.version,
Expand Down Expand Up @@ -242,9 +240,9 @@ function createWebpackConfig(defines, output) {
};
}

function webpack2Stream(config) {
function webpack2Stream(webpackConfig) {
// Replacing webpack1 to webpack2 in the webpack-stream.
return webpackStream(config, webpack2);
return webpackStream(webpackConfig, webpack2);
}

function getVersionJSON() {
Expand Down Expand Up @@ -377,26 +375,26 @@ function createImageDecodersBundle(defines) {
.pipe(replaceJSRootName(imageDecodersAMDName, "pdfjsImageDecoders"));
}

function checkFile(path) {
function checkFile(filePath) {
try {
var stat = fs.lstatSync(path);
var stat = fs.lstatSync(filePath);
return stat.isFile();
} catch (e) {
return false;
}
}

function checkDir(path) {
function checkDir(dirPath) {
try {
var stat = fs.lstatSync(path);
var stat = fs.lstatSync(dirPath);
return stat.isDirectory();
} catch (e) {
return false;
}
}

function replaceInFile(path, find, replacement) {
var content = fs.readFileSync(path).toString();
function replaceInFile(filePath, find, replacement) {
var content = fs.readFileSync(filePath).toString();
content = content.replace(find, replacement);
fs.writeFileSync(path, content);
}
Expand All @@ -406,9 +404,9 @@ function getTempFile(prefix, suffix) {
var bytes = require("crypto")
.randomBytes(6)
.toString("hex");
var path = BUILD_DIR + "tmp/" + prefix + bytes + suffix;
fs.writeFileSync(path, "");
return path;
var filePath = BUILD_DIR + "tmp/" + prefix + bytes + suffix;
fs.writeFileSync(filePath, "");
return filePath;
}

function createTestSource(testsName, bot) {
Expand Down Expand Up @@ -526,10 +524,10 @@ gulp.task("buildnumber", function(done) {

var version = config.versionPrefix + buildNumber;

exec('git log --format="%h" -n 1', function(err, stdout, stderr) {
exec('git log --format="%h" -n 1', function(err2, stdout2, stderr2) {
var buildCommit = "";
if (!err) {
buildCommit = stdout.replace("\n", "");
if (!err2) {
buildCommit = stdout2.replace("\n", "");
}

createStringSource(
Expand Down Expand Up @@ -558,9 +556,9 @@ gulp.task("default_preferences-pre", function() {
function babelPluginReplaceNonWebPackRequire(babel) {
return {
visitor: {
Identifier(path, state) {
if (path.node.name === "__non_webpack_require__") {
path.replaceWith(babel.types.identifier("require"));
Identifier(curPath, state) {
if (curPath.node.name === "__non_webpack_require__") {
curPath.replaceWith(babel.types.identifier("require"));
}
},
},
Expand Down Expand Up @@ -642,8 +640,8 @@ gulp.task("locale", function() {
var locales = [];
for (var i = 0; i < subfolders.length; i++) {
var locale = subfolders[i];
var path = L10N_DIR + locale;
if (!checkDir(path)) {
var dirPath = L10N_DIR + locale;
if (!checkDir(dirPath)) {
continue;
}
if (!/^[a-z][a-z]([a-z])?(-[A-Z][A-Z])?$/.test(locale)) {
Expand All @@ -655,7 +653,7 @@ gulp.task("locale", function() {

locales.push(locale);

if (checkFile(path + "/viewer.properties")) {
if (checkFile(dirPath + "/viewer.properties")) {
viewerOutput +=
"[" +
locale +
Expand Down Expand Up @@ -1134,9 +1132,9 @@ gulp.task(
function babelPluginReplaceNonWebPackRequire(babel) {
return {
visitor: {
Identifier(path, state) {
if (path.node.name === "__non_webpack_require__") {
path.replaceWith(babel.types.identifier("require"));
Identifier(curPath, state) {
if (curPath.node.name === "__non_webpack_require__") {
curPath.replaceWith(babel.types.identifier("require"));
}
},
},
Expand Down Expand Up @@ -1324,9 +1322,9 @@ gulp.task("baseline", function(done) {
}

exec("git checkout " + baselineCommit, { cwd: workingDirectory }, function(
error
error2
) {
if (error) {
if (error2) {
done(new Error("Baseline commit checkout failed."));
return;
}
Expand Down
10 changes: 5 additions & 5 deletions src/core/chunked_stream.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,15 +519,15 @@ class ChunkedStreamManager {
}

const loadedRequests = [];
for (let chunk = beginChunk; chunk < endChunk; ++chunk) {
for (let curChunk = beginChunk; curChunk < endChunk; ++curChunk) {
// The server might return more chunks than requested.
const requestIds = this.requestsByChunk[chunk] || [];
delete this.requestsByChunk[chunk];
const requestIds = this.requestsByChunk[curChunk] || [];
delete this.requestsByChunk[curChunk];

for (const requestId of requestIds) {
const chunksNeeded = this.chunksNeededByRequest[requestId];
if (chunk in chunksNeeded) {
delete chunksNeeded[chunk];
if (curChunk in chunksNeeded) {
delete chunksNeeded[curChunk];
}

if (!isEmptyObj(chunksNeeded)) {
Expand Down
4 changes: 2 additions & 2 deletions src/core/document.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ class Page {
// Fetching the individual streams from the array.
const xref = this.xref;
const streams = [];
for (const stream of content) {
streams.push(xref.fetchIfRef(stream));
for (const subStream of content) {
streams.push(xref.fetchIfRef(subStream));
}
stream = new StreamsSequenceStream(streams);
} else if (isStream(content)) {
Expand Down
34 changes: 17 additions & 17 deletions src/core/evaluator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
pdfFunctionFactory: this.pdfFunctionFactory,
})
.then(imageObj => {
var imgData = imageObj.createImageData(/* forceRGBA = */ false);
imgData = imageObj.createImageData(/* forceRGBA = */ false);

if (this.parsingType3Font) {
return this.handler.sendWithPromise(
Expand Down Expand Up @@ -2387,16 +2387,16 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
properties.hasEncoding = !!baseEncodingName || differences.length > 0;
properties.dict = dict;
return toUnicodePromise
.then(toUnicode => {
properties.toUnicode = toUnicode;
.then(readToUnicode => {
properties.toUnicode = readToUnicode;
return this.buildToUnicode(properties);
})
.then(toUnicode => {
properties.toUnicode = toUnicode;
.then(builtToUnicode => {
properties.toUnicode = builtToUnicode;
if (cidToGidBytes) {
properties.cidToGidMap = this.readCidToGidMap(
cidToGidBytes,
toUnicode
builtToUnicode
);
}
return properties;
Expand Down Expand Up @@ -2983,21 +2983,21 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
};
const widths = dict.get("Widths");
return this.extractDataStructures(dict, dict, properties).then(
properties => {
newProperties => {
if (widths) {
const glyphWidths = [];
let j = firstChar;
for (let i = 0, ii = widths.length; i < ii; i++) {
glyphWidths[j++] = this.xref.fetchIfRef(widths[i]);
}
properties.widths = glyphWidths;
newProperties.widths = glyphWidths;
} else {
properties.widths = this.buildCharCodeToWidth(
newProperties.widths = this.buildCharCodeToWidth(
metrics.widths,
properties
newProperties
);
}
return new Font(baseFontName, null, properties);
return new Font(baseFontName, null, newProperties);
}
);
}
Expand Down Expand Up @@ -3104,13 +3104,13 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
.then(() => {
return this.extractDataStructures(dict, baseDict, properties);
})
.then(properties => {
this.extractWidths(dict, descriptor, properties);
.then(newProperties => {
this.extractWidths(dict, descriptor, newProperties);

if (type === "Type3") {
properties.isType3Font = true;
newProperties.isType3Font = true;
}
return new Font(fontName.name, fontFile, properties);
return new Font(fontName.name, fontFile, newProperties);
});
},
};
Expand Down Expand Up @@ -3245,8 +3245,8 @@ var TranslatedFont = (function TranslatedFontClosure() {
})
.catch(function(reason) {
warn(`Type3 font resource "${key}" is not available.`);
var operatorList = new OperatorList();
charProcOperatorList[key] = operatorList.getIR();
const dummyOperatorList = new OperatorList();
charProcOperatorList[key] = dummyOperatorList.getIR();
});
});
}
Expand Down
14 changes: 7 additions & 7 deletions src/core/font_renderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,7 @@ var FontRendererFactory = (function FontRendererFactoryClosure() {
}
}

function compileCharString(code, cmds, font, glyphId) {
var stack = [];
var x = 0,
y = 0;
var stems = 0;

function compileCharString(charStringCode, cmds, font, glyphId) {
function moveTo(x, y) {
cmds.push({ cmd: "moveTo", args: [x, y] });
}
Expand All @@ -361,6 +356,11 @@ var FontRendererFactory = (function FontRendererFactoryClosure() {
cmds.push({ cmd: "bezierCurveTo", args: [x1, y1, x2, y2, x, y] });
}

var stack = [];
var x = 0,
y = 0;
var stems = 0;

function parse(code) {
var i = 0;
while (i < code.length) {
Expand Down Expand Up @@ -719,7 +719,7 @@ var FontRendererFactory = (function FontRendererFactoryClosure() {
}
}
}
parse(code);
parse(charStringCode);
}

const NOOP = [];
Expand Down
Loading

0 comments on commit 9207c17

Please sign in to comment.