Skip to content

Commit

Permalink
Standardize on mustBe*() method name prefix (#450)
Browse files Browse the repository at this point in the history
This PR standardizes on `mustBe*()` as a method name prefix for methods
which are checking that an argument is of a particular type (or type-ish
thing). This replaces two "competing" prefixes that had evolved
organically (`check*()` and `expect*()`) which _both_ ended up being in
at least a little conflict with other uses of those prefixes
(specifically in tests).
  • Loading branch information
danfuzz authored Feb 3, 2025
2 parents 20f890d + f145cc7 commit cfc31af
Show file tree
Hide file tree
Showing 45 changed files with 122 additions and 115 deletions.
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ versioning principles. Unstable releases do not.
Breaking changes:
* framework API (general):
* Defined a new `*ElseNull()` method naming convention, to use instead of
`*OrNull*()`, clarifying the contexts in which each is appropriate. As a
result, renamed a bunch of methods throughout the system.
`*OrNull*()`, clarifying the contexts in which each is appropriate.
* Reworked method naming convention for type/value-checking methods to be
`mustBe*()`, instead of using either `expect*()` or `check*()`.
* As a result of the above, renamed a bunch of methods throughout the system.

Other notable changes:
* `net-util`:
Expand Down
7 changes: 6 additions & 1 deletion doc/coding-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,14 @@ use the following comment in place of an intentionally omitted constructor:
With type conversion methods, the distinction is sometimes a bit arbitrary,
but the pattern `*ElseNull()` can also be used in non-conversion contexts.
For example, in `findFooElseNull()`, the distinction is more meaningful. And
in `expectFooOrNull()` it is clear that the expectation is for a "nullable"
in `mustBeFooOrNull()` it is clear that the expectation is for a "nullable"
`foo` and not that the method is allowed to return `null` in case of error.

* `mustBe<thing>()` &mdash; Names a method which is checking that its argument
is of the given type (or type-ish thing), returning the given value if it
matches the type, or `throw`ing if not. This parallels the library class
`typey.MustBe`.

### Ledger of arbitrary decisions

Every enduring project of nontrivial size ends up having the results of myriad
Expand Down
8 changes: 4 additions & 4 deletions src/compy/export/BaseComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { ThisModule } from '#p/ThisModule';
*
* **Note:** If a concrete subclass uses a configuration object with a `name`
* property, then this class requires that that name honor the contract of
* {@link Names#checkName}.
* {@link Names#mustBeName}.
*/
export class BaseComponent {
/**
Expand Down Expand Up @@ -78,7 +78,7 @@ export class BaseComponent {

const name = this.#config?.name;
if (name) {
Names.checkName(name);
Names.mustBeName(name);
}

if (rootContext !== null) {
Expand Down Expand Up @@ -544,7 +544,7 @@ export class BaseComponent {
* The item's name, or `null` if it does not have a configured name. If
* `null`, the corresponding component will get a synthesized name as soon
* as it is attached to a hierarchy. If non-`null`, it must adhere to the
* syntax defined by {@link Names#checkName}. Names are used when finding
* syntax defined by {@link Names#mustBeName}. Names are used when finding
* a component in its hierarchy, and when logging.
*
* @param {?string} [value] Proposed configuration value. Default `null`.
Expand All @@ -553,7 +553,7 @@ export class BaseComponent {
_config_name(value = null) {
return (value === null)
? null
: Names.checkName(value);
: Names.mustBeName(value);
}
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/compy/export/BaseRootComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class BaseRootComponent extends BaseComponent {
* @returns {string} Accepted configuration value.
*/
_config_name(value = 'root') {
return Names.checkName(value);
return Names.mustBeName(value);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/compy/export/ControlContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ export class ControlContext {
* @param {string} state State to wait for.
*/
async whenState(state) {
ControlContext.#checkState(state);
ControlContext.#mustBeState(state);

while (this.#state !== state) {
await this.#stateChangeCondition.whenTrue();
Expand Down Expand Up @@ -280,7 +280,7 @@ export class ControlContext {
* @param {string} state The new state.
*/
[ThisModule.SYM_setState](state) {
ControlContext.#checkState(state);
ControlContext.#mustBeState(state);

if (state !== this.#state) {
this.#state = state;
Expand Down Expand Up @@ -343,7 +343,7 @@ export class ControlContext {
* @param {string} state State value to check.
* @throws {Error} Thrown if `state` is invalid.
*/
static #checkState(state) {
static #mustBeState(state) {
MustBe.string(state);

if (!this.#VALID_STATES.has(state)) {
Expand Down
2 changes: 1 addition & 1 deletion src/compy/export/Names.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class Names {
* pattern.
* @throws {Error} Thrown if `value` does not match.
*/
static checkName(value) {
static mustBeName(value) {
if (this.isName(value)) {
return value;
}
Expand Down
2 changes: 1 addition & 1 deletion src/compy/tests/Names.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Names } from '@this/compy';

describe.each`
methodName | throws
${'checkName'} | ${true}
${'mustBeName'} | ${true}
${'isName'} | ${false}
`('$methodName()', ({ methodName, throws }) => {
// Non-string errors. Always expected to throw.
Expand Down
2 changes: 1 addition & 1 deletion src/fs-util/export/FileAppender.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class FileAppender {
* stuff to write, or `null` not to do any buffering.
*/
constructor(filePath, maxBufferTime = null) {
this.#filePath = Paths.checkAbsolutePath(filePath);
this.#filePath = Paths.mustBeAbsolutePath(filePath);
this.#maxBufferTime = maxBufferTime
? MustBe.instanceOf(maxBufferTime, Duration)
: Duration.ZERO;
Expand Down
4 changes: 2 additions & 2 deletions src/fs-util/export/Paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class Paths {
* @returns {string} `value` if it is a string which matches the pattern.
* @throws {Error} Thrown if `value` does not match.
*/
static checkAbsolutePath(value) {
static mustBeAbsolutePath(value) {
const pattern = '^' +
'(?!.*/[.]{1,2}/)' + // No dot or double-dot internal component.
'(?!.*/[.]{1,2}$)' + // No dot or double-dot final component.
Expand All @@ -45,7 +45,7 @@ export class Paths {
* pattern.
* @throws {Error} Thrown if `value` does not match.
*/
static checkFileName(value) {
static mustBeFileName(value) {
const pattern = /^(?![.]{1,2}$)[^/]+$/;

try {
Expand Down
16 changes: 8 additions & 8 deletions src/fs-util/tests/Paths.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { Paths } from '@this/fs-util';


describe('checkAbsolutePath()', () => {
describe('mustBeAbsolutePath()', () => {
// Errors: Wrong argument type.
test.each`
arg
Expand All @@ -16,7 +16,7 @@ describe('checkAbsolutePath()', () => {
${['/x/y']}
${new Map()}
`('fails for $arg', ({ arg }) => {
expect(() => Paths.checkAbsolutePath(arg)).toThrow();
expect(() => Paths.mustBeAbsolutePath(arg)).toThrow();
});

// Errors: Invalid string syntax.
Expand All @@ -36,7 +36,7 @@ describe('checkAbsolutePath()', () => {
${'/a/../b'}
${'/a/b/'}
`('fails for $arg', ({ arg }) => {
expect(() => Paths.checkAbsolutePath(arg)).toThrow();
expect(() => Paths.mustBeAbsolutePath(arg)).toThrow();
});

// Correct cases.
Expand All @@ -47,11 +47,11 @@ describe('checkAbsolutePath()', () => {
${'/abc'}
${'/abc/def'}
`('succeeds for $arg', ({ arg }) => {
expect(Paths.checkAbsolutePath(arg)).toBe(arg);
expect(Paths.mustBeAbsolutePath(arg)).toBe(arg);
});
});

describe('checkFileName()', () => {
describe('mustBeFileName()', () => {
// Errors: Wrong argument type.
test.each`
arg
Expand All @@ -63,7 +63,7 @@ describe('checkFileName()', () => {
${['/x/y']}
${new Map()}
`('fails for $arg', ({ arg }) => {
expect(() => Paths.checkFileName(arg)).toThrow();
expect(() => Paths.mustBeFileName(arg)).toThrow();
});

// Errors: Invalid string syntax.
Expand All @@ -77,7 +77,7 @@ describe('checkFileName()', () => {
${'a/'}
${'a/b'}
`('fails for $arg', ({ arg }) => {
expect(() => Paths.checkFileName(arg)).toThrow();
expect(() => Paths.mustBeFileName(arg)).toThrow();
});

// Correct cases.
Expand All @@ -92,6 +92,6 @@ describe('checkFileName()', () => {
${'a..'}
${'a..b'}
`('succeeds for $arg', ({ arg }) => {
expect(Paths.checkFileName(arg)).toBe(arg);
expect(Paths.mustBeFileName(arg)).toBe(arg);
});
});
6 changes: 3 additions & 3 deletions src/loggy-intf/export/IntfLogger.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class IntfLogger {
* @returns {IntfLogger} `logger` if it is a logger.
* @throws {Error} Thrown if `logger` is not actually a logger.
*/
static expectInstance(logger) {
static mustBeInstance(logger) {
if (logger instanceof IntfLogger) {
return logger;
} else if (AskIf.callableFunction(logger) && logger.$env) {
Expand All @@ -116,8 +116,8 @@ export class IntfLogger {
* @returns {?IntfLogger} `logger` if it is a logger or `null`.
* @throws {Error} Thrown if `logger` is not actually a logger or `null`.
*/
static expectInstanceOrNull(logger) {
return (logger === null) ? null : this.expectInstance(logger);
static mustBeInstanceOrNull(logger) {
return (logger === null) ? null : this.mustBeInstance(logger);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/loggy-intf/export/LogTag.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ export class LogTag extends IntfDeconstructable {
constructor(main, ...context) {
super();

this.#main = LogTag.#checkMainString(main);
this.#main = LogTag.#mustBeMainString(main);

for (const c of context) {
LogTag.#checkContextString(c);
LogTag.#mustBeContextString(c);
}

this.#context = Object.freeze(context);
Expand Down Expand Up @@ -215,7 +215,7 @@ export class LogTag extends IntfDeconstructable {
* @returns {string} `value`, if it is valid.
* @throws {Error} Thrown if `value` is invalid.
*/
static #checkContextString(value) {
static #mustBeContextString(value) {
return MustBe.string(value, /^.{1,30}$/);
}

Expand All @@ -226,7 +226,7 @@ export class LogTag extends IntfDeconstructable {
* @returns {string} `value`, if it is valid.
* @throws {Error} Thrown if `value` is invalid.
*/
static #checkMainString(value) {
static #mustBeMainString(value) {
return MustBe.string(value, /^(?![-.])(?:[-._a-zA-Z0-9]{1,20}|\(top\))(?<![-.])$/);
}
}
2 changes: 1 addition & 1 deletion src/metacomp/export/LimitedLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class LimitedLoader {
*/
constructor(context = null, logger = null) {
this.#context = context;
this.#logger = IntfLogger.expectInstanceOrNull(logger);
this.#logger = IntfLogger.mustBeInstanceOrNull(logger);

if (context && !vm.isContext(context)) {
vm.createContext(context);
Expand Down
2 changes: 1 addition & 1 deletion src/net-protocol/export/ProtocolWrangler.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export class ProtocolWrangler {
* logging.
*/
async init(logger) {
this.#logger = IntfLogger.expectInstanceOrNull(logger);
this.#logger = IntfLogger.mustBeInstanceOrNull(logger);

// Confusion alert!: This is not the same as the `requestLogger` (a "request
// logger") per se) passed in as an option. This is the sub-logger of the
Expand Down
2 changes: 1 addition & 1 deletion src/net-protocol/export/ProtocolWranglers.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class ProtocolWranglers {
* @throws {Error} Thrown if `name` is not a known protocol (or is not a
* string).
*/
static checkProtocol(name) {
static mustBeProtocol(name) {
MustBe.string(name);

if (!this.#WRANGLER_CLASSES.has(name)) {
Expand Down
2 changes: 1 addition & 1 deletion src/net-protocol/private/AsyncServerSocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class AsyncServerSocket {
// Note: `interface` is a reserved word.
this.#interface = MustBe.instanceOf(iface, InterfaceAddress);
this.#protocol = MustBe.string(protocol);
this.#logger = IntfLogger.expectInstanceOrNull(logger);
this.#logger = IntfLogger.mustBeInstanceOrNull(logger);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/net-protocol/private/WranglerContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ export class WranglerContext {
ctx.#socket = socket;

if (logger) {
ctx.#connectionLogger = IntfLogger.expectInstanceOrNull(logger);
ctx.#connectionLogger = IntfLogger.mustBeInstanceOrNull(logger);
ctx.#connectionId = logger?.$meta.lastContext ?? null;
}

Expand Down
4 changes: 2 additions & 2 deletions src/net-util/export/CertUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class CertUtil {
* pattern.
* @throws {Error} Thrown if `value` does not match.
*/
static checkCertificateChain(value) {
static mustBeCertificateChain(value) {
const pattern = this.#makePemPattern('CERTIFICATE', true);
return MustBe.string(value, pattern);
}
Expand All @@ -38,7 +38,7 @@ export class CertUtil {
* pattern.
* @throws {Error} Thrown if `value` does not match.
*/
static checkPrivateKey(value) {
static mustBePrivateKey(value) {
const pattern = this.#makePemPattern('((RSA|EC) )?PRIVATE KEY');
return MustBe.string(value, pattern);
}
Expand Down
2 changes: 1 addition & 1 deletion src/net-util/export/DispatchInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class DispatchInfo extends IntfDeconstructable {

this.#base = MustBe.instanceOf(base, PathKey);
this.#extra = MustBe.instanceOf(extra, PathKey);
this.#logger = IntfLogger.expectInstanceOrNull(logger);
this.#logger = IntfLogger.mustBeInstanceOrNull(logger);
}

/** @override */
Expand Down
4 changes: 2 additions & 2 deletions src/net-util/export/EtagGenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class EtagGenerator {
* exist.
*/
async etagFromFileData(absolutePath) {
Paths.checkAbsolutePath(absolutePath);
Paths.mustBeAbsolutePath(absolutePath);

const stats = await Statter.statElseNull(absolutePath);
if (!stats) {
Expand Down Expand Up @@ -205,7 +205,7 @@ export class EtagGenerator {
throw new Error('Cannot use with data-only instance.');
}

Paths.checkAbsolutePath(absolutePath);
Paths.mustBeAbsolutePath(absolutePath);

// Converts a number to hex.
const hex = (num) => {
Expand Down
2 changes: 1 addition & 1 deletion src/net-util/export/FullResponse.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export class FullResponse extends BaseResponse {
* header based on the file's stats. Defaults to `true`.
*/
async setBodyFile(absolutePath, options = null) {
Paths.checkAbsolutePath(absolutePath);
Paths.mustBeAbsolutePath(absolutePath);
const {
offset = null,
length = null,
Expand Down
2 changes: 1 addition & 1 deletion src/net-util/export/HostInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ export class HostInfo {
const { hostname, port } = topParse;

// Refined `hostname` check, along with IP address canonicalization.
const canonicalHostname = HostUtil.checkHostnameElseNull(hostname, false);
const canonicalHostname = HostUtil.canonicalizeHostnameElseNull(hostname, false);

if (!canonicalHostname) {
return null;
Expand Down
Loading

0 comments on commit cfc31af

Please sign in to comment.