Skip to content

Commit

Permalink
lib: fix JSDoc issues
Browse files Browse the repository at this point in the history
Updating ESLint and dependencies will start flagging a few additional
JSDoc issues. One or two of these are simple fixes. The ESM stuff
requires throwing explicitly in JSDoc'ed functions rather than calling
another function to throw. I think this makes the code easier to
understand--you don't need to know that a particular function that
starts with `throwsIf` *might* throw but something that starts with
`throwsAnythingElse` will always throw. Instead, it's right there in the
code. This also might make it easier to improve stack traces if that's
something we'd like to do at some point.

PR-URL: #45243
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
  • Loading branch information
Trott authored and RafaelGSS committed Nov 10, 2022
1 parent 1f0edde commit a8e9015
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 19 deletions.
2 changes: 1 addition & 1 deletion lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function createConnection(port, host, options) {
* maxCachedSessions?: number;
* servername?: string;
* }} [options]
* @returns {Agent}
* @constructor
*/
function Agent(options) {
if (!(this instanceof Agent))
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/esm/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function validateAssertions(url, format,
// `type` wasn't specified at all.
throw new ERR_IMPORT_ASSERTION_TYPE_MISSING(url, validType);
}
handleInvalidType(url, importAssertions.type);
return handleInvalidType(url, importAssertions.type);
}
}

Expand Down
58 changes: 42 additions & 16 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,8 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
* @param {URL} packageJSONUrl
* @param {string | URL | undefined} base
*/
function throwImportNotDefined(specifier, packageJSONUrl, base) {
throw new ERR_PACKAGE_IMPORT_NOT_DEFINED(
function importNotDefined(specifier, packageJSONUrl, base) {
return new ERR_PACKAGE_IMPORT_NOT_DEFINED(
specifier, packageJSONUrl && fileURLToPath(new URL('.', packageJSONUrl)),
fileURLToPath(base));
}
Expand All @@ -288,8 +288,8 @@ function throwImportNotDefined(specifier, packageJSONUrl, base) {
* @param {URL} packageJSONUrl
* @param {string | URL | undefined} base
*/
function throwExportsNotFound(subpath, packageJSONUrl, base) {
throw new ERR_PACKAGE_PATH_NOT_EXPORTED(
function exportsNotFound(subpath, packageJSONUrl, base) {
return new ERR_PACKAGE_PATH_NOT_EXPORTED(
fileURLToPath(new URL('.', packageJSONUrl)), subpath,
base && fileURLToPath(base));
}
Expand All @@ -310,14 +310,14 @@ function throwInvalidSubpath(request, match, packageJSONUrl, internal, base) {
base && fileURLToPath(base));
}

function throwInvalidPackageTarget(
function invalidPackageTarget(
subpath, target, packageJSONUrl, internal, base) {
if (typeof target === 'object' && target !== null) {
target = JSONStringify(target, null, '');
} else {
target = `${target}`;
}
throw new ERR_INVALID_PACKAGE_TARGET(
return new ERR_INVALID_PACKAGE_TARGET(
fileURLToPath(new URL('.', packageJSONUrl)), subpath, target,
internal, base && fileURLToPath(base));
}
Expand All @@ -327,6 +327,19 @@ const deprecatedInvalidSegmentRegEx = /(^|\\|\/)((\.|%2e)(\.|%2e)?|(n|%6e|%4e)(o
const invalidPackageNameRegEx = /^\.|%|\\/;
const patternRegEx = /\*/g;

/**
*
* @param {string} target
* @param {*} subpath
* @param {*} match
* @param {*} packageJSONUrl
* @param {*} base
* @param {*} pattern
* @param {*} internal
* @param {*} isPathMap
* @param {*} conditions
* @returns {URL}
*/
function resolvePackageTargetString(
target,
subpath,
Expand All @@ -340,7 +353,7 @@ function resolvePackageTargetString(
) {

if (subpath !== '' && !pattern && target[target.length - 1] !== '/')
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
throw invalidPackageTarget(match, target, packageJSONUrl, internal, base);

if (!StringPrototypeStartsWith(target, './')) {
if (internal && !StringPrototypeStartsWith(target, '../') &&
Expand All @@ -360,7 +373,7 @@ function resolvePackageTargetString(
exportTarget, packageJSONUrl, conditions);
}
}
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
throw invalidPackageTarget(match, target, packageJSONUrl, internal, base);
}

if (RegExpPrototypeExec(invalidSegmentRegEx, StringPrototypeSlice(target, 2)) !== null) {
Expand All @@ -375,7 +388,7 @@ function resolvePackageTargetString(
emitInvalidSegmentDeprecation(resolvedTarget, request, match, packageJSONUrl, internal, base, true);
}
} else {
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
throw invalidPackageTarget(match, target, packageJSONUrl, internal, base);
}
}

Expand All @@ -384,7 +397,7 @@ function resolvePackageTargetString(
const packagePath = new URL('.', packageJSONUrl).pathname;

if (!StringPrototypeStartsWith(resolvedPath, packagePath))
throwInvalidPackageTarget(match, target, packageJSONUrl, internal, base);
throw invalidPackageTarget(match, target, packageJSONUrl, internal, base);

if (subpath === '') return resolved;

Expand Down Expand Up @@ -421,6 +434,19 @@ function isArrayIndex(key) {
return keyNum >= 0 && keyNum < 0xFFFF_FFFF;
}

/**
*
* @param {*} packageJSONUrl
* @param {string|[string]} target
* @param {*} subpath
* @param {*} packageSubpath
* @param {*} base
* @param {*} pattern
* @param {*} internal
* @param {*} isPathMap
* @param {*} conditions
* @returns {URL|null}
*/
function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
base, pattern, internal, isPathMap, conditions) {
if (typeof target === 'string') {
Expand Down Expand Up @@ -485,8 +511,8 @@ function resolvePackageTarget(packageJSONUrl, target, subpath, packageSubpath,
} else if (target === null) {
return null;
}
throwInvalidPackageTarget(packageSubpath, target, packageJSONUrl, internal,
base);
throw invalidPackageTarget(packageSubpath, target, packageJSONUrl, internal,
base);
}

/**
Expand Down Expand Up @@ -543,7 +569,7 @@ function packageExportsResolve(
);

if (resolveResult == null) {
throwExportsNotFound(packageSubpath, packageJSONUrl, base);
throw exportsNotFound(packageSubpath, packageJSONUrl, base);
}

return resolveResult;
Expand Down Expand Up @@ -594,12 +620,12 @@ function packageExportsResolve(
conditions);

if (resolveResult == null) {
throwExportsNotFound(packageSubpath, packageJSONUrl, base);
throw exportsNotFound(packageSubpath, packageJSONUrl, base);
}
return resolveResult;
}

throwExportsNotFound(packageSubpath, packageJSONUrl, base);
throw exportsNotFound(packageSubpath, packageJSONUrl, base);
}

function patternKeyCompare(a, b) {
Expand Down Expand Up @@ -679,7 +705,7 @@ function packageImportsResolve(name, base, conditions) {
}
}
}
throwImportNotDefined(name, packageJSONUrl, base);
throw importNotDefined(name, packageJSONUrl, base);
}

/**
Expand Down
1 change: 0 additions & 1 deletion lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,6 @@ function validateBooleanArray(value, name) {
}
}

// eslint-disable-next-line jsdoc/require-returns-check
/**
* @param {*} signal
* @param {string} [name='signal']
Expand Down

0 comments on commit a8e9015

Please sign in to comment.