Skip to content

Commit

Permalink
refactor(manager): more strict null checks (#15166)
Browse files Browse the repository at this point in the history
  • Loading branch information
viceice authored Apr 19, 2022
1 parent dfa04b6 commit 6c7e79f
Show file tree
Hide file tree
Showing 31 changed files with 239 additions and 215 deletions.
12 changes: 6 additions & 6 deletions lib/modules/manager/homebrew/__snapshots__/extract.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ Object {
"deps": Array [
Object {
"currentValue": null,
"datasource": null,
"datasource": undefined,
"depName": "Acmetool",
"managerData": Object {
"ownerName": null,
Expand All @@ -78,7 +78,7 @@ Object {
"deps": Array [
Object {
"currentValue": null,
"datasource": null,
"datasource": undefined,
"depName": "Ibazel",
"managerData": Object {
"ownerName": null,
Expand All @@ -97,7 +97,7 @@ Object {
"deps": Array [
Object {
"currentValue": null,
"datasource": null,
"datasource": undefined,
"depName": "Ibazel",
"managerData": Object {
"ownerName": null,
Expand Down Expand Up @@ -154,7 +154,7 @@ Object {
"deps": Array [
Object {
"currentValue": null,
"datasource": null,
"datasource": undefined,
"depName": "Ibazel",
"managerData": Object {
"ownerName": null,
Expand All @@ -173,7 +173,7 @@ Object {
"deps": Array [
Object {
"currentValue": null,
"datasource": null,
"datasource": undefined,
"depName": "Aalib",
"managerData": Object {
"ownerName": null,
Expand All @@ -192,7 +192,7 @@ Object {
"deps": Array [
Object {
"currentValue": null,
"datasource": null,
"datasource": undefined,
"depName": "Aap",
"managerData": Object {
"ownerName": null,
Expand Down
16 changes: 9 additions & 7 deletions lib/modules/manager/homebrew/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ function extractUrl(content: string): string | null {
return parseUrl(i, content);
}

export function parseUrlPath(urlStr: string): UrlPathParsedResult | null {
export function parseUrlPath(
urlStr: string | null | undefined
): UrlPathParsedResult | null {
if (!urlStr) {
return null;
}
Expand All @@ -69,7 +71,7 @@ export function parseUrlPath(urlStr: string): UrlPathParsedResult | null {
s = s.filter((val) => val);
const ownerName = s[0];
const repoName = s[1];
let currentValue: string;
let currentValue: string | undefined;
if (s[2] === 'archive') {
currentValue = s[3];
const targz = currentValue.slice(
Expand Down Expand Up @@ -146,10 +148,10 @@ export function extractPackageFile(content: string): PackageFile | null {
logger.debug('Invalid URL field');
}
const urlPathResult = parseUrlPath(url);
let skipReason: SkipReason;
let currentValue: string = null;
let ownerName: string = null;
let repoName: string = null;
let skipReason: SkipReason | undefined;
let currentValue: string | null = null;
let ownerName: string | null = null;
let repoName: string | null = null;
if (urlPathResult) {
currentValue = urlPathResult.currentValue;
ownerName = urlPathResult.ownerName;
Expand All @@ -173,7 +175,7 @@ export function extractPackageFile(content: string): PackageFile | null {
dep.skipReason = skipReason;
if (skipReason === 'unsupported-url') {
dep.depName = className;
dep.datasource = null;
dep.datasource = undefined;
}
}
const deps = [dep];
Expand Down
10 changes: 5 additions & 5 deletions lib/modules/manager/homebrew/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function getUrlTestContent(
content: string,
oldUrl: string,
newUrl: string
): string {
): string | null {
const urlRegExp = /(^|\s)url(\s)/;
const cleanContent = removeComments(content);
let j = cleanContent.search(urlRegExp);
Expand Down Expand Up @@ -60,7 +60,7 @@ function updateUrl(
if (!newContent || !testContent) {
return null;
}
while (removeComments(newContent) !== testContent) {
while (newContent && removeComments(newContent) !== testContent) {
i += 'url'.length;
i += content.substring(i).search(urlRegExp);
if (isSpace(content[i])) {
Expand Down Expand Up @@ -124,7 +124,7 @@ function updateSha256(
if (!newContent || !testContent) {
return null;
}
while (removeComments(newContent) !== testContent) {
while (newContent && removeComments(newContent) !== testContent) {
i += 'sha256'.length;
i += content.substring(i).search(sha256RegExp);
if (isSpace(content[i])) {
Expand All @@ -148,8 +148,8 @@ export async function updateDependency({
// Example urls:
// "https://github.com/bazelbuild/bazel-watcher/archive/v0.8.2.tar.gz"
// "https://github.com/aide/aide/releases/download/v0.16.1/aide-0.16.1.tar.gz"
const oldParsedUrlPath = parseUrlPath(upgrade.managerData.url);
if (!oldParsedUrlPath) {
const oldParsedUrlPath = parseUrlPath(upgrade.managerData?.url);
if (!oldParsedUrlPath || !upgrade.managerData) {
logger.debug(
`Failed to update - upgrade.managerData.url is invalid ${upgrade.depName}`
);
Expand Down
3 changes: 2 additions & 1 deletion lib/modules/manager/jsonnet-bundler/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import type {
} from '../types';

function dependencyUrl(dep: PackageDependency): string {
const url = dep.packageName;
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const url = dep.packageName!;
if (dep.managerData?.subdir) {
return url.concat('/', dep.managerData.subdir);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/modules/manager/jsonnet-bundler/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function extractDependency(dependency: Dependency): PackageDependency | null {

return {
depName:
dependency.name || match.groups.depName || dependency.source.git.remote,
dependency.name ?? match?.groups?.depName ?? dependency.source.git.remote,
packageName: dependency.source.git.remote,
currentValue: dependency.version,
managerData: { subdir: dependency.source.git.subdir },
Expand Down
53 changes: 31 additions & 22 deletions lib/modules/manager/maven/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ export function parsePom(raw: string): XmlDocument | null {
return null;
}

function containsPlaceholder(str: string): boolean {
return regEx(/\${.*?}/g).test(str);
function containsPlaceholder(str: string | null | undefined): boolean {
return !!str && regEx(/\${.*?}/g).test(str);
}

function depFromNode(
Expand All @@ -46,15 +46,15 @@ function depFromNode(
let groupId = node.valueWithPath('groupId')?.trim();
const artifactId = node.valueWithPath('artifactId')?.trim();
const currentValue = node.valueWithPath('version')?.trim();
let depType: string;
let depType: string | undefined;

if (!groupId && node.name === 'plugin') {
groupId = 'org.apache.maven.plugins';
}

if (groupId && artifactId && currentValue) {
const depName = `${groupId}:${artifactId}`;
const versionNode = node.descendantWithPath('version');
const versionNode = node.descendantWithPath('version')!;
const fileReplacePosition = versionNode.position;
const datasource = MavenDatasource.id;
const registryUrls = [MAVEN_REPO];
Expand Down Expand Up @@ -162,7 +162,8 @@ function applyPropsInternal(
const replaceAll = (str: string): string =>
str.replace(regEx(/\${.*?}/g), (substr) => {
const propKey = substr.slice(2, -1).trim();
const propValue = props[propKey];
// TODO: wrong types here, props is already `MavenProp`
const propValue = (props as any)[propKey] as MavenProp;
if (propValue) {
anyChange = true;
if (alreadySeenProps.find((it) => it === propKey)) {
Expand All @@ -175,23 +176,27 @@ function applyPropsInternal(
return substr;
});

const depName = replaceAll(dep.depName);
const registryUrls = dep.registryUrls.map((url) => replaceAll(url));
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const depName = replaceAll(dep.depName!);
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const registryUrls = dep.registryUrls!.map((url) => replaceAll(url));

let fileReplacePosition = dep.fileReplacePosition;
let propSource = dep.propSource;
let groupName = null;
const currentValue = dep.currentValue.replace(
let groupName: string | null = null;
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const currentValue = dep.currentValue!.replace(
regEx(/^\${.*?}$/),
(substr) => {
const propKey = substr.slice(2, -1).trim();
const propValue = props[propKey];
// TODO: wrong types here, props is already `MavenProp`
const propValue = (props as any)[propKey] as MavenProp;
if (propValue) {
if (!groupName) {
groupName = propKey;
}
fileReplacePosition = propValue.fileReplacePosition;
propSource = propValue.packageFile;
propSource = propValue.packageFile ?? undefined;
anyChange = true;
if (alreadySeenProps.find((it) => it === propKey)) {
fatal = true;
Expand Down Expand Up @@ -273,16 +278,17 @@ export function extractPackage(

const repositories = project.childNamed('repositories');
if (repositories?.children) {
const repoUrls = [];
const repoUrls: string[] = [];
for (const repo of repositories.childrenNamed('repository')) {
const repoUrl = repo.valueWithPath('url')?.trim();
if (repoUrl) {
repoUrls.push(repoUrl);
}
}
result.deps.forEach((dep) => {
if (dep.registryUrls) {
repoUrls.forEach((url) => dep.registryUrls.push(url));
if (is.array(dep.registryUrls)) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
repoUrls.forEach((url) => dep.registryUrls!.push(url));
}
});
}
Expand All @@ -294,7 +300,7 @@ export function extractPackage(
}

if (project.childNamed('version')) {
result.packageFileVersion = project.valueWithPath('version').trim();
result.packageFileVersion = project.valueWithPath('version')!.trim();
}

return result;
Expand All @@ -310,7 +316,7 @@ export function extractRegistries(rawContent: string): string[] {
return [];
}

const urls = [];
const urls: string[] = [];

const mirrorUrls = parseUrls(settings, 'mirrors');
urls.push(...mirrorUrls);
Expand All @@ -326,7 +332,7 @@ export function extractRegistries(rawContent: string): string[] {

function parseUrls(xmlNode: XmlElement, path: string): string[] {
const children = xmlNode.descendantWithPath(path);
const urls = [];
const urls: string[] = [];
if (children?.children) {
children.eachChild((child) => {
const url = child.valueWithPath('url');
Expand Down Expand Up @@ -362,7 +368,8 @@ export function resolveParents(packages: PackageFile[]): PackageFile[] {
const extractedProps: Record<string, MavenProp> = {};
const registryUrls: Record<string, Set<string>> = {};
packages.forEach((pkg) => {
const name = pkg.packageFile;
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const name = pkg.packageFile!;
packageFileNames.push(name);
extractedPackages[name] = pkg;
extractedDeps[name] = [];
Expand All @@ -375,9 +382,10 @@ export function resolveParents(packages: PackageFile[]): PackageFile[] {
registryUrls[name] = new Set();
const propsHierarchy: Record<string, MavenProp>[] = [];
const visitedPackages: Set<string> = new Set();
let pkg = extractedPackages[name];
let pkg: PackageFile | null = extractedPackages[name];
while (pkg) {
propsHierarchy.unshift(pkg.mavenProps);
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
propsHierarchy.unshift(pkg.mavenProps!);

if (pkg.deps) {
pkg.deps.forEach((dep) => {
Expand All @@ -404,7 +412,8 @@ export function resolveParents(packages: PackageFile[]): PackageFile[] {
packageFileNames.forEach((name) => {
const pkg = extractedPackages[name];
pkg.deps.forEach((rawDep) => {
const urlsSet = new Set([...rawDep.registryUrls, ...registryUrls[name]]);
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const urlsSet = new Set([...rawDep.registryUrls!, ...registryUrls[name]]);
rawDep.registryUrls = [...urlsSet];
});
});
Expand Down Expand Up @@ -442,7 +451,7 @@ export async function extractAllPackageFiles(
packageFiles: string[]
): Promise<PackageFile[]> {
const packages: PackageFile[] = [];
const additionalRegistryUrls = [];
const additionalRegistryUrls: string[] = [];

for (const packageFile of packageFiles) {
const content = await readLocalFile(packageFile, 'utf8');
Expand Down
2 changes: 1 addition & 1 deletion lib/modules/manager/maven/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export interface MavenProp {
val: string;
fileReplacePosition: number;
packageFile: string;
packageFile?: string | null;
}
6 changes: 4 additions & 2 deletions lib/modules/manager/maven/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ export function updateAtPosition(
return fileContent;
}
if (version === currentValue || upgrade.groupName) {
const replacedPart = versionPart.replace(version, newValue);
// TODO: validate newValue
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
const replacedPart = versionPart.replace(version, newValue!);
return leftPart + replacedPart + restPart;
}
logger.debug({ depName, version, currentValue, newValue }, 'Unknown value');
Expand Down Expand Up @@ -78,7 +80,7 @@ export function bumpPackageVersion(

try {
const project = new XmlDocument(content);
const versionNode = project.childNamed('version');
const versionNode = project.childNamed('version')!;
const startTagPosition = versionNode.startTagPosition;
const versionPosition = content.indexOf(versionNode.val, startTagPosition);

Expand Down
8 changes: 6 additions & 2 deletions lib/modules/manager/mix/artifacts.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import is from '@sindresorhus/is';
import { quote } from 'shlex';
import { TEMPORARY_ERROR } from '../../../constants/error-messages';
import { logger } from '../../../logger';
Expand Down Expand Up @@ -70,7 +71,7 @@ export async function updateArtifacts({
}

return acc;
}, []);
}, [] as string[]);

const execOptions: ExecOptions = {
cwdFile: packageFileName,
Expand All @@ -82,7 +83,10 @@ export async function updateArtifacts({
const command = [
'mix',
'deps.update',
...updatedDeps.map((dep) => quote(dep.depName)),
...updatedDeps
.map((dep) => dep.depName)
.filter(is.string)
.map((dep) => quote(dep)),
].join(' ');

try {
Expand Down
2 changes: 1 addition & 1 deletion lib/modules/manager/pip-compile/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function constructPipCompileCmd(
): string {
const headers = constraintLineRegex.exec(content);
const args = ['pip-compile'];
if (headers) {
if (headers?.groups) {
logger.debug({ header: headers[0] }, 'Found pip-compile header');
for (const argument of split(headers.groups.arguments)) {
if (['--allow-unsafe', '--generate-hashes'].includes(argument)) {
Expand Down
Loading

0 comments on commit 6c7e79f

Please sign in to comment.