Fix linters

bin/reflection-linter

 #!/usr/bin/env bash
 
-warnings=`lein check 2>&1 | grep Reflection | grep metabase`
+echo -e "\e[1;34mChecking for reflection warnings. This may take a few minutes, so sit tight...\e[0m"
+
+warnings=`lein check-reflection-warnings 2>&1 | grep Reflection | grep metabase | uniq`
 
 if [ ! -z "$warnings" ]; then
     echo -e "\e[1;31mYour code has cased introduced some reflection warnings.\e[0m 😞"

circle.yml

@@ -26,10 +26,10 @@ test:
     # 1) runs unit tests w/ Postgres local DB. Runs against H2, SQL Server
     # 2) runs unit tests w/ MySQL local DB. Runs against H2, Postgres, SQLite
     # 3) runs unit tests w/ H2 local DB. Runs against H2, Redshift, Druid
-    # 4) runs docstring-checker linter && Eastwood linter & Bikeshed linter && ./bin/reflection-linter
+    # 4) runs Eastwood linter, Bikeshed linter, docstring-checker & ./bin/reflection-linter
     # 5) runs JS linter + JS test
     # 6) runs lein uberjar. (We don't run bin/build because we're not really concerned about `npm install` (etc) in this test, which runs elsewhere)
-    - case $CIRCLE_NODE_INDEX in 0) ENGINES=h2,mongo,mysql,bigquery lein test ;; 1) ENGINES=h2,sqlserver MB_DB_TYPE=postgres MB_DB_DBNAME=circle_test MB_DB_PORT=5432 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 2) ENGINES=h2,postgres,sqlite MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 3) ENGINES=h2,redshift,druid lein test ;; 4) lein eastwood 2>&1 | grep -v Reflection && lein bikeshed 2>&1 | grep -v Reflection && ./bin/reflection-linter && lein docstring-checker ;; 5) npm install && npm run lint && npm run build && npm run test ;; 6) lein uberjar ;; esac:
+    - case $CIRCLE_NODE_INDEX in 0) ENGINES=h2,mongo,mysql,bigquery lein test ;; 1) ENGINES=h2,sqlserver MB_DB_TYPE=postgres MB_DB_DBNAME=circle_test MB_DB_PORT=5432 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 2) ENGINES=h2,postgres,sqlite MB_DB_TYPE=mysql MB_DB_DBNAME=circle_test MB_DB_PORT=3306 MB_DB_USER=ubuntu MB_DB_HOST=localhost lein test ;; 3) ENGINES=h2,redshift,druid lein test ;; 4) lein eastwood && lein bikeshed && lein docstring-checker && ./bin/reflection-linter ;; 5) npm install && npm run lint && npm run build && npm run test ;; 6) lein uberjar ;; esac:
         parallel: true
 deployment:
   master:

docs/developers-guide.md

