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.

Sign up for GitHub

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

Jump to bottom

lib: introduce no-mixed-operators eslint rule to lib #29834

Closed
wants to merge 2 commits into from
Closed

lib: introduce no-mixed-operators eslint rule to lib #29834

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49bd9ca
lib: introduce no-mixed-operators eslint rule to lib
ZYSzys Oct 3, 2019
b8d8f79
fixup! lib: introduce no-mixed-operators eslint rule to lib
ZYSzys Oct 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/.eslintrc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ env:
rules:
prefer-object-spread: error
no-buffer-constructor: error
no-mixed-operators:
- error
- groups: [[ "&&", "||" ]]
no-restricted-globals:
- error
- name: JSON
Expand Down
2 changes: 1 addition & 1 deletion lib/_http_client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ function ClientRequest(input, options, cb) {
}

const defaultPort = options.defaultPort ||
this.agent && this.agent.defaultPort;
(this.agent && this.agent.defaultPort);

const port = options.port = options.port || defaultPort || 80;
const host = options.host = validateHost(options.hostname, 'hostname') ||
Expand Down
2 changes: 1 addition & 1 deletion lib/_stream_readable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ function readableAddChunk(stream, chunk, encoding, addToFront) {
er = chunkInvalid(state, chunk);
if (er) {
errorOrDestroy(stream, er);
} else if (state.objectMode || chunk && chunk.length > 0) {
} else if (state.objectMode || (chunk && chunk.length > 0)) {
if (typeof chunk !== 'string' &&
!state.objectMode &&
// Do not use Object.getPrototypeOf as it is slower since V8 7.3.
Expand Down
4 changes: 2 additions & 2 deletions lib/_tls_wrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ function loadSession(hello) {

if (hello.sessionId.length <= 0 ||
hello.tlsTicket ||
owner.server &&
!owner.server.emit('resumeSession', hello.sessionId, onSession)) {
(owner.server &&
!owner.server.emit('resumeSession', hello.sessionId, onSession))) {
// Sessions without identifiers can't be resumed.
// Sessions with tickets can be resumed directly from the ticket, no server
// session storage is necessary.
Expand Down
6 changes: 3 additions & 3 deletions lib/assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ function expectedException(actual, expected, message, fn) {
message = 'The error is expected to be an instance of ' +
`"${expected.name}". Received `;
if (isError(actual)) {
const name = actual.constructor && actual.constructor.name ||
const name = (actual.constructor && actual.constructor.name) ||
actual.name;
if (expected.name === name) {
message += 'an error with identical name but a different prototype.';
Expand Down Expand Up @@ -685,9 +685,9 @@ function checkIsPromise(obj) {
// way. Do not accept thenables that use a function as `obj` and that have no
// `catch` handler.
return isPromise(obj) ||
obj !== null && typeof obj === 'object' &&
(obj !== null && typeof obj === 'object' &&
typeof obj.then === 'function' &&
typeof obj.catch === 'function';
typeof obj.catch === 'function');
}

async function waitForActual(promiseFn) {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/assert/assertion_error.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ class AssertionError extends Error {
// In case "actual" is an object or a function, it should not be
// reference equal.
if (operator === 'notStrictEqual' &&
(typeof actual === 'object' && actual !== null ||
((typeof actual === 'object' && actual !== null) ||
typeof actual === 'function')) {
base = kReadableOperator.notStrictEqualObject;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/child_process.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,7 @@ function getValidStdio(stdio, sync) {

if (stdio === 'ignore') {
acc.push({ type: 'ignore' });
} else if (stdio === 'pipe' || typeof stdio === 'number' && stdio < 0) {
} else if (stdio === 'pipe' || (typeof stdio === 'number' && stdio < 0)) {
var a = {
type: 'pipe',
readable: i === 0,
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/cluster/child.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ cluster._getServer = function(obj, options, cb) {
cluster.worker.state = 'listening';
const address = obj.address();
message.act = 'listening';
message.port = address && address.port || options.port;
message.port = (address && address.port) || options.port;
send(message);
});
};
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,8 +666,8 @@ const fatalExceptionStackEnhancers = {
colors: defaultColors
}
} = lazyInternalUtilInspect();
const colors = internalBinding('util').guessHandleType(2) === 'TTY' &&
require('internal/tty').hasColors() ||
const colors = (internalBinding('util').guessHandleType(2) === 'TTY' &&
require('internal/tty').hasColors()) ||
defaultColors;
try {
return inspect(error, { colors });
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/fs/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,9 @@ const nullCheck = hideStackFrames((path, propName, throwError = true) => {
const pathIsUint8Array = isUint8Array(path);

// We can only perform meaningful checks on strings and Uint8Arrays.
if (!pathIsString && !pathIsUint8Array ||
pathIsString && !path.includes('\u0000') ||
pathIsUint8Array && !path.includes(0)) {
if ((!pathIsString && !pathIsUint8Array) ||
(pathIsString && !path.includes('\u0000')) ||
(pathIsUint8Array && !path.includes(0))) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {
if (Array.isArray(options.paths)) {
const isRelative = request.startsWith('./') ||
request.startsWith('../') ||
(isWindows && request.startsWith('.\\') ||
((isWindows && request.startsWith('.\\')) ||
request.startsWith('..\\'));

if (isRelative) {
Expand Down
26 changes: 13 additions & 13 deletions lib/internal/readline/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,30 +108,30 @@ if (internalBinding('config').hasIntl) {
code === 0x2329 || // LEFT-POINTING ANGLE BRACKET
code === 0x232a || // RIGHT-POINTING ANGLE BRACKET
// CJK Radicals Supplement .. Enclosed CJK Letters and Months
code >= 0x2e80 && code <= 0x3247 && code !== 0x303f ||
(code >= 0x2e80 && code <= 0x3247 && code !== 0x303f) ||
// Enclosed CJK Letters and Months .. CJK Unified Ideographs Extension A
code >= 0x3250 && code <= 0x4dbf ||
(code >= 0x3250 && code <= 0x4dbf) ||
// CJK Unified Ideographs .. Yi Radicals
code >= 0x4e00 && code <= 0xa4c6 ||
(code >= 0x4e00 && code <= 0xa4c6) ||
// Hangul Jamo Extended-A
code >= 0xa960 && code <= 0xa97c ||
(code >= 0xa960 && code <= 0xa97c) ||
// Hangul Syllables
code >= 0xac00 && code <= 0xd7a3 ||
(code >= 0xac00 && code <= 0xd7a3) ||
// CJK Compatibility Ideographs
code >= 0xf900 && code <= 0xfaff ||
(code >= 0xf900 && code <= 0xfaff) ||
// Vertical Forms
code >= 0xfe10 && code <= 0xfe19 ||
(code >= 0xfe10 && code <= 0xfe19) ||
// CJK Compatibility Forms .. Small Form Variants
code >= 0xfe30 && code <= 0xfe6b ||
(code >= 0xfe30 && code <= 0xfe6b) ||
// Halfwidth and Fullwidth Forms
code >= 0xff01 && code <= 0xff60 ||
code >= 0xffe0 && code <= 0xffe6 ||
(code >= 0xff01 && code <= 0xff60) ||
(code >= 0xffe0 && code <= 0xffe6) ||
// Kana Supplement
code >= 0x1b000 && code <= 0x1b001 ||
(code >= 0x1b000 && code <= 0x1b001) ||
// Enclosed Ideographic Supplement
code >= 0x1f200 && code <= 0x1f251 ||
(code >= 0x1f200 && code <= 0x1f251) ||
// CJK Unified Ideographs Extension B .. Tertiary Ideographic Plane
code >= 0x20000 && code <= 0x3fffd
(code >= 0x20000 && code <= 0x3fffd)
);
};
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/tty.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ function getColorDepth(env = process.env) {

function hasColors(count, env) {
if (env === undefined &&
(count === undefined || typeof count === 'object' && count !== null)) {
(count === undefined || (typeof count === 'object' && count !== null))) {
env = count;
count = 16;
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1363,7 +1363,7 @@ function pathToFileURL(filepath) {
// path.resolve strips trailing slashes so we must add them back
const filePathLast = filepath.charCodeAt(filepath.length - 1);
if ((filePathLast === CHAR_FORWARD_SLASH ||
isWindows && filePathLast === CHAR_BACKWARD_SLASH) &&
(isWindows && filePathLast === CHAR_BACKWARD_SLASH)) &&
resolved[resolved.length - 1] !== path.sep)
resolved += '/';
const outURL = new URL('file://');
Expand Down
10 changes: 5 additions & 5 deletions lib/internal/util/comparisons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,9 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
}

if (aKeys.length === 0 &&
(iterationType === kNoIterator ||
iterationType === kIsArray && val1.length === 0 ||
val1.size === 0)) {
((iterationType === kNoIterator ||
iterationType === kIsArray) && (val1.length === 0 ||
val1.size === 0))) {
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be a typo. The condition does not seem to be identical to the original one.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, yes, this is different. For example, if aKeys.length === 0 and iterationType === kNoIterator are both true but the remaining three conditions are all false, the current code evaluates to true but the modified code evaluates to false. Might be good to add a test for this if possible, since all tests pass with this change. If it's not possible to add a test, it may indicate that this conditional can be simplified.

Copy link
Member Author

@ZYSzys ZYSzys Oct 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch.

So here I found that always && should be within parenthesis in every mixed operators && and ||.

Might be good to add a test for this if possible, since all tests pass with this change. If it's not possible to add a test, it may indicate that this conditional can be simplified.

@Trott It seems a little hard to add tests here for me. Is there a user case could actually catch this ?
And if this condition need to be simplified, maybe it's better to do it in another PR.

Copy link
Member

Choose a reason for hiding this comment

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

This is a fast path. If it returns false when it should return true, it will still work. Testing this is almost impossible (only the timing could be tested for plus the coverage).

return true;
}

Expand Down Expand Up @@ -383,7 +383,7 @@ function mapMightHaveLoosePrim(a, b, prim, item, memo) {
return altValue;
}
const curB = b.get(altValue);
if (curB === undefined && !b.has(altValue) ||
if ((curB === undefined && !b.has(altValue)) ||
!innerDeepEqual(item, curB, false, memo)) {
return false;
}
Expand Down Expand Up @@ -470,7 +470,7 @@ function mapEquiv(a, b, strict, memo) {
// By directly retrieving the value we prevent another b.has(key) check in
// almost all possible cases.
const item2 = b.get(key);
if ((item2 === undefined && !b.has(key) ||
if (((item2 === undefined && !b.has(key)) ||
!innerDeepEqual(item1, item2, strict, memo))) {
if (strict)
return false;
Expand Down
24 changes: 12 additions & 12 deletions lib/internal/util/inspect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,10 +599,10 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
// Only list the tag in case it's non-enumerable / not an own property.
// Otherwise we'd print this twice.
if (typeof tag !== 'string' ||
tag !== '' &&
(tag !== '' &&
(ctx.showHidden ? hasOwnProperty : propertyIsEnumerable)(
value, Symbol.toStringTag
)) {
))) {
tag = '';
}
let base = '';
Expand Down Expand Up @@ -686,7 +686,7 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
const prefix = getPrefix(constructor, tag, 'RegExp');
if (prefix !== 'RegExp ')
base = `${prefix}${base}`;
if (keys.length === 0 || recurseTimes > ctx.depth && ctx.depth !== null)
if (keys.length === 0 || (recurseTimes > ctx.depth && ctx.depth !== null))
return ctx.stylize(base, 'regexp');
} else if (isDate(value)) {
// Make dates with properties first say the date
Expand Down Expand Up @@ -914,14 +914,14 @@ function formatError(err, constructor, tag, ctx) {
const name = err.name || 'Error';
let len = name.length;
if (constructor === null ||
name.endsWith('Error') &&
(name.endsWith('Error') &&
stack.startsWith(name) &&
(stack.length === len || stack[len] === ':' || stack[len] === '\n')) {
(stack.length === len || stack[len] === ':' || stack[len] === '\n'))) {
let fallback = 'Error';
if (constructor === null) {
const start = stack.match(/^([A-Z][a-z_ A-Z0-9[\]()-]+)(?::|\n {4}at)/) ||
stack.match(/^([a-z_A-Z0-9-]*Error)$/);
fallback = start && start[1] || '';
fallback = (start && start[1]) || '';
len = fallback.length;
fallback = fallback || 'Error';
}
Expand All @@ -939,7 +939,7 @@ function formatError(err, constructor, tag, ctx) {
}
}
// Ignore the error message if it's contained in the stack.
let pos = err.message && stack.indexOf(err.message) || -1;
let pos = (err.message && stack.indexOf(err.message)) || -1;
if (pos !== -1)
pos += err.message.length;
// Wrap the error in brackets in case it has no stack trace.
Expand Down Expand Up @@ -1419,8 +1419,8 @@ function formatProperty(ctx, value, recurseTimes, key, type) {
const s = ctx.stylize;
const sp = 'special';
if (ctx.getters && (ctx.getters === true ||
ctx.getters === 'get' && desc.set === undefined ||
ctx.getters === 'set' && desc.set !== undefined)) {
(ctx.getters === 'get' && desc.set === undefined) ||
(ctx.getters === 'set' && desc.set !== undefined))) {
try {
const tmp = value[key];
ctx.indentationLvl += 2;
Expand Down Expand Up @@ -1597,15 +1597,15 @@ function formatWithOptions(inspectOptions, ...args) {
let constr;
if (typeof tempArg !== 'object' ||
tempArg === null ||
typeof tempArg.toString === 'function' &&
(typeof tempArg.toString === 'function' &&
// A direct own property.
(hasOwnProperty(tempArg, 'toString') ||
// A direct own property on the constructor prototype in
// case the constructor is not an built-in object.
(constr = tempArg.constructor) &&
((constr = tempArg.constructor) &&
!builtInObjects.has(constr.name) &&
constr.prototype &&
hasOwnProperty(constr.prototype, 'toString'))) {
hasOwnProperty(constr.prototype, 'toString'))))) {
tempStr = String(tempArg);
} else {
tempStr = inspect(tempArg, {
Expand Down
12 changes: 6 additions & 6 deletions lib/path.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ function isPosixPathSeparator(code) {
}

function isWindowsDeviceRoot(code) {
return code >= CHAR_UPPERCASE_A && code <= CHAR_UPPERCASE_Z ||
code >= CHAR_LOWERCASE_A && code <= CHAR_LOWERCASE_Z;
return (code >= CHAR_UPPERCASE_A && code <= CHAR_UPPERCASE_Z) ||
(code >= CHAR_LOWERCASE_A && code <= CHAR_LOWERCASE_Z);
}

// Resolves . and .. elements in a path with directory names
Expand Down Expand Up @@ -155,8 +155,8 @@ const win32 = {
// Verify that a cwd was found and that it actually points
// to our drive. If not, default to the drive's root.
if (path === undefined ||
path.slice(0, 2).toLowerCase() !== resolvedDevice.toLowerCase() &&
path.charCodeAt(2) === CHAR_BACKWARD_SLASH) {
(path.slice(0, 2).toLowerCase() !== resolvedDevice.toLowerCase() &&
path.charCodeAt(2) === CHAR_BACKWARD_SLASH)) {
path = `${resolvedDevice}\\`;
}
}
Expand Down Expand Up @@ -358,10 +358,10 @@ const win32 = {
const code = path.charCodeAt(0);
return isPathSeparator(code) ||
// Possible device root
len > 2 &&
(len > 2 &&
isWindowsDeviceRoot(code) &&
path.charCodeAt(1) === CHAR_COLON &&
isPathSeparator(path.charCodeAt(2));
isPathSeparator(path.charCodeAt(2)));
},

join(...args) {
Expand Down
4 changes: 2 additions & 2 deletions lib/readline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,8 @@ Interface.prototype._wordRight = function() {
function charLengthLeft(str, i) {
if (i <= 0)
return 0;
if (i > 1 && str.codePointAt(i - 2) >= kUTF16SurrogateThreshold ||
str.codePointAt(i - 1) >= kUTF16SurrogateThreshold) {
if (i > 1 && (str.codePointAt(i - 2) >= kUTF16SurrogateThreshold ||
str.codePointAt(i - 1) >= kUTF16SurrogateThreshold)) {
ZYSzys marked this conversation as resolved.
Show resolved Hide resolved
return 2;
}
return 1;
Expand Down
10 changes: 5 additions & 5 deletions lib/url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,13 +765,13 @@ Url.prototype.resolveObject = function resolveObject(relative) {

const isSourceAbs = (result.pathname && result.pathname.charAt(0) === '/');
const isRelAbs = (
relative.host || relative.pathname && relative.pathname.charAt(0) === '/'
relative.host || (relative.pathname && relative.pathname.charAt(0) === '/')
);
var mustEndAbs = (isRelAbs || isSourceAbs ||
(result.host && relative.pathname));
const removeAllDots = mustEndAbs;
var srcPath = result.pathname && result.pathname.split('/') || [];
const relPath = relative.pathname && relative.pathname.split('/') || [];
var srcPath = (result.pathname && result.pathname.split('/')) || [];
const relPath = (relative.pathname && relative.pathname.split('/')) || [];
const noLeadingSlashes = result.protocol &&
!slashedProtocol.has(result.protocol);

Expand Down Expand Up @@ -869,8 +869,8 @@ Url.prototype.resolveObject = function resolveObject(relative) {
// then it must NOT get a trailing slash.
var last = srcPath.slice(-1)[0];
const hasTrailingSlash = (
(result.host || relative.host || srcPath.length > 1) &&
(last === '.' || last === '..') || last === '');
((result.host || relative.host || srcPath.length > 1) &&
(last === '.' || last === '..')) || last === '');

// Strip single dots, resolve double dots to parent dir
// if the path tries to go above the root, `up` ends up > 0
Expand Down
2 changes: 1 addition & 1 deletion lib/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function isFunction(arg) {

function isPrimitive(arg) {
return arg === null ||
typeof arg !== 'object' && typeof arg !== 'function';
(typeof arg !== 'object' && typeof arg !== 'function');
}

function pad(n) {
Expand Down