Skip to content
This repository has been archived by the owner on Mar 5, 2023. It is now read-only.

Commit

Permalink
checkstyle: add SingleSpaceSeparator
Browse files Browse the repository at this point in the history
Most of the code in our the code base only puts a single space between
non-whitespace characters,

    "Like " + "this."

It is highly likely that there will not be a case where we will need to
put more than a single space between non-whitespace characters,

    "Like "  + "this."

To enforce consistency throughout the code base, let's add the
SingleSpaceSeparator module to our checkstyle configuration, and fix all
of the existing violations in our code base.
  • Loading branch information
pyokagan committed Aug 17, 2018
1 parent 385809b commit 83699f1
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 26 deletions.
19 changes: 19 additions & 0 deletions config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,25 @@
<property name="severity" value="warning"/>
</module>

<!-- Checks that non-whitespace characters are separated by no more than one whitespace character.
a = 1; // Allowed
a = 1; // Not allowed (more than one space before =)
-->
<module name="SingleSpaceSeparator">
<!-- Validate whitespace surrounding comments as well.
a = 1; // Allowed (single space before start of comment)
a = 1; /* Allowed (single space before start of comment) */
/* Allowed (single space after end of comment) */ a = 1;
a = 1; // Not allowed (more than one space before start of comment)
a = 1; /* Not allowed (more than one space before start of comment) */
/* Not allowed (more than one space after end of comment) */ a = 1;
This doesn't validate whitespace within comments so a comment /* like this */ is allowed.
-->
<property name="validateComments" value="true"/>
</module>

