Issue checkstyle#6311: OrderedPropertiesCheck check that properties a…

…re ordered

.ci/jsoref-spellchecker/whitelist.words

@@ -933,6 +933,7 @@ optgroup
 optimisation
 Optimizable
 oraclejdk
+orderedproperties
 orderingandspacing
 orekit
 Orgorgan

config/checkstyle_checks.xml

@@ -69,6 +69,7 @@
     <property name="requiredTranslations" value="de, fr, fi, es, pt, ja, tr, zh"/>
   </module>
   <module name="UniqueProperties"/>
+  <module name="OrderedProperties" />
 
   <!-- Regexp -->
   <module name="RegexpMultiline"/>

config/pmd.xml

@@ -379,11 +379,12 @@
   </rule>
   <rule ref="category/java/multithreading.xml/AvoidSynchronizedAtMethodLevel">
     <properties>
-      <!-- UniqueProperties#put overloads a synchronized method, so it should have synchronized
-           modifier. -->
+      <!-- UniqueProperties#put and OrderedProperties#keys
+      overloads a synchronized method, so it should have synchronized
+      modifier. -->
       <property name="violationSuppressXPath"
-                value="//ClassOrInterfaceDeclaration[@Image='UniqueProperties']
-                           //MethodDeclarator[@Image='put']"/>
+        value="//ClassOrInterfaceDeclaration[@Image='UniqueProperties'
+          or @Image='SequencedProperties']//MethodDeclarator[@Image='keys' or @Image='put']"/>
     </properties>
   </rule>
 

config/spotbugs-exclude.xml

@@ -178,6 +178,12 @@
         <Method name="processFiltered"/>
         <Bug pattern="NP_GUARANTEED_DEREF_ON_EXCEPTION_PATH"/>
     </Match>
+    <Match>
+        <!-- false-positive. See details at https://github.com/checkstyle/checkstyle/pull/6343 -->
+        <Class
+        name="com.puppycrawl.tools.checkstyle.checks.OrderedPropertiesCheck$SequencedProperties"/>
+        <Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>
+    </Match>
     <Match>
         <!-- It is better to catch all exceptions since it can throw a runtime exception. -->
         <Class name="com.puppycrawl.tools.checkstyle.checks.TranslationCheck"/>

pom.xml

@@ -1807,6 +1807,7 @@
                 <param>com.puppycrawl.tools.checkstyle.checks.FinalParametersCheck*</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.LineSeparatorOption*</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.NewlineAtEndOfFileCheck*</param>
+                <param>com.puppycrawl.tools.checkstyle.checks.OrderedPropertiesCheck*</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.OuterTypeFilenameCheck*</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.SuppressWarningsHolder*</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.TodoCommentCheck*</param>
@@ -1824,6 +1825,7 @@
                 <param>com.puppycrawl.tools.checkstyle.checks.DescendantTokenCheckTest</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.FinalParametersCheckTest</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.NewlineAtEndOfFileCheckTest</param>
+                <param>com.puppycrawl.tools.checkstyle.checks.OrderedPropertiesCheckTest</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.OuterTypeFilenameCheckTest</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.SuppressWarningsHolderTest</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.TodoCommentCheckTest</param>

src/main/java/com/puppycrawl/tools/checkstyle/PackageObjectFactory.java

@@ -805,6 +805,8 @@ private static void fillModulesFromChecksPackage() {
                 BASE_PACKAGE + ".checks.NewlineAtEndOfFileCheck");
         NAME_TO_FULL_MODULE_NAME.put("OuterTypeFilenameCheck",
                 BASE_PACKAGE + ".checks.OuterTypeFilenameCheck");
