From 228f645d1cbdc85c70f96f8ad84bdcdbd9fb5c12 Mon Sep 17 00:00:00 2001 From: Unknown Date: Thu, 14 Dec 2017 15:18:12 +0800 Subject: [PATCH 1/3] AddCommandParser: add check to ensure preamble is empty AddCommandParser accepts invalid command format. A check to ensure the preamble is empty is more user-friendly as the user is notified of the invalid input. Let's add a check to ensure that argMultimap.getPreamble() is empty. --- src/main/java/seedu/address/logic/parser/AddCommandParser.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/seedu/address/logic/parser/AddCommandParser.java b/src/main/java/seedu/address/logic/parser/AddCommandParser.java index 670abf84e164..ece2bb7bbf28 100644 --- a/src/main/java/seedu/address/logic/parser/AddCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/AddCommandParser.java @@ -35,7 +35,8 @@ public AddCommand parse(String args) throws ParseException { ArgumentMultimap argMultimap = ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_TAG); - if (!arePrefixesPresent(argMultimap, PREFIX_NAME, PREFIX_ADDRESS, PREFIX_PHONE, PREFIX_EMAIL)) { + if (!arePrefixesPresent(argMultimap, PREFIX_NAME, PREFIX_ADDRESS, PREFIX_PHONE, PREFIX_EMAIL) + || !argMultimap.getPreamble().isEmpty()) { throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE)); } From 50b7b3c9101a428494e1a64a965aa138c1145356 Mon Sep 17 00:00:00 2001 From: Unknown Date: Thu, 14 Dec 2017 15:19:18 +0800 Subject: [PATCH 2/3] CommandTestUtil: add non-empty preamble constant --- src/test/java/seedu/address/logic/commands/CommandTestUtil.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java index 34a5c7959f18..f6821a1ba999 100644 --- a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java +++ b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java @@ -53,6 +53,8 @@ public class CommandTestUtil { public static final String INVALID_ADDRESS_DESC = " " + PREFIX_ADDRESS; // empty string not allowed for addresses public static final String INVALID_TAG_DESC = " " + PREFIX_TAG + "hubby*"; // '*' not allowed in tags + public static final String NON_EMPTY_PREAMBLE = " " + "FormattingProblem"; + public static final EditCommand.EditPersonDescriptor DESC_AMY; public static final EditCommand.EditPersonDescriptor DESC_BOB; From 37974c92f052350ae1e434c4bfac3e5541650297 Mon Sep 17 00:00:00 2001 From: Unknown Date: Thu, 14 Dec 2017 15:26:42 +0800 Subject: [PATCH 3/3] AddCommandParserTest: update to test if preamble check works There are many instances of the AddCommand command word in the userInput. They need to be removed to prevent them from being detected as a preamble. Let's remove all instances of AddCommand.COMMAND_WORD. --- .../logic/parser/AddCommandParserTest.java | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java b/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java index 7502f801465d..4a6c51d0a252 100644 --- a/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java @@ -12,6 +12,7 @@ import static seedu.address.logic.commands.CommandTestUtil.INVALID_TAG_DESC; import static seedu.address.logic.commands.CommandTestUtil.NAME_DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.NAME_DESC_BOB; +import static seedu.address.logic.commands.CommandTestUtil.NON_EMPTY_PREAMBLE; import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_BOB; import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_FRIEND; @@ -49,26 +50,26 @@ public void parse_allFieldsPresent_success() { .withEmail(VALID_EMAIL_BOB).withAddress(VALID_ADDRESS_BOB).withTags(VALID_TAG_FRIEND).build(); // multiple names - last name accepted - assertParseSuccess(parser, AddCommand.COMMAND_WORD + NAME_DESC_AMY + NAME_DESC_BOB + PHONE_DESC_BOB + assertParseSuccess(parser, NAME_DESC_AMY + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson)); // multiple phones - last phone accepted - assertParseSuccess(parser, AddCommand.COMMAND_WORD + NAME_DESC_BOB + PHONE_DESC_AMY + PHONE_DESC_BOB + assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_AMY + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson)); // multiple emails - last email accepted - assertParseSuccess(parser, AddCommand.COMMAND_WORD + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_AMY + assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_AMY + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson)); // multiple addresses - last address accepted - assertParseSuccess(parser, AddCommand.COMMAND_WORD + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_AMY + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson)); // multiple tags - all accepted Person expectedPersonMultipleTags = new PersonBuilder().withName(VALID_NAME_BOB).withPhone(VALID_PHONE_BOB) .withEmail(VALID_EMAIL_BOB).withAddress(VALID_ADDRESS_BOB) .withTags(VALID_TAG_FRIEND, VALID_TAG_HUSBAND).build(); - assertParseSuccess(parser, AddCommand.COMMAND_WORD + NAME_DESC_BOB + PHONE_DESC_BOB + assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, new AddCommand(expectedPersonMultipleTags)); } @@ -78,7 +79,7 @@ public void parse_optionalFieldsMissing_success() { // zero tags Person expectedPerson = new PersonBuilder().withName(VALID_NAME_AMY).withPhone(VALID_PHONE_AMY) .withEmail(VALID_EMAIL_AMY).withAddress(VALID_ADDRESS_AMY).withTags().build(); - assertParseSuccess(parser, AddCommand.COMMAND_WORD + NAME_DESC_AMY + PHONE_DESC_AMY + assertParseSuccess(parser, NAME_DESC_AMY + PHONE_DESC_AMY + EMAIL_DESC_AMY + ADDRESS_DESC_AMY, new AddCommand(expectedPerson)); } @@ -110,28 +111,33 @@ public void parse_compulsoryFieldMissing_failure() { @Test public void parse_invalidValue_failure() { // invalid name - assertParseFailure(parser, AddCommand.COMMAND_WORD + INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Name.MESSAGE_NAME_CONSTRAINTS); // invalid phone - assertParseFailure(parser, AddCommand.COMMAND_WORD + NAME_DESC_BOB + INVALID_PHONE_DESC + EMAIL_DESC_BOB + assertParseFailure(parser, NAME_DESC_BOB + INVALID_PHONE_DESC + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Phone.MESSAGE_PHONE_CONSTRAINTS); // invalid email - assertParseFailure(parser, AddCommand.COMMAND_WORD + NAME_DESC_BOB + PHONE_DESC_BOB + INVALID_EMAIL_DESC + assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + INVALID_EMAIL_DESC + ADDRESS_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Email.MESSAGE_EMAIL_CONSTRAINTS); // invalid address - assertParseFailure(parser, AddCommand.COMMAND_WORD + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Address.MESSAGE_ADDRESS_CONSTRAINTS); // invalid tag - assertParseFailure(parser, AddCommand.COMMAND_WORD + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + INVALID_TAG_DESC + VALID_TAG_FRIEND, Tag.MESSAGE_TAG_CONSTRAINTS); // two invalid values, only first invalid value reported - assertParseFailure(parser, AddCommand.COMMAND_WORD + INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC, Name.MESSAGE_NAME_CONSTRAINTS); + + // non-empty preamble + assertParseFailure(parser, NON_EMPTY_PREAMBLE + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + + ADDRESS_DESC_BOB + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE)); } }