From ee9f390761b61d2adb1027897f3505c9c591360c Mon Sep 17 00:00:00 2001 From: Gabriel Horner Date: Thu, 9 Jun 2022 15:13:20 -0400 Subject: [PATCH] Localize bb-tasks for deps Deps need to be independent to not introduce testing bugs. These bb tasks are good candidates to move into bb-tasks gitlib --- .github/workflows/build.yml | 4 +-- .github/workflows/db.yml | 10 +++--- .github/workflows/graph-parser.yml | 22 +++++++----- bb.edn | 25 +++++++++----- deps/db/README.md | 52 ++++++++++++++++++++++++++++ deps/db/bb.edn | 35 +++++++++++++++++++ deps/db/src/logseq/db/rules.cljc | 3 +- deps/graph-parser/bb.edn | 23 +++++++++++++ docs/dev-practices.md | 16 +++++---- scripts/carve.clj | 43 ------------------------ scripts/large_vars.clj | 54 ------------------------------ scripts/lint_rules.clj | 52 ---------------------------- scripts/src/logseq/tasks/dev.clj | 10 +++--- scripts/src/logseq/tasks/nbb.clj | 25 -------------- 14 files changed, 162 insertions(+), 212 deletions(-) create mode 100644 deps/db/README.md create mode 100644 deps/db/bb.edn create mode 100644 deps/graph-parser/bb.edn delete mode 100755 scripts/carve.clj delete mode 100755 scripts/large_vars.clj delete mode 100755 scripts/lint_rules.clj delete mode 100644 scripts/src/logseq/tasks/nbb.clj diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9e8ed48ae..a876a0c07 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -101,10 +101,10 @@ jobs: run: clojure -M:clj-kondo --parallel --lint src - name: Carve lint for unused vars - run: scripts/carve.clj + run: bb lint:carve - name: Lint for vars that are too large - run: scripts/large_vars.clj + run: bb lint:large-vars - name: Lint invalid translation entries run: bb lang:invalid-translations diff --git a/.github/workflows/db.yml b/.github/workflows/db.yml index 9d7c51873..9ace7e37c 100644 --- a/.github/workflows/db.yml +++ b/.github/workflows/db.yml @@ -24,7 +24,7 @@ env: JAVA_VERSION: '8' # This is the latest node version we can run. NODE_VERSION: '16' - BABASHKA_VERSION: '0.8.2' + BABASHKA_VERSION: '0.8.156' jobs: test: @@ -58,7 +58,7 @@ jobs: # In this job because it depends on an npm package - name: Load namespaces into nbb-logseq - run: cd ../.. && bb test:load-all-namespaces-with-nbb deps/db + run: bb test:load-all-namespaces-with-nbb . lint: runs-on: ubuntu-latest @@ -83,10 +83,10 @@ jobs: run: clojure -M:clj-kondo --parallel --lint src - name: Carve lint for unused vars - run: ../../scripts/carve.clj + run: bb lint:carve - name: Lint for vars that are too large - run: ../../scripts/large_vars.clj src '{:max-lines-count 30}' + run: bb lint:large-vars - name: Lint datalog rules - run: ../../scripts/lint_rules.clj + run: bb lint:rules diff --git a/.github/workflows/graph-parser.yml b/.github/workflows/graph-parser.yml index b81a3fdbe..20c3df6f8 100644 --- a/.github/workflows/graph-parser.yml +++ b/.github/workflows/graph-parser.yml @@ -17,6 +17,10 @@ on: - 'deps/db/**' - '!deps/graph-parser/**.md' +defaults: + run: + working-directory: deps/graph-parser + env: CLOJURE_VERSION: '1.10.1.727' # setup-java@v2 dropped support for legacy Java version syntax. @@ -24,7 +28,7 @@ env: JAVA_VERSION: '8' # This is the latest node version we can run. NODE_VERSION: '16' - BABASHKA_VERSION: '0.8.2' + BABASHKA_VERSION: '0.8.156' jobs: test: @@ -69,20 +73,20 @@ jobs: - name: Fetch Clojure deps if: steps.clojure-deps.outputs.cache-hit != 'true' - run: cd deps/graph-parser && clojure -A:test -P + run: clojure -A:test -P - name: Fetch yarn deps - run: cd deps/graph-parser && yarn install --frozen-lockfile + run: yarn install --frozen-lockfile - name: Run ClojureScript tests - run: cd deps/graph-parser && clojure -M:test + run: clojure -M:test - name: Run nbb-logseq tests - run: cd deps/graph-parser && yarn nbb-logseq -cp src:test:../db/src -m logseq.graph-parser.nbb-test-runner/run-tests + run: yarn nbb-logseq -cp src:test:../db/src -m logseq.graph-parser.nbb-test-runner/run-tests # In this job because it depends on an npm package - name: Load namespaces into nbb-logseq - run: bb test:load-all-namespaces-with-nbb deps/graph-parser + run: bb test:load-all-namespaces-with-nbb . lint: runs-on: ubuntu-latest @@ -108,10 +112,10 @@ jobs: babashka-version: ${{ env.BABASHKA_VERSION }} - name: Run clj-kondo lint - run: cd deps/graph-parser && clojure -M:clj-kondo --parallel --lint src test + run: clojure -M:clj-kondo --parallel --lint src test - name: Carve lint for unused vars - run: cd deps/graph-parser && ../../scripts/carve.clj + run: bb lint:carve - name: Lint for vars that are too large - run: scripts/large_vars.clj deps/graph-parser/src '{:max-lines-count 75}' + run: bb lint:large-vars diff --git a/bb.edn b/bb.edn index 1b2cdee83..5bc432d99 100644 --- a/bb.edn +++ b/bb.edn @@ -6,7 +6,7 @@ logseq/bb-tasks #_{:local/root "../bb-tasks"} {:git/url "https://github.com/logseq/bb-tasks" - :git/sha "4b3e623fb475cacb992425aa9dac770d6dd63e38"} + :git/sha "abb32ccd26405d56fd28a29d56f3cb902b8c4334"} logseq/graph-parser {:local/root "deps/graph-parser"}} :pods @@ -33,18 +33,18 @@ dev:lint logseq.tasks.dev/lint + lint:large-vars + logseq.bb-tasks.lint.large-vars/-main + + lint:carve + logseq.bb-tasks.lint.carve/-main + nbb:watch logseq.bb-tasks.nbb.watch/watch nbb:portal-watch logseq.bb-tasks.nbb.watch/portal-watch - test:load-namespaces-with-nbb - logseq.tasks.nbb/load-compatible-namespaces - - test:load-all-namespaces-with-nbb - logseq.tasks.nbb/load-all-namespaces - lang:list logseq.tasks.lang/list-langs @@ -58,4 +58,13 @@ logseq.tasks.lang/invalid-translations file-sync:integration-tests - logseq.tasks.file-sync/integration-tests}} + logseq.tasks.file-sync/integration-tests} + + :tasks/config + {:large-vars + ;; TODO: Get to a smaller max-lines-count + {:max-lines-count 100 + ;; TODO: Address vars tagged with cleanup-todo. These + ;; are left mostly because they are not high priority + ;; or not well understood + :metadata-exceptions #{:large-vars/cleanup-todo}}}} diff --git a/deps/db/README.md b/deps/db/README.md new file mode 100644 index 000000000..27c6940aa --- /dev/null +++ b/deps/db/README.md @@ -0,0 +1,52 @@ +## Description + +This library provides a minimal API for using a +[datascript](https://github.com/tonsky/datascript) database from the Logseq app +and the CLI. This library is compatible with ClojureScript and with +[nbb-logseq](https://github.com/logseq/nbb-logseq) to respectively provide +frontend and commandline functionality. + +## API + +This library is under the parent namespace `logseq.db`. This library provides +two main namespaces, `logseq.db` and `logseq.db.rules`. + +## Usage + +See usage in `deps/graph-parser` and in the Logseq app. + +## Dev + +This follows the practices that [the Logseq frontend +follows](/docs/dev-practices.md). Most of the same linters are used, with +configurations that are specific to this library. See [this library's CI +file](/.github/workflows/db.yml) for linting examples. + +### Setup + +To run linters, you'll want to install yarn dependencies once: +``` +yarn install +``` + +This step is not needed if you're just running the application. + +## Linting + +### Datalog linting + +Our rules are linted through a script that also uses the datalog-parser. To run this linter: +``` +bb lint:rules +``` + + +### Managing dependencies + +The package.json dependencies are just for testing and should be updated if there is +new behavior to test. + +The deps.edn dependecies are used by both ClojureScript and nbb-logseq. Their +versions should be backwards compatible with each other with priority given to +the frontend. _No new dependency_ should be introduced to this library without +an understanding of the tradeoffs of adding this to nbb-logseq. diff --git a/deps/db/bb.edn b/deps/db/bb.edn new file mode 100644 index 000000000..e46d067a9 --- /dev/null +++ b/deps/db/bb.edn @@ -0,0 +1,35 @@ +{:paths ["src"] + :min-bb-version "0.8.156" + :deps + {logseq/bb-tasks + #_{:local/root "../../../bb-tasks"} + {:git/url "https://github.com/logseq/bb-tasks" + :git/sha "abb32ccd26405d56fd28a29d56f3cb902b8c4334"}} + + :pods + {clj-kondo/clj-kondo {:version "2022.02.09"}} + + :tasks + {test:load-all-namespaces-with-nbb + logseq.bb-tasks.nbb.test/load-all-namespaces + + lint:large-vars + logseq.bb-tasks.lint.large-vars/-main + + lint:carve + logseq.bb-tasks.lint.carve/-main + + lint:rules + {:requires ([logseq.bb-tasks.lint.datalog :as datalog] + [logseq.db.rules :as rules]) + :doc "Lint datalog rules for parsability and unbound variables" + :task (datalog/lint-rules + (into rules/rules + (-> rules/query-dsl-rules + ;; TODO: Update linter to handle false positive on ?str-val + (dissoc :property) + vals)))}} + + :tasks/config + {:large-vars + {:max-lines-count 30}}} diff --git a/deps/db/src/logseq/db/rules.cljc b/deps/db/src/logseq/db/rules.cljc index b4b21cce8..6cd78fdf5 100644 --- a/deps/db/src/logseq/db/rules.cljc +++ b/deps/db/src/logseq/db/rules.cljc @@ -1,4 +1,5 @@ -(ns ^:bb-compatible logseq.db.rules) +(ns ^:bb-compatible logseq.db.rules + "Datalog rules for use with logseq.db.schema") (def ^:large-vars/data-var rules ;; rule "parent" is optimized for child node -> parent node nesting queries diff --git a/deps/graph-parser/bb.edn b/deps/graph-parser/bb.edn new file mode 100644 index 000000000..25800eeb8 --- /dev/null +++ b/deps/graph-parser/bb.edn @@ -0,0 +1,23 @@ +{:min-bb-version "0.8.156" + :deps + {logseq/bb-tasks + #_{:local/root "../../../bb-tasks"} + {:git/url "https://github.com/logseq/bb-tasks" + :git/sha "abb32ccd26405d56fd28a29d56f3cb902b8c4334"}} + + :pods + {clj-kondo/clj-kondo {:version "2022.02.09"}} + + :tasks + {test:load-all-namespaces-with-nbb + logseq.bb-tasks.nbb.test/load-all-namespaces + + lint:large-vars + logseq.bb-tasks.lint.large-vars/-main + + lint:carve + logseq.bb-tasks.lint.carve/-main} + + :tasks/config + {:large-vars + {:max-lines-count 75}}} diff --git a/docs/dev-practices.md b/docs/dev-practices.md index 85e2ccc9d..764d47644 100644 --- a/docs/dev-practices.md +++ b/docs/dev-practices.md @@ -27,7 +27,7 @@ We use https://github.com/borkdude/carve to detect unused vars in our codebase. To run this linter: ``` -scripts/carve.clj +bb lint:carve ``` By default, the script runs in CI mode which prints unused vars if they are @@ -35,7 +35,7 @@ found. The script can be run in an interactive mode which prompts for keeping (ignoring) an unused var or removing it. Run this mode with: ``` -scripts/carve.clj '{:interactive true}' +bb lint:carve '{:interactive true}' ``` When a var is ignored, it is added to `.carve/ignore`. Please add a comment for @@ -46,17 +46,19 @@ why a var is ignored to help others understand why it's unused. Large vars have a lot of complexity and make it hard for the team to maintain and understand them. To run this linter: ``` -scripts/large_vars.clj +bb lint:large-vars ``` To configure the linter, see its `config` var. ### Datalog linting -We use [datascript](https://github.com/tonsky/datascript)'s datalog to power our modeling and querying layer. Since datalog is concise, it is easy to write something invalid. To avoid typos and other preventable mistakes, we lint our queries and rules. Our queries are linted through clj-kondo and [datalog-parser](https://github.com/lambdaforge/datalog-parser). clj-kondo will error if it detects an invalid query. Our rules are linted through a script that also uses the datalog-parser. To run this linter: -``` -scripts/lint_rules.clj -``` +We use [datascript](https://github.com/tonsky/datascript)'s datalog to power our +modeling and querying layer. Since datalog is concise, it is easy to write +something invalid. To avoid typos and other preventable mistakes, we lint our +queries and rules. Our queries are linted through clj-kondo and +[datalog-parser](https://github.com/lambdaforge/datalog-parser). clj-kondo will +error if it detects an invalid query. ## Testing diff --git a/scripts/carve.clj b/scripts/carve.clj deleted file mode 100755 index 142b6a790..000000000 --- a/scripts/carve.clj +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/env bb -;; This file is copied from -;; https://github.com/borkdude/carve/blob/df552797a198b6701fb2d92390fce7c59205ea77/carve.clj -;; and thus this file is under the same EPL license. -;; The script is modified to run latest clj-kondo and carve versions and to add -;; a more friendly commandline interface through -main - -(require '[babashka.pods :as pods]) - -(pods/load-pod 'clj-kondo/clj-kondo "2022.02.09") -(require '[pod.borkdude.clj-kondo :as clj-kondo]) -;; define clj-kondo.core ns which is used by carve -(intern (create-ns 'clj-kondo.core) 'run! clj-kondo/run!) - -(require '[babashka.deps :as deps]) -(deps/add-deps '{:deps {borkdude/carve ;; {:local/root "."} - {:git/url "https://github.com/borkdude/carve" - :git/sha "df552797a198b6701fb2d92390fce7c59205ea77"} - borkdude/spartan.spec {:git/url "https://github.com/borkdude/spartan.spec" - :sha "12947185b4f8b8ff8ee3bc0f19c98dbde54d4c90"}}}) - -(require '[spartan.spec]) ;; defines clojure.spec - -(with-out-str ;; silence warnings about spartan.spec + with-gen - (binding [*err* *out*] - (require '[carve.api :as carve]))) - -;; again to make clj-kondo happy -(require '[carve.main]) -(require '[clojure.edn :as edn]) - -(defn -main - "Wrapper around carve.main that defaults to .carve/config.edn and merges -in an optional string of options" - [args] - (let [default-opts (slurp ".carve/config.edn") - opts (if-let [more-opts (first args)] - (pr-str (merge (select-keys (edn/read-string default-opts) [:paths :api-namespaces]) - (edn/read-string more-opts))) - default-opts)] - (apply carve.main/-main ["--opts" opts]))) - -(-main *command-line-args*) diff --git a/scripts/large_vars.clj b/scripts/large_vars.clj deleted file mode 100755 index 32a63c292..000000000 --- a/scripts/large_vars.clj +++ /dev/null @@ -1,54 +0,0 @@ -#!/usr/bin/env bb - -(ns large-vars - "This script detects vars that are too large and that make it difficult for - the team to maintain and understand them." - (:require [babashka.pods :as pods] - [clojure.pprint :as pprint] - [clojure.edn :as edn] - [clojure.set :as set])) - -(pods/load-pod 'clj-kondo/clj-kondo "2022.02.09") -(require '[pod.borkdude.clj-kondo :as clj-kondo]) - -(def default-config - ;; TODO: Discuss with team and agree on lower number - {:max-lines-count 100 - ;; Vars with these metadata flags are allowed. Name should indicate the reason - ;; it is allowed - :metadata-exceptions #{::data-var - ;; TODO: Address vars tagged with cleanup-todo. These - ;; are left mostly because they are not high priority - ;; or not well understood - ::cleanup-todo}}) - -(defn -main - [args] - (let [paths [(or (first args) "src")] - config (or (some->> (second args) edn/read-string (merge default-config)) - default-config) - {{:keys [var-definitions]} :analysis} - (clj-kondo/run! - {:lint paths - :config {:output {:analysis {:var-definitions {:meta true - :lang :cljs}}}}}) - vars (->> var-definitions - (keep (fn [m] - (let [lines-count (inc (- (:end-row m) (:row m)))] - (when (and (> lines-count (:max-lines-count config)) - (empty? (set/intersection (set (keys (:meta m))) - (:metadata-exceptions config)))) - {:var (:name m) - :lines-count lines-count - :filename (:filename m)})))) - (sort-by :lines-count (fn [x y] (compare y x))))] - (if (seq vars) - (do - (println (format "\nThe following vars exceed the line count max of %s:" - (:max-lines-count config))) - (pprint/print-table vars) - (System/exit 1)) - (println "All vars are below the max size!")))) - -(when (= *file* (System/getProperty "babashka.file")) - (-main *command-line-args*)) diff --git a/scripts/lint_rules.clj b/scripts/lint_rules.clj deleted file mode 100755 index 26b0f58c9..000000000 --- a/scripts/lint_rules.clj +++ /dev/null @@ -1,52 +0,0 @@ -#!/usr/bin/env bb - -(require '[babashka.deps :as deps]) -(deps/add-deps '{:deps {me.tagaholic/dlint {:mvn/version "0.1.0"} - io.lambdaforge/datalog-parser {:mvn/version "0.1.11"}} - :paths ["src"]}) - -(ns lint-rules - "Lint datalog rules for parse-ability and unbound variables" - (:require [datalog.parser.impl :as parser-impl] - [dlint.core :as dlint] - [logseq.db.rules :as rules])) - -(defn- lint-unbound-rule [rule] - (->> (dlint/lint [rule]) - (keep - (fn [[k v]] - (when (seq v) - {:success false :name k :rule rule :unbound-vars v}))))) - -(defn- lint-rule [rule] - (try (parser-impl/parse-rule rule) - {:success true :rule rule} - (catch Exception e - {:success false :rule rule :error (.getMessage e)}))) - -(defn- collect-logseq-rules - "Collects logseq rules and prepares them for linting" - [] - (into rules/rules - (-> rules/query-dsl-rules - ;; TODO: Update linter to handle false positive on ?str-val - (dissoc :property) - vals))) - -(defn -main [rules] - (let [invalid-unbound-rules (->> rules - (mapcat lint-unbound-rule) - (remove :success)) - invalid-rules (->> rules - (map lint-rule) - (remove :success)) - lint-results (concat invalid-unbound-rules invalid-rules)] - (if (seq lint-results) - (do - (println (count lint-results) "rules failed to lint:") - (println lint-results) - (System/exit 1)) - (println (count rules) "datalog rules linted fine!")))) - -(when (= *file* (System/getProperty "babashka.file")) - (-main (collect-logseq-rules))) diff --git a/scripts/src/logseq/tasks/dev.clj b/scripts/src/logseq/tasks/dev.clj index 1ab1c9e98..787f6bb73 100644 --- a/scripts/src/logseq/tasks/dev.clj +++ b/scripts/src/logseq/tasks/dev.clj @@ -31,13 +31,11 @@ - clj-kondo lint - carve lint for unused vars - lint for vars that are too large - - lint invalid translation entries - - Lint datalog rules" + - lint invalid translation entries" [] (doseq [cmd ["clojure -M:clj-kondo --parallel --lint src --cache false" - "scripts/carve.clj" - "scripts/large_vars.clj" - "bb lang:invalid-translations" - "scripts/lint_rules.clj"]] + "bb lint:carve" + "bb lint:large-vars" + "bb lang:invalid-translations"]] (println cmd) (shell cmd))) diff --git a/scripts/src/logseq/tasks/nbb.clj b/scripts/src/logseq/tasks/nbb.clj deleted file mode 100644 index a05f4bc95..000000000 --- a/scripts/src/logseq/tasks/nbb.clj +++ /dev/null @@ -1,25 +0,0 @@ -(ns logseq.tasks.nbb - (:require [pod.borkdude.clj-kondo :as clj-kondo] - [clojure.string :as str] - [babashka.tasks :refer [shell]])) - -(defn- validate-namespaces - [namespaces classpath dir] - (assert (seq namespaces) "There must be some namespaces to check") - ;; distinct b/c sometimes namespaces are duplicated with .cljc analysis - (doseq [n (distinct namespaces)] - (println "Requiring" n "...") - ;; Run from current dir so that yarn command runs correctly - (shell {:dir dir} "yarn nbb-logseq -cp" classpath "-e" (format "(require '[%s])" n))) - (println "Success!")) - -(defn load-all-namespaces - "Check all namespaces in a directory can be required by nbb-logseq" - [dir] - (let [{{:keys [namespace-definitions]} :analysis} - (clj-kondo/run! - {:lint (map #(str dir "/" %) ["src"]) - :config {:output {:analysis {:namespace-definitions {:lang :cljs}}}}})] - (validate-namespaces (map :name namespace-definitions) - (str/trim (:out (shell {:dir dir :out :string} "clojure -Spath"))) - dir)))