+        NAME_TO_FULL_MODULE_NAME.put("OrderedPropertiesCheck",
+                BASE_PACKAGE + ".checks.OrderedPropertiesCheck");
         NAME_TO_FULL_MODULE_NAME.put("SuppressWarningsHolder",
                 BASE_PACKAGE + ".checks.SuppressWarningsHolder");
         NAME_TO_FULL_MODULE_NAME.put("TodoCommentCheck",

src/main/java/com/puppycrawl/tools/checkstyle/checks/OrderedPropertiesCheck.java

@@ -0,0 +1,237 @@
+////////////////////////////////////////////////////////////////////////////////
+// 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.puppycrawl.tools.checkstyle.checks;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import com.puppycrawl.tools.checkstyle.StatelessCheck;
+import com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck;
+import com.puppycrawl.tools.checkstyle.api.FileText;
+
+/**
+ * <p>Since Checkstyle 8.21</p>
+ * <p>Detects if keys in properties files are in correct order.</p>
+ * <p>
+ *   Rationale: Unsorted property makes merges more easy. While there are no errors in
+ *   runtime.
+ *   This check is valuable only on files with string resources where order of lines
+ *   does not matter at all, but this can be improved.
+ *   E.g.: checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/messages.properties
+ *   You may suppress warnings of this check for files that have an logical structure like
+ *   build files or log4j configuration files. See SuppressionFilter.
+ *   <pre />
+ * </p>
+ * <p>Known limitation: The key should not contain a newline.
+ * The string compare will work, but not the line number reporting.</p>
+ * <ul>
+ *   <li>Property {@code fileExtensions} - file type extension of the files to check.
+ *   Default value is .properties.</li>
+ * </ul>
+ * <p>To configure the check:</p>
+ * <pre>&lt;module name="OrderedProperties" /&gt;</pre>
+ * <p>Example properties file:</p>
+ * <pre>
+ * @=64
+ *           A=65
+ *           a=97
+ *           key=107 than nothing
+ *           key.sub=k is 107 and dot is 46
+ *           key.png=value - violation
+ * </pre>
+ * <p>We check order of key's only. Here we would like to use an Locale independent
+ * order mechanism, an binary order. The order is case insensitive and ascending.</p>
+ * <ul>
+ *   <li>The @ sign is on position 64 and comes first.</li>
+ *   <li>The capital A is on 65 and the lowercase a is on position 97 on the ascii table.</li>
+ *   <li>Key and key.sub are in correct order here, because only keys are relevant.
+ *   Therefore on line 5 you have only "key" an nothing behind.
+ *   On line 6 you have "key." The dot is on position 46 which is higher than nothing.
+ *   key.png will reported as violation because "png" comes before "sub".</li>
+ * </ul>
+ * @since null
+ */
+@StatelessCheck
+public class OrderedPropertiesCheck extends AbstractFileSetCheck {
+
+    /**
+     * Localization key for check violation.
+     */
+    public static final String MSG_KEY = "properties.notSorted.property";
+    /**
+     * Localization key for IO exception occurred on file open.
+     */
+    public static final String MSG_IO_EXCEPTION_KEY = "unable.open.cause";
+    /**
+     * Pattern matching single space.
+     */
+    private static final Pattern SPACE_PATTERN = Pattern.compile(" ");
+
+    /**
+     * Construct the check with default values.
+     */
+    public OrderedPropertiesCheck() {
+        setFileExtensions("properties");
+    }
+
+    /**
+     * Processes the file and check order.
+     * @param file the file to be processed
+     * @param fileText the contents of the file.
+     * @noinspection EnumerationCanBeIteration
+     */
+    @Override
+    protected void processFiltered(File file, FileText fileText) {
+        final SequencedProperties properties = new SequencedProperties();
+        try (InputStream inputStream = Files.newInputStream(file.toPath())) {
+            properties.load(inputStream);
+        }
+        catch (IOException | IllegalArgumentException ex) {
+            log(1, MSG_IO_EXCEPTION_KEY, file.getPath(), ex.getLocalizedMessage());
+        }
+
+        String previousProp = "";
+        int startLineNo = 0;
+
+        final Enumeration<Object> keys = properties.keys();
+
+        while (keys.hasMoreElements()) {
+
+            final String propKey = (String) keys.nextElement();
+
+            if (String.CASE_INSENSITIVE_ORDER.compare(previousProp, propKey) > 0) {
+
+                final int lineNo = getLineNumber(startLineNo, fileText, previousProp, propKey);
+                log(lineNo + 1, MSG_KEY, propKey, previousProp);
+                // start searching at position of the last reported validation
+                startLineNo = lineNo;
+            }
+
+            previousProp = propKey;
+        }
+    }
+
+    /**
+     * Method returns the index number where the key is detected (starting at 0).
+     * To assure that we get the correct line it starts at the point
+     * of the last occurrence.
+     * Also the previousProp should be in file before propKey.
+     *
+     * @param startLineNo start searching at line
+     * @param fileText {@link FileText} object contains the lines to process
+     * @param previousProp key name found last iteration, works only if valid
+     * @param propKey key name to look for
+     * @return index number of first occurrence. If no key found in properties file, 0 is returned
+     */
+    private static int getLineNumber(int startLineNo, FileText fileText,
+                                     String previousProp, String propKey) {
+
+        final int indexOfPreviousProp = getIndex(startLineNo, fileText, previousProp);
+        return getIndex(indexOfPreviousProp, fileText, propKey);
+    }
+
+    /**
+     * Inner method to get the index number of the position of keyName.
+     *
+     * @param startLineNo start searching at line
+     * @param fileText {@link FileText} object contains the lines to process
+     * @param keyName key name to look for
+     * @return index number of first occurrence. If no key found in properties file, 0 is returned
+     */
+    private static int getIndex(int startLineNo, FileText fileText, String keyName) {
+        final Pattern keyPattern = getKeyPattern(keyName);
+        int indexNumber = 0;
+        final Matcher matcher = keyPattern.matcher("");
+        for (int index = startLineNo; index < fileText.size(); index++) {
+            final String line = fileText.get(index);
+            matcher.reset(line);
+            if (matcher.matches()) {
+                indexNumber = index;
+                break;
+            }
+        }
+        return indexNumber;
+    }
+
+    /**
+     * Method returns regular expression pattern given key name.
+     *
+     * @param keyName
+     *            key name to look for
+     * @return regular expression pattern given key name
+     */
+    private static Pattern getKeyPattern(String keyName) {
+
+        final String keyPatternString = "^" + SPACE_PATTERN.matcher(keyName)
+                .replaceAll(Matcher.quoteReplacement("\\\\ ")) + "[\\s:=].*";
+        return Pattern.compile(keyPatternString);
+    }
+
+    /**
+     * Private property implementation that keeps order of properties like in file.
+     *
+     * @noinspection ClassExtendsConcreteCollection, SerializableHasSerializationMethods,
+     *     NonSerializableFieldInSerializableClass
+     */
+    private static class SequencedProperties extends Properties {
+
+        private static final long serialVersionUID = 1L;
+
+        /**
+         * Holding the keys in the same order than in the file.
+         */
+        private final List<Object> keyList = new ArrayList<>();
+
+        /**
+         * Returns a copy of the keys.
+         */
+        @Override
+        public synchronized Enumeration<Object> keys() {
+            return Collections.enumeration(keyList);
+        }
+
+        /**
+         * Puts the value into list by its key.
+         * @noinspection UseOfPropertiesAsHashtable
+         *
+         * @param key the hashtable key
+         * @param value the value
+         * @return the previous value of the specified key in this hashtable,
+         *      or null if it did not have one
+         * @throws NullPointerException - if the key or value is null
+         */
+        @Override
+        public synchronized Object put(Object key, Object value) {
+
+            keyList.add(key);
+
+            return super.put(key, value);
+        }
+    }
+}

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages.properties

@@ -7,6 +7,7 @@ final.parameter=Parameter {0} should be final.
 forbid.escaped.unicode.char=Unicode escape(s) usage should be avoided.
 noNewlineAtEOF=File does not end with a newline.
 properties.duplicate.property=Duplicated property ''{0}'' ({1} occurrence(s)).
+properties.notSorted.property=Property key ''{0}'' is not in the right order with previous property ''{1}''.
 suppress.warnings.invalid.target=Invalid target for @SuppressWarnings.
 todo.match=Comment matches to-do format ''{0}''.
 trailing.comments=Don''t use trailing comments.

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages_de.properties

@@ -7,6 +7,7 @@ final.parameter=Der Parameter {0} sollte als ''final'' deklariert sein.
 forbid.escaped.unicode.char=Unicode-Escapes sollten vermieden werden.
 noNewlineAtEOF=Datei endet nicht mit einem Zeilenumbruch.
 properties.duplicate.property=Duplizierte Property ''{0}'' (Anzahl {1}).
+properties.notSorted.property=Property Schlüssel ''{0}'' ist nicht in der richtigen Sortierreihenfolge mit dem Vorgänger ''{1}''.
 suppress.warnings.invalid.target=Ungültiges Ziel für SuppressWarnings.
 todo.match=Kommentar entspricht to-do-Format ''{0}''.
 trailing.comments=Kommentare und Code sollten nicht in einer Zeile gemischt werden.

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages_es.properties

@@ -7,6 +7,7 @@ final.parameter=El parámetro {0} debería ser final.
 forbid.escaped.unicode.char=El uso de Unicode de escape (s) debe ser evitado.
 noNewlineAtEOF=El fichero no termina con un retorno de carro.
 properties.duplicate.property=Propiedad duplicado ''{0}'' {1} ocurrencia (s)).
