Fix FP S2871 (no-alphabetical-sort): Rework the rule to emit a dedi…

…cated message for arrays of strings (#4539)
SonarSource · Jan 31, 2024 · 9870bba · 9870bba
1 parent 745f452
commit 9870bba
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 3 deletions.
diff --git a/packages/jsts/src/rules/S2871/rule.ts b/packages/jsts/src/rules/S2871/rule.ts
@@ -53,6 +53,8 @@ export const rule: Rule.RuleModule = {
     messages: {
       provideCompareFunction:
         'Provide a compare function to avoid sorting elements alphabetically.',
+      provideCompareFunctionForArrayOfStrings:
+        'Provide a compare function that depends on "String.localeCompare", to reliably sort elements alphabetically.',
       suggestNumericOrder: 'Add a comparator function to sort in ascending order',
       suggestLanguageSensitiveOrder:
         'Add a comparator function to sort in ascending language-sensitive order',
@@ -75,7 +77,8 @@ export const rule: Rule.RuleModule = {
 
         if ([...sortLike, ...copyingSortLike].includes(text) && isArrayLikeType(type, services)) {
           const suggest = getSuggestions(call, type);
-          context.report({ node, suggest, messageId: 'provideCompareFunction' });
+          const messageId = getMessageId(type);
+          context.report({ node, suggest, messageId });
         }
       },
     };
@@ -101,6 +104,14 @@ export const rule: Rule.RuleModule = {
       return suggestions;
     }
 
+    function getMessageId(type: ts.Type) {
+      if (isStringArray(type, services)) {
+        return 'provideCompareFunctionForArrayOfStrings';
+      }
+
+      return 'provideCompareFunction';
+    }
+
     function fixer(call: estree.CallExpression, ...placeholder: string[]): Rule.ReportFixer {
       const closingParenthesis = sourceCode.getLastToken(call, token => token.value === ')')!;
       const indent = ' '.repeat(call.loc?.start.column!);

diff --git a/packages/jsts/src/rules/S2871/unit.test.ts b/packages/jsts/src/rules/S2871/unit.test.ts
@@ -606,6 +606,8 @@ ruleTester.run(
         code: 'const array = ["foo", "bar"]; const sortedArray = array.toSorted();',
         errors: [
           {
+            message:
+              'Provide a compare function that depends on "String.localeCompare", to reliably sort elements alphabetically.',
             suggestions: [
               {
                 desc: 'Add a comparator function to sort in ascending language-sensitive order',

diff --git a/...avascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S2871.html b/...avascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S2871.html
@@ -22,8 +22,9 @@ <h2>Why is this an issue?</h2>
 console.log(numbers); // Output: [1, 2, 5, 10, 30]
 </pre>
 <p>Even to sort strings, the default sort order may give unexpected results. Not only does it not support localization, it also doesn’t fully support
-Unicode, as it only considers UTF-16 code units. For example, in the code below, <code>"eΔ"</code> is surprisingly before and after
-<code>"éΔ"</code>.</p>
+Unicode, as it only considers UTF-16 code units. For example, in the code below, <code>"eΔ"</code> is surprisingly before and after <code>"éΔ"</code>.
+To guarantee that the sorting is reliable and remains as such in the long run, it is necessary to provide a compare function that is both locale and
+Unicode aware - typically <code>String.localeCompare</code>.</p>
 <pre>
 const code1 = '\u00e9\u0394'; // "éΔ"
 const code2 = '\u0065\u0301\u0394'; // "éΔ" using Unicode combining marks