Repository: checkstyle/checkstyle

Problem statement:
MultipleVariableDeclarations missing from sun style
The rational of the check says the sun style demands this, but it is not part of the sun checks XML.

https://checkstyle.org/config_coding.html#MultipleVariableDeclarations


Hints:
https://checkstyle.org/sun_style.html this page is empty, because style guide is not properly created.
Sun config is seriously in unclear state.

One day we will find contributor to make like https://checkstyle.org/google_style.html , but this is a big project https://github.com/checkstyle/checkstyle/wiki/Checkstyle-GSoC-2019-Project-Ideas#project-name-sun-and-open-jdk-code-convention-coverage
Did you maybe misunderstand me?
Because I cannot relate your comment to my report @romani.
The rationale of the check says that the sun style demands what this check validates.
But the `sun_checks.xml` file does not include this check which is what I reported.
@Vampire Our sun and google configuration files are validated against the written guides from sun and google respectively. We create a table to show what line of text in the guide related to the check we have setup and configured to prove we follow the guide.

We already have a complete mapping for google:
http://checkstyle.sourceforge.net/google_style.html#Google_Coverage_table
http://checkstyle.sourceforge.net/reports/google-java-style-20170228.html

Sun's mapping is empty because no one has sat down and worked on it. Any text that says style is a demand of sun could be wrong.
https://checkstyle.org/sun_style.html#Sun_Coverage_table
Ah, now I understand what you mean, but actually the rational is correct:
![grafik](https://user-images.githubusercontent.com/325196/53899541-b62a0a00-403a-11e9-9af7-4db133ec2aea.png)


Patch:
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/DeclarationOrderCheck.java
@@ -37,7 +37,7 @@
  * </p>
  * <p>
  * According to
- * <a href="https://www.oracle.com/technetwork/java/javase/documentation/codeconventions-141855.html#1852">
+ * <a href="https://checkstyle.org/styleguides/sun-code-conventions-19990420/CodeConventions.doc2.html#a1852">
  * Code Conventions for the Java Programming Language</a>, the parts of a class
  * or interface declaration should appear in the following order:
  * </p>
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/MultipleVariableDeclarationsCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/MultipleVariableDeclarationsCheck.java
--- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/MultipleVariableDeclarationsCheck.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/MultipleVariableDeclarationsCheck.java
@@ -32,7 +32,7 @@
  * </p>
  * <p>
  * Rationale: <a
- * href="https://www.oracle.com/technetwork/java/javase/documentation/codeconventions-141270.html">
+ * href="https://checkstyle.org/styleguides/sun-code-conventions-19990420/CodeConventions.doc5.html#a2992">
  * the SUN Code conventions chapter 6.1</a> recommends that
  * declarations should be one per line.
  * </p>


Test patch:
diff --git a/src/it/java/com/sun/checkstyle/test/chapter6declarations/rule61numberperline/MultipleVariableDeclarationsTest.java b/src/it/java/com/sun/checkstyle/test/chapter6declarations/rule61numberperline/MultipleVariableDeclarationsTest.java
new file mode 100644
--- /dev/null
+++ b/src/it/java/com/sun/checkstyle/test/chapter6declarations/rule61numberperline/MultipleVariableDeclarationsTest.java
@@ -0,0 +1,72 @@
+////////////////////////////////////////////////////////////////////////////////
+// checkstyle: Checks Java source code for adherence to a set of rules.
+// Copyright (C) 2001-2019 the original author or authors.
+//
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2.1 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+////////////////////////////////////////////////////////////////////////////////
+
+package com.sun.checkstyle.test.chapter6declarations.rule61numberperline;
+
+import org.junit.Test;
+
+import com.google.checkstyle.test.base.AbstractModuleTestSupport;
+import com.puppycrawl.tools.checkstyle.api.Configuration;
+import com.puppycrawl.tools.checkstyle.checks.coding.MultipleVariableDeclarationsCheck;
+
+public class MultipleVariableDeclarationsTest extends AbstractModuleTestSupport {
+
+    @Override
+    protected String getPackageLocation() {
+        return "com/sun/checkstyle/test/chapter6declarations/rule61numberperline";
+    }
+
+    @Test
+    public void testMultipleVariableDeclarations() throws Exception {
+        final String msgComma = getCheckMessage(MultipleVariableDeclarationsCheck.class,
+            "multiple.variable.declarations.comma");
+        final String msg = getCheckMessage(MultipleVariableDeclarationsCheck.class,
+            "multiple.variable.declarations");
+
+        final String[] expected = {
+            "5:5: " + msgComma,
+            "6:5: " + msg,
+            "9:9: " + msgComma,
+            "10:9: " + msg,
+            "14:5: " + msg,
+            "17:5: " + msg,
+            "31:9: " + msgComma,
+            "32:9: " + msg,
+            "35:13: " + msgComma,
+            "36:13: " + msg,
+            "40:9: " + msg,
+            "43:9: " + msg,
+            "57:13: " + msgComma,
+            "58:13: " + msg,
+            "61:17: " + msgComma,
+            "62:17: " + msg,
+            "66:13: " + msg,
+            "69:13: " + msg,
+            "86:5: " + msgComma,
+            "89:5: " + msgComma,
+        };
+
+        final Configuration checkConfig = getModuleConfig("MultipleVariableDeclarations");
+        final String filePath = getPath("InputMultipleVariableDeclarations.java");
+
+        final Integer[] warnList = getLinesWithWarn(filePath);
+        verify(checkConfig, filePath, expected, warnList);
+    }
+
+}
diff --git a/src/it/resources/com/sun/checkstyle/test/chapter6declarations/rule61numberperline/InputMultipleVariableDeclarations.java b/src/it/resources/com/sun/checkstyle/test/chapter6declarations/rule61numberperline/InputMultipleVariableDeclarations.java
new file mode 100644
--- /dev/null
+++ b/src/it/resources/com/sun/checkstyle/test/chapter6declarations/rule61numberperline/InputMultipleVariableDeclarations.java
@@ -0,0 +1,90 @@
+package com.sun.checkstyle.test.chapter6declarations.rule61numberperline;
+
+public class InputMultipleVariableDeclarations
+{
+    int i, j; //warn
+    int i1; int j1; //warn
+
+    void method1() {
+        String str, str1; //warn
+        java.lang.Object obj; Object obj1; //warn
+    }
+    // second definition is wrapped
+    // line of VARIABLE_DEF is not the same as first line of the definition
+    java.lang.String string; java.lang.String //warn
+        strings[];
+    //both definitions is wrapped
+    java.lang. //warn
+        String string1; java.lang.String
+            strings1[];
+
+    void method2() {
+        for (int i=0, j=0; i < 10; i++, j--) { //ok
+        }
+
+        for(int i=0; i<4;i++) {
+
+        }
+    }
+
+    class Inner {
+        int i, j; //warn
+        int i1; int j1; //warn
+
+        void method1() {
+            String str, str1; //warn
+            java.lang.Object obj; Object obj1; //warn
+        }
+        // second definition is wrapped
+        // line of VARIABLE_DEF is not the same as first line of the definition
+        java.lang.String string; java.lang.String //warn
+            strings[];
+        //both definitions is wrapped
+        java.lang. //warn
+            String string1; java.lang.String
+                strings1[];
+
+        void method2() {
+            for (int i=0, j=0; i < 10; i++, j--) { //ok
+            }
+
+            for(int i=0; i<4;i++) {
+
+            }
+        }
+        Inner anon = new Inner()
+        {
+            int i, j; //warn
+            int i1; int j1; //warn
+
+            void method1() {
+                String str, str1; //warn
+                java.lang.Object obj; Object obj1; //warn
+            }
+            // second definition is wrapped
+            // line of VARIABLE_DEF is not the same as first line of the definition
+            java.lang.String string; java.lang.String //warn
+                strings[];
+            //both definitions is wrapped
+            java.lang. //warn
+                String string1; java.lang.String
+                    strings1[];
+
+            void method2() {
+                for (int i=0, j=0; i < 10; i++, j--) { //ok
+                }
+
+                for(int i=0; i<4;i++) {
+
+                }
+            }
+        };
+    }
+}
+
+class Suppress {
+    @SuppressWarnings("unused") //warn
+    long q1, q2, q3;
+
+    @SuppressWarnings("unused") long q4, q5, q6;  //warn
+}
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/internal/XdocsPagesTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/internal/XdocsPagesTest.java
--- a/src/test/java/com/puppycrawl/tools/checkstyle/internal/XdocsPagesTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/internal/XdocsPagesTest.java
@@ -19,7 +19,10 @@
 
 package com.puppycrawl.tools.checkstyle.internal;
 
+import static java.lang.Integer.parseInt;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.hamcrest.CoreMatchers.describedAs;
+import static org.hamcrest.CoreMatchers.is;
 
 import java.beans.PropertyDescriptor;
 import java.io.File;
@@ -46,6 +49,7 @@
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.regex.Pattern;
+import java.util.stream.IntStream;
 
 import org.apache.commons.beanutils.PropertyUtils;
 import org.junit.Assert;
@@ -143,6 +147,72 @@ public class XdocsPagesTest {
 
     private static final Set<String> SUN_MODULES = Collections.unmodifiableSet(
         new HashSet<>(CheckUtil.getConfigSunStyleModules()));
+    // ignore the not yet properly covered modules while testing newly added ones
+    // add proper sections to the coverage report and integration tests
+    // and then remove this list eventually
+    private static final List<String> IGNORED_SUN_MODULES = Arrays.asList(
+            "ArrayTypeStyle",
+            "AvoidNestedBlocks",
+            "AvoidStarImport",
+            "ConstantName",
+            "DesignForExtension",
+            "EmptyBlock",
+            "EmptyForIteratorPad",
+            "EmptyStatement",
+            "EqualsHashCode",
+            "FileLength",
+            "FileTabCharacter",
+            "FinalClass",
+            "FinalParameters",
+            "GenericWhitespace",
+            "HiddenField",
+            "HideUtilityClassConstructor",
+            "IllegalImport",
+            "IllegalInstantiation",
+            "InnerAssignment",
+            "InterfaceIsType",
+            "JavadocMethod",
+            "JavadocPackage",
+            "JavadocStyle",
+            "JavadocType",
+            "JavadocVariable",
+            "LeftCurly",
+            "LineLength",
+            "LocalFinalVariableName",
+            "LocalVariableName",
+            "MagicNumber",
+            "MemberName",
+            "MethodLength",
+            "MethodName",
+            "MethodParamPad",
+            "MissingSwitchDefault",
+            "ModifierOrder",
+            "NeedBraces",
+            "NewlineAtEndOfFile",
+            "NoWhitespaceAfter",
+            "NoWhitespaceBefore",
+            "OperatorWrap",
+            "PackageName",
+            "ParameterName",
+            "ParameterNumber",
+            "ParenPad",
+            "RedundantImport",
+            "RedundantModifier",
+            "RegexpSingleline",
+            "RightCurly",
+            "SimplifyBooleanExpression",
+            "SimplifyBooleanReturn",
+            "StaticVariableName",
+            "TodoComment",
+            "Translation",
+            "TypecastParenPad",
+            "TypeName",
+            "UnusedImports",
+            "UpperEll",
+            "VisibilityModifier",
+            "WhitespaceAfter",
+            "WhitespaceAround"
+    );
     private static final Set<String> GOOGLE_MODULES = Collections.unmodifiableSet(
         new HashSet<>(CheckUtil.getConfigGoogleStyleModules()));
 
@@ -1334,19 +1404,29 @@ private static Set<String> getProperties(Class<?> clss) {
     public void testAllStyleRules() throws Exception {
         for (Path path : XdocUtil.getXdocsStyleFilePaths(XdocUtil.getXdocsFilePaths())) {
             final String fileName = path.getFileName().toString();
+            final String styleName = fileName.substring(0, fileName.lastIndexOf('_'));
             final String input = new String(Files.readAllBytes(path), UTF_8);
             final Document document = XmlUtil.getRawXml(fileName, input, input);
             final NodeList sources = document.getElementsByTagName("tr");
-            Set<String> styleChecks = null;
 
-            if (path.toFile().getName().contains("google")) {
-                styleChecks = new HashSet<>(GOOGLE_MODULES);
-            }
-            else if (path.toFile().getName().contains("sun")) {
-                styleChecks = new HashSet<>();
+            final Set<String> styleChecks;
+            switch (styleName) {
+                case "google":
+                    styleChecks = new HashSet<>(GOOGLE_MODULES);
+                    break;
+
+                case "sun":
+                    styleChecks = new HashSet<>(SUN_MODULES);
+                    styleChecks.removeAll(IGNORED_SUN_MODULES);
+                    break;
+
+                default:
+                    Assert.fail("Missing modules list for style file '" + fileName + "'");
+                    styleChecks = null;
             }
 
             String lastRuleName = null;
+            String[] lastRuleNumberParts = null;
 
             for (int position = 0; position < sources.getLength(); position++) {
                 final Node row = sources.item(position);
@@ -1358,14 +1438,8 @@ else if (path.toFile().getName().contains("sun")) {
                 }
 
                 final String ruleName = columns.get(1).getTextContent().trim();
-
-                if (lastRuleName != null) {
-                    Assert.assertTrue(
-                            fileName + " rule '" + ruleName + "' is out of order compared to '"
-                                    + lastRuleName + "'",
-                            ruleName.toLowerCase(Locale.ENGLISH).compareTo(
-                                    lastRuleName.toLowerCase(Locale.ENGLISH)) >= 0);
-                }
+                lastRuleNumberParts = validateRuleNameOrder(
+                        fileName, lastRuleName, lastRuleNumberParts, ruleName);
 
                 if (!"--".equals(ruleName)) {
                     validateStyleAnchors(XmlUtil.findChildElementsByTag(columns.get(0), "a"),
@@ -1373,7 +1447,7 @@ else if (path.toFile().getName().contains("sun")) {
                 }
 
                 validateStyleModules(XmlUtil.findChildElementsByTag(columns.get(2), "a"),
-                        XmlUtil.findChildElementsByTag(columns.get(3), "a"), styleChecks, fileName,
+                        XmlUtil.findChildElementsByTag(columns.get(3), "a"), styleChecks, styleName,
                         ruleName);
 
                 lastRuleName = ruleName;
@@ -1389,6 +1463,70 @@ else if (path.toFile().getName().contains("sun")) {
         }
     }
 
+    private static String[] validateRuleNameOrder(String fileName, String lastRuleName,
+                                                  String[] lastRuleNumberParts, String ruleName) {
+        final String[] ruleNumberParts = ruleName.split(" ", 2)[0].split("\\.");
+
+        if (lastRuleName != null) {
+            final int ruleNumberPartsAmount = ruleNumberParts.length;
+            final int lastRuleNumberPartsAmount = lastRuleNumberParts.length;
+            final String outOfOrderReason = fileName + " rule '" + ruleName
+                    + "' is out of order compared to '" + lastRuleName + "'";
+            boolean lastRuleNumberPartWasEqual = false;
+            int partIndex;
+            for (partIndex = 0; partIndex < ruleNumberPartsAmount; partIndex++) {
+                if (lastRuleNumberPartsAmount <= partIndex) {
+                    // equal up to here and last rule has less parts,
+                    // thus order is correct, stop comparing
+                    break;
+                }
+
+                final String ruleNumberPart = ruleNumberParts[partIndex];
+                final String lastRuleNumberPart = lastRuleNumberParts[partIndex];
+                final boolean ruleNumberPartsAreNumeric = IntStream.concat(
+                        ruleNumberPart.chars(),
+                        lastRuleNumberPart.chars()
+                ).allMatch(Character::isDigit);
+
+                if (ruleNumberPartsAreNumeric) {
+                    final int numericRuleNumberPart = parseInt(ruleNumberPart);
+                    final int numericLastRuleNumberPart = parseInt(lastRuleNumberPart);
+                    Assert.assertThat(
+                            outOfOrderReason,
+                            numericRuleNumberPart < numericLastRuleNumberPart,
+                            describedAs("'%0' should not be less than '%1'",
+                                    is(false),
+                                    numericRuleNumberPart, numericLastRuleNumberPart));
+                }
+                else {
+                    Assert.assertThat(
+                            outOfOrderReason,
+                            ruleNumberPart.compareToIgnoreCase(lastRuleNumberPart) < 0,
+                            describedAs("'%0' should not be less than '%1'",
+                                    is(false),
+                                    ruleNumberPart, lastRuleNumberPart));
+                }
+                lastRuleNumberPartWasEqual = ruleNumberPart.equalsIgnoreCase(lastRuleNumberPart);
+                if (!lastRuleNumberPartWasEqual) {
+                    // number part is not equal but properly ordered,
+                    // thus order is correct, stop comparing
+                    break;
+                }
+            }
+            if (ruleNumberPartsAmount == partIndex && lastRuleNumberPartWasEqual) {
+                if (lastRuleNumberPartsAmount == partIndex) {
+                    Assert.fail(fileName + " rule '" + ruleName + "' and rule '"
+                            + lastRuleName + "' have the same rule number");
+                }
+                else {
+                    Assert.fail(outOfOrderReason);
+                }
+            }
+        }
+
+        return ruleNumberParts;
+    }
+
     private static void validateStyleAnchors(Set<Node> anchors, String fileName, String ruleName) {
         Assert.assertEquals(fileName + " rule '" + ruleName + "' must have two row anchors", 2,
                 anchors.size());
@@ -1423,7 +1561,7 @@ private static void validateStyleAnchors(Set<Node> anchors, String fileName, Str
     }
 
     private static void validateStyleModules(Set<Node> checks, Set<Node> configs,
-            Set<String> styleChecks, String fileName, String ruleName) {
+            Set<String> styleChecks, String styleName, String ruleName) {
         final Iterator<Node> itrChecks = checks.iterator();
         final Iterator<Node> itrConfigs = configs.iterator();
 
@@ -1436,7 +1574,7 @@ private static void validateStyleModules(Set<Node> checks, Set<Node> configs,
                 continue;
             }
 
-            Assert.assertTrue(fileName + " rule '" + ruleName + "' module '" + moduleName
+            Assert.assertTrue(styleName + "_style.xml rule '" + ruleName + "' module '" + moduleName
                     + "' shouldn't end with 'Check'", !moduleName.endsWith("Check"));
 
             styleChecks.remove(moduleName);
@@ -1448,43 +1586,45 @@ private static void validateStyleModules(Set<Node> checks, Set<Node> configs,
                     config = itrConfigs.next();
                 }
                 catch (NoSuchElementException ignore) {
-                    Assert.fail(fileName + " rule '" + ruleName + "' module '" + moduleName
-                            + "' is missing the config link: " + configName);
+                    Assert.fail(styleName + "_style.xml rule '" + ruleName + "' module '"
+                            + moduleName + "' is missing the config link: " + configName);
                 }
 
-                Assert.assertEquals(fileName + " rule '" + ruleName + "' module '" + moduleName
-                        + "' has mismatched config/test links", configName, config.getTextContent()
-                        .trim());
+                Assert.assertEquals(styleName + "_style.xml rule '" + ruleName + "' module '"
+                        + moduleName + "' has mismatched config/test links", configName,
+                        config.getTextContent().trim());
 
                 final String configUrl = config.getAttributes().getNamedItem("href")
                         .getTextContent();
 
                 if ("config".equals(configName)) {
                     final String expectedUrl = "https://github.com/search?q="
-                            + "path%3Asrc%2Fmain%2Fresources+filename%3Agoogle_checks.xml+"
-                            + "repo%3Acheckstyle%2Fcheckstyle+" + moduleName;
+                            + "path%3Asrc%2Fmain%2Fresources+filename%3A" + styleName
+                            + "_checks.xml+repo%3Acheckstyle%2Fcheckstyle+" + moduleName;
 
-                    Assert.assertEquals(fileName + " rule '" + ruleName + "' module '" + moduleName
-                            + "' should have matching " + configName + " url", expectedUrl,
-                            configUrl);
+                    Assert.assertEquals(styleName + "_style.xml rule '" + ruleName + "' module '"
+                                    + moduleName + "' should have matching " + configName + " url",
+                            expectedUrl, configUrl);
                 }
                 else if ("test".equals(configName)) {
-                    Assert.assertTrue(fileName + " rule '" + ruleName + "' module '" + moduleName
-                            + "' should have matching " + configName + " url",
+                    Assert.assertTrue(styleName + "_style.xml rule '" + ruleName + "' module '"
+                                    + moduleName + "' should have matching " + configName + " url",
                             configUrl.startsWith("https://github.com/checkstyle/checkstyle/"
-                                    + "blob/master/src/it/java/com/google/checkstyle/test/"));
-                    Assert.assertTrue(fileName + " rule '" + ruleName + "' module '" + moduleName
-                            + "' should have matching " + configName + " url",
+                                    + "blob/master/src/it/java/com/" + styleName
+                                    + "/checkstyle/test/"));
+                    Assert.assertTrue(styleName + "_style.xml rule '" + ruleName + "' module '"
+                                    + moduleName + "' should have matching " + configName + " url",
                             configUrl.endsWith("/" + moduleName + "Test.java"));
 
-                    Assert.assertTrue(fileName + " rule '" + ruleName + "' module '" + moduleName
-                            + "' should have a test that exists", new File(configUrl.substring(53)
-                            .replace('/', File.separatorChar)).exists());
+                    Assert.assertTrue(styleName + "_style.xml rule '" + ruleName + "' module '"
+                            + moduleName + "' should have a test that exists",
+                            new File(configUrl.substring(53)
+                                    .replace('/', File.separatorChar)).exists());
                 }
             }
         }
 
-        Assert.assertFalse(fileName + " rule '" + ruleName + "' has too many configs",
+        Assert.assertFalse(styleName + "_style.xml rule '" + ruleName + "' has too many configs",
                 itrConfigs.hasNext());
     }