Skip to content

Commit

Permalink
Implement fix for #1881
Browse files Browse the repository at this point in the history
Signed-off-by: Andrew W. Harn <[email protected]>
  • Loading branch information
awharn committed Aug 20, 2024
1 parent 6ab6ea9 commit a0fb714
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 9 deletions.
4 changes: 4 additions & 0 deletions packages/imperative/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

All notable changes to the Imperative package will be documented in this file.

## Recent Changes

- LTS Breaking: Fixed command parsing error where `string` typed options would be converted into `number`s if the value provided by the user consists only of numeric characters. [#1881](https://github.com/zowe/zowe-cli/issues/1881)

## `8.0.0-next.202408191401`

- Update: See `5.26.3` and `5.27.0` for details
Expand Down
10 changes: 6 additions & 4 deletions packages/imperative/src/cmd/src/syntax/SyntaxValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ export class SyntaxValidator {
}
if (positional.type === "number") {
valid = this.validateNumeric(commandArguments[positional.name], positional, responseObject, true) && valid;
// Convert to number for backwards compatability
if (valid) { commandArguments[positional.name] = parseInt(commandArguments[positional.name]); }
}

if (!(positional.stringLengthRange == null) &&
Expand Down Expand Up @@ -373,11 +375,11 @@ export class SyntaxValidator {
commandArguments[optionDef.name]);
}
} else if (optionDef.type === "boolean") {
valid = this.validateBoolean(commandArguments[optionDef.name], optionDef,
responseObject) && valid;
valid = this.validateBoolean(commandArguments[optionDef.name], optionDef, responseObject) && valid;
} else if (optionDef.type === "number") {
valid = this.validateNumeric(commandArguments[optionDef.name], optionDef,
responseObject) && valid;
valid = this.validateNumeric(commandArguments[optionDef.name], optionDef, responseObject) && valid;
// Convert to numbers for backwards compatibility
if (valid) { commandArguments[optionDef.name] = parseInt(commandArguments[optionDef.name]); }
}
/**
* Validate that the option's value is valid json.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@ import { CommandResponse, ICommandDefinition } from "../../../";
import { ValidationTestCommand } from "../../../../../__tests__/src/packages/cmd/ValidationTestCommand";
import { SyntaxValidator } from "../SyntaxValidator";
import { Constants } from "../../../../constants";
import { YargsConfigurer } from "../../yargs/YargsConfigurer";


describe("Imperative should provide advanced syntax validation rules", () => {
const logger = TestLogger.getTestLogger();
const configuration = {
configuration: YargsConfigurer.yargsConfiguration
};

describe("Advanced syntax validation for commands using a test command", () => {
const yargsParser = require("yargs-parser");
Expand All @@ -33,7 +37,7 @@ describe("Imperative should provide advanced syntax validation rules", () => {

function tryOptions(optionString: string, shouldSucceed: boolean, expectedText?: string[]) {

const options = yargsParser(optionString);
const options = yargsParser(optionString, configuration);
options._ = ["test", "validation-test"].concat(options._ || []); // fake out command structure
options[Constants.JSON_OPTION] = true;
delete options["--"]; // delete extra yargs parse field
Expand Down Expand Up @@ -362,6 +366,43 @@ describe("Imperative should provide advanced syntax validation rules", () => {
minValidOptions + "--always-required-string hello",
false, ["multiple", "--always-required-string"])();
});

it("should validate that typed numbers are numbers, and convert strings that are numbers", async () => {
const options = yargsParser(minValidOptions + " --should-be-number 4", configuration);
options._ = ["test", "validation-test"].concat(options._ || []); // fake out command structure
options[Constants.JSON_OPTION] = true;
delete options["--"]; // delete extra yargs parse field
logger.debug("Executing test syntax command with arguments: " + inspect(options));
const response = new CommandResponse({responseFormat: "json"});
const fakeParent: ICommandDefinition = {
name: undefined,
description: "", type: "group",
children: [ValidationTestCommand]
};
const svResponse = await new SyntaxValidator(ValidationTestCommand, fakeParent).validate(response, options);
expect(options["should-be-number"]).toBe(4);
expect(options["should-be-number"]).not.toBe("4");
expect(svResponse.valid).toEqual(true);
});

it("should validate that typed strings are strings and not numbers", async () => {
const options = yargsParser(minValidOptions + " --fluffy 9001", configuration);
options._ = ["test", "validation-test"].concat(options._ || []); // fake out command structure
options[Constants.JSON_OPTION] = true;
delete options["--"]; // delete extra yargs parse field
logger.debug("Executing test syntax command with arguments: " + inspect(options));
const response = new CommandResponse({responseFormat: "json"});
const fakeParent: ICommandDefinition = {
name: undefined,
description: "", type: "group",
children: [ValidationTestCommand]
};
const svResponse = await new SyntaxValidator(ValidationTestCommand, fakeParent).validate(response, options);
expect(options["fluffy"]).toBe("9001");
expect(options["fluffy"]).not.toBe(9001);
expect(svResponse.valid).toEqual(true);
});

describe("We should be able to validate positional arguments of type 'number'", () => {
const numberCommand: ICommandDefinition = {
name: "gimme-number", aliases: [],
Expand Down
7 changes: 3 additions & 4 deletions packages/imperative/src/cmd/src/yargs/CommandYargs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,10 @@ export class CommandYargs extends AbstractCommandYargs {
if (!(option.type == null)) {
// don't let yargs handle any types that we are validating ourselves
// and don't use custom types as the yargs type since yargs won't understand
if (option.type !== "number" &&
option.type !== "json") {
definition.type = option.type as any;
} else if (option.type === "json") {
if (option.type === "json" || option.type === "number") {
definition.type = "string";
} else {
definition.type = option.type as any;
}
}
// If this is a boolean type option, default it to undefined so that we can distinguish
Expand Down
6 changes: 6 additions & 0 deletions packages/imperative/src/cmd/src/yargs/YargsConfigurer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ export class YargsConfigurer {
) {
}

public static yargsConfiguration: Record<string, boolean> = {
"parse-numbers": false,
"parse-positional-numbers": false
};

public configure() {

/**
Expand All @@ -50,6 +55,7 @@ export class YargsConfigurer {
this.yargs.help(false);
this.yargs.version(false);
this.yargs.showHelpOnFail(false);
this.yargs.parserConfiguration(YargsConfigurer.yargsConfiguration);
// finally, catch any undefined commands
this.yargs.command({
command: "*",
Expand Down

0 comments on commit a0fb714

Please sign in to comment.