Skip to content

Commit

Permalink
Issue checkstyle#3101: Add new 'useContainerOrderingForStatic' option…
Browse files Browse the repository at this point in the history
… for ImportOrderCheck
  • Loading branch information
MEZk committed Jul 6, 2016
1 parent a211730 commit 4911e7d
Show file tree
Hide file tree
Showing 5 changed files with 213 additions and 6 deletions.
2 changes: 1 addition & 1 deletion config/suppressions.xml
Expand Up @@ -158,7 +158,7 @@
<!-- equals() - a lot of fields to check -->
<suppress checks="CyclomaticComplexity" files="LocalizedMessage\.java" lines="210"/>
<!-- SWITCH was transformed into IF-ELSE -->
<suppress checks="CyclomaticComplexity" files="ImportOrderCheck\.java" lines="344"/>
<suppress checks="CyclomaticComplexity" files="ImportOrderCheck\.java" lines="357"/>

<!-- Just big SWITCH block which contains IF blocks in 'visitToken'.
If we split the block to several methods it will demage readibility. -->
Expand Down
Expand Up @@ -64,6 +64,8 @@
* Case sensitive sorting is in ASCII sort order</td><td>Boolean</td><td>true</td></tr>
* <tr><td>sortStaticImportsAlphabetically</td><td>whether static imports grouped by top or
* bottom option are sorted alphabetically or not</td><td>Boolean</td><td>false</td></tr>
* <tr><td>useContainerOrderingForStatic</td><td>whether to use container ordering
* (Eclipse IDE term) for static imports or not</td><td>Boolean</td><td>false</td></tr>
* </table>
*
* <p>
Expand Down Expand Up @@ -176,6 +178,7 @@
* @author David DIDIER
* @author Steve McKay
* @author <a href="mailto:nesterenko-aleksey@list.ru">Aleksey Nesterenko</a>
* @author Andrei Selkin
*/
public class ImportOrderCheck
extends AbstractCheck {
Expand Down Expand Up @@ -219,6 +222,8 @@ public class ImportOrderCheck
private boolean beforeFirstImport;
/** Whether static imports should be sorted alphabetically or not. */
private boolean sortStaticImportsAlphabetically;
/** Whether to use container ordering (Eclipse IDE term) for static imports or not. */
private boolean useContainerOrderingForStatic;

/** The policy to enforce. */
private ImportOrderOption option = ImportOrderOption.UNDER;
Expand Down Expand Up @@ -317,6 +322,14 @@ public void setSortStaticImportsAlphabetically(boolean sortAlphabetically) {
sortStaticImportsAlphabetically = sortAlphabetically;
}

/**
* Sets whether to use container ordering (Eclipse IDE term) for static imports or not.
* @param useContainerOrdering whether to use container ordering for static imports or not.
*/
public void setUseContainerOrderingForStatic(boolean useContainerOrdering) {
useContainerOrderingForStatic = useContainerOrdering;
}

@Override
public int[] getDefaultTokens() {
return getAcceptableTokens();
Expand Down Expand Up @@ -459,8 +472,7 @@ private void doVisitTokenInSameGroup(boolean isStatic,
boolean previous, String name, int line) {
if (ordered) {
if (option == ImportOrderOption.INFLOW) {
// out of lexicographic order
if (compare(lastImport, name, caseSensitive) > 0) {
if (isWrongOrder(name, isStatic)) {
log(line, MSG_ORDERING, name);
}
}
Expand All @@ -474,9 +486,7 @@ private void doVisitTokenInSameGroup(boolean isStatic,
// current and previous static or current and
// previous non-static
lastImportStatic == isStatic
&&
// and out of lexicographic order
compare(lastImport, name, caseSensitive) > 0;
&& isWrongOrder(name, isStatic);

if (shouldFireError) {
log(line, MSG_ORDERING, name);
Expand All @@ -485,6 +495,90 @@ private void doVisitTokenInSameGroup(boolean isStatic,
}
}

/**
* Checks whether import name is in wrong order.
* @param name import name.
* @param isStatic whether it is a static import name.
* @return true if import name is in wrong order.
*/
private boolean isWrongOrder(String name, boolean isStatic) {
final boolean result;
if (isStatic && useContainerOrderingForStatic) {
result = compareContainerOrder(lastImport, name, caseSensitive) > 0;
}
else {
// out of lexicographic order
result = compare(lastImport, name, caseSensitive) > 0;
}
return result;
}

/**
* Compares two import strings.
* We first compare the container of the static import, container being the type enclosing
* the static element being imported. When this returns 0, we compare the qualified
* import name. For e.g. this is what is considered to be container names:
* <p>
* import static HttpConstants.COLON => HttpConstants
* import static HttpHeaders.addHeader => HttpHeaders
* import static HttpHeaders.setHeader => HttpHeaders
* import static HttpHeaders.Names.DATE => HttpHeaders.Names
* </p>
* <p>
* According to this logic, HttpHeaders.Names would come after HttpHeaders.
*
* For more details, see <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=473629#c3">
* static imports comparison method</a> in Eclipse.
* </p>
*
* @param importName1 first import name.
* @param importName2 second import name.
* @param caseSensitive whether the comparison of fully qualified import names is case
* sensitive.
* @return the value {@code 0} if str1 is equal to str2; a value
* less than {@code 0} if str is less than the str2 (container order
* or lexicographical); and a value greater than {@code 0} if str1 is greater than str2
* (container order or lexicographically).
*/
private static int compareContainerOrder(String importName1, String importName2,
boolean caseSensitive) {
final String container1 = getImportContainer(importName1);
final String container2 = getImportContainer(importName2);
final int compareContainersOrderResult;
if (caseSensitive) {
compareContainersOrderResult = container1.compareTo(container2);
}
else {
compareContainersOrderResult = container1.compareToIgnoreCase(container2);
}
final int result;
if (compareContainersOrderResult == 0) {
result = compare(importName1, importName2, caseSensitive);
}
else {
result = compareContainersOrderResult;
}
return result;
}

/**
* Extracts import container name from fully qualified import name.
* An import container name is the type which encloses the static element being imported.
* For example, HttpConstants, HttpHeaders, HttpHeaders.Names are import container names:
* <p>
* import static HttpConstants.COLON => HttpConstants
* import static HttpHeaders.addHeader => HttpHeaders
* import static HttpHeaders.setHeader => HttpHeaders
* import static HttpHeaders.Names.DATE => HttpHeaders.Names
* </p>
* @param qualifiedImportName fully qualified import name.
* @return import container name.
*/
private static String getImportContainer(String qualifiedImportName) {
final int lastDotIndex = qualifiedImportName.lastIndexOf('.');
return qualifiedImportName.substring(0, lastDotIndex);
}

/**
* Finds out what group the specified import belongs to.
*
Expand Down
Expand Up @@ -483,4 +483,51 @@ public void testEclipseDefaultNegative() throws Exception {

verify(checkConfig, getPath("InputImportOrder_EclipseDefaultNegative.java"), expected);
}

@Test
public void testUseContainerOrderingForStaticTrue() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(ImportOrderCheck.class);
checkConfig.addAttribute("groups", "/^javax?\\./,org");
checkConfig.addAttribute("ordered", "true");
checkConfig.addAttribute("separated", "true");
checkConfig.addAttribute("option", "top");
checkConfig.addAttribute("caseSensitive", "false");
checkConfig.addAttribute("sortStaticImportsAlphabetically", "true");
checkConfig.addAttribute("useContainerOrderingForStatic", "true");
final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
verify(checkConfig, getNonCompilablePath("InputEclipseStaticImportsOrder.java"), expected);
}

@Test
public void testUseContainerOrderingForStaticFalse() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(ImportOrderCheck.class);
checkConfig.addAttribute("groups", "/^javax?\\./,org");
checkConfig.addAttribute("ordered", "true");
checkConfig.addAttribute("separated", "true");
checkConfig.addAttribute("option", "top");
checkConfig.addAttribute("caseSensitive", "false");
checkConfig.addAttribute("sortStaticImportsAlphabetically", "true");
checkConfig.addAttribute("useContainerOrderingForStatic", "false");
final String[] expected = {
"6: " + getCheckMessage(MSG_ORDERING,
"io.netty.handler.codec.http.HttpHeaders.Names.addDate"),
};
verify(checkConfig, getNonCompilablePath("InputEclipseStaticImportsOrder.java"), expected);
}

@Test
public void testUseContainerOrderingForStaticTrueCaseSensitive() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(ImportOrderCheck.class);
checkConfig.addAttribute("groups", "/^javax?\\./,org");
checkConfig.addAttribute("ordered", "true");
checkConfig.addAttribute("separated", "true");
checkConfig.addAttribute("option", "top");
checkConfig.addAttribute("sortStaticImportsAlphabetically", "true");
checkConfig.addAttribute("useContainerOrderingForStatic", "true");
final String[] expected = {
"7: " + getCheckMessage(MSG_ORDERING,
"io.netty.handler.codec.http.HttpHeaders.Names.DATE"),
};
verify(checkConfig, getNonCompilablePath("InputEclipseStaticImportsOrder.java"), expected);
}
}
@@ -0,0 +1,10 @@
package com.puppycrawl.tools.checkstyle.checks.imports;

import static io.netty.handler.codec.http.HttpConstants.COLON;
import static io.netty.handler.codec.http.HttpHeaders.addHeader;
import static io.netty.handler.codec.http.HttpHeaders.setHeader;
import static io.netty.handler.codec.http.HttpHeaders.Names.addDate;
import static io.netty.handler.codec.http.HttpHeaders.Names.DATE;

public class InputEclipseStaticImportsOrder {
}
56 changes: 56 additions & 0 deletions src/xdocs/config_imports.xml
Expand Up @@ -878,6 +878,12 @@ import android.*;
<td><a href="property_types.html#boolean">Boolean</a></td>
<td>false</td>
</tr>
<tr>
<td>useContainerOrderingForStatic</td>
<td>whether to use container ordering (Eclipse IDE term) for static imports or not</td>
<td><a href="property_types.html#boolean">Boolean</a></td>
<td>false</td>
</tr>

<tr>
<td>tokens</td>
Expand Down Expand Up @@ -1007,6 +1013,56 @@ import java.util.Set; // Wrong order for 'java.util.Set' import.
public class SomeClass { ... }
</source>

<p>
The following example shows the idea of 'useContainerOrderingForStatic' option that is
useful for Eclipse IDE users to match ordering validation.
This is how the import comparison works for static imports: we first compare
the container of the static import, container is the type enclosing the static element
being imported. When the result of the comparison is 0 (containers are equal),
we compare the fully qualified import names.
For e.g. this is what is considered to be container names for the given example:

import static HttpConstants.COLON => HttpConstants
import static HttpHeaders.addHeader => HttpHeaders
import static HttpHeaders.setHeader => HttpHeaders
import static HttpHeaders.Names.DATE => HttpHeaders.Names

According to this logic, HttpHeaders.Names should come after HttpHeaders.
</p>
<source>
&lt;module name=&quot;ImportOrder&quot;&gt;
&lt;property name=&quot;useContainerOrderingForStatic&quot; value=&quot;true&quot;/&gt;
&lt;property name=&quot;ordered&quot; value=&quot;true&quot;/&gt;
&lt;property name=&quot;option&quot; value=&quot;top&quot;/&gt;
&lt;property name=&quot;caseSensitive&quot; value=&quot;false&quot;/&gt;
&lt;property name=&quot;sortStaticImportsAlphabetically&quot; value=&quot;true&quot;/&gt;
&lt;/module&gt;
</source>
<source>
import static io.netty.handler.codec.http.HttpConstants.COLON;
import static io.netty.handler.codec.http.HttpHeaders.addHeader;
import static io.netty.handler.codec.http.HttpHeaders.setHeader;
import static io.netty.handler.codec.http.HttpHeaders.Names.DATE;

public class InputEclipseStaticImportsOrder { }
</source>
<source>
&lt;module name=&quot;ImportOrder&quot;&gt;
&lt;property name=&quot;useContainerOrderingForStatic&quot; value=&quot;false&quot;/&gt;
&lt;property name=&quot;ordered&quot; value=&quot;true&quot;/&gt;
&lt;property name=&quot;option&quot; value=&quot;top&quot;/&gt;
&lt;property name=&quot;caseSensitive&quot; value=&quot;false&quot;/&gt;
&lt;property name=&quot;sortStaticImportsAlphabetically&quot; value=&quot;true&quot;/&gt;
&lt;/module&gt;
</source>
<source>
import static io.netty.handler.codec.http.HttpConstants.COLON;
import static io.netty.handler.codec.http.HttpHeaders.addHeader;
import static io.netty.handler.codec.http.HttpHeaders.setHeader;
import static io.netty.handler.codec.http.HttpHeaders.Names.DATE; // violation

public class InputEclipseStaticImportsOrder { }
</source>
</subsection>

<subsection name="Example of Usage">
Expand Down

0 comments on commit 4911e7d

Please sign in to comment.