@@ -142,7 +142,7 @@ when testing since they are impossible to run locally (such as Redshift and Bigq
 
 Run the linters:
 
-    lein eastwood && lein bikeshed
+    lein eastwood && lein bikeshed && lein docstring-checker && ./bin/reflection-linter
 
 
 #### Developing with Emacs
@@ -159,13 +159,6 @@ You'll probably want to tell Emacs to store customizations in a different file.
   (load-file custom-file))
 ```
 
-#### Checking for Out-of-Date Dependencies
-
-    lein ancient                   # list all out-of-date dependencies
-    lein ancient latest lein-ring  # list latest version of artifact lein-ring
-
-Will give you a list of out-of-date dependencies.
-
 ## Documentation
 
 #### Instant Cheatsheet

project.clj

@@ -5,7 +5,8 @@
   :description "Metabase Community Edition"
   :url "http://metabase.com/"
   :min-lein-version "2.5.0"
-  :aliases {"bikeshed" ["with-profile" "+bikeshed" "bikeshed" "--max-line-length" "240"]
+  :aliases {"bikeshed" ["bikeshed" "--max-line-length" "240"]
+            "check-reflection-warnings" ["with-profile" "+reflection-warnings" "check"]
             "test" ["with-profile" "+expectations" "expectations"]
             "generate-sample-dataset" ["with-profile" "+generate-sample-dataset" "run"]}
   :dependencies [[org.clojure/clojure "1.8.0"]
@@ -76,7 +77,8 @@
   :ring {:handler metabase.core/app
          :init metabase.core/init!
          :destroy metabase.core/destroy}
-  :eastwood {:exclude-namespaces [:test-paths]
+  :eastwood {:exclude-namespaces [:test-paths
+                                  metabase.driver.generic-sql]        ; ISQLDriver causes Eastwood to fail. Skip this ns until issue is fixed: https://github.com/jonase/eastwood/issues/191
              :add-linters [:unused-private-vars]
              :exclude-linters [:constant-test                         ; korma macros generate some forms with if statements that are always logically true or false
                                :suspicious-expression                 ; core.match macros generate some forms like (and expr) which is "suspicious"
@@ -88,11 +90,9 @@
   :profiles {:dev {:dependencies [[org.clojure/tools.nrepl "0.2.12"]  ; REPL <3
                                   [expectations "2.1.3"]              ; unit tests
                                   [ring/ring-mock "0.3.0"]]
-                   :plugins [[docstring-checker "1.0.0"]              ; Check that all public vars have docstrings
+                   :plugins [[docstring-checker "1.0.0"]              ; Check that all public vars have docstrings. Run with 'lein docstring-checker'
                              [jonase/eastwood "0.2.3"
                               :exclusions [org.clojure/clojure]]      ; Linting
-                             [lein-ancient "0.6.8"                    ; Check project for outdated dependencies + plugins w/ 'lein ancient'
-                              :exclusions [org.clojure/clojure]]
                              [lein-bikeshed "0.3.0"]                  ; Linting
                              [lein-expectations "0.0.8"]              ; run unit tests with 'lein expectations'
                              [lein-instant-cheatsheet "2.2.1"         ; use awesome instant cheatsheet created by yours truly w/ 'lein instant-cheatsheet'
@@ -104,6 +104,7 @@
                               "-Xmx2048m"                             ; hard limit of 2GB so we stop hitting the 4GB container limit on CircleCI
                               "-XX:+CMSClassUnloadingEnabled"         ; let Clojure's dynamically generated temporary classes be GC'ed from PermGen
                               "-XX:+UseConcMarkSweepGC"]}             ; Concurrent Mark Sweep GC needs to be used for Class Unloading (above)
+             :reflection-warnings {:global-vars {*warn-on-reflection* true}} ; run `lein check-reflection-warnings` to check for reflection warnings
              :expectations {:injections [(require 'metabase.test-setup)]
                             :resource-paths ["test_resources"]
                             :env {:mb-test-setting-1 "ABCDEFG"

src/metabase/driver/druid/query_processor.clj

@@ -97,7 +97,7 @@
                 :fnAggregate "function(current, x) { return current + (parseFloat(x) || 0); }"
                 :fnCombine   "function(x, y) { return x + y; }"}))
 
-(defn- ag:filtered  [filter aggregator] {:type :filtered, :filter filter, :aggregator aggregator})
+(defn- ag:filtered  [filtr aggregator] {:type :filtered, :filter filtr, :aggregator aggregator})
 
 (defn- ag:count
   ([output-name]       {:type :count, :name output-name})
@@ -208,11 +208,11 @@
 
 ;;; ### handle-filter
 
-(defn- filter:not [filter]
-  {:pre [filter]}
-  (if (= (:type filter) :not)     ; it looks like "two nots don't make an identity" with druid
-    (:field filter)
-    {:type :not, :field filter}))
+(defn- filter:not [filtr]
+  {:pre [filtr]}
+  (if (= (:type filtr) :not)     ; it looks like "two nots don't make an identity" with druid
+    (:field filtr)
+    {:type :not, :field filtr}))
 
 (defn- filter:= [field value]
   {:type      :selector
@@ -282,8 +282,13 @@
     nil  (parse-filter-subclause:filter clause)))
 
 
-(defn- make-intervals [min max & more]
-  (vec (concat [(str (or (->rvalue min) -5000) "/" (or (->rvalue max) 5000))]
+(defn- make-intervals
+  "Make a value for the `:intervals` in a Druid query.
+
+     ;; Return results in 2012 or 2015
+     (make-intervals 2012 2013 2015 2016) -> [\"2012/2013\" \"2015/2016\"]"
+  [interval-min interval-max & more]
+  (vec (concat [(str (or (->rvalue interval-min) -5000) "/" (or (->rvalue interval-max) 5000))]
                (when (seq more)
                  (apply make-intervals more)))))
 

src/metabase/driver/generic_sql/query_processor.clj

@@ -122,7 +122,9 @@
                                       :when (not (contains? (set fields-fields) field))]
                                   (as (formatted field) field)))))
 
-(defn- apply-fields [_ korma-form {fields :fields}]
+(defn apply-fields
+  "Apply a `fields` clause to KORMA-FORM. Default implementation of `apply-fields` for SQL drivers."
+  [_ korma-form {fields :fields}]
   (apply k/fields korma-form (for [field fields]
                                 (as (formatted field) field))))
 
@@ -150,10 +152,14 @@
     :not (kfns/pred-not (kengine/pred-map (filter-subclause->predicate subclause)))
     nil  (filter-subclause->predicate clause)))
 
-(defn- apply-filter [_ korma-form {clause :filter}]
+(defn apply-filter
+  "Apply a `filter` clause to KORMA-FORM. Default implementation of `apply-filter` for SQL drivers."
+  [_ korma-form {clause :filter}]
   (k/where korma-form (filter-clause->predicate clause)))
 
-(defn- apply-join-tables [_ korma-form {join-tables :join-tables, {source-table-name :name, source-schema :schema} :source-table}]
+(defn apply-join-tables
+  "Apply expanded query `join-tables` clause to KORMA-FORM. Default implementation of `apply-join-tables` for SQL drivers."
+  [_ korma-form {join-tables :join-tables, {source-table-name :name, source-schema :schema} :source-table}]
   (loop [korma-form korma-form, [{:keys [table-name pk-field source-field schema]} & more] join-tables]
     (let [table-name        (if (seq schema)
                               (str schema \. table-name)
@@ -168,10 +174,14 @@
         (recur korma-form more)
         korma-form))))
 
-(defn- apply-limit [_ korma-form {value :limit}]
+(defn apply-limit
+  "Apply `limit` clause to KORMA-FORM. Default implementation of `apply-limit` for SQL drivers."
+  [_ korma-form {value :limit}]
   (k/limit korma-form value))
 
-(defn- apply-order-by [_ korma-form {subclauses :order-by}]
+(defn apply-order-by
+  "Apply `order-by` clause to KORMA-FORM. Default implementation of `apply-order-by` for SQL drivers."
+  [_ korma-form {subclauses :order-by}]
   (loop [korma-form korma-form, [{:keys [field direction]} & more] subclauses]
     (let [korma-form (k/order korma-form (formatted field) (case direction
                                                                :ascending  :ASC
@@ -180,7 +190,9 @@
         (recur korma-form more)
         korma-form))))
 
-(defn- apply-page [_ korma-form {{:keys [items page]} :page}]
+(defn apply-page
+  "Apply `page` clause to KORMA-FORM. Default implementation of `apply-page` for SQL drivers."
+  [_ korma-form {{:keys [items page]} :page}]
   (-> korma-form
       (k/limit items)
       (k/offset (* items (dec page)))))

src/metabase/events/activity_feed.clj

@@ -149,5 +149,7 @@
 ;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
 
 
-(defn- events-init []
+(defn events-init
+  "Automatically called during startup; start the events listener for the activity feed."
+  []
   (events/start-event-listener activity-feed-topics activity-feed-channel process-activity-event))

src/metabase/events/dependencies.clj

@@ -7,7 +7,7 @@
                              [metric :refer [Metric]])))
 
 
-(def ^:const dependencies-topics
+(def ^:private ^:const dependencies-topics
   "The `Set` of event topics which are subscribed to for use in dependencies tracking."
   #{:card-create
     :card-update
@@ -47,5 +47,7 @@
 ;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
 
 
-(defn- events-init []
+(defn events-init
+  "Automatically called during startup; start the events listener for dependencies topics."
+  []
   (events/start-event-listener dependencies-topics dependencies-channel process-dependencies-event))

src/metabase/events/last_login.clj

@@ -37,5 +37,7 @@
 ;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
 
 
-(defn- events-init []
+(defn events-init
+  "Automatically called during startup; start the events listener for last login events."
+  []
   (events/start-event-listener last-login-topics last-login-channel process-last-login-event))

src/metabase/events/notifications.clj

@@ -92,5 +92,7 @@
 ;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
 
 
-(defn- events-init []
+(defn events-init
+  "Automatically called during startup; start event listener for notifications events."
+  []
   (events/start-event-listener notifications-topics notifications-channel process-notifications-event))

src/metabase/events/revision.clj

@@ -77,5 +77,7 @@
 ;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
 
 
-(defn- events-init []
+(defn events-init
+  "Automatically called during startup; start event listener for revision events."
+  []
   (events/start-event-listener revisions-topics revisions-channel process-revision-event))

src/metabase/events/sync_database.clj

@@ -40,5 +40,7 @@
 ;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
 
 
-(defn- events-init []
+(defn events-init
+  "Automatically called during startup; start event listener for database sync events."
+  []
   (events/start-event-listener sync-database-topics sync-database-channel process-sync-database-event))

src/metabase/events/view_log.clj

@@ -6,7 +6,7 @@
             [metabase.models.view-log :refer [ViewLog]]))
 
 
-(def ^:const view-counts-topics
+(def ^:private ^:const view-counts-topics
   "The `Set` of event topics which we subscribe to for view counting."
   #{:card-create
     :card-read
@@ -46,5 +46,7 @@
 ;;; ## ---------------------------------------- LIFECYLE ----------------------------------------
 
 
-(defn- events-init []
+(defn events-init
+  "Automatically called during startup; start the events listener for view events."
+  []
   (events/start-event-listener view-counts-topics view-counts-channel process-view-count-event))

src/metabase/task/send_pulses.clj

@@ -64,7 +64,9 @@
         curr-monthweek     (monthweek now)]
     (send-pulses! curr-hour curr-weekday curr-monthday curr-monthweek)))
 
-(defn- task-init []
+(defn task-init
+  "Automatically called during startup; start the job for sending pulses."
+  []
   ;; build our job
   (reset! send-pulses-job (jobs/build
                                (jobs/of-type SendPulses)

src/metabase/task/sync_databases.clj

@@ -26,7 +26,9 @@
         (catch Throwable e
           (log/error "Error syncing database: " (:id database) e))))))
 
-(defn- task-init []
+(defn task-init
+  "Automatically called during startup; start the job for syncing databases."
+  []
   ;; build our job
   (reset! sync-databases-job (jobs/build
                                (jobs/of-type SyncDatabases)