Special case more trailing-blank diff logic.

Follow-up to #102 which triggers this by trimming multiple trailing blanks.

src/cljstyle/format/core.clj

@@ -133,6 +133,8 @@
         (vary-meta assoc ::durations (into {} durations)))))
 
 
+;; ## String Formatting
+
 (defn reformat-string*
   "Transform a string by parsing it, formatting it, then rendering it. Returns
   a map with the revised string under `:formatted` and a map of the durations
@@ -153,6 +155,8 @@
   (:formatted (reformat-string* form-string rules-config)))
 
 
+;; ## File Formatting
+
 (defn- split-shebang
   "Separate out a shebang line if it the first text present in the string.
   Returns a vector with two entries, the potential shebang (or nil, if not
@@ -166,24 +170,35 @@
     [nil text]))
 
 
+(defn- fix-eof-newlines
+  "Change to the formatted string so it has the correct number of newlines at
+  the end. Returns the updated string, or the original if the rule is not
+  enabled."
+  [string rule]
+  (if (:enabled? rule)
+    (if (:trailing-blanks? rule)
+      ;; Any number of newlines are allowed, so ensure there is at least one.
+      (if-not (str/ends-with? string "\n")
+        (str string "\n")
+        string)
+      ;; Exactly one newline is allowed, clobber any.
+      (str/replace-first string #"\n*\z" "\n"))
+    ;; Rule disabled.
+    string))
+
+
 (defn reformat-file*
   "Like `reformat-string*` but applies to an entire file. Will add a final
   newline if configured to do so. Returns a map with the revised text and other
   information."
   [file-text rules-config]
-  (let [[shebang text] (split-shebang file-text)
-        result (reformat-string* text rules-config)]
-    (cond-> result
-      shebang
-      (update :formatted (partial str shebang))
-
-      (get-in rules-config [:eof-newline :enabled?])
-      (as-> result
-        (if (get-in rules-config [:eof-newline :trailing-blanks?])
-          (if (not (str/ends-with? (:formatted result) "\n"))
-            (update result :formatted str "\n")
-            result)
-          (update result :formatted str/replace-first #"\n*\z" "\n"))))))
+  (let [[shebang text] (split-shebang file-text)]
+    (->
+      (reformat-string* text rules-config)
+      (update :formatted fix-eof-newlines (:eof-newline rules-config))
+      (cond->
+        shebang
+        (update :formatted (partial str shebang))))))
 
 
 (defn reformat-file

src/cljstyle/task/check.clj

@@ -20,12 +20,25 @@
     path))
 
 
+(defn- chomp-blank-tail
+  "Remove all trailing newlines from the string."
+  [text]
+  (str/replace-first text #"\n+\z" ""))
+
+
+(defn- blank-tail-diff?
+  "True if the two strings only differ by the number of newlines at the end."
+  [original revised]
+  (and (not= original revised)
+       (= (chomp-blank-tail original)
+          (chomp-blank-tail revised))))
+
+
 (defn- diff-lines
   "Compare the two strings and return a diff from `DiffUtils`."
   [path original revised]
   (let [original-lines (str/split original #"\n")
-        revised-lines (str/split revised #"\n")
-        path (chomp-preslash path)]
+        revised-lines (str/split revised #"\n")]
     (str/join
       "\n"
       (DiffUtils/generateUnifiedDiff
@@ -36,26 +49,39 @@
         3))))
 
 
+(defn- diff-blank-tail
+  "Special-case diff construction when dealing with two strings which only
+  differ by a 'tail' of blank lines."
+  [path original revised]
+  (let [diff (diff-lines path original (str revised "x"))
+        lines (butlast (str/split diff #"\n"))
+        header (str/join "\n" (butlast lines))
+        last-line (last lines)
+        delta (- (count revised) (count original))]
+    (if (= 1 delta)
+      ;; Special case where we added a newline to the end of the text.
+      (str header "\n"
+           "-" (subs last-line 1) "\n"
+           "+" (subs last-line 1)
+           "\n\\ No newline at end of file")
+      ;; Some number of removed or added newlines
+      (str header "\n"
+           last-line "\n"
+           (if (neg? delta)
+             (str/join "\n" (repeat (- (count original) (count revised)) "-"))
+             (str/join "\n" (repeat (- (count revised) (count original)) "+")))))))
+
+
 (defn unified-diff
   "Produce a unified diff string comparing the original and revised texts."
   [path original revised]
   (let [path (chomp-preslash path)]
     ;; DiffUtils does not render anything in the case where the only difference
-    ;; is a trailing newline. Handle this case specially. Note that one
+    ;; is trailing newlines. Handle this case specially. Note that one
     ;; limitation of this approach is that if there are other errors _and_ a
     ;; missing EOF newline, this won't show the newline error in the diff.
-    (if (and (= (count revised) (inc (count original)))
-             (= revised (str original "\n")))
-      ;; Special-case newline diff.
-      (let [diff (diff-lines path original (str revised "x"))
-            lines (butlast (str/split diff #"\n"))
-            header (str/join "\n" (butlast lines))
-            last-line (last lines)]
-        (str header "\n"
-             "-" (subs last-line 1) "\n"
-             "+" (subs last-line 1) "\n"
-             "\\ No newline at end of file"))
-      ;; Standard diff.
+    (if (blank-tail-diff? original revised)
+      (diff-blank-tail path original revised)
       (diff-lines path original revised))))
 
 

test/cljstyle/task/check_test.clj

@@ -82,3 +82,33 @@
             (is (str/blank? stdout))
             (is (str/blank? stderr))
             (is (.exists stats-file))))))))
+
+
+(deftest diff-output
+  (testing "with changed content"
+    (is (= (str "--- a/path/to/file.txt\n"
+                "+++ b/path/to/file.txt\n"
+                "@@ -1,4 +1,4 @@\n"
+                " 1\n"
+                "-2\n"
+                "+two\n"
+                "+e\n"
+                " 3\n"
+                "-4")
+           (check/unified-diff "path/to/file.txt" "1\n2\n3\n4\n" "1\ntwo\ne\n3\n"))))
+  (testing "with missing eof newline"
+    (is (= (str "--- a/path/to/file.txt\n"
+                "+++ b/path/to/file.txt\n"
+                "@@ -1,1 +1,2 @@\n"
+                "-content\n"
+                "+content\n"
+                "\\ No newline at end of file")
+           (check/unified-diff "path/to/file.txt" "content" "content\n"))))
+  (testing "with trimmed eof newlines"
+    (is (= (str "--- a/path/to/file.txt\n"
+                "+++ b/path/to/file.txt\n"
+                "@@ -1,1 +1,2 @@\n"
+                " content\n"
+                "-\n"
+                "-")
+           (check/unified-diff "path/to/file.txt" "content\n\n\n" "content\n")))))