From 4913b287c0739f88eed8bea42e63d416c3e1cf7c Mon Sep 17 00:00:00 2001 From: Aleksandr Ivanov Date: Tue, 28 Jul 2015 23:13:00 +0300 Subject: [PATCH] fixed problem with lexical order in CustomImportOrder #1469 --- .../imports/CustomImportOrderCheck.java | 34 +++++++++++-------- .../imports/CustomImportOrderCheckTest.java | 21 ++++++++++++ .../InputCustomImportOrderSamePackage.java | 24 ++++++++----- .../InputCustomImportOrderSamePackage2.java | 18 ++++++---- ...CustomImportOrderThirdPartyAndSpecial.java | 13 +++++-- 5 files changed, 79 insertions(+), 31 deletions(-) diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java index 1b7fd2ef5..b54c24e9c 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java @@ -460,24 +460,25 @@ public class CustomImportOrderCheck extends Check { final ImportDetails firstImport = importToGroupList.get(0); String currentGroup = getImportGroup(firstImport.isStaticImport(), firstImport.getImportFullPath()); - int groupNumber = customImportOrderRules.indexOf(currentGroup); - String previousImport = null; + int currentGroupNumber = customImportOrderRules.indexOf(currentGroup); + String previousImportFromCurrentGroup = null; for (ImportDetails importObject : importToGroupList) { final String importGroup = importObject.getImportGroup(); - final String fullImportIdent = importObject.importFullPath; + final String fullImportIdent = importObject.getImportFullPath(); if (!importGroup.equals(currentGroup)) { - if (customImportOrderRules.size() > groupNumber + 1) { - final String nextGroup = getNextImportGroup(groupNumber + 1); + //not the last group, last one is always NON_GROUP + if (customImportOrderRules.size() > currentGroupNumber + 1) { + final String nextGroup = getNextImportGroup(currentGroupNumber + 1); if (importGroup.equals(nextGroup)) { if (separateLineBetweenGroups && !hasEmptyLineBefore(importObject.getLineNumber())) { - log(importObject.getLineNumber(), MSG_LINE_SEPARATOR, - fullImportIdent); + log(importObject.getLineNumber(), MSG_LINE_SEPARATOR, fullImportIdent); } currentGroup = nextGroup; - groupNumber = customImportOrderRules.indexOf(nextGroup); + currentGroupNumber = customImportOrderRules.indexOf(nextGroup); + previousImportFromCurrentGroup = fullImportIdent; } else { logWrongImportGroupOrder(importObject.getLineNumber(), @@ -489,14 +490,17 @@ public class CustomImportOrderCheck extends Check { importGroup, currentGroup, fullImportIdent); } } - else if (sortImportsInGroupAlphabetically - && previousImport != null - && matchesImportGroup(importObject.isStaticImport(), - fullImportIdent, currentGroup) - && compareImports(fullImportIdent, previousImport) < 0) { - log(importObject.getLineNumber(), MSG_LEX, fullImportIdent, previousImport); + else { + if (sortImportsInGroupAlphabetically + && previousImportFromCurrentGroup != null + && compareImports(fullImportIdent, previousImportFromCurrentGroup) < 0) { + log(importObject.getLineNumber(), MSG_LEX, + fullImportIdent, previousImportFromCurrentGroup); + } + else { + previousImportFromCurrentGroup = fullImportIdent; + } } - previousImport = fullImportIdent; } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheckTest.java index ac4169ffb..fc711b08b 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheckTest.java @@ -70,6 +70,7 @@ public class CustomImportOrderCheckTest extends BaseCheckTestSupport { checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); final String[] expected = { "4: " + getCheckMessage(MSG_LEX, "java.awt.Button.ABORT", "java.io.File.createTempFile"), + "5: " + getCheckMessage(MSG_LEX, "java.awt.print.Paper.*", "java.io.File.createTempFile"), "8: " + getCheckMessage(MSG_ORDER, STD, SAME, "java.awt.Button"), "9: " + getCheckMessage(MSG_ORDER, STD, SAME, "java.awt.Frame"), "10: " + getCheckMessage(MSG_ORDER, STD, SAME, "java.awt.Dialog"), @@ -101,8 +102,12 @@ public class CustomImportOrderCheckTest extends BaseCheckTestSupport { checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); final String[] expected = { "4: " + getCheckMessage(MSG_LEX, "java.awt.Button.ABORT", "java.io.File.createTempFile"), + "5: " + getCheckMessage(MSG_LEX, "java.awt.print.Paper.*", "java.io.File.createTempFile"), "10: " + getCheckMessage(MSG_LEX, "java.awt.Dialog", "java.awt.Frame"), "15: " + getCheckMessage(MSG_LEX, "java.io.File", "javax.swing.JTable"), + "16: " + getCheckMessage(MSG_LEX, "java.io.IOException", "javax.swing.JTable"), + "17: " + getCheckMessage(MSG_LEX, "java.io.InputStream", "javax.swing.JTable"), + "18: " + getCheckMessage(MSG_LEX, "java.io.Reader", "javax.swing.JTable"), "22: " + getCheckMessage(MSG_LEX, "com.google.common.collect.*", "com.puppycrawl.tools.*"), }; @@ -123,8 +128,12 @@ public class CustomImportOrderCheckTest extends BaseCheckTestSupport { checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); final String[] expected = { "4: " + getCheckMessage(MSG_LEX, "java.awt.Button.ABORT", "java.io.File.createTempFile"), + "5: " + getCheckMessage(MSG_LEX, "java.awt.print.Paper.*", "java.io.File.createTempFile"), "10: " + getCheckMessage(MSG_LEX, "java.awt.Dialog", "java.awt.Frame"), "15: " + getCheckMessage(MSG_LEX, "java.io.File", "javax.swing.JTable"), + "16: " + getCheckMessage(MSG_LEX, "java.io.IOException", "javax.swing.JTable"), + "17: " + getCheckMessage(MSG_LEX, "java.io.InputStream", "javax.swing.JTable"), + "18: " + getCheckMessage(MSG_LEX, "java.io.Reader", "javax.swing.JTable"), "20: " + getCheckMessage(MSG_ORDER, SAME, THIRD, "com.puppycrawl.tools.*"), "22: " + getCheckMessage(MSG_NONGROUP_IMPORT, "com.google.common.collect.*"), "23: " + getCheckMessage(MSG_LINE_SEPARATOR, "org.junit.*"), @@ -143,11 +152,15 @@ public class CustomImportOrderCheckTest extends BaseCheckTestSupport { "STANDARD_JAVA_PACKAGE"); checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); final String[] expected = { + "4: " + getCheckMessage(MSG_LEX, "java.awt.Button.ABORT", "java.io.File.createTempFile"), "7: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STD, "java.util.List"), "8: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STD, "java.util.StringTokenizer"), "9: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STD, "java.util.*"), "10: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STD, "java.util.concurrent.AbstractExecutorService"), "11: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STD, "java.util.concurrent.*"), + "13: " + getCheckMessage(MSG_LEX, "com.puppycrawl.tools.*", "javax.swing.WindowConstants.*"), + "14: " + getCheckMessage(MSG_LEX, "com.*", "javax.swing.WindowConstants.*"), + "16: " + getCheckMessage(MSG_LEX, "com.google.common.base.*", "javax.swing.WindowConstants.*"), }; verify(checkConfig, getPath("imports" + File.separator @@ -163,11 +176,14 @@ public class CustomImportOrderCheckTest extends BaseCheckTestSupport { "STATIC###SAME_PACKAGE(3)"); checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); final String[] expected = { + "5: " + getCheckMessage(MSG_LEX, "java.util.*", "java.util.StringTokenizer"), "6: " + getCheckMessage(MSG_NONGROUP_EXPECTED, SAME, "java.util.concurrent.*"), "7: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STATIC, "java.awt.Button.ABORT"), "8: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STATIC, "javax.swing.WindowConstants.*"), + "9: " + getCheckMessage(MSG_LEX, "com.puppycrawl.tools.*", "java.util.StringTokenizer"), "10: " + getCheckMessage(MSG_NONGROUP_EXPECTED, SAME, "java.util.concurrent.AbstractExecutorService"), "11: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STATIC, "java.io.File.createTempFile"), + "12: " + getCheckMessage(MSG_LEX, "com.*", "java.util.StringTokenizer"), }; verify(checkConfig, new File("src/test/resources-noncompilable/com/puppycrawl/tools/" @@ -185,11 +201,14 @@ public class CustomImportOrderCheckTest extends BaseCheckTestSupport { "STATIC###SAME_PACKAGE(3)"); checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); final String[] expected = { + "5: " + getCheckMessage(MSG_LEX, "java.util.*", "java.util.StringTokenizer"), "6: " + getCheckMessage(MSG_NONGROUP_EXPECTED, SAME, "java.util.concurrent.*"), "7: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STATIC, "java.awt.Button.ABORT"), "8: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STATIC, "javax.swing.WindowConstants.*"), + "9: " + getCheckMessage(MSG_LEX, "com.puppycrawl.tools.*", "java.util.StringTokenizer"), "10: " + getCheckMessage(MSG_NONGROUP_EXPECTED, SAME, "java.util.concurrent.AbstractExecutorService"), "11: " + getCheckMessage(MSG_NONGROUP_EXPECTED, STATIC, "java.io.File.createTempFile"), + "12: " + getCheckMessage(MSG_LEX, "com.*", "java.util.StringTokenizer"), }; verify(checkConfig, new File("src/test/resources-noncompilable/com/puppycrawl/tools/" @@ -207,6 +226,7 @@ public class CustomImportOrderCheckTest extends BaseCheckTestSupport { checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); final String[] expected = { "4: " + getCheckMessage(MSG_LEX, "java.io.File.createTempFile", "javax.swing.WindowConstants.*"), + "8: " + getCheckMessage(MSG_LEX, "com.*", "com.puppycrawl.tools.*"), }; verify(checkConfig, getPath("imports" + File.separator @@ -508,4 +528,5 @@ public class CustomImportOrderCheckTest extends BaseCheckTestSupport { fail(); } } + } diff --git a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderSamePackage.java b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderSamePackage.java index 54afeeb05..8cfadfd7a 100644 --- a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderSamePackage.java +++ b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderSamePackage.java @@ -2,15 +2,23 @@ package java.util.concurrent; import com.google.common.*; import java.util.StringTokenizer; -import java.util.*; -import java.util.concurrent.*; -import static java.awt.Button.ABORT; -import static javax.swing.WindowConstants.*; -import com.puppycrawl.tools.*; -import java.util.concurrent.AbstractExecutorService; -import static java.io.File.createTempFile; -import com.*; +import java.util.*; //warn, LEX, should be before "java.util.StringTokenizer" +import java.util.concurrent.*; //warn, ORDER, should be on SAME_PACKAGE, now NOT_ASSIGNED +import static java.awt.Button.ABORT; //warn, ORDER, should be on STATIC, now NOT_ASSIGNED +import static javax.swing.WindowConstants.*; //warn, ORDER, should be on STATIC, now NOT_ASSIGNED +import com.puppycrawl.tools.*; //warn, LEX, should be before "java.util.StringTokenizer" +import java.util.concurrent.AbstractExecutorService; //warn, ORDER, should be on SAME_PACKAGE, now NOT_ASSIGNED +import static java.io.File.createTempFile; //warn, ORDER, should be on STATIC, now NOT_ASSIGNED +import com.*; //warn, LEX, should be before "java.util.StringTokenizer" import org.apache.*; public class InputCustomImportOrderSamePackage { } +/* +test: testStaticSamePackage() +configuration: + checkConfig.addAttribute("thirdPartyPackageRegExp", "org."); + checkConfig.addAttribute("customImportOrderRules", + "STATIC###SAME_PACKAGE(3)"); + checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); +*/ diff --git a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderSamePackage2.java b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderSamePackage2.java index e40eedfc6..4d4a2eddc 100644 --- a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderSamePackage2.java +++ b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderSamePackage2.java @@ -1,12 +1,18 @@ //Moved to noncompilable because UT requires imports from the same package package java.util.concurrent; import java.util.regex.Pattern; -import java.util.List; -import java.util.regex.Matcher; -import java.util.StringTokenizer; -import java.util.*; -import java.util.concurrent.AbstractExecutorService; -import java.util.concurrent.*; +import java.util.List; //warn, LEX, should be before "java.util.regex.Pattern" +import java.util.regex.Matcher; //warn, LEX, should be before "java.util.regex.Pattern" +import java.util.StringTokenizer; //warn, LEX, should be before "java.util.regex.Pattern" +import java.util.*; //warn, LEX, should be before "java.util.regex.Pattern" +import java.util.concurrent.AbstractExecutorService; //warn, ORDER, should be on SAME_PACKAGE, now NOT_ASSIGNED +import java.util.concurrent.*; //warn, ORDER, should be on SAME_PACKAGE, now NOT_ASSIGNED public class InputCustomImportOrderSamePackage2 { } +/* +test: testOnlySamePackage() +configuration: + checkConfig.addAttribute("customImportOrderRules", "SAME_PACKAGE(3)"); + checkConfig.addAttribute("sortImportsInGroupAlphabetically", "true"); +*/ diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderThirdPartyAndSpecial.java b/src/test/resources/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderThirdPartyAndSpecial.java index c8096327b..2bc26b1c8 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderThirdPartyAndSpecial.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/imports/InputCustomImportOrderThirdPartyAndSpecial.java @@ -8,11 +8,20 @@ import org.apache.commons.io.ByteOrderMark; import static sun.tools.util.ModifierFilter.ALL_ACCESS; -import com.google.common.annotations.GwtCompatible; +import com.google.common.annotations.GwtCompatible; //warn, ORDER, should be on THIRD_PARTY_PACKAGE, now SPECIAL_IMPORTS import antlr.*; +import antlr.CommonASTWithHiddenTokens; +import antlr.Token; +import antlr.collections.AST; public class InputCustomImportOrderThirdPartyAndSpecial { - } +/* +test: testThirdPartyAndSpecialImports() +configuration: + checkConfig.addAttribute("specialImportsRegExp", "antlr.*"); + checkConfig.addAttribute("customImportOrderRules", + "SAME_PACKAGE(3)###THIRD_PARTY_PACKAGE###STATIC###SPECIAL_IMPORTS"); +*/