Fix handling of FileInfo in comparisions

Author: dmiller

src/main/clojure/clojure/tools/namespace/dir.clj

@@ -18,7 +18,9 @@
             [clojure.set :as set]
             [clojure.string :as string])
   (:import (System.IO DirectoryInfo FileSystemInfo Path) (System.Text.RegularExpressions Regex)))    ;;; (java.io File) (java.util.regex Pattern)
+  
 (declare make-dir-info)
+
 (defn- find-files [dirs platform]
   (->> dirs
        (map make-dir-info)                         ;;; (map io/file)
@@ -27,18 +29,24 @@
        (mapcat #(find/find-sources-in-dir % platform))
        ))                                          ;;; ditto:  (map #(.getCanonicalFile ^File %))
 
+(defn- milliseconds-since-epoch [^DateTime time]
+  (long
+    (/ (- (.-Ticks time)
+          (.-Ticks DateTime/UnixEpoch))
+       TimeSpan/TicksPerMillisecond)))
+
 (defn- modified-files [tracker files]
-  (filter #(DateTime/op_LessThan ^DateTime (::time tracker 0) (.LastWriteTimeUTC ^FileSystemInfo %)) files))         ;;; (.lastModified ^File %)
+  (filter #(< (::time tracker 0) (milliseconds-since-epoch (.-LastWriteTimeUtc ^FileSystemInfo %))) files))   ;;; #(< (::time tracker 0) (.lastModified ^File %))
 
 (defn- deleted-files [tracker files]
-  (set/difference (::files tracker #{}) (set files)))
+  (set (remove #(file/some-file files %) (::files tracker #{}))))                                             ;;; (set/difference (::files tracker #{}) (set files))
 
 (defn- update-files [tracker deleted modified {:keys [read-opts]}]
-  (let [now (DateTime/UtcNow)]                                             ;;; (System/currentTimeMillis)
+  (let [now (milliseconds-since-epoch DateTime/UtcNow)]                        ;;; (System/currentTimeMillis)
     (-> tracker
         (update-in [::files] #(if % (apply disj % deleted) #{}))
         (file/remove-files deleted)
-        (update-in [::files] into modified)
+        (update-in [::files] file/into-files modified)                         ;;; (update-in [::files] into modified)
         (file/add-files modified read-opts)
         (assoc ::time now))))
 
@@ -59,7 +67,7 @@
   Optional third argument is map of options:
 
     :platform  Either clj (default) or cljs, both defined in
-               clojure.tools.namespace.find, controls reader options for 
+               clojure.tools.namespace.find, controls reader options for
                parsing files.
 
     :add-all?  If true, assumes all extant files are modified regardless
@@ -85,8 +93,8 @@
 
   Optional third argument is map of options:
 
-    :platform  Either clj (default) or cljs, both defined in 
-               clojure.tools.namespace.find, controls file extensions 
+    :platform  Either clj (default) or cljs, both defined in
+               clojure.tools.namespace.find, controls file extensions
                and reader options.
 
     :add-all?  If true, assumes all extant files are modified regardless
@@ -118,14 +126,15 @@
  dependency tracker to reload files. If no dirs given, defaults to
   all directories on the classpath."
   {:added "0.2.0"
-   :deprecated "0.3.0"}[tracker & dirs]
+   :deprecated "0.3.0"}
+  [tracker & dirs]
   (scan-dirs tracker dirs {:platform find/cljr :add-all? true}))          ;;; find/clj  -- is this correct?
 
 ;;; ADDED
 
-(defn- make-dir-info 
+(defn- make-dir-info
   ^DirectoryInfo [x]
-  (cond 
+  (cond
     (instance? DirectoryInfo x) x
 	(string? x) (DirectoryInfo. ^String x)
 	:default (DirectoryInfo. (str x))))

src/main/clojure/clojure/tools/namespace/file.clj

@@ -67,7 +67,38 @@
   read by the Clojure (JVM) compiler."
   [^System.IO.FileSystemInfo file]                                         ;;; java.io.File
   (file-with-extension? file clojure-clr-extensions))
+
+;; Dealing with FileInfo.Equals is reference-based, not structuaral  -- via Brandon Correa
+
+(defn- files= [file-1 file-2]
+  (= (.-FullName file-1) (.-FullName file-2)))
+
+(defn some-file [coll file]
+  (reduce #(when (files= file %2) (reduced %2)) nil coll))
 
+(defn into-files [files others]
+  (into files (remove #(some-file files %) others)))
+
+(defn- dissoc-files [m files]
+  (when m
+    (select-keys m (remove #(some-file files %) (keys m)))))
+
+(defn- get-file [filemap file]
+  (reduce #(when (files= file (first %2)) (reduced (second %2))) nil filemap))
+
+(defn- files->symbols [tracker files]
+  (let [filemap (::filemap tracker {})]
+    (keep #(get-file filemap %) files)))
+
+(defn- merge-file-map [m other]
+  (merge (dissoc-files m (keys other)) other))
+
+(defn- distinct-files [files]
+  (reduce #(-> (disj %1 (some-file %1 %2))
+               (conj %2)) #{} files))  
+
+;; 
+ 
 ;;; Dependency tracker
 
 (defn- files-and-deps [files read-opts]
@@ -79,9 +110,10 @@
                     (assoc-in [:depmap name] deps)
                     (assoc-in [:filemap file] name)))
               m))
-          {} files))
-
-(def ^:private merge-map (fnil merge {}))
+          {} (distinct-files files)))                                      ;;; files
+ 
+ 
+(def ^:private merge-map (fnil merge-file-map {}))                         ;;;  (fnil merge {})
 
 (defn add-files
   "Reads ns declarations from files; returns an updated dependency
@@ -93,21 +125,20 @@
    (let [{:keys [depmap filemap]} (files-and-deps files read-opts)]
      (-> tracker
          (track/add depmap)
-         (update-in [::filemap] merge-map filemap)))))
+         (update ::filemap merge-map filemap)))))                          ;;; (update-in [::filemap] merge-map filemap)
 
 (defn remove-files
   "Returns an updated dependency tracker with files removed. The files
   must have been previously added with add-files."
   [tracker files]
   (-> tracker
-      (track/remove (keep (::filemap tracker {}) files))
-      (update-in [::filemap] #(apply dissoc % files))))
+      (track/remove (files->symbols tracker files))                        ;;; (track/remove (keep (::filemap tracker {}) files))
+      (update ::filemap dissoc-files files)))                              ;;; (update-in [::filemap] #(apply dissoc % files))
 
 ;;;  Added
 
 (defn is-file? [^System.IO.FileSystemInfo file]
   (not= (enum-and (.Attributes file) System.IO.FileAttributes/Directory) System.IO.FileAttributes/Directory))
-  
+
 (defn is-directory? [^System.IO.FileSystemInfo file]
   (= (enum-and (.Attributes file) System.IO.FileAttributes/Directory) System.IO.FileAttributes/Directory))  
-  

src/test/clojure/clojure/tools/namespace/file_test.clj

@@ -0,0 +1,80 @@
+(ns clojure.tools.namespace.file-test
+    (:require [clojure.test :refer [deftest is]]
+      [clojure.tools.namespace.dependency :as dep]
+      [clojure.tools.namespace.file :as file]
+      [clojure.tools.namespace.test-helpers :as help]
+      [clojure.tools.namespace.track :as track])
+    (:import (System.IO FileInfo)))
+	
+;; Tests compliments of Brandon Correa
+
+(deftest t-add-no-files
+  (let [tracker (file/add-files (track/tracker) nil)]
+    (is (= (dep/->MapDependencyGraph {} {}) (::track/deps tracker)))
+    (is (= {} (::file/filemap tracker)))
+    (is (= '() (::track/unload tracker)))
+    (is (= '() (::track/load tracker)))))
+
+(deftest t-add-one-file
+  (let [dir     (help/create-temp-dir "t-add-one-file")
+        one-clj (help/create-source dir 'example.one :clj)
+        tracker (file/add-files (track/tracker) [one-clj])]
+    (is (= (dep/->MapDependencyGraph {} {}) (::track/deps tracker)))
+    (is (= {one-clj 'example.one} (::file/filemap tracker)))
+    (is (= (list 'example.one) (::track/unload tracker)))
+    (is (= (list 'example.one) (::track/load tracker)))))
+
+(deftest t-add-file-with-dependency
+  (let [dir      (help/create-temp-dir "t-add-file-with-dependency")
+        main-clj (help/create-source dir 'example.main :clj '[example.one])
+        tracker  (file/add-files (track/tracker) [main-clj])]
+    (is (= {'example.main #{'example.one}} (:dependencies (::track/deps tracker))))
+    (is (= {'example.one #{'example.main}} (:dependents (::track/deps tracker))))
+    (is (= {main-clj 'example.main} (::file/filemap tracker)))
+    (is (= (list 'example.main) (::track/unload tracker)))
+    (is (= (list 'example.main) (::track/load tracker)))))
+
+(deftest t-add-file-that-already-exists
+  (let [dir        (help/create-temp-dir "t-add-file-that-already-exists")
+        file-ref-1 (help/create-source dir 'example.main :clj)
+        file-ref-2 (FileInfo. (.-FullName file-ref-1))
+        tracker    (-> (track/tracker)
+                       (file/add-files [file-ref-1])
+                       (file/add-files [file-ref-2]))]
+       (is (= {} (:dependencies (::track/deps tracker))))
+       (is (= {} (:dependents (::track/deps tracker))))
+       (is (= {file-ref-2 'example.main} (::file/filemap tracker)))
+       (is (= (list 'example.main) (::track/unload tracker)))
+       (is (= (list 'example.main) (::track/load tracker)))))
+
+(deftest t-add-file-that-already-exists-in-the-same-call
+  (let [dir        (help/create-temp-dir "t-add-file-that-already-exists-in-the-same-call")
+        file-ref-1 (help/create-source dir 'example.main :clj)
+        file-ref-2 (FileInfo. (.-FullName file-ref-1))
+        tracker    (-> (track/tracker)
+                       (file/add-files [file-ref-1 file-ref-2]))]
+    (is (= {} (:dependencies (::track/deps tracker))))
+    (is (= {} (:dependents (::track/deps tracker))))
+    (is (= {file-ref-2 'example.main} (::file/filemap tracker)))
+    (is (= (list 'example.main) (::track/unload tracker)))
+    (is (= (list 'example.main) (::track/load tracker)))))
+
+(deftest t-remove-no-files-from-empty-tracker
+  (let [tracker (file/remove-files {} nil)]
+    (is (= (dep/->MapDependencyGraph {} {}) (::track/deps tracker)))
+    (is (nil? (::file/filemap tracker)))
+    (is (= '() (::track/unload tracker)))
+    (is (= '() (::track/load tracker)))))
+
+(deftest t-remove-file-with-dependency-from-filemap
+  (let [dir        (help/create-temp-dir "t-remove-file-with-dependency-from-filemap")
+        file-ref-1 (help/create-source dir 'example.main :clj '[example.one])
+        file-ref-2 (FileInfo. (.-FullName file-ref-1))
+        tracker    (-> (track/tracker)
+                       (file/add-files [file-ref-1])
+                       (file/remove-files [file-ref-2]))]
+       (is (= {} (:dependencies (::track/deps tracker))))
+       (is (= {'example.one #{}} (:dependents (::track/deps tracker))))
+       (is (= {} (::file/filemap tracker)))
+       (is (= (list 'example.main) (::track/unload tracker)))
+       (is (= (list) (::track/load tracker)))))

src/test/clojure/clojure/tools/namespace/repl_test.clj

@@ -0,0 +1,63 @@
+(ns clojure.tools.namespace.repl-test
+    (:require [clojure.test :refer [deftest is use-fixtures]]
+              [clojure.tools.namespace.dir :as dir]
+              [clojure.tools.namespace.find :as find]
+              [clojure.tools.namespace.repl :as repl]
+              [clojure.tools.namespace.test-helpers :as help]))
+			
+;; Tests contributed by Brandon Correa
+
+(defn reset-repl! []
+  (repl/clear)
+  (repl/set-refresh-dirs))
+  
+(defn reset-repl-fixture [test-fn]
+  (reset-repl!)
+  (test-fn)
+  (reset-repl!))
+  
+(use-fixtures :each reset-repl-fixture)
+			
+(defn current-time-millis []
+  (long
+    (/ (- (.-Ticks DateTime/UtcNow)
+          (.-Ticks DateTime/UnixEpoch))
+       TimeSpan/TicksPerMillisecond)))
+	   
+(deftest t-repl-scan-time-component
+  (let [before (current-time-millis)
+        scan   (repl/scan {:platform find/clj})
+        after  (current-time-millis)
+        time   (::dir/time scan)]
+    (is (<= before time after))
+    (is (integer? (::dir/time scan)))))
+	
+(deftest t-repl-scan-twice
+  (let [dir       (help/create-temp-dir "t-repl-scan")
+        other-dir (help/create-temp-dir "t-repl-scan-other")
+        main-clj  (help/create-source dir 'example.main :clj '[example.one])
+        one-cljc  (help/create-source dir 'example.one :clj)
+        _         (repl/set-refresh-dirs dir other-dir)
+        scan-1    (repl/scan {:platform find/clj})
+        scan-2    (repl/scan {:platform find/clj})
+        paths-1   (map str (::dir/files scan-1))
+        paths-2   (map str (::dir/files scan-2))
+        paths     (set paths-1)]
+       (is (= 2 (count paths-1)))
+       (is (= paths-1 paths-2))
+       (is (contains? paths (str main-clj)))
+       (is (contains? paths (str one-cljc)))))
+
+(deftest t-repl-scan-after-file-modified
+  (let [dir       (help/create-temp-dir "t-repl-scan-after-file-modified")
+        main-clj  (help/create-source dir 'example.main :clj)
+        _         (repl/set-refresh-dirs dir)
+        scan-1    (repl/scan {:platform find/clj})
+        _         (System.IO.File/SetLastWriteTimeUtc (.-FullName main-clj) DateTime/UtcNow)
+        scan-2    (repl/scan {:platform find/clj})
+        paths-1   (map str (::dir/files scan-1))
+        paths-2   (map str (::dir/files scan-2))
+        paths     (set paths-1)]
+    (is (= 1 (count paths-1)))
+    (is (= paths-1 paths-2))
+    (is (contains? paths (str main-clj)))))