Report minimal ranges

fsoikin · fsoikin · commit 94eb56dc8193 · 2025-01-25T22:54:50.000-05:00
diff --git a/integration-tests/cases/0-normal/baseline.txt b/integration-tests/cases/0-normal/baseline.txt
@@ -1,28 +1,22 @@
-[36msrc/B.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/B.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
-[36msrc/A.purs[0m:9:5:  [31mCommaFirstArrays[0m: Prefer comma-first style in array literals, items aligned vertically
+[36msrc/A.purs[0m:9:5:  [31mCommaFirstArrays[0m: Format array literals comma-first, align items vertically
 [93m
      9: y = [
     10:   true,
     11:   false
     12: ]
 [0m
-[36msrc/nested/C.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/C.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
-[36msrc/nested/doubly-nested/A.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/doubly-nested/A.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
diff --git a/integration-tests/cases/0-normal/glob-nested-and-b.txt b/integration-tests/cases/0-normal/glob-nested-and-b.txt
@@ -1,21 +1,15 @@
-[36msrc/nested/C.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/C.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
-[36msrc/nested/doubly-nested/A.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/doubly-nested/A.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
-[36msrc/B.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/B.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
diff --git a/integration-tests/cases/0-normal/glob-only-a.txt b/integration-tests/cases/0-normal/glob-only-a.txt
@@ -1,14 +1,12 @@
-[36msrc/A.purs[0m:9:5:  [31mCommaFirstArrays[0m: Prefer comma-first style in array literals, items aligned vertically
+[36msrc/A.purs[0m:9:5:  [31mCommaFirstArrays[0m: Format array literals comma-first, align items vertically
 [93m
      9: y = [
     10:   true,
     11:   false
     12: ]
 [0m
-[36msrc/nested/doubly-nested/A.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/doubly-nested/A.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
diff --git a/integration-tests/cases/0-normal/glob-only-doubly-nested.txt b/integration-tests/cases/0-normal/glob-only-doubly-nested.txt
@@ -1,7 +1,5 @@
-[36msrc/nested/doubly-nested/A.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/doubly-nested/A.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
diff --git a/integration-tests/cases/0-normal/glob-only-nested.txt b/integration-tests/cases/0-normal/glob-only-nested.txt
@@ -1,14 +1,10 @@
-[36msrc/nested/C.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/C.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
-[36msrc/nested/doubly-nested/A.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/doubly-nested/A.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
-    3: f = case 0 of
-    4:   1 -> true
     5:   2 ->
     6:     false
 [0m
diff --git a/integration-tests/cases/0-normal/output-short.txt b/integration-tests/cases/0-normal/output-short.txt
@@ -1,4 +1,4 @@
-[36msrc/B.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
-[36msrc/A.purs[0m:9:5:  [31mCommaFirstArrays[0m: Prefer comma-first style in array literals, items aligned vertically
-[36msrc/nested/C.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
-[36msrc/nested/doubly-nested/A.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/B.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/A.purs[0m:9:5:  [31mCommaFirstArrays[0m: Format array literals comma-first, align items vertically
+[36msrc/nested/C.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
+[36msrc/nested/doubly-nested/A.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
diff --git a/spago.yaml b/spago.yaml
@@ -49,7 +49,7 @@ package:
     dependencies:
       - spec-node
   publish:
-    version: 0.0.23
+    version: 0.0.24
     license: MIT
     location:
       githubOwner: collegevine
diff --git a/src/Whine/Core/CaseBranchIndentation.purs b/src/Whine/Core/CaseBranchIndentation.purs
@@ -33,12 +33,13 @@ rule _ = emptyRule { onExpr = onExpr }
   where
     onExpr :: Handle Expr
     onExpr = Handle case _ of
-      e@(ExprCase { branches }) -> do
+      ExprCase { branches } -> do
         let { yes, no } = partition isOneLine branchesAndGuards
             consistentIndent = null yes || null no
+            smaller = if length yes <= length no then yes else no
         unless consistentIndent $
           reportViolation
-            { source: Just $ rangeOf e
+            { source: mergeRanges $ smaller # concatMap \(head /\ body) -> [head, body]
             , message: "Inconsistent indentation in case branches: keep either all single-line or all multi-line"
             }
         where
diff --git a/src/Whine/Core/CommaFirstArrays.purs b/src/Whine/Core/CommaFirstArrays.purs
@@ -60,30 +60,46 @@ rule _ = emptyRule { onExpr = onExpr }
     onExpr :: Handle Expr
     onExpr = Handle case _ of
       ExprArray (Wrapped { open, value: Just (Separated items), close }) -> do
-        when (multiLine && (anyMisalignedPrefixes || not correctCloseBracket)) $
+        for_ (mergeRanges violations) \range ->
           reportViolation
-            { source: Just { start: open.range.start, end: close.range.end }
-            , message: "Prefer comma-first style in array literals, items aligned vertically"
+            { source: Just range
+            , message: "Format array literals comma-first, align items vertically"
             }
         where
           multiLine = open.range.start.line /= close.range.end.line
           lastItem = last items.tail <#> snd # fromMaybe items.head
-          correctCloseBracket = close.range.start.line > (rangeOf lastItem).end.line
 
           firstItem = open /\ items.head
           newLineStartingItems = firstItem : do
             (_ /\ prevItem) /\ (nextComma /\ nextItem) <- zip (firstItem : items.tail) items.tail
             guard $ (rangeOf prevItem).end.line < (rangeOf nextItem).start.line
             pure $ nextComma /\ nextItem
 
-          anyMisalignedPrefixes =
-            newLineStartingItems # any \(prefix /\ item) ->
-              or
+          violations
+            | multiLine = misalignedPrefixes <> closeBracket
+            | otherwise = []
+
+          closeBracket = do
+            guard $ or
+              [ close.range.start.line /= (rangeOf lastItem).end.line + 1
+              , close.range.start.column /= open.range.start.column
+              ]
+            pure
+              { start: (rangeOf lastItem).end
+              , end: close.range.end
+              }
+
+          misalignedPrefixes = do
+            prefix /\ item <- newLineStartingItems
+            guard $ or
               [ prefix.range.end.line /= (rangeOf item).start.line
               , prefix.range.end.column /= (rangeOf item).start.column - 1
               , prefix.range.start.column /= open.range.start.column
-              , open.range.start.column /= close.range.start.column
               ]
+            pure
+              { start: prefix.range.start
+              , end: (rangeOf item).end
+              }
 
       _ ->
         pure unit
diff --git a/src/Whine/Prelude.purs b/src/Whine/Prelude.purs
@@ -1,6 +1,7 @@
 module Whine.Prelude
   ( module Prelude
   , module Reexport
+  , module Whine.Prelude
   ) where
 
 import Prelude
@@ -10,7 +11,7 @@ import Control.Monad.Error.Class (try) as Reexport
 import Control.Monad.Except (mapExcept, mapExceptT, runExcept, runExceptT) as Reexport
 import Control.Monad.Writer (class MonadWriter, WriterT, execWriterT, mapWriterT, tell) as Reexport
 
-import Data.Array ((!!), (:), (..), (\\), nub, null, cons, drop, dropWhile, elem, filter, length, catMaybes, mapMaybe, last, partition, take, uncons, zip, zipWith) as Reexport
+import Data.Array ((!!), (:), (..), (\\), concatMap, foldl, nub, null, cons, drop, dropWhile, elem, filter, length, catMaybes, mapMaybe, last, partition, take, uncons, zip, zipWith) as Reexport
 import Data.Array.NonEmpty (NonEmptyArray) as Reexport
 import Data.Bifunctor (bimap, lmap, rmap) as Reexport
 import Data.Either (Either(..), fromRight, either, hush, note) as Reexport
@@ -30,7 +31,19 @@ import Data.Tuple.Nested (type (/\), (/\)) as Reexport
 import Effect (Effect) as Reexport
 import Effect.Class (class MonadEffect, liftEffect) as Reexport
 
+import PureScript.CST.Types (SourceRange) as Reexport
+
 import JSON (JSON) as Reexport
 import Node.Encoding (Encoding(..)) as Reexport
 import Node.Path (FilePath) as Reexport
 import Type.Row (type (+)) as Reexport
+
+mergeRanges :: Array Reexport.SourceRange -> Reexport.Maybe Reexport.SourceRange
+mergeRanges = Reexport.foldl go Reexport.Nothing
+  where
+    go Reexport.Nothing r = Reexport.Just r
+    go (Reexport.Just a) b = Reexport.Just { start: min a.start b.start, end: max a.end b.end }
+
+    lt x y = x.line < y.line || (x.line == y.line && x.column < y.column)
+    min x y = if x `lt` y then x else y
+    max x y = if x `lt` y then y else x
diff --git a/test/Core/CaseBranchIndentation.purs b/test/Core/CaseBranchIndentation.purs
@@ -9,18 +9,18 @@ import Whine.Core.CaseBranchIndentation as CaseBranchIndentation
 import Whine.Test (runRule)
 
 spec :: Spec Unit
-spec = describe "CommaFirstArrays" do
+spec = describe "CaseBranchIndentation" do
 
   it "Reports inconsistent indentation" $
-    hasViolations ["0:4-3:5"] """
+    hasViolations ["1:2-1:8"] """
       x = case a of
         b -> c
         d ->
           e
     """
 
   it "Reports inconsistent guards" $
-    hasViolations ["0:4-5:8","7:4-12:5"] """
+    hasViolations ["2:4-3:9","11:2-12:5"] """
       x = case a of
         b
           | x ->
@@ -36,6 +36,36 @@ spec = describe "CommaFirstArrays" do
           e
     """
 
+  it "Reports the smallest range that encompasses all violations" $
+    hasViolations ["2:2-8:9"] """
+      x = case a of
+        b -> c
+        d ->
+          e
+        x -> y
+        z
+          | a -> b
+          | c ->
+              d
+          | e -> f
+    """
+
+  it "Reports either just single-lines or just multilines, whichever are in the minority" $
+    hasViolations ["2:2-3:5", "9:2-9:8"] """
+      x = case a of
+        b -> c
+        d ->
+          e
+        x -> y
+
+      y = case b of
+        c ->
+          d
+        e -> f
+        g ->
+          h
+    """
+
   it "Allows consistent indentation" $
     hasViolations [] """
       x = case a of
diff --git a/test/Core/CommaFirstArrays.purs b/test/Core/CommaFirstArrays.purs
@@ -20,31 +20,52 @@ spec = describe "CommaFirstArrays" do
     """
 
   it "Reports misaligned commas" $
-    hasViolations ["2:4-4:5"] """
+    hasViolations ["3:4-3:15"] """
       x =
         [ []
         , [ 1
-          ,  2
+          ,  "foobar"
           ]
         , "more stuff"
         ]
     """
 
   it "Reports misaligned open bracket" $
-    hasViolations ["1:2-3:3"] """
+    hasViolations ["1:2-1:6"] """
       x =
         [  1
         , 2
         ]
     """
 
+  it "Reports misaligned close bracket" $
+    hasViolations ["2:5-3:2"] """
+      x =
+        [ 1
+        , 2
+       ]
+    """
+
   it "Reports closing bracket on same line" $
-    hasViolations ["1:2-2:7"] """
+    hasViolations ["2:5-2:7"] """
       x =
         [ 1
         , 2 ]
     """
 
+  it "Reports the smallest range that encompasses all violations" $
+    hasViolations ["3:2-6:3"] """
+      x =
+        [ 1
+        , 2
+        ,  3
+        , 4
+        , 5, 6,
+        7
+        , 8
+        ]
+    """
+
   it "Allows comma-first" $
     hasViolations [] """
       x = length
@@ -71,6 +92,6 @@ spec = describe "CommaFirstArrays" do
       runRule
         { rule: CommaFirstArrays.rule JSON.null
         , module: mod
-        , assertViolationMessage: (_ `shouldEqual` "Prefer comma-first style in array literals, items aligned vertically")
+        , assertViolationMessage: (_ `shouldEqual` "Format array literals comma-first, align items vertically")
         }
       >>= shouldEqual vs

-Original file line number
+Diff line change
@@ @@ -1,28 +1,22 @@ @@
 -[36msrc/B.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 +[36msrc/B.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
 -    3: f = case 0 of
 -    4:   1 -> true
 :   2 ->
 :     false
 [0m
 -[36msrc/A.purs[0m:9:5:  [31mCommaFirstArrays[0m: Prefer comma-first style in array literals, items aligned vertically
 +[36msrc/A.purs[0m:9:5:  [31mCommaFirstArrays[0m: Format array literals comma-first, align items vertically
 [93m
 : y = [
 :   true,
 :   false
 : ]
 [0m
 -[36msrc/nested/C.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 +[36msrc/nested/C.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
 -    3: f = case 0 of
 -    4:   1 -> true
 :   2 ->
 :     false
 [0m
 -[36msrc/nested/doubly-nested/A.purs[0m:3:5:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 +[36msrc/nested/doubly-nested/A.purs[0m:5:3:  [31mCaseBranchIndentation[0m: Inconsistent indentation in case branches: keep either all single-line or all multi-line
 [93m
 -    3: f = case 0 of
 -    4:   1 -> true
 :   2 ->
 :     false
 [0m