+properties.notSorted.property=Clave de propiedad ''{0}'' no está en el orden correcto con la propiedad anterior ''{1}''.
 suppress.warnings.invalid.target=Objetivo no válido paraSuppressWarnings.
 todo.match=El comentario coincide con el formato to-do ''{0}''.
 trailing.comments=No usar comentarios de final de línea.

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages_fi.properties

@@ -7,6 +7,7 @@ final.parameter=Parametri {0} pitäisi olla lopullinen.
 forbid.escaped.unicode.char=Unicode paeta (t) käyttö tulee välttää.
 noNewlineAtEOF=Tiedosto ei pääty rivinvaihtoon.
 properties.duplicate.property=Monistaa omaisuus ''{0}'' ( {1} esiintyminen (t)).
+properties.notSorted.property=Ominaisuusavain ''{0}'' ei ole oikeassa järjestyksessä aiemman ominaisuuden ''{1}'' kanssa.
 suppress.warnings.invalid.target=Virheellinen kohdeSuppressWarnings.
 todo.match=Kommentti on to-do formaatin ''{0}'' mukainen.
 trailing.comments=Mä käyttää perään kommentteja.

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages_fr.properties

@@ -7,6 +7,7 @@ final.parameter=Le paramètre {0} devrait être final.
 forbid.escaped.unicode.char=Les séquences d''échappement Unicode devraient être évitées.
 noNewlineAtEOF=Il manque un caractère NewLine à la fin du fichier.
 properties.duplicate.property=Propriété dupliquée ''{0} '' ({1} occurrence(s)).
