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

Commit

Permalink
[#319] Checkstyle: enforce operators' placement in line wraps (#382)
Browse files Browse the repository at this point in the history
Our coding standard for Java requires operators' placement in line
wraps to follow specific rules, such as having assignment operator (=)
stay attached to the token that precedes it and having the arithmetric
operator (+) in a new line. These rules are not enforced by
checkstyle.

Let's teach Checkstyle to be stricter about line wrapping around more
symbols by adding the relevant OperatorWrap rules.
  • Loading branch information
eugenepeh authored and yamgent committed May 22, 2017
1 parent a74cc95 commit cc22ed6
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 15 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ allprojects {
junitVersion = '4.12'
testFxVersion = '4.0.5-alpha'
monocleVersion = '1.8.0_20'
checkstyleVersion = '7.1.2'
checkstyleVersion = '7.2'

libDir = 'lib'
}
Expand Down
22 changes: 22 additions & 0 deletions config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,28 @@
<property name="message" value="Trailing whitespace"/>
</module>

<module name="OperatorWrap">
<!-- Checks that the non-assignment type operator is at the next line in a line wrap.
This includes "?", ":", "==", "!=", "/", "+", "-", "*", "%", ">>", ">>>",
">=", ">", "<<", "<=", "<", "^", "|", "||", "&", "&&", "instanceof",
"&" when used in a generic upper or lower bounds constraints,
e.g. <T extends Foo & Bar>
"::" when used as a reference to a method or constructor without arguments.
e.g. String::compareToIgnoreCase
-->
<property name="tokens" value="QUESTION, COLON, EQUAL, NOT_EQUAL, DIV, PLUS, MINUS, STAR, MOD, SR, BSR,
GE, GT, SL, LE, LT, BXOR, BOR, LOR, BAND, LAND, LITERAL_INSTANCEOF, TYPE_EXTENSION_AND, METHOD_REF"/>
<property name="option" value="nl"/>
</module>
<module name="OperatorWrap">
<!-- Checks that the assignment type operator is at the previous end of line in a line wrap.
This includes "=", "/=", "+=", "-=", "*=", "%=", ">>=", ">>>=", "<<=", "^=", "&=".
-->
<property name="tokens" value="ASSIGN, DIV_ASSIGN, PLUS_ASSIGN, MINUS_ASSIGN, STAR_ASSIGN, MOD_ASSIGN,
SR_ASSIGN, BSR_ASSIGN, SL_ASSIGN, BXOR_ASSIGN, BOR_ASSIGN, BAND_ASSIGN"/>
<property name="option" value="eol"/>
</module>

<module name="SeparatorWrap">
<!-- Checks that the ".", "@" is at the next line in a line wrap. -->
<property name="tokens" value="DOT, AT"/>
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/seedu/address/MainApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ protected Config initConfig(String configFilePath) {
Optional<Config> configOptional = ConfigUtil.readConfig(configFilePathUsed);
initializedConfig = configOptional.orElse(new Config());
} catch (DataConversionException e) {
logger.warning("Config file at " + configFilePathUsed + " is not in the correct format. " +
"Using default config properties");
logger.warning("Config file at " + configFilePathUsed + " is not in the correct format. "
+ "Using default config properties");
initializedConfig = new Config();
}

Expand All @@ -144,8 +144,8 @@ protected UserPrefs initPrefs(UserPrefsStorage storage) {
Optional<UserPrefs> prefsOptional = storage.readUserPrefs();
initializedPrefs = prefsOptional.orElse(new UserPrefs());
} catch (DataConversionException e) {
logger.warning("UserPrefs file at " + prefsFilePath + " is not in the correct format. " +
"Using default user prefs");
logger.warning("UserPrefs file at " + prefsFilePath + " is not in the correct format. "
+ "Using default user prefs");
initializedPrefs = new UserPrefs();
} catch (IOException e) {
logger.warning("Problem while reading from the file. Will be starting with an empty AddressBook");
Expand Down
21 changes: 16 additions & 5 deletions src/main/java/seedu/address/commons/core/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,22 @@ public String toString() {

@Override
public int compareTo(Version other) {
return this.major != other.major ? this.major - other.major :
this.minor != other.minor ? this.minor - other.minor :
this.patch != other.patch ? this.patch - other.patch :
this.isEarlyAccess == other.isEarlyAccess() ? 0 :
this.isEarlyAccess ? -1 : 1;
if (this.major != other.major) {
return this.major - other.major;
}
if (this.minor != other.minor) {
return this.minor - other.minor;
}
if (this.patch != other.patch) {
return this.patch - other.patch;
}
if (this.isEarlyAccess == other.isEarlyAccess()) {
return 0;
}
if (this.isEarlyAccess) {
return -1;
}
return 1;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/guitests/guihandles/PersonListPanelHandle.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ public boolean isListMatching(ReadOnlyPerson... persons) {
*/
public boolean isListMatching(int startPosition, ReadOnlyPerson... persons) throws IllegalArgumentException {
if (persons.length + startPosition != getListView().getItems().size()) {
throw new IllegalArgumentException("List size mismatched\n" +
"Expected " + (getListView().getItems().size() - 1) + " persons");
throw new IllegalArgumentException("List size mismatched\n"
+ "Expected " + (getListView().getItems().size() - 1) + " persons");
}
assertTrue(this.containsInOrder(startPosition, persons));
for (int i = 0; i < persons.length; i++) {
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/seedu/address/commons/core/ConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ public class ConfigTest {

@Test
public void toString_defaultObject_stringReturned() {
String defaultConfigAsString = "App title : Address App\n" +
"Current log level : INFO\n" +
"Preference file Location : preferences.json";
String defaultConfigAsString = "App title : Address App\n"
+ "Current log level : INFO\n"
+ "Preference file Location : preferences.json";

assertEquals(defaultConfigAsString, new Config().toString());
}
Expand Down

0 comments on commit cc22ed6

Please sign in to comment.