Skip to content

Commit

Permalink
refactor: renames functions that assert things so they reflect that (#…
Browse files Browse the repository at this point in the history
…848)

## Description

* in the cli renames assertion that checks whether the node environment
is suitable
* in the main 'option' check renames functions that assert whether the
options are valid
* in the main 'rule-set' renames functions that assert whether the rule
set is valid

## Motivation and Context

A lot of the assertions dependency-cruiser makes before starting were
called 'validate' - which is not as clear/ descriptive of the actual
functionality of the functions as 'assert' is. Renaming

## How Has This Been Tested?

- [x] green ci
- [x] updated automated non-regression tests

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [x] Refactor (non-breaking change which fixes an issue without
changing functionality)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist

- [x] 📖

  - My change doesn't require a documentation update, or ...
  - it _does_ and I have updated it

- [x] ⚖️
- The contribution will be subject to [The MIT
license](https://github.com/sverweij/dependency-cruiser/blob/main/LICENSE),
and I'm OK with that.
  - The contribution is my own original work.
- I am ok with the stuff in
[**CONTRIBUTING.md**](https://github.com/sverweij/dependency-cruiser/blob/main/.github/CONTRIBUTING.md).
  • Loading branch information
sverweij authored Sep 30, 2023
1 parent ddd2ee6 commit c2d731c
Show file tree
Hide file tree
Showing 12 changed files with 112 additions and 111 deletions.
4 changes: 2 additions & 2 deletions bin/depcruise-baseline.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env node
import { program } from "commander";
import validateNodeEnvironment from "../src/cli/validate-node-environment.mjs";
import assertNodeEnvironmentSuitable from "../src/cli/assert-node-environment-suitable.mjs";
import meta from "../src/meta.js";
import cli from "../src/cli/index.mjs";

Expand All @@ -10,7 +10,7 @@ function formatError(pError) {
}

try {
validateNodeEnvironment();
assertNodeEnvironmentSuitable();

program
.description(
Expand Down
4 changes: 2 additions & 2 deletions bin/depcruise-fmt.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env node

import { program } from "commander";
import validateNodeEnvironment from "../src/cli/validate-node-environment.mjs";
import assertNodeEnvironmentSuitable from "../src/cli/assert-node-environment-suitable.mjs";
import meta from "../src/meta.js";
import format from "../src/cli/format.mjs";

Expand All @@ -11,7 +11,7 @@ function formatError(pError) {
}

try {
validateNodeEnvironment();
assertNodeEnvironmentSuitable();

program
.description(
Expand Down
4 changes: 2 additions & 2 deletions bin/dependency-cruise.mjs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#!/usr/bin/env node
import { EOL } from "node:os";
import { program } from "commander";
import validateNodeEnvironment from "../src/cli/validate-node-environment.mjs";
import assertNodeEnvironmentSuitable from "../src/cli/assert-node-environment-suitable.mjs";
import meta from "../src/meta.js";
import cli from "../src/cli/index.mjs";

try {
validateNodeEnvironment();
assertNodeEnvironmentSuitable();

program
.description(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import satisfies from "semver/functions/satisfies.js";
import meta from "../meta.js";

export default function validateNodeEnvironment(pNodeVersion) {
export default function assertNodeEnvironmentSuitable(pNodeVersion) {
// not using default parameter here because the check should run
// run on node 4 as well
const lNodeVersion = pNodeVersion || process.versions.node;
Expand Down
24 changes: 12 additions & 12 deletions src/main/cruise.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable no-return-await */
/* eslint-disable no-magic-numbers */
import { bus } from "../utl/bus.mjs";
import { validateCruiseOptions } from "./options/validate.mjs";
import { assertCruiseOptionsValid } from "./options/assert-validity.mjs";
import { normalizeCruiseOptions } from "./options/normalize.mjs";
import reportWrap from "./report-wrap.mjs";

Expand All @@ -17,20 +17,20 @@ export default async function cruise(
pFileAndDirectoryArray,
pCruiseOptions,
pResolveOptions,
pTranspileOptions
pTranspileOptions,
) {
bus.summary("parsing options", c(1));
/** @type {import("../../types/strict-options.js").IStrictCruiseOptions} */
let lCruiseOptions = normalizeCruiseOptions(
validateCruiseOptions(pCruiseOptions),
pFileAndDirectoryArray
assertCruiseOptionsValid(pCruiseOptions),
pFileAndDirectoryArray,
);
let lCache = null;

if (lCruiseOptions.cache) {
bus.summary(
`cache: checking freshness with ${lCruiseOptions.cache.strategy}`,
c(2)
c(2),
);

const { default: Cache } = await import("../cache/cache.mjs");
Expand All @@ -46,7 +46,7 @@ export default async function cruise(
bus.summary("importing analytical modules", c(3));
const [
{ default: normalizeRuleSet },
{ default: validateRuleSet },
{ default: assertRuleSetValid },
{ default: normalizeFilesAndDirectories },
{ default: normalizeResolveOptions },
{ default: extract },
Expand All @@ -55,7 +55,7 @@ export default async function cruise(
// despite rule set parsing being behind an if, it's the 'normal' use case
// for dependency-cruiser, so import it unconditionally nonetheless
import("./rule-set/normalize.mjs"),
import("./rule-set/validate.mjs"),
import("./rule-set/assert-validity.mjs"),
import("./files-and-dirs/normalize.mjs"),
import("./resolve-options/normalize.mjs"),
import("../extract/index.mjs"),
Expand All @@ -65,34 +65,34 @@ export default async function cruise(
if (Boolean(lCruiseOptions.ruleSet)) {
bus.summary("parsing rule set", c(4));
lCruiseOptions.ruleSet = normalizeRuleSet(
validateRuleSet(lCruiseOptions.ruleSet)
assertRuleSetValid(lCruiseOptions.ruleSet),
);
}

const lNormalizedFileAndDirectoryArray = normalizeFilesAndDirectories(
pFileAndDirectoryArray
pFileAndDirectoryArray,
);

bus.summary("determining how to resolve", c(5));
const lNormalizedResolveOptions = await normalizeResolveOptions(
pResolveOptions,
lCruiseOptions,
pTranspileOptions?.tsConfig
pTranspileOptions?.tsConfig,
);

bus.summary("reading files", c(6));
const lExtractionResult = extract(
lNormalizedFileAndDirectoryArray,
lCruiseOptions,
lNormalizedResolveOptions,
pTranspileOptions
pTranspileOptions,
);

bus.summary("analyzing", c(7));
const lCruiseResult = enrich(
lExtractionResult,
lCruiseOptions,
lNormalizedFileAndDirectoryArray
lNormalizedFileAndDirectoryArray,
);

if (lCruiseOptions.cache) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/format.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Ajv from "ajv";

import cruiseResultSchema from "../schema/cruise-result.schema.mjs";
import { validateFormatOptions } from "./options/validate.mjs";
import { assertFormatOptionsValid } from "./options/assert-validity.mjs";
import { normalizeFormatOptions } from "./options/normalize.mjs";
import reportWrap from "./report-wrap.mjs";

Expand All @@ -10,14 +10,14 @@ function validateResultAgainstSchema(pResult) {

if (!ajv.validate(cruiseResultSchema, pResult)) {
throw new Error(
`The supplied dependency-cruiser result is not valid: ${ajv.errorsText()}.\n`
`The supplied dependency-cruiser result is not valid: ${ajv.errorsText()}.\n`,
);
}
}
/** @type {import("../../types/dependency-cruiser.js").format} */
export default async function format(pResult, pFormatOptions = {}) {
const lFormatOptions = normalizeFormatOptions(pFormatOptions);
validateFormatOptions(lFormatOptions);
assertFormatOptionsValid(lFormatOptions);

validateResultAgainstSchema(pResult);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,29 @@ import report from "../../report/index.mjs";
const MODULE_SYSTEM_LIST_RE = /^((cjs|amd|es6|tsd)(,|$))+$/gi;
const VALID_DEPTH_RE = /^\d{1,2}$/g;

function validateSystems(pModuleSystems) {
function assertModuleSystemsValid(pModuleSystems) {
if (
Boolean(pModuleSystems) &&
Array.isArray(pModuleSystems) &&
!pModuleSystems.every((pModuleSystem) =>
Boolean(pModuleSystem.match(MODULE_SYSTEM_LIST_RE))
Boolean(pModuleSystem.match(MODULE_SYSTEM_LIST_RE)),
)
) {
throw new Error(
`Invalid module system list: '${pModuleSystems.join(", ")}'\n`
`Invalid module system list: '${pModuleSystems.join(", ")}'\n`,
);
}
}

function validateRegExpSafety(pPattern) {
function assertRegExpSafety(pPattern) {
if (Boolean(pPattern) && !safeRegex(pPattern)) {
throw new Error(
`The pattern '${pPattern}' will probably run very slowly - cowardly refusing to run.\n`
`The pattern '${pPattern}' will probably run very slowly - cowardly refusing to run.\n`,
);
}
}

function validateOutputType(pOutputType) {
function assertOutputTypeValid(pOutputType) {
if (
Boolean(pOutputType) &&
!report.getAvailableReporters().includes(pOutputType) &&
Expand All @@ -38,15 +38,15 @@ function validateOutputType(pOutputType) {
}
}

function validateMaxDepth(pDepth) {
function assertMaxDepthValid(pDepth) {
if (Boolean(pDepth) && !pDepth.toString().match(VALID_DEPTH_RE)) {
throw new Error(
`'${pDepth}' is not a valid depth - use an integer between 0 and 99`
`'${pDepth}' is not a valid depth - use an integer between 0 and 99`,
);
}
}

function validateFocusDepth(pFocusDepth) {
function assertFocusDepthValid(pFocusDepth) {
const lFocusDepth = Number.parseInt(pFocusDepth, 10);
const lMaxFocusDepth = 99;

Expand All @@ -57,51 +57,51 @@ function validateFocusDepth(pFocusDepth) {
lFocusDepth > lMaxFocusDepth)
) {
throw new Error(
`'${pFocusDepth}' is not a valid focus depth - use an integer between 0 and ${lMaxFocusDepth}`
`'${pFocusDepth}' is not a valid focus depth - use an integer between 0 and ${lMaxFocusDepth}`,
);
}
}

function validatePathsSafety(pFilterOption) {
function assertPathsSafety(pFilterOption) {
if (typeof pFilterOption === "string") {
validateRegExpSafety(pFilterOption);
assertRegExpSafety(pFilterOption);
}

validateRegExpSafety(pFilterOption?.path ?? "");
validateRegExpSafety(pFilterOption?.pathNot ?? "");
assertRegExpSafety(pFilterOption?.path ?? "");
assertRegExpSafety(pFilterOption?.pathNot ?? "");
}

/**
* @param {any} pOptions
* @throws {Error}
* @returns {import("../../../types/dependency-cruiser.js").ICruiseOptions}
*/
export function validateCruiseOptions(pOptions) {
export function assertCruiseOptionsValid(pOptions) {
let lReturnValue = {};

if (Boolean(pOptions)) {
// necessary because can slip through the cracks when passed as a cli parameter
validateSystems(pOptions.moduleSystems);
assertModuleSystemsValid(pOptions.moduleSystems);

// necessary because this safety check can't be done in json schema (a.f.a.i.k.)
validatePathsSafety(pOptions.doNotFollow);
validatePathsSafety(pOptions.exclude);
validateRegExpSafety(pOptions.includeOnly);
validateRegExpSafety(pOptions.focus);
validateRegExpSafety(pOptions.reaches);
validateRegExpSafety(pOptions.highlight);
validateRegExpSafety(pOptions.collapse);
assertPathsSafety(pOptions.doNotFollow);
assertPathsSafety(pOptions.exclude);
assertRegExpSafety(pOptions.includeOnly);
assertRegExpSafety(pOptions.focus);
assertRegExpSafety(pOptions.reaches);
assertRegExpSafety(pOptions.highlight);
assertRegExpSafety(pOptions.collapse);

// necessary because not in the config schema
validateOutputType(pOptions.outputType);
assertOutputTypeValid(pOptions.outputType);

// necessary because not found a way to do this properly in JSON schema
validateMaxDepth(pOptions.maxDepth);
assertMaxDepthValid(pOptions.maxDepth);

validateFocusDepth(pOptions.focusDepth);
assertFocusDepthValid(pOptions.focusDepth);

if (has(pOptions, "ruleSet.options")) {
lReturnValue = validateCruiseOptions(pOptions.ruleSet.options);
lReturnValue = assertCruiseOptionsValid(pOptions.ruleSet.options);
}
return merge({}, lReturnValue, pOptions);
}
Expand All @@ -113,12 +113,12 @@ export function validateCruiseOptions(pOptions) {
* @param {import("../../../types/dependency-cruiser.js").IFormatOptions} pFormatOptions
* @throws {Error}
*/
export function validateFormatOptions(pFormatOptions) {
validatePathsSafety(pFormatOptions.exclude);
validatePathsSafety(pFormatOptions.focus);
validatePathsSafety(pFormatOptions.reaches);
validatePathsSafety(pFormatOptions.includeOnly);
validateRegExpSafety(pFormatOptions.collapse);
validateOutputType(pFormatOptions.outputType);
validateFocusDepth(pFormatOptions.focusDepth);
export function assertFormatOptionsValid(pFormatOptions) {
assertPathsSafety(pFormatOptions.exclude);
assertPathsSafety(pFormatOptions.focus);
assertPathsSafety(pFormatOptions.reaches);
assertPathsSafety(pFormatOptions.includeOnly);
assertRegExpSafety(pFormatOptions.collapse);
assertOutputTypeValid(pFormatOptions.outputType);
assertFocusDepthValid(pFormatOptions.focusDepth);
}
Loading

0 comments on commit c2d731c

Please sign in to comment.