diff --git a/src/commands/MatrixInterfaceCommand.ts b/src/commands/MatrixInterfaceCommand.ts index a7cce237..c0dbf700 100644 --- a/src/commands/MatrixInterfaceCommand.ts +++ b/src/commands/MatrixInterfaceCommand.ts @@ -41,7 +41,7 @@ type ParserSignature Promise> = ( mjolnir: Mjolnir, roomId: string, event: any, - parts: string[]) => Promise, ValidationError>>; + parts: string[]) => Promise>>; type RendererSignature> = ( mjolnir: Mjolnir, diff --git a/src/commands/UnbanBanCommand.ts b/src/commands/UnbanBanCommand.ts index c86a88e1..6d91a4b1 100644 --- a/src/commands/UnbanBanCommand.ts +++ b/src/commands/UnbanBanCommand.ts @@ -37,7 +37,7 @@ import { ValidationError, ValidationResult } from "./Validation"; type Arguments = Parameters<(mjolnir: Mjolnir, list: PolicyList, ruleType: string, entity: string, reason: string) => void>; // Exported for tests -export async function parseArguments(mjolnir: Mjolnir, roomId: string, event: any, parts: string[]): Promise> { +export async function parseArguments(mjolnir: Mjolnir, roomId: string, event: any, parts: string[]): Promise> { let defaultShortcode: string | null = null; try { const data: { shortcode: string } = await mjolnir.client.getAccountData(DEFAULT_LIST_EVENT_TYPE); @@ -49,19 +49,19 @@ export async function parseArguments(mjolnir: Mjolnir, roomId: string, event: an // Assume no default. } - const findList = (mjolnir: Mjolnir, shortcode: string): ValidationResult => { + const findList = (mjolnir: Mjolnir, shortcode: string): ValidationResult => { const foundList = mjolnir.lists.find(b => b.listShortcode.toLowerCase() === shortcode.toLowerCase()); if (foundList !== undefined) { return ValidationResult.Ok(foundList); } else { - return ValidationResult.Err(ValidationError.makeValidationError('shortcode not found', `A list with the shourtcode ${shortcode} could not be found.`)); + return ValidationError.Result('shortcode not found', `A list with the shourtcode ${shortcode} could not be found.`); } } let argumentIndex = 2; let ruleType: string | null = null; let entity: string | null = null; - let list: ValidationResult|null = null; + let list: ValidationResult|null = null; let force = false; while (argumentIndex < 7 && argumentIndex < parts.length) { const arg = parts[argumentIndex++]; @@ -104,34 +104,28 @@ export async function parseArguments(mjolnir: Mjolnir, roomId: string, event: an if (defaultShortcode) { list = await findList(mjolnir, defaultShortcode); if (list.isErr()) { - return ValidationResult.Err(ValidationError.makeValidationError( + return ValidationError.Result( "shortcode not found", - `A shortcode was not provided for this command, and a list couldn't be found with the default shortcode ${defaultShortcode}`)) + `A shortcode was not provided for this command, and a list couldn't be found with the default shortcode ${defaultShortcode}`) } } else { // FIXME: should be turned into a utility function to find the default list. // and in general, why is there a default shortcode instead of a default list? - return ValidationResult.Err(ValidationError.makeValidationError( + return ValidationError.Result( "no default shortcode", `A shortcode was not provided for this command, and a default shortcode was not set either.` - )) + ) } } if (list.isErr()) { return ValidationResult.Err(list.err); } else if (!ruleType) { - return ValidationResult.Err( - ValidationError.makeValidationError('uknown rule type', "Please specify the type as either 'user', 'room', or 'server'") - ); + return ValidationError.Result('uknown rule type', "Please specify the type as either 'user', 'room', or 'server'"); } else if (!entity) { - return ValidationResult.Err( - ValidationError.makeValidationError('no entity', "No entity was able to be parsed from this command") - ); + return ValidationError.Result('no entity', "No entity was able to be parsed from this command"); } else if (mjolnir.config.commands.confirmWildcardBan && /[*?]/.test(entity) && !force) { - return ValidationResult.Err( - ValidationError.makeValidationError("wildcard required", "Wildcard bans require an additional `--force` argument to confirm") - ); + return ValidationError.Result("wildcard required", "Wildcard bans require an additional `--force` argument to confirm"); } return ValidationResult.Ok([ diff --git a/src/commands/Validation.ts b/src/commands/Validation.ts index 969d290e..f0826a21 100644 --- a/src/commands/Validation.ts +++ b/src/commands/Validation.ts @@ -28,48 +28,30 @@ limitations under the License. type ValidationMatchExpression = { ok?: (ok: Ok) => any, err?: (err: Err) => any}; /** - * Why do we need a Result Monad for the parser signiture. - * I (Gnuxie) don't like monadic error handling, simply because - * I'm a strong believer in failing early, yes i may be misinformed. - * The only reason we don't use an exception in this case is because - * these are NOT to be used nilly willy and thrown out of context - * from an unrelated place. The Monad ensures locality (in terms of call chain) - * to the user interface by being infuriating to deal with. - * It also does look different to an exception - * to a naive programmer. Ideally though, if the world had adopted - * condition based error handling i would simply create a condition - * type for validation errors that can be translated/overidden by - * the command handler, and it wouldn't have to look like this. - * It's important to remember the errors we are reporting are to do with user input, - * we're trying to tell the user they did something wrong and what that is. + * This is a utility specifically for validating user input, and reporting + * what was wrong back to the end user in a way that makes sense. + * We are trying to tell the user they did something wrong and what that is. * This is something completely different to a normal exception, * where we are saying to ourselves that our assumptions in our code about * the thing we're doing are completely wrong. The user never - * should see these as there is nothing they can do about it. - * - * OK, it would be too annoying even for me to have a real Monad. - * So this is dumb as hell, no worries though - * - * OK I'm beginning to regret my decision. - * - * TODO: Can we make ValidationResult include ValidationError + * should see exceptions as there is nothing they can do about it. */ - export class ValidationResult { + export class ValidationResult { private constructor( private readonly okValue: Ok|null, - private readonly errValue: Err|null, + private readonly errValue: ValidationError|null, ) { } - public static Ok(value: Ok): ValidationResult { - return new ValidationResult(value, null); + public static Ok(value: Ok): ValidationResult { + return new ValidationResult(value, null); } - public static Err(value: Err): ValidationResult { - return new ValidationResult(null, value); + public static Err(value: ValidationError): ValidationResult { + return new ValidationResult(null, value); } - public async match(expression: ValidationMatchExpression) { + public async match(expression: ValidationMatchExpression) { return this.okValue ? await expression.ok!(this.ok) : await expression.err!(this.err); } @@ -89,7 +71,7 @@ type ValidationMatchExpression = { ok?: (ok: Ok) => any, err?: (err: Er } } - public get err(): Err { + public get err(): ValidationError { if (this.isErr()) { return this.errValue!; } else { @@ -128,6 +110,10 @@ export class ValidationError { } } + public static Result(code: string, message: string): ValidationResult { + return ValidationResult.Err(this.makeValidationError(code, message)); + } + public static makeValidationError(code: string, message: string) { return new ValidationError(ValidationError.ensureErrorCode(code), message); }