From 461be74976ef360c0bccff4328ae5e776a71a647 Mon Sep 17 00:00:00 2001
From: Ng Zhi An <ngzhian@gmail.com>
Date: Sat, 22 Dec 2018 15:42:28 +0800
Subject: [PATCH] Realign virtual tabs when applying fix

Fix an off-by-one error, in the case that is commented `should never happen`.
It happens when the end of a range is the at the end of a line.
In that case we should update the real column count (probably just by +1)
instead of returning it.

I modified makeNonVirtual to use a helper, realign, that works on
Ranged. That way we can share the code to realign a PositionedComment
and also a Replacement.

Fixes #1420
---
 src/ShellCheck/Fixer.hs            | 18 +++++++++++++++++
 src/ShellCheck/Formatter/Format.hs | 32 ++++++++++++++++++++----------
 src/ShellCheck/Formatter/TTY.hs    |  5 ++++-
 3 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/ShellCheck/Fixer.hs b/src/ShellCheck/Fixer.hs
index 78abfeb..9c4837c 100644
--- a/src/ShellCheck/Fixer.hs
+++ b/src/ShellCheck/Fixer.hs
@@ -16,20 +16,38 @@ class Ranged a where
             yEnd = end y
             xStart = start x
             xEnd = end x
+    -- Set a new start and end position on a Ranged
+    setRange :: (Position, Position) -> a -> a
+
+instance Ranged PositionedComment where
+    start = pcStartPos
+    end = pcEndPos
+    setRange (s, e) pc = pc {
+        pcStartPos = s,
+        pcEndPos = e
+    }
 
 instance Ranged Replacement where
     start = repStartPos
     end   = repEndPos
+    setRange (s, e) r = r {
+        repStartPos = s,
+        repEndPos = e
+    }
 
 instance Ranged a => Ranged [a] where
     start [] = newPosition
     start xs = (minimum . map start) xs
     end []   = newPosition
     end xs   = (maximum . map end) xs
+    setRange (s, e) rs = map (setRange (s, e)) rs
 
 instance Ranged Fix where
     start = start . fixReplacements
     end   = end . fixReplacements
+    setRange (s, e) f = f {
+        fixReplacements = setRange (s, e) (fixReplacements f)
+    }
 
 -- The Monoid instance for Fix merges replacements that do not overlap.
 instance Monoid Fix where
diff --git a/src/ShellCheck/Formatter/Format.hs b/src/ShellCheck/Formatter/Format.hs
index 5e46713..5c3c444 100644
--- a/src/ShellCheck/Formatter/Format.hs
+++ b/src/ShellCheck/Formatter/Format.hs
@@ -21,6 +21,7 @@ module ShellCheck.Formatter.Format where
 
 import ShellCheck.Data
 import ShellCheck.Interface
+import ShellCheck.Fixer
 
 -- A formatter that carries along an arbitrary piece of data
 data Formatter = Formatter {
@@ -51,20 +52,29 @@ makeNonVirtual comments contents =
     map fix comments
   where
     ls = lines contents
-    fix c = c {
-        pcStartPos = (pcStartPos c) {
-            posColumn = realignColumn lineNo colNo c
-        }
-      , pcEndPos = (pcEndPos c) {
-            posColumn = realignColumn endLineNo endColNo c
-        }
-    }
+    fix c =  realign c ls
+
+-- Realign a Ranged from a tabstop of 8 to 1
+realign :: Ranged a => a -> [String] -> a
+realign range ls =
+    let startColumn = realignColumn lineNo colNo range
+        endColumn = realignColumn endLineNo endColNo range
+        startPosition = (start range) { posColumn = startColumn }
+        endPosition = (end range) { posColumn = endColumn } in
+    setRange (startPosition, endPosition) range
+  where
     realignColumn lineNo colNo c =
       if lineNo c > 0 && lineNo c <= fromIntegral (length ls)
       then real (ls !! fromIntegral (lineNo c - 1)) 0 0 (colNo c)
       else colNo c
     real _ r v target | target <= v = r
-    real [] r v _ = r -- should never happen
-    real ('\t':rest) r v target =
-        real rest (r+1) (v + 8 - (v `mod` 8)) target
+    -- hit this case at the end of line, and if we don't hit the target
+    -- return real + (target - v)
+    real [] r v target = r + (target - v)
+    real ('\t':rest) r v target = real rest (r+1) (v + 8 - (v `mod` 8)) target
     real (_:rest) r v target = real rest (r+1) (v+1) target
+    lineNo = posLine . start
+    endLineNo = posLine . end
+    colNo = posColumn . start
+    endColNo = posColumn . end
+
diff --git a/src/ShellCheck/Formatter/TTY.hs b/src/ShellCheck/Formatter/TTY.hs
index adcc277..ffc5e93 100644
--- a/src/ShellCheck/Formatter/TTY.hs
+++ b/src/ShellCheck/Formatter/TTY.hs
@@ -153,10 +153,13 @@ showFixedString color comments lineNum fileLines =
         -- fixes for that single line. We can fold the fixes (which removes
         -- overlaps), and apply it as a single fix with multiple replacements.
         applicableComments -> do
-            let mergedFix = (fold . catMaybes . (map pcFix)) applicableComments
+            let mergedFix = (realignFix . fold . catMaybes . (map pcFix)) applicableComments
             -- in the spirit of error prone
             putStrLn $ color "message" "Did you mean: "
             putStrLn $ unlines $ fixedString mergedFix fileLines
+        where
+            realignFix f = f { fixReplacements = map fix (fixReplacements f) }
+            fix r = realign r fileLines
 
 fixedString :: Fix -> [String] -> [String]
 fixedString fix fileLines =