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

First attempt at enable noUncheckedIndexedAccess TS compiler option #4039

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 8 additions & 5 deletions _tools/check_circular_submodule_dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ type Dep = {
const root = new URL("../", import.meta.url).href;
const deps: Record<string, Dep> = {};

function getSubmoduleNameFromUrl(url: string) {
return url.replace(root, "").split("/")[0];
function getSubmoduleNameFromUrl(url: string): string {
const name = url.replace(root, "").split("/")[0];
if (typeof name !== "string") {
throw new Error("Submodule url is not valid");
}
return name;
}

async function check(
Expand Down Expand Up @@ -172,10 +176,9 @@ function stateToNodeStyle(state: DepState) {

if (Deno.args.includes("--graph")) {
console.log("digraph std_deps {");
for (const mod of Object.keys(deps)) {
const info = deps[mod];
for (const [mod, info] of Object.entries(deps)) {
console.log(` ${formatLabel(mod)} ${stateToNodeStyle(info.state)};`);
for (const dep of deps[mod].set) {
for (const dep of info.set) {
console.log(` ${formatLabel(mod)} -> ${dep};`);
}
}
Expand Down
2 changes: 1 addition & 1 deletion _tools/check_doc_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function checkCodeBlocks(
lineNumber = 1,
): void {
for (const codeBlockMatch of content.matchAll(RX_CODE_BLOCK)) {
const [, language, codeBlock] = codeBlockMatch;
const [, language = "", codeBlock = ""] = codeBlockMatch;
const codeBlockLineNumber =
content.slice(0, codeBlockMatch.index).split("\n").length;

Expand Down
2 changes: 1 addition & 1 deletion _tools/check_licence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ for await (
console.log("Copyright header automatically added to " + path);
}
} else if (
parseInt(match[1]) !== FIRST_YEAR || parseInt(match[2]) !== CURRENT_YEAR
parseInt(match[1]!) !== FIRST_YEAR || parseInt(match[2]!) !== CURRENT_YEAR
) {
if (CHECK) {
console.error(`Incorrect copyright year: ${path}`);
Expand Down
1 change: 1 addition & 0 deletions deno.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"compilerOptions": {
"strict": true,
"useUnknownInCatchVariables": true,
"noUncheckedIndexedAccess": true,
"noImplicitOverride": true
},
"imports": {
Expand Down
14 changes: 9 additions & 5 deletions path/_common/glob_to_reg_exp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function _globToRegExp(

// Remove trailing separators.
let newLength = glob.length;
for (; newLength > 1 && c.seps.includes(glob[newLength - 1]); newLength--);
for (; newLength > 1 && c.seps.includes(glob[newLength - 1]!); newLength--);
glob = glob.slice(0, newLength);
Comment on lines +68 to 69
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like there might be a bug here, the semicolon at the end of the for statement means line below is only ever run once. The for loop itself does nothing.


let regExpString = "";
Expand All @@ -80,11 +80,13 @@ export function _globToRegExp(
let i = j;

// Terminates with `i` at the non-inclusive end of the current segment.
for (; i < glob.length && !c.seps.includes(glob[i]); i++) {
for (; i < glob.length && !c.seps.includes(glob[i] ?? ""); i++) {
if (inEscape) {
inEscape = false;
const escapeChars = inRange ? rangeEscapeChars : regExpEscapeChars;
segment += escapeChars.includes(glob[i]) ? `\\${glob[i]}` : glob[i];
segment += escapeChars.includes(glob[i] ?? "")
? `\\${glob[i]}`
: glob[i];
continue;
}

Expand Down Expand Up @@ -247,7 +249,9 @@ export function _globToRegExp(
continue;
}

segment += regExpEscapeChars.includes(glob[i]) ? `\\${glob[i]}` : glob[i];
segment += regExpEscapeChars.includes(glob[i] ?? "")
? `\\${glob[i]}`
: glob[i];
}

// Check for unclosed groups or a dangling backslash.
Expand All @@ -267,7 +271,7 @@ export function _globToRegExp(
}

// Terminates with `i` at the start of the next segment.
while (c.seps.includes(glob[i])) i++;
while (c.seps.includes(glob[i] ?? "")) i++;

// Check that the next value of `j` is indeed higher than the current value.
if (!(i > j)) {
Expand Down
3 changes: 1 addition & 2 deletions path/posix/join.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ export function join(...paths: string[]): string {
if (paths.length === 0) return ".";

let joined: string | undefined;
for (let i = 0, len = paths.length; i < len; ++i) {
const path = paths[i];
for (const path of paths) {
assertPath(path);
if (path.length > 0) {
if (!joined) joined = path;
Expand Down
5 changes: 3 additions & 2 deletions path/posix/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ export function resolve(...pathSegments: string[]): string {
for (let i = pathSegments.length - 1; i >= -1 && !resolvedAbsolute; i--) {
let path: string;

if (i >= 0) path = pathSegments[i];
else {
if (i >= 0) {
path = pathSegments[i]!;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i starts at pathSegments.length and decrements down to -1, however this statement is protected with i >= 0 so the i should always be inbounds.

} else {
// deno-lint-ignore no-explicit-any
const { Deno } = globalThis as any;
if (typeof Deno?.cwd !== "function") {
Expand Down
3 changes: 1 addition & 2 deletions path/windows/join.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ export function join(...paths: string[]): string {

let joined: string | undefined;
let firstPart: string | null = null;
for (let i = 0; i < paths.length; ++i) {
const path = paths[i];
for (const path of paths) {
assertPath(path);
if (path.length > 0) {
if (joined === undefined) joined = firstPart = path;
Expand Down
6 changes: 5 additions & 1 deletion path/windows/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ export function resolve(...pathSegments: string[]): string {
// deno-lint-ignore no-explicit-any
const { Deno } = globalThis as any;
if (i >= 0) {
path = pathSegments[i];
const segment = pathSegments[i];
if (typeof segment !== "string") {
throw new Error("Unexpected out of bounds on resolve() pathSegments");
}
path = segment;
} else if (!resolvedDevice) {
if (typeof Deno?.cwd !== "function") {
throw new TypeError("Resolved a drive-letter-less path without a CWD.");
Expand Down
3 changes: 3 additions & 0 deletions path/windows/to_file_url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export function toFileUrl(path: string): URL {
const [, hostname, pathname] = path.match(
/^(?:[/\\]{2}([^/\\]+)(?=[/\\](?:[^/\\]|$)))?(.*)/,
)!;
if (typeof pathname !== "string") {
throw new Error("Missing pathname in file path");
}
const url = new URL("file:///");
url.pathname = encodeWhitespace(pathname.replace(/%/g, "%25"));
if (hostname !== undefined && hostname !== "localhost") {
Expand Down
12 changes: 6 additions & 6 deletions semver/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,19 @@ export function parse(version: string | SemVer): SemVer {
if (!groups) throw new TypeError(`Invalid Version: ${version}`);

// these are actually numbers
const major = parseInt(groups.major);
const minor = parseInt(groups.minor);
const patch = parseInt(groups.patch);
const major = parseInt(groups.major!);
const minor = parseInt(groups.minor!);
const patch = parseInt(groups.patch!);
Comment on lines +44 to +46
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ignoring the lint error with ! as parseInt(undefined) will parse to NaN and I've added a new isNaN check below.


if (major > Number.MAX_SAFE_INTEGER || major < 0) {
if (major > Number.MAX_SAFE_INTEGER || major < 0 || Number.isNaN(major)) {
throw new TypeError("Invalid major version");
}

if (minor > Number.MAX_SAFE_INTEGER || minor < 0) {
if (minor > Number.MAX_SAFE_INTEGER || minor < 0 || Number.isNaN(minor)) {
throw new TypeError("Invalid minor version");
}

if (patch > Number.MAX_SAFE_INTEGER || patch < 0) {
if (patch > Number.MAX_SAFE_INTEGER || patch < 0 || Number.isNaN(patch)) {
throw new TypeError("Invalid patch version");
}

Expand Down
33 changes: 26 additions & 7 deletions semver/parse_range.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function replaceTildes(comp: string): string {
}

function replaceTilde(comp: string): string {
const groups = comp.match(TILDE_REGEXP)?.groups;
const groups = parseGroup(comp.match(TILDE_REGEXP));
if (!groups) return comp;
const { major, minor, patch, prerelease } = groups;
if (isWildcard(major)) {
Expand Down Expand Up @@ -59,7 +59,7 @@ function replaceCarets(comp: string): string {
}

function replaceCaret(comp: string): string {
const groups = comp.match(CARET_REGEXP)?.groups;
const groups = parseGroup(comp.match(CARET_REGEXP));
if (!groups) return comp;
const { major, minor, patch, prerelease } = groups;

Expand Down Expand Up @@ -183,19 +183,19 @@ function replaceStars(comp: string): string {
function hyphenReplace(range: string) {
// convert `1.2.3 - 1.2.4` into `>=1.2.3 <=1.2.4`
const leftMatch = range.match(new RegExp(`^${XRANGE_PLAIN}`));
const leftGroup = leftMatch?.groups;
const leftGroup = parseGroup(leftMatch);
if (!leftGroup) return range;
const leftLength = leftMatch[0].length;
const leftLength = leftMatch?.[0].length ?? 0;
const hyphenMatch = range.slice(leftLength).match(/^\s+-\s+/);
if (!hyphenMatch) return range;
const hyphenLength = hyphenMatch[0].length;
const rightMatch = range.slice(leftLength + hyphenLength).match(
new RegExp(`^${XRANGE_PLAIN}\\s*$`),
);
const rightGroups = rightMatch?.groups;
const rightGroups = parseGroup(rightMatch);
if (!rightGroups) return range;
let from = leftMatch[0];
let to = rightMatch[0];
let from = leftMatch?.[0];
let to = rightMatch?.[0];

if (isWildcard(leftGroup.major)) {
from = "";
Expand Down Expand Up @@ -227,6 +227,25 @@ function isWildcard(id: string): boolean {
return !id || id.toLowerCase() === "x" || id === "*";
}

type Group = {
major: string;
minor: string;
patch: string;
prerelease: string;
};
function parseGroup(match: RegExpMatchArray | null): Group | null {
if (!match || !match?.groups?.length) {
return null;
}
const version = match.groups;
return {
major: version.major ?? "",
minor: version.minor ?? "",
patch: version.patch ?? "",
prerelease: version.prerelease ?? "",
};
}

/**
* Parses a range string into a SemVerRange object or throws a TypeError.
* @param range The range string
Expand Down
3 changes: 1 addition & 2 deletions semver/range_intersects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ function rangesSatisfiable(ranges: SemVerRange[]): boolean {

function comparatorsSatisfiable(comparators: SemVerComparator[]): boolean {
// Comparators are satisfiable if they all intersect with each other
for (let i = 0; i < comparators.length - 1; i++) {
const c0 = comparators[i];
for (const [i, c0] of comparators.entries()) {
for (const c1 of comparators.slice(i + 1)) {
if (!comparatorIntersects(c0, c1)) {
return false;
Expand Down
Loading