<!--
JAVADOC CHECKS
-->
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/commons/util/JsonUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static <T> Optional<T> readJsonFile(
requireNonNull(filePath);

if (!Files.exists(filePath)) {
logger.info("Json file " + filePath + " not found");
logger.info("Json file " + filePath + " not found");
return Optional.empty();
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/model/person/Email.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/
public class Email {

private static final String SPECIAL_CHARACTERS = "!#$%&'*+/=?`{|}~^.-";
private static final String SPECIAL_CHARACTERS = "!#$%&'*+/=?`{|}~^.-";
public static final String MESSAGE_EMAIL_CONSTRAINTS = "Emails should be of the format local-part@domain "
+ "and adhere to the following constraints:\n"
+ "1. The local-part should only contain alphanumeric characters and these special characters, excluding "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public Optional<ReadOnlyAddressBook> readAddressBook(Path filePath) throws DataC
requireNonNull(filePath);

if (!Files.exists(filePath)) {
logger.info("AddressBook file " + filePath + " not found");
logger.info("AddressBook file " + filePath + " not found");
return Optional.empty();
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/guitests/GuiRobot.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public int getNumberOfWindowsShown(String stageTitle) {
*/
public Stage getStage(String stageTitle) {
Optional<Stage> targetStage = listTargetWindows().stream()
.filter(Stage.class::isInstance) // checks that the window is of type Stage
.filter(Stage.class::isInstance) // checks that the window is of type Stage
.map(Stage.class::cast)
.filter(stage -> stage.getTitle().equals(stageTitle))
.findFirst();
Expand Down
26 changes: 13 additions & 13 deletions src/test/java/seedu/address/commons/core/VersionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,56 +56,56 @@ public void versionComparable_validVersion_compareToIsCorrect() {

// Tests equality
one = new Version(0, 0, 0, true);
another = new Version(0, 0, 0, true);
another = new Version(0, 0, 0, true);
assertTrue(one.compareTo(another) == 0);

one = new Version(11, 12, 13, false);
another = new Version(11, 12, 13, false);
another = new Version(11, 12, 13, false);
assertTrue(one.compareTo(another) == 0);

// Tests different patch
one = new Version(0, 0, 5, false);
another = new Version(0, 0, 0, false);
another = new Version(0, 0, 0, false);
assertTrue(one.compareTo(another) > 0);

// Tests different minor
one = new Version(0, 0, 0, false);
another = new Version(0, 5, 0, false);
another = new Version(0, 5, 0, false);
assertTrue(one.compareTo(another) < 0);

// Tests different major
one = new Version(10, 0, 0, true);
another = new Version(0, 0, 0, true);
another = new Version(0, 0, 0, true);
assertTrue(one.compareTo(another) > 0);

// Tests high major vs low minor
one = new Version(10, 0, 0, true);
another = new Version(0, 1, 0, true);
another = new Version(0, 1, 0, true);
assertTrue(one.compareTo(another) > 0);

// Tests high patch vs low minor
one = new Version(0, 0, 10, false);
another = new Version(0, 1, 0, false);
another = new Version(0, 1, 0, false);
assertTrue(one.compareTo(another) < 0);

// Tests same major minor different patch
one = new Version(2, 15, 0, false);
another = new Version(2, 15, 5, false);
another = new Version(2, 15, 5, false);
assertTrue(one.compareTo(another) < 0);

// Tests early access vs not early access on same version number
one = new Version(2, 15, 0, true);
another = new Version(2, 15, 0, false);
another = new Version(2, 15, 0, false);
assertTrue(one.compareTo(another) < 0);

// Tests early access lower version vs not early access higher version compare by version number first
one = new Version(2, 15, 0, true);
another = new Version(2, 15, 5, false);
another = new Version(2, 15, 5, false);
assertTrue(one.compareTo(another) < 0);

// Tests early access higher version vs not early access lower version compare by version number first
one = new Version(2, 15, 0, false);
another = new Version(2, 15, 5, true);
another = new Version(2, 15, 5, true);
assertTrue(one.compareTo(another) < 0);
}

Expand All @@ -124,11 +124,11 @@ public void versionComparable_validVersion_equalIsCorrect() {
Version another;

one = new Version(0, 0, 0, false);
another = new Version(0, 0, 0, false);
another = new Version(0, 0, 0, false);
assertTrue(one.equals(another));

one = new Version(100, 191, 275, true);
another = new Version(100, 191, 275, true);
another = new Version(100, 191, 275, true);
assertTrue(one.equals(another));
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/seedu/address/commons/util/AppUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ public void checkArgument_falseWithErrorMessage_throwsIllegalArgumentException()
String errorMessage = "error message";
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage(errorMessage);
AppUtil.checkArgument(false, errorMessage);
AppUtil.checkArgument(false, errorMessage);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void isUnsignedPositiveInteger() {

// EP: numbers with white space
assertFalse(StringUtil.isNonZeroUnsignedInteger(" 10 ")); // Leading/trailing spaces
assertFalse(StringUtil.isNonZeroUnsignedInteger("1 0")); // Spaces in the middle
assertFalse(StringUtil.isNonZeroUnsignedInteger("1 0")); // Spaces in the middle

// EP: number larger than Integer.MAX_VALUE
assertFalse(StringUtil.isNonZeroUnsignedInteger(Long.toString(Integer.MAX_VALUE + 1)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public void parse_oneFieldSpecified_success() {
@Test
public void parse_multipleRepeatedFields_acceptsLast() {
Index targetIndex = INDEX_FIRST_PERSON;
String userInput = targetIndex.getOneBased() + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY
String userInput = targetIndex.getOneBased() + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY
+ TAG_DESC_FRIEND + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY + TAG_DESC_FRIEND
+ PHONE_DESC_BOB + ADDRESS_DESC_BOB + EMAIL_DESC_BOB + TAG_DESC_HUSBAND;

Expand Down
10 changes: 5 additions & 5 deletions src/test/java/seedu/address/model/person/EmailTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ public void isValidEmail() {

// valid email
assertTrue(Email.isValidEmail("[email protected]"));
assertTrue(Email.isValidEmail("a@bc")); // minimal
assertTrue(Email.isValidEmail("test@localhost")); // alphabets only
assertTrue(Email.isValidEmail("a@bc")); // minimal
assertTrue(Email.isValidEmail("test@localhost")); // alphabets only
assertTrue(Email.isValidEmail("!#$%&'*+/=?`{|}~^[email protected]")); // special characters local part
assertTrue(Email.isValidEmail("123@145")); // numeric local part and domain name
assertTrue(Email.isValidEmail("123@145")); // numeric local part and domain name
assertTrue(Email.isValidEmail("[email protected]")); // mixture of alphanumeric and special characters
assertTrue(Email.isValidEmail("[email protected]")); // long domain name
assertTrue(Email.isValidEmail("[email protected]")); // long local part
assertTrue(Email.isValidEmail("[email protected]")); // long domain name
assertTrue(Email.isValidEmail("[email protected]")); // long local part
}
}
2 changes: 1 addition & 1 deletion src/test/java/systemtests/ClearCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void clear() {
/* Case: undo clearing address book -> original address book restored */
String command = UndoCommand.COMMAND_WORD;
String expectedResultMessage = UndoCommand.MESSAGE_SUCCESS;
assertCommandSuccess(command, expectedResultMessage, defaultModel);
assertCommandSuccess(command, expectedResultMessage, defaultModel);
assertSelectedCardUnchanged();

/* Case: redo clearing address book -> cleared */
Expand Down

0 comments on commit 83699f1

Please sign in to comment.