+properties.notSorted.property=La clé de propriété ''{0}'' n´est pas dans le bon ordre avec la propriété précédente ''{1}''.
 suppress.warnings.invalid.target=Cible non valide pour @SuppressWarnings.
 todo.match=Le commentaire correspond au format TODO ''{0}''.
 trailing.comments=Ne mélangez pas instructions Java et commentaires sur la même ligne.

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages_ja.properties

@@ -7,6 +7,7 @@ final.parameter=パラメータ {0} は final にしてください。
 forbid.escaped.unicode.char=Unicode エスケープは使用しないでください。
 noNewlineAtEOF=ファイルが新しい行で終了していません。
 properties.duplicate.property=プロパティ ''{0}'' が重複しています（{1} 回）。
+properties.notSorted.property=プロパティキー ''{0}'' は前のプロパティ ''{1}'' と正しい順序にありません。
 suppress.warnings.invalid.target=@SuppressWarnings のターゲットが無効です。
 todo.match=コメントが to-do の形式 ''{0}'' に一致しています。
 trailing.comments=後続コメントは使用しないでください。

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages_pt.properties

@@ -7,6 +7,7 @@ final.parameter=O parâmetro {0} deve ser final.
 forbid.escaped.unicode.char=A utilização de escape Unicode deve ser evitada.
 noNewlineAtEOF=O arquivo não termina com uma linha em branco.
 properties.duplicate.property=Propriedade duplicada ''{0}'' ({1} ocorrência(s)).
+properties.notSorted.property=Chave de propriedade ''{0}'' não está na ordem correta com a propriedade anterior ''{1}''.
 suppress.warnings.invalid.target=Destino inválido para @SuppressWarnings.
 todo.match=O comentário condiz com o padrão ''{0}'' de tarefa pendente.
 trailing.comments=Não use comentários no final de linhas.

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages_tr.properties

@@ -7,6 +7,7 @@ final.parameter={0} parametresi ''final'' olarak tanımlanmalı.
 forbid.escaped.unicode.char=Unicode kaçış (ler) kullanımından kaçınılmalıdır.
 noNewlineAtEOF=Dosyanın sonunda yeni satır karakteri yok.
 properties.duplicate.property=Yinelenen özellik {0} {1} olay (lar)).
+properties.notSorted.property=Özellik anahtarı ''{0}'' önceki ''{1}'' özelliğiyle doğru sırada değil.
 suppress.warnings.invalid.target=SuppressWarnings Için geçersiz hedef.
 todo.match=Açıklamalar, ''to-do'' formatı olan ''{0}'' ile çakışıyor.
 trailing.comments=İzleyen (trailing) açıklamalar kullanılmamalıdır.

src/main/resources/com/puppycrawl/tools/checkstyle/checks/messages_zh.properties

@@ -7,6 +7,7 @@ final.parameter=参数： {0} 应定义为 final 的。
 forbid.escaped.unicode.char=不要使用Unicode转义字符。
 noNewlineAtEOF=文件未以空行结尾。
 properties.duplicate.property=重复属性： ''{0}'' ({1} 次).
+properties.notSorted.property=属性键 ''{0}'' 与前一个属性 ''{1}'' 的顺序不正确。
 suppress.warnings.invalid.target=@SuppressWarnings 目标错误。
 todo.match=TODO块： ''{0}'' 。
 trailing.comments=不要使用行尾注释。