Improve sort-flags to fix unknown patterns (#338)

* Improve `sort-flags` to fix unknown patterns

* Added test for non-owned pattern

Author: RunDevelopment

lib/rules/sort-flags.ts

@@ -42,7 +42,7 @@ export default createRule("sort-flags", {
                         loc: getFlagsLocation(),
                         messageId: "sortFlags",
                         data: { flags: flagsString, sortedFlags },
-                        fix: fixReplaceFlags(sortedFlags),
+                        fix: fixReplaceFlags(sortedFlags, false),
                     })
                 }
             }

lib/utils/index.ts

@@ -71,6 +71,7 @@ type RegExpContextBase = {
 
     fixReplaceFlags: (
         newFlags: string | (() => string | null),
+        includePattern?: boolean,
     ) => (fixer: Rule.RuleFixer) => Rule.Fix[] | Rule.Fix | null
 
     /**
@@ -118,6 +119,7 @@ type UnparsableRegExpContextBase = {
 
     fixReplaceFlags: (
         newFlags: string | (() => string | null),
+        includePattern?: boolean,
     ) => (fixer: Rule.RuleFixer) => Rule.Fix[] | Rule.Fix | null
 }
 export type RegExpContextForInvalid = {
@@ -609,12 +611,13 @@ function buildRegExpContextBase({
         fixReplaceQuant: (qNode, replacement) => {
             return fixReplaceQuant(patternSource, qNode, replacement)
         },
-        fixReplaceFlags: (newFlags) => {
+        fixReplaceFlags: (newFlags, includePattern) => {
             return fixReplaceFlags(
                 patternSource,
                 regexpNode,
                 flagsNode,
                 newFlags,
+                includePattern ?? true,
             )
         },
         getUsageOfPattern: () =>
@@ -665,15 +668,13 @@ function buildUnparsableRegExpContextBase({
         getFlagLocation: (flag) =>
             getFlagLocation(sourceCode, regexpNode, flagsNode, flag),
 
-        fixReplaceFlags: (newFlags) => {
-            if (!patternSource) {
-                return () => null
-            }
+        fixReplaceFlags: (newFlags, includePattern) => {
             return fixReplaceFlags(
                 patternSource,
                 regexpNode,
                 flagsNode,
                 newFlags,
+                includePattern ?? true,
             )
         },
     }
@@ -863,14 +864,35 @@ function fixReplaceQuant(
 
 /**
  * Returns a new fixer that replaces the current flags with the given flags.
+ *
+ * @param includePattern Whether the whole pattern is to be included in the fix.
+ *
+ * Fixes that change the pattern generally assume that the flags don't change,
+ * so changing the flags should conflict with all pattern fixes.
  */
 function fixReplaceFlags(
-    patternSource: PatternSource,
+    patternSource: PatternSource | null,
     regexpNode: ESTree.CallExpression | ESTree.RegExpLiteral,
     flagsNode: ESTree.Expression | null,
     replacement: string | (() => string | null),
+    includePattern: boolean,
 ) {
     return (fixer: Rule.RuleFixer): Rule.Fix[] | Rule.Fix | null => {
+        let patternFix = null
+        if (includePattern) {
+            if (!patternSource) {
+                return null
+            }
+            const patternRange = patternSource.getReplaceRange({
+                start: 0,
+                end: patternSource.value.length,
+            })
+            if (patternRange == null) {
+                return null
+            }
+            patternFix = patternRange.replace(fixer, patternSource.value)
+        }
+
         let newFlags
         if (typeof replacement === "string") {
             newFlags = replacement
@@ -916,18 +938,10 @@ function fixReplaceFlags(
             )
         }
 
-        // fixes that change the pattern generally assume that flags don't
-        // change, so we have to create conflicts.
-
-        const patternRange = patternSource.getReplaceRange({
-            start: 0,
-            end: patternSource.value.length,
-        })
-        if (patternRange == null) {
-            return null
+        if (!patternFix) {
+            return flagsFix
         }
-
-        return [patternRange.replace(fixer, patternSource.value), flagsFix]
+        return [patternFix, flagsFix]
     }
 }
 

tests/lib/rules/sort-flags.ts

@@ -71,5 +71,27 @@ tester.run("sort-flags", rule as any, {
                 },
             ],
         },
+        {
+            // sort flags even on unknown
+            code: String.raw`RegExp('a' + b, 'us');`,
+            output: String.raw`RegExp('a' + b, 'su');`,
+            errors: [
+                {
+                    message: "The flags 'us' should be in the order 'su'.",
+                    column: 18,
+                },
+            ],
+        },
+        {
+            // sort flags even on non-owned pattern
+            code: String.raw`var a = "foo"; RegExp(foo, 'us'); RegExp(foo, 'u');`,
+            output: String.raw`var a = "foo"; RegExp(foo, 'su'); RegExp(foo, 'u');`,
+            errors: [
+                {
+                    message: "The flags 'us' should be in the order 'su'.",
+                    column: 29,
+                },
+            ],
+        },
     ],
 })