Merge pull request #3 from metabase/fix-reflection-warnings

Fix reflection warnings; add reflection linter
metabase · Jul 8, 2019 · a4e66d6 · a4e66d6
2 parents 7ded45d + 1dcba76
commit a4e66d6
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 30 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -27,12 +27,12 @@ commands:
       - attach_workspace:
           at: /home/circleci/
 
-  restore-be-deps-cache:
+  restore-deps-cache:
     steps:
       - restore_cache:
           keys:
-            - be-deps-{{ checksum "project.clj" }}
-            - be-deps-
+            - deps-{{ checksum "project.clj" }}
+            - deps-
 
 jobs:
 
@@ -54,14 +54,14 @@ jobs:
           paths:
             - metabase/connection-pool
 
-  be-deps:
+  deps:
     executor: default
     steps:
       - attach-workspace
-      - restore-be-deps-cache
+      - restore-deps-cache
       - run: lein deps
       - save_cache:
-          key: be-deps-{{ checksum "project.clj" }}
+          key: deps-{{ checksum "project.clj" }}
           paths:
             - /home/circleci/.m2
 
@@ -75,11 +75,21 @@ jobs:
     executor: << parameters.e >>
     steps:
       - attach-workspace
-      - restore-be-deps-cache
+      - restore-deps-cache
       - run:
           command: lein << parameters.lein-command >>
           no_output_timeout: 5m
 
+  linter-reflection-warnings:
+    executor: default
+    steps:
+      - attach-workspace
+      - restore-deps-cache
+      - run:
+          name: Run reflection warnings checker
+          command: ./bin/reflection-linter
+          no_output_timeout: 2m
+
   deploy:
     executor: default
     steps:
@@ -99,55 +109,60 @@ workflows:
     jobs:
       - checkout
 
-      - be-deps:
+      - deps:
           requires:
             - checkout
 
       - lein:
-          name: be-tests
+          name: tests
           requires:
-            - be-deps
+            - deps
           lein-command: test
 
       - lein:
-          name: be-tests-java-11
+          name: tests-java-11
           requires:
-            - be-deps
+            - deps
           e: java-11
           lein-command: test
 
       - lein:
-          name: be-linter-eastwood
+          name: linter-eastwood
           requires:
-            - be-deps
+            - deps
           lein-command: eastwood
 
       - lein:
-          name: be-linter-docstring-checker
+          name: linter-docstring-checker
           requires:
-            - be-deps
+            - deps
           lein-command: docstring-checker
 
       - lein:
-          name: be-linter-namespace-decls
+          name: linter-namespace-decls
           requires:
-            - be-deps
+            - deps
           lein-command: check-namespace-decls
 
       - lein:
-          name: be-linter-bikeshed
+          name: linter-bikeshed
           requires:
-            - be-deps
+            - deps
           lein-command: bikeshed
 
+      - linter-reflection-warnings:
+          requires:
+            - deps
+
       - deploy:
           requires:
-            - be-linter-bikeshed
-            - be-linter-docstring-checker
-            - be-linter-eastwood
-            - be-linter-namespace-decls
-            - be-tests
-            - be-tests-java-11
+            - linter-bikeshed
+            - linter-docstring-checker
+            - linter-eastwood
+            - linter-namespace-decls
+            - linter-reflection-warnings
+            - tests
+            - tests-java-11
           filters:
             branches:
               only: master
diff --git a/bin/reflection-linter b/bin/reflection-linter
@@ -0,0 +1,13 @@
+#! /usr/bin/env bash
+
+printf "\e[1;34mChecking for reflection warnings. This may take a few minutes, so sit tight...\e[0m\n"
+
+warnings=`lein check 2>&1 | grep Reflection | grep metabase | sort | uniq`
+
+if [ ! -z "$warnings" ]; then
+    printf "\e[1;31mYour code has introduced some reflection warnings.\e[0m 😞\n"
+    echo "$warnings";
+    exit -1;
+fi
+
+printf "\e[1;32mNo reflection warnings! Success.\e[0m\n"
diff --git a/project.clj b/project.clj
@@ -1,4 +1,4 @@
-(defproject metabase/connection-pool "1.0.2-SNAPSHOT"
+(defproject metabase/connection-pool "1.0.2"
   :description "Connection pools for JDBC databases. Simple wrapper around C3P0."
   :url "https://github.com/metabase/connection-pool"
   :min-lein-version "2.5.0"
@@ -11,7 +11,6 @@
    "bikeshed"                  ["with-profile" "+bikeshed" "bikeshed" "--max-line-length" "120"]
    "check-namespace-decls"     ["with-profile" "+check-namespace-decls" "check-namespace-decls"]
    "eastwood"                  ["with-profile" "+eastwood" "eastwood"]
-   "check-reflection-warnings" ["with-profile" "+reflection-warnings" "check"]
    "docstring-checker"         ["with-profile" "+docstring-checker" "docstring-checker"]
    ;; `lein lint` will run all linters
    "lint"                      ["do" ["eastwood"] ["bikeshed"] ["check-namespace-decls"] ["docstring-checker"]]}
@@ -37,7 +36,7 @@
 
    :eastwood
    {:plugins
-    [[jonase/eastwood "0.3.5" :exclusions [org.clojure/clojure]]]
+    [[jonase/eastwood "0.3.6" :exclusions [org.clojure/clojure]]]
 
     :add-linters
     [:unused-private-vars

diff --git a/src/metabase/connection_pool.clj b/src/metabase/connection_pool.clj
@@ -47,7 +47,7 @@
   (map->properties (dissoc spec :classname :subprotocol :subname)))
 
 (defn ^:private unpooled-data-source
-  [{:keys [subname subprotocol], :as spec}]
+  ^DataSource [{:keys [subname subprotocol], :as spec}]
   (proxy-data-source (format "jdbc:%s:%s" subprotocol subname) (spec->properties spec)))
 
 (defn- pooled-data-source ^DataSource