Compare commits
18 Commits
python-run
...
test-creat
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4a4820f8cf | ||
|
|
dfd311f64f | ||
|
|
cf35abf7c8 | ||
|
|
f17cf1f627 | ||
|
|
1b6ef871ba | ||
|
|
2090fd5618 | ||
|
|
acaf29fec7 | ||
|
|
4b76bbf497 | ||
|
|
df328ac63b | ||
|
|
234a29ff6d | ||
|
|
7c54c58cf1 | ||
|
|
2796a9a0e6 | ||
|
|
69d5d70c5a | ||
|
|
668f81e5a4 | ||
|
|
5964d0e3ee | ||
|
|
10e8c852b3 | ||
|
|
1c440f8913 | ||
|
|
023da744c5 |
@@ -1,12 +0,0 @@
|
||||
Generate a CI failure report for PR $PR_NUM_OR_URL (or current branch if no argument given).
|
||||
|
||||
Run `./bin/mage ci-report $PR_NUM_OR_URL` to generate the report, then:
|
||||
|
||||
1. **Summarize the failures** - What tests failed and in which jobs?
|
||||
2. **Categorize** - Are these real failures, flaky tests, or infrastructure issues?
|
||||
3. **Recommend next steps**:
|
||||
- If flaky: suggest rerunning with `gh run rerun <run-id> --failed -R metabase/metabase`
|
||||
- If real: identify the root cause and suggest fixes
|
||||
- If infrastructure: note the issue type (timeout, OOM, network)
|
||||
|
||||
Focus on actionable insights. If all checks are passing, confirm that briefly.
|
||||
@@ -1 +0,0 @@
|
||||
Fix flakey test $ARGUMENTS
|
||||
@@ -1 +0,0 @@
|
||||
Fix issue $ARGUMENTS
|
||||
@@ -1 +0,0 @@
|
||||
Fetch unresolved comments for this branch's PR, then fix them
|
||||
@@ -1 +0,0 @@
|
||||
Run mutation testing for $ARGUMENTS
|
||||
@@ -1 +0,0 @@
|
||||
Repro issue $ARGUMENTS in a failing test
|
||||
@@ -1,79 +0,0 @@
|
||||
## Linting and Formatting
|
||||
|
||||
- **Lint PR:** `./bin/mage kondo-updated master` (or whatever target branch)
|
||||
- Call the command one time at the beginning, record the results, then work through the problems one at a time.
|
||||
- If the solution is obvious, then please apply the fix. Otherwise skip it.
|
||||
- If you fix all the issues (and verify by rerunning the kondo-updated command):
|
||||
- commit the change with a succinct and descriptive commit message
|
||||
- **Lint File:** `./bin/mage kondo <file or files>`
|
||||
- Use the linter as a way to know that you are adhering to conventions in place in the codebase
|
||||
- **Lint Changes:** `./bin/mage kondo-updated HEAD`
|
||||
- **Format:** `./bin/mage cljfmt-files [path]`
|
||||
|
||||
## Testing
|
||||
|
||||
Use `./bin/test-agent` to run backend tests. See the top-level CLAUDE.md for usage.
|
||||
|
||||
## Code Readability
|
||||
|
||||
- **Check Code Readability:** `./bin/mage -check-readable <file> [line-number]`
|
||||
- Run after every change to Clojure code
|
||||
- Check specific line first, then entire file if readable
|
||||
|
||||
## REPL Usage
|
||||
|
||||
> **Note:** If you have `clojure-mcp` tools available (check for tools like `clojure_eval`),
|
||||
> **always prefer those over `./bin/mage -repl`**. The MCP tools provide better integration,
|
||||
> richer feedback, and avoid shell escaping issues. Only use `./bin/mage -repl` as a fallback
|
||||
> when clojure-mcp is not available.
|
||||
|
||||
- **Evaluating Clojure Code:** `./bin/mage -repl '<code>'`
|
||||
- See "Sending Code to the REPL" section for more details
|
||||
|
||||
### Sending Code to the REPL
|
||||
|
||||
- Send code to the metabase process REPL using: `./bin/mage -repl '(+ 1 1)'` where `(+ 1 1)` is your Clojure code.
|
||||
- See `./bin/mage -repl -h` for more details.
|
||||
- If the Metabase backend is not running, you'll see an error message with instructions on how to start it.
|
||||
|
||||
#### Working with Files and Namespaces
|
||||
|
||||
1. **Load a file and call functions with fully qualified names**:
|
||||
|
||||
To call `your.namespace/your-function` on `arg1` and `arg2`:
|
||||
|
||||
```
|
||||
./bin/mage -repl --namespace your.namespace '(your-function arg1 arg2)'
|
||||
```
|
||||
|
||||
DO NOT use "require", "load-file" etc in the code string argument.
|
||||
|
||||
#### Understanding the Response
|
||||
|
||||
The `./bin/mage -repl` command returns three separate, independent outputs:
|
||||
|
||||
- `value`: The return value of the last expression (best for data structures)
|
||||
- `stdout`: Any printed output from `println` etc. (best for messages)
|
||||
- `stderr`: Any error messages (best for warnings and errors)
|
||||
|
||||
Example call:
|
||||
|
||||
```bash
|
||||
./bin/mage -repl '(println "Hello, world!") '\''({0 1, 1 3, 2 0, 3 2} {0 2, 1 0, 2 3, 3 1})'
|
||||
```
|
||||
|
||||
Example response:
|
||||
|
||||
```
|
||||
ns: user
|
||||
session: 32a35206-871c-4553-9bc9-f49491173d1c
|
||||
value: ({0 1, 1 3, 2 0, 3 2} {0 2, 1 0, 2 3, 3 1})
|
||||
stdout: Hello, world!
|
||||
stderr:
|
||||
```
|
||||
|
||||
For effective REPL usage:
|
||||
|
||||
- Return data structures as function return values
|
||||
- Use `println` for human-readable messages
|
||||
- Print errors to stderr
|
||||
@@ -1,153 +0,0 @@
|
||||
# Metabase Clojure Style Guide
|
||||
|
||||
This guide covers Clojure and ClojureScript coding conventions for Metabase. See also: `CLOJURE_STYLE_GUIDE.adoc` for the Community Clojure Style Guide.
|
||||
|
||||
## Naming Conventions
|
||||
|
||||
**General Naming:**
|
||||
|
||||
- Acceptable abbreviations: `acc`, `i`, `pred`, `coll`, `n`, `s`, `k`, `f`
|
||||
- Use `kebab-case` for all variables, functions, and constants
|
||||
|
||||
**Function Naming:**
|
||||
|
||||
- Pure functions should be nouns describing the value they return (e.g., `age` not `calculate-age` or `get-age`)
|
||||
- Functions with side effects must end with `!`
|
||||
- Don't repeat namespace alias in function names
|
||||
|
||||
**Destructuring:**
|
||||
|
||||
- Map destructuring should use kebab-case local bindings even if the map uses `snake_case` keys
|
||||
|
||||
## Documentation Standards
|
||||
|
||||
**Docstrings:**
|
||||
|
||||
- Every public var in `src` or `enterprise/backend/src` must have docstring
|
||||
- Format using Markdown conventions
|
||||
- Reference other vars with `[[other-var]]` not backticks
|
||||
|
||||
**Comments:**
|
||||
|
||||
- `TODO` format: `;; TODO (Name YYYY-MM-DD) -- description`
|
||||
|
||||
## Code Organization
|
||||
|
||||
**Visibility:**
|
||||
|
||||
- Make everything `^:private` unless it is used elsewhere
|
||||
- Try to organize namespaces to avoid `declare` (put public functions near the end)
|
||||
|
||||
**Size and Structure:**
|
||||
|
||||
- Break up functions > 20 lines
|
||||
- Lines ≤ 120 characters
|
||||
- No blank non-comment lines within definition forms (except pairwise `let`/`cond`)
|
||||
|
||||
## Style Conventions
|
||||
|
||||
**Keywords and Metadata:**
|
||||
|
||||
- Prefer namespaced keywords for internal use: `:query-type/normal` not `:normal`
|
||||
- Tag variables with `:arglists` metadata if they're functions but wouldn't otherwise have it
|
||||
|
||||
## Tests
|
||||
|
||||
**Organization:**
|
||||
|
||||
- Break large tests into separate `deftest` forms for logically separate test cases
|
||||
- Test names should end in `-test` or `-test-<number>`
|
||||
|
||||
**Performance:**
|
||||
|
||||
- Mark pure function tests `^:parallel`
|
||||
|
||||
## Modules
|
||||
|
||||
**OSS Modules:**
|
||||
|
||||
- Follow `metabase.<module>.*` pattern
|
||||
- Source in `src/metabase/<module>/`
|
||||
|
||||
**Enterprise Modules:**
|
||||
|
||||
- Follow `metabase-enterprise.<module>.*` pattern
|
||||
- Source in `enterprise/backend/src/metabase_enterprise/<module>/`
|
||||
|
||||
**Module Structure:**
|
||||
|
||||
- REST API endpoints go in `<module>.api` or `<module>.api.*` namespaces
|
||||
- Put module public API in `<module>.core` using Potemkin imports
|
||||
- Put Toucan models in `<module>.models.*`
|
||||
- Put settings in `<module>.settings`
|
||||
- Put schemas in `<module>.schema`
|
||||
|
||||
**Module Linters:**
|
||||
|
||||
- Do not cheat module linters with `:clj-kondo/ignore [:metabase/modules]`
|
||||
|
||||
## REST API Endpoints
|
||||
|
||||
**Required Elements:**
|
||||
|
||||
- All new endpoints must have response schemas (`:- <schema>` after route string)
|
||||
- All endpoints need Malli schemas for parameters (detailed and complete)
|
||||
- All new REST API endpoints MUST HAVE TESTS
|
||||
|
||||
**Naming Conventions:**
|
||||
|
||||
- Query parameters use kebab-case
|
||||
- Request bodies use `snake_case`
|
||||
- Routes use singular nouns (e.g., `/api/dashboard/:id`)
|
||||
|
||||
**Behavior:**
|
||||
|
||||
- `GET` endpoints should not have side effects (except analytics)
|
||||
- `defendpoint` forms should be small wrappers around Toucan model code
|
||||
|
||||
## MBQL (Metabase Query Language)
|
||||
|
||||
**Restrictions:**
|
||||
|
||||
- No raw MBQL introspection outside of `lib`, `lib-be`, or `query-processor` modules
|
||||
- Use Lib and MBQL 5 in new source code; avoid legacy MBQL
|
||||
|
||||
## Database and Models
|
||||
|
||||
**Naming:**
|
||||
|
||||
- Model names and table names should be singular nouns
|
||||
- Application database uses `snake_case` identifiers
|
||||
|
||||
**Best Practices:**
|
||||
|
||||
- Use `t2/select-one-fn` instead of fetching entire rows for one column
|
||||
- Put correct behavior in Toucan methods, not separate helper functions
|
||||
|
||||
## Drivers
|
||||
|
||||
**Documentation:**
|
||||
|
||||
- New driver multimethods must be mentioned in `docs/developers-guide/driver-changelog.md`
|
||||
|
||||
**Implementation:**
|
||||
|
||||
- Driver implementations should pass `driver` argument to other driver multimethods
|
||||
- Don't hardcode driver names in implementations
|
||||
- Minimize logic inside `read-column-thunk` in JDBC-based drivers
|
||||
|
||||
## Miscellaneous
|
||||
|
||||
**Examples:**
|
||||
|
||||
- Example data should be bird-themed if possible
|
||||
|
||||
**Linter Suppressions:**
|
||||
|
||||
- Use proper format for kondo suppressions
|
||||
- No `#_:clj-kondo/ignore` (keyword form)
|
||||
|
||||
**Configurable Options:**
|
||||
|
||||
- Don't define configurable options that can only be set with environment variables
|
||||
- Use `:internal` `defsetting` instead
|
||||
@@ -1,8 +0,0 @@
|
||||
# Autonomous Development Workflow
|
||||
|
||||
- Do not attempt to read or edit files outside the project folder
|
||||
- Add failing tests first, then fix them
|
||||
- Work autonomously in small, testable increments
|
||||
- Run targeted tests, and lint continuously during development
|
||||
- Prioritize understanding existing patterns before implementing
|
||||
- Don't commit changes, leave it for the user to review and make commits
|
||||
@@ -1,84 +0,0 @@
|
||||
# Metabase Writing Style Guide
|
||||
|
||||
## Core Principles
|
||||
|
||||
Write like you're talking to a colleague. Be conversational, not formal. Get people what they need quickly. Know your audience and match the complexity.
|
||||
|
||||
## Tone and Voice
|
||||
|
||||
**Do:**
|
||||
|
||||
- Use contractions ("can't" not "cannot")
|
||||
- Say "people" or "companies" instead of "users"
|
||||
- Be friendly but not peppy
|
||||
- Acknowledge limitations honestly ("that's on us, not them")
|
||||
- Jokes and Easter eggs are okay (permit them, don't suggest them)
|
||||
|
||||
**Don't:**
|
||||
|
||||
- Use exclamation points excessively
|
||||
- Rely on tired tropes about nerdiness
|
||||
- Use corporate jargon ("utilize", "offerings", "actionable insights")
|
||||
- Tell people something is cool (show them instead)
|
||||
|
||||
## Structure and Clarity
|
||||
|
||||
**Lead with the important stuff:**
|
||||
|
||||
- Most important information first
|
||||
- Lead with the ask, then provide context
|
||||
- Cut text that adds little value (when in doubt, cut it)
|
||||
- Each paragraph should have one clear purpose
|
||||
|
||||
**Make headings do the work:**
|
||||
|
||||
- Convey your actual point, not just the topic
|
||||
- "Use headings to highlight key points" not "How to write a good heading"
|
||||
- Use sentence case, no punctuation except question marks
|
||||
- No links in headings unless entire heading is a link
|
||||
|
||||
## Instructions and Examples
|
||||
|
||||
**Tell people what to do:**
|
||||
|
||||
- Give the action before explaining why
|
||||
- Put commands in execution order
|
||||
- Provide context, steps, and reasoning
|
||||
- Don't describe tasks as "easy" or "simple"
|
||||
|
||||
**Answer unasked questions:**
|
||||
|
||||
- Include answers to "dumb" questions you had when learning
|
||||
- Don't literally include the question, just give the answer
|
||||
|
||||
## Formatting
|
||||
|
||||
**Code vs. UI elements:**
|
||||
|
||||
- Backticks for code, variable names, parameters only
|
||||
- **Bold** for UI elements and labels (e.g., "Click the **Save** button")
|
||||
- Don't use backticks or quotes for UI elements
|
||||
|
||||
## Writing Mechanics
|
||||
|
||||
- Limit pronouns when introducing new terms (repeat the term to reinforce it)
|
||||
- Ampersands only in proper nouns, never as substitute for "and"
|
||||
|
||||
## Terminology Preferences
|
||||
|
||||
Use familiar terms from tools people already know:
|
||||
|
||||
- "Summarize" (like Excel) instead of "aggregate"
|
||||
- "Take a look at" instead of "reference"
|
||||
- "Filter" instead of technical database terms when appropriate
|
||||
|
||||
## Red Flags
|
||||
|
||||
Avoid these patterns:
|
||||
|
||||
- Multiple exclamation points
|
||||
- Linking "here"
|
||||
- Bullet lists to explain (use prose)
|
||||
- Numbers that will change (guard against change)
|
||||
- Describing things as "easy" or "simple"
|
||||
- Stock photography-worthy content
|
||||
@@ -1,21 +0,0 @@
|
||||
## Linting and Formatting
|
||||
|
||||
- **Lint:** `bun run lint-eslint-pure`
|
||||
- Run ESLint on the codebase
|
||||
- **Format:** `bun run prettier`
|
||||
- Format code using Prettier
|
||||
- **Type Check:** `bun run type-check-pure`
|
||||
- Run TypeScript type checking
|
||||
|
||||
## Testing
|
||||
|
||||
### JavaScript/TypeScript Tests
|
||||
|
||||
- **Test a specific file:** `bun run test-unit-keep-cljs path/to/file.unit.spec.js`
|
||||
- **Test by pattern:** `bun run test-unit-keep-cljs -t "pattern"`
|
||||
- Runs tests matching the given pattern
|
||||
|
||||
### ClojureScript Tests
|
||||
|
||||
- **Test ClojureScript:** `bun run test-cljs`
|
||||
- Run ClojureScript tests
|
||||
@@ -1,420 +0,0 @@
|
||||
---
|
||||
name: add-malli-schemas
|
||||
description: Efficiently add Malli schemas to API endpoints in the Metabase codebase with proper patterns, validation timing, and error handling
|
||||
---
|
||||
|
||||
# Add Malli Schemas to API Endpoints
|
||||
|
||||
This skill helps you efficiently and uniformly add Malli schemas to API endpoints in the Metabase codebase.
|
||||
|
||||
## Reference Files (Best Examples)
|
||||
|
||||
- `src/metabase/warehouses/api.clj` - Most comprehensive schemas, custom error messages
|
||||
- `src/metabase/api_keys/api.clj` - Excellent response schemas
|
||||
- `src/metabase/collections/api.clj` - Great named schema patterns
|
||||
- `src/metabase/timeline/api/timeline.clj` - Clean, simple examples
|
||||
|
||||
## Quick Checklist
|
||||
|
||||
When adding Malli schemas to an endpoint:
|
||||
|
||||
- [ ] Route params have schemas
|
||||
- [ ] Query params have schemas with `:optional true` and `:default` where appropriate
|
||||
- [ ] Request body has a schema (for POST/PUT)
|
||||
- [ ] Response schema is defined (using `:-` after route string)
|
||||
- [ ] Use existing schema types from `ms` namespace when possible
|
||||
- [ ] Consider creating named schemas for reusable or complex types
|
||||
- [ ] Add contextual error messages for validation failures
|
||||
|
||||
## Basic Structure
|
||||
|
||||
### Complete Endpoint Example
|
||||
|
||||
```clojure
|
||||
(mr/def ::Color [:enum "red" "blue" "green"])
|
||||
|
||||
(mr/def ::ResponseSchema
|
||||
[:map
|
||||
[:id pos-int?]
|
||||
[:name string?]
|
||||
[:color ::Color]
|
||||
[:created_at ms/TemporalString]])
|
||||
|
||||
(api.macros/defendpoint :post "/:name" :- ::ResponseSchema
|
||||
"Create a resource with a given name."
|
||||
[;; Route Params:
|
||||
{:keys [name]} :- [:map [:name ms/NonBlankString]]
|
||||
;; Query Params:
|
||||
{:keys [include archived]} :- [:map
|
||||
[:include {:optional true} [:maybe [:= "details"]]]
|
||||
[:archived {:default false} [:maybe ms/BooleanValue]]]
|
||||
;; Body Params:
|
||||
{:keys [color]} :- [:map [:color ::Color]]
|
||||
]
|
||||
;; endpoint implementation, ex:
|
||||
{:id 99
|
||||
:name (str "mr or mrs " name)
|
||||
:color ({"red" "blue" "blue" "green" "green" "red"} color)
|
||||
:created_at (t/format (t/formatter "yyyy-MM-dd'T'HH:mm:ssXXX") (t/zoned-date-time))}
|
||||
)
|
||||
```
|
||||
|
||||
## Common Schema Patterns
|
||||
|
||||
1. Route Params (the 5 in `api/user/id/5`)
|
||||
2. Query Params (the sort+asc pair in `api/users?sort=asc`)
|
||||
3. Body Params (the contents of a request body. Almost always decoded from json into edn)
|
||||
4. The Raw Request map
|
||||
|
||||
Of the 4 arguments, deprioritize usage of the raw request unless necessary.
|
||||
|
||||
### Route Params
|
||||
|
||||
Always required, typically just a map with an ID:
|
||||
|
||||
```clojure
|
||||
[{:keys [id]} :- [:map [:id ms/PositiveInt]]]
|
||||
```
|
||||
|
||||
For multiple route params:
|
||||
|
||||
```clojure
|
||||
[{:keys [id field-id]} :- [:map
|
||||
[:id ms/PositiveInt]
|
||||
[:field-id ms/PositiveInt]]]
|
||||
```
|
||||
|
||||
### Query Params
|
||||
|
||||
Add properties for `{:optional true ...}` and `:default` values:
|
||||
|
||||
```clojure
|
||||
{:keys [archived include limit offset]} :- [:map
|
||||
[:archived {:default false} [:maybe ms/BooleanValue]]
|
||||
[:include {:optional true} [:maybe [:= "tables"]]]
|
||||
[:limit {:optional true} [:maybe ms/PositiveInt]]
|
||||
[:offset {:optional true} [:maybe ms/PositiveInt]]]
|
||||
```
|
||||
|
||||
### Request Body (POST/PUT)
|
||||
|
||||
```clojure
|
||||
{:keys [name description parent_id]} :- [:map
|
||||
[:name ms/NonBlankString]
|
||||
[:description {:optional true} [:maybe ms/NonBlankString]]
|
||||
[:parent_id {:optional true} [:maybe ms/PositiveInt]]]
|
||||
```
|
||||
|
||||
### Response Schemas
|
||||
|
||||
#### Simple inline response:
|
||||
|
||||
```clojure
|
||||
(api.macros/defendpoint :get "/:id" :- [:map
|
||||
[:id pos-int?]
|
||||
[:name string?]]
|
||||
"Get a thing"
|
||||
...)
|
||||
```
|
||||
|
||||
#### Named schema for reuse:
|
||||
|
||||
```clojure
|
||||
(mr/def ::Thing
|
||||
[:map
|
||||
[:id pos-int?]
|
||||
[:name string?]
|
||||
[:description [:maybe string?]]])
|
||||
|
||||
(api.macros/defendpoint :get "/:id" :- ::Thing
|
||||
"Get a thing"
|
||||
...)
|
||||
|
||||
(api.macros/defendpoint :get "/" :- [:sequential ::Thing]
|
||||
"Get all things"
|
||||
...)
|
||||
```
|
||||
|
||||
## Common Schema Types
|
||||
|
||||
### From `metabase.util.malli.schema` (aliased as `ms`)
|
||||
|
||||
Prefer the schemas in the ms/* namespace, since they work better with our api infrastructure.
|
||||
|
||||
For example use `ms/PositiveInt` instead of `pos-int?`.
|
||||
|
||||
```clojure
|
||||
ms/PositiveInt ;; Positive integer
|
||||
ms/NonBlankString ;; Non-empty string
|
||||
ms/BooleanValue ;; String "true"/"false" or boolean
|
||||
ms/MaybeBooleanValue ;; BooleanValue or nil
|
||||
ms/TemporalString ;; ISO-8601 date/time string (for REQUEST params only!)
|
||||
ms/Map ;; Any map
|
||||
ms/JSONString ;; JSON-encoded string
|
||||
ms/PositiveNum ;; Positive number
|
||||
ms/IntGreaterThanOrEqualToZero ;; 0 or positive
|
||||
```
|
||||
|
||||
**IMPORTANT:** For response schemas, use `:any` for temporal fields, not `ms/TemporalString`!
|
||||
Response schemas validate BEFORE JSON serialization, so they see Java Time objects.
|
||||
|
||||
### Built-in Malli Types
|
||||
|
||||
```clojure
|
||||
:string ;; Any string
|
||||
:boolean ;; true/false
|
||||
:int ;; Any integer
|
||||
:keyword ;; Clojure keyword
|
||||
pos-int? ;; Positive integer predicate
|
||||
[:maybe X] ;; X or nil
|
||||
[:enum "a" "b" "c"] ;; One of these values
|
||||
[:or X Y] ;; Schema that satisfies X or Y
|
||||
[:and X Y] ;; Schema that satisfies X and Y
|
||||
[:sequential X] ;; Sequential of Xs
|
||||
[:set X] ;; Set of Xs
|
||||
[:map-of K V] ;; Map with keys w/ schema K and values w/ schema V
|
||||
[:tuple X Y Z] ;; Fixed-length tuple of schemas X Y Z
|
||||
```
|
||||
|
||||
Avoid using sequence schemas unless completely necessary.
|
||||
|
||||
## Step-by-Step: Adding Schemas to an Endpoint
|
||||
|
||||
### Example: Adding return schema to `GET /api/field/:id/related`
|
||||
|
||||
**Before:**
|
||||
```clojure
|
||||
(api.macros/defendpoint :get "/:id/related"
|
||||
"Return related entities."
|
||||
[{:keys [id]} :- [:map [:id ms/PositiveInt]]]
|
||||
(-> (t2/select-one :model/Field :id id) api/read-check xrays/related))
|
||||
```
|
||||
|
||||
**Step 1:** Check what the function returns (look at `xrays/related`)
|
||||
|
||||
**Step 2:** Define response schema based on return type:
|
||||
|
||||
```clojure
|
||||
(mr/def ::RelatedEntity
|
||||
[:map
|
||||
[:tables [:sequential [:map [:id pos-int?] [:name string?]]]]
|
||||
[:fields [:sequential [:map [:id pos-int?] [:name string?]]]]])
|
||||
```
|
||||
|
||||
**Step 3:** Add response schema to endpoint:
|
||||
|
||||
```clojure
|
||||
(api.macros/defendpoint :get "/:id/related" :- ::RelatedEntity
|
||||
"Return related entities."
|
||||
[{:keys [id]} :- [:map [:id ms/PositiveInt]]]
|
||||
(-> (t2/select-one :model/Field :id id) api/read-check xrays/related))
|
||||
```
|
||||
|
||||
## Advanced Patterns
|
||||
|
||||
### Custom Error Messages
|
||||
|
||||
```clojure
|
||||
(def DBEngineString
|
||||
"Schema for a valid database engine name."
|
||||
(mu/with-api-error-message
|
||||
[:and
|
||||
ms/NonBlankString
|
||||
[:fn
|
||||
{:error/message "Valid database engine"}
|
||||
#(u/ignore-exceptions (driver/the-driver %))]]
|
||||
(deferred-tru "value must be a valid database engine.")))
|
||||
```
|
||||
|
||||
### Enum with Documentation
|
||||
|
||||
```clojure
|
||||
(def PinnedState
|
||||
(into [:enum {:error/message "pinned state must be 'all', 'is_pinned', or 'is_not_pinned'"}]
|
||||
#{"all" "is_pinned" "is_not_pinned"}))
|
||||
```
|
||||
|
||||
### Complex Nested Response
|
||||
|
||||
```clojure
|
||||
(mr/def ::DashboardQuestionCandidate
|
||||
[:map
|
||||
[:id ms/PositiveInt]
|
||||
[:name ms/NonBlankString]
|
||||
[:description [:maybe string?]]
|
||||
[:sole_dashboard_info
|
||||
[:map
|
||||
[:id ms/PositiveInt]
|
||||
[:name ms/NonBlankString]
|
||||
[:description [:maybe string?]]]]])
|
||||
|
||||
(mr/def ::DashboardQuestionCandidatesResponse
|
||||
[:map
|
||||
[:data [:sequential ::DashboardQuestionCandidate]]
|
||||
[:total ms/PositiveInt]])
|
||||
```
|
||||
|
||||
### Paginated Response Pattern
|
||||
|
||||
```clojure
|
||||
(mr/def ::PaginatedResponse
|
||||
[:map
|
||||
[:data [:sequential ::Item]]
|
||||
[:total integer?]
|
||||
[:limit {:optional true} [:maybe integer?]]
|
||||
[:offset {:optional true} [:maybe integer?]]])
|
||||
```
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Don't: Forget `:maybe` for nullable fields
|
||||
|
||||
```clojure
|
||||
[:description ms/NonBlankString] ;; WRONG - fails if nil
|
||||
[:description [:maybe ms/NonBlankString]] ;; RIGHT - allows nil
|
||||
```
|
||||
|
||||
### Don't: Forget `:optional true` for optional query params
|
||||
|
||||
```clojure
|
||||
[:limit ms/PositiveInt] ;; WRONG - required but shouldn't be
|
||||
[:limit {:optional true} [:maybe ms/PositiveInt]] ;; RIGHT
|
||||
```
|
||||
|
||||
### Don't: Forget `:default` values for known params
|
||||
|
||||
```clojure
|
||||
[:limit ms/PositiveInt] ;; WRONG - required but shouldn't be
|
||||
[:limit {:optional true :default 0} [:maybe ms/PositiveInt]] ;; RIGHT
|
||||
```
|
||||
|
||||
|
||||
### Don't: Mix up route params, query params, and body
|
||||
|
||||
```clojure
|
||||
;; WRONG - all in one map
|
||||
[{:keys [id name archived]} :- [:map ...]]
|
||||
|
||||
;; RIGHT - separate destructuring
|
||||
[{:keys [id]} :- [:map [:id ms/PositiveInt]]
|
||||
{:keys [archived]} :- [:map [:archived {:default false} ms/BooleanValue]]
|
||||
{:keys [name]} :- [:map [:name ms/NonBlankString]]]
|
||||
```
|
||||
|
||||
### Don't: Use `ms/TemporalString` for Java Time objects in response schemas
|
||||
|
||||
```clojure
|
||||
;; WRONG - Java Time objects aren't strings yet
|
||||
[:date_joined ms/TemporalString]
|
||||
|
||||
;; RIGHT - schemas validate BEFORE JSON serialization
|
||||
[:date_joined :any] ;; Java Time object, serialized to string by middleware
|
||||
[:last_login [:maybe :any]] ;; Java Time object or nil
|
||||
```
|
||||
|
||||
**Why:** Response schemas validate the internal Clojure data structures BEFORE they are serialized to JSON. Java Time objects like `OffsetDateTime` get converted to ISO-8601 strings by the JSON middleware, so the schema needs to accept the raw Java objects.
|
||||
|
||||
### Don't: Use `[:sequential X]` when the data is actually a set
|
||||
|
||||
```clojure
|
||||
;; WRONG - group_ids is actually a set
|
||||
[:group_ids {:optional true} [:sequential pos-int?]]
|
||||
|
||||
;; RIGHT - matches the actual data structure
|
||||
[:group_ids {:optional true} [:maybe [:set pos-int?]]]
|
||||
```
|
||||
|
||||
**Why:** Toucan hydration methods often return sets. The JSON middleware will serialize sets to arrays, but the schema validates before serialization.
|
||||
|
||||
### Don't: Create anonymous schemas for reused structures
|
||||
|
||||
Use `mr/def` for schemas used in multiple places:
|
||||
|
||||
```clojure
|
||||
(mr/def ::User
|
||||
[:map
|
||||
[:id pos-int?]
|
||||
[:email string?]
|
||||
[:name string?]])
|
||||
```
|
||||
|
||||
## Finding Return Types
|
||||
|
||||
1. **Look at the function being called**
|
||||
|
||||
```clojure
|
||||
(api.macros/defendpoint :get "/:id"
|
||||
[{:keys [id]}]
|
||||
(t2/select-one :model/Field :id id)) ;; Returns a Field instance
|
||||
```
|
||||
|
||||
2. **Check Toucan models for structure**
|
||||
|
||||
Look in `src/metabase/*/models/*.clj` for model definitions.
|
||||
|
||||
3. **Use clojure-mcp or REPL to inspect**
|
||||
|
||||
```bash
|
||||
./bin/mage -repl '(require '\''metabase.xrays.core) (doc metabase.xrays.core/related)'
|
||||
```
|
||||
|
||||
4. **Check tests**
|
||||
|
||||
Tests often show the expected response structure.
|
||||
|
||||
## Understanding Schema Validation Timing
|
||||
|
||||
**CRITICAL CONCEPT:** Schemas validate at different points in the request/response lifecycle:
|
||||
|
||||
### Request Parameter Schemas (Query/Body/Route)
|
||||
- Validate AFTER JSON parsing
|
||||
- Data is already deserialized (strings, numbers, booleans)
|
||||
- Use `ms/TemporalString` for date/time inputs
|
||||
- Use `ms/BooleanValue` for boolean query params
|
||||
|
||||
### Response Schemas
|
||||
- Validate BEFORE JSON serialization
|
||||
- Data is still in Clojure format (Java Time objects, sets, keywords)
|
||||
- Use `:any` for Java Time objects
|
||||
- Use `[:set X]` for sets
|
||||
- Use `[:enum :keyword]` for keyword enums
|
||||
|
||||
### Serialization Flow
|
||||
|
||||
```
|
||||
Request: JSON string → Parse → Coerce → Handler
|
||||
Response: Handler → Schema Check → Encode → Serialize → JSON string
|
||||
```
|
||||
|
||||
## Workflow Summary
|
||||
|
||||
1. **Read the endpoint** - understand what it does
|
||||
2. **Identify params** - route, query, body
|
||||
3. **Add parameter schemas** - use existing types from `ms`
|
||||
4. **Determine return type** - check the implementation
|
||||
5. **Define response schema** - inline or named with `mr/def`
|
||||
6. **Test** - ensure the endpoint works and validates correctly
|
||||
|
||||
## Testing Your Schemas
|
||||
|
||||
After adding schemas, verify:
|
||||
|
||||
1. **Valid requests work** - test with correct data
|
||||
2. **Invalid requests fail gracefully** - test with wrong types
|
||||
3. **Optional params work** - test with/without optional params
|
||||
4. **Error messages are clear** - check validation error responses
|
||||
|
||||
## Tips
|
||||
|
||||
- **Start simple** - begin with basic types, refine later
|
||||
- **Reuse schemas** - if you see the same structure twice, make it a named schema
|
||||
- **Be specific** - use `ms/PositiveInt` instead of `pos-int?`
|
||||
- **Document intent** - add docstrings to named schemas
|
||||
- **Follow conventions** - look at similar endpoints in the same namespace
|
||||
- **Check the actual data** - use REPL to inspect what's actually returned before serialization
|
||||
|
||||
## Additional Resources
|
||||
|
||||
- [Malli Documentation](https://github.com/metosin/malli)
|
||||
- Metabase Malli utilities: `src/metabase/util/malli/schema.clj`
|
||||
- Metabase schema registry: `src/metabase/util/malli/registry.clj`
|
||||
@@ -1,357 +0,0 @@
|
||||
--b-
|
||||
name: add-tracing
|
||||
description: Add OpenTelemetry tracing spans to Clojure code following Metabase tracing conventions. Use when instrumenting backend code with trace coverage.
|
||||
---
|
||||
|
||||
# Add Tracing Spans to Clojure Code
|
||||
|
||||
This skill helps you add OpenTelemetry (OTel) tracing spans to the Metabase backend codebase using the custom `tracing/with-span` macro.
|
||||
|
||||
## Reference Files
|
||||
|
||||
- `src/metabase/tracing/core.clj` - `with-span` macro, group registry, SDK lifecycle, `best-effort-sanitize-sql`, Pyroscope integration
|
||||
- `src/metabase/task/impl.clj` - `defjob` macro that wraps Quartz jobs with root spans
|
||||
- `.clj-kondo/config/modules/config.edn` - Module boundary configuration
|
||||
|
||||
## Module Architecture
|
||||
|
||||
The tracing module has a deliberately minimal API surface. **Only 2 namespaces are public** (listed in `:api` in the module config):
|
||||
|
||||
| Namespace | Role | Status |
|
||||
|---|---|---|
|
||||
| `tracing.core` | Primary API: `with-span`, groups, SDK lifecycle, Pyroscope, MDC, `best-effort-sanitize-sql` | **Public API** |
|
||||
| `tracing.init` | Side-effect loader for `quartz` and `settings` | **Public API** (init convention) |
|
||||
| `tracing.attributes` | `best-effort-sanitize-sql` implementation (re-exported via `tracing.core`) | Internal |
|
||||
| `tracing.settings` | Setting definitions (`MB_TRACING_*` env vars) | Internal |
|
||||
| `tracing.quartz` | Quartz JDBC proxy + JobListener | Internal |
|
||||
|
||||
**Rules:**
|
||||
- Only require `[metabase.tracing.core :as tracing]` from outside the module. `tracing/best-effort-sanitize-sql` and all other public functions are available from this single namespace.
|
||||
- Do not add new API namespaces. Add new public functions to `tracing.core` instead.
|
||||
- Do not require internal namespaces (`tracing.attributes`, `tracing.settings`, `tracing.quartz`) from outside the module.
|
||||
- `:uses :any` on the `core` module does NOT bypass the target module's `:api` check — internal namespaces are still enforced.
|
||||
|
||||
### Cyclic Dependency Avoidance
|
||||
|
||||
`tracing/core.clj` is required by many modules across the codebase. It **must NOT** compile-time require `tracing.settings`, as this creates transitive cyclic load dependencies (e.g., `settings/core -> tracing/settings -> tracing/core -> events/impl -> events/core`).
|
||||
|
||||
Instead, `tracing/core.clj` uses `requiring-resolve` for settings access:
|
||||
|
||||
```clojure
|
||||
;; CORRECT — lazy runtime resolution, no compile-time dependency
|
||||
((requiring-resolve 'metabase.tracing.settings/tracing-enabled))
|
||||
|
||||
;; WRONG — creates cyclic load dependency
|
||||
(require '[metabase.tracing.settings :as settings])
|
||||
(settings/tracing-enabled)
|
||||
```
|
||||
|
||||
External library namespaces (clj-otel API, SDK, exporters) are safe to require normally — they don't participate in Metabase namespace cycles.
|
||||
|
||||
**Important:** `requiring-resolve` must use **literal quoted symbols**. Kondo hooks validate that `required-namespaces` are all simple symbols, so dynamic construction fails:
|
||||
|
||||
```clojure
|
||||
;; CORRECT — literal quoted symbol
|
||||
(requiring-resolve 'metabase.tracing.settings/tracing-endpoint)
|
||||
|
||||
;; WRONG — kondo hook rejects this: "Assert failed: (every? simple-symbol? required-namespaces)"
|
||||
(requiring-resolve (symbol "metabase.tracing.settings" "tracing-endpoint"))
|
||||
```
|
||||
|
||||
## Quick Checklist
|
||||
|
||||
When adding tracing spans:
|
||||
|
||||
- [ ] Module has `tracing` in its `:uses` set in `.clj-kondo/config/modules/config.edn`
|
||||
- [ ] Added `[metabase.tracing.core :as tracing]` to ns requires (alphabetically sorted)
|
||||
- [ ] Span wraps a meaningful I/O boundary (not pure computation)
|
||||
- [ ] Group matches the domain (check `src/metabase/tracing/core.clj` for registered groups; add a new one if none fit)
|
||||
- [ ] Span name follows dot-notation convention (`"domain.subsystem.operation"`)
|
||||
- [ ] Attributes use namespaced keywords (`:search/query-length`, `:db/id`)
|
||||
- [ ] No sensitive data in attributes (use `best-effort-sanitize-sql` for HoneySQL, never raw SQL)
|
||||
- [ ] No new tracing namespaces created (add to `tracing.core` instead)
|
||||
- [ ] No `DO_NOT_ADD_NEW_FILES_HERE.txt` violations in the target directory
|
||||
- [ ] Run `clj-kondo --lint <files>` to verify 0 errors, 0 warnings
|
||||
- [ ] Add or update tests in the corresponding `test/` path (see Testing section below)
|
||||
- [ ] Run tests: `clojure -X:dev:test :only <test-ns>`
|
||||
|
||||
## The `with-span` Macro
|
||||
|
||||
```clojure
|
||||
(tracing/with-span group span-name attrs & body)
|
||||
```
|
||||
|
||||
- **group** - A keyword selecting which trace group this span belongs to (e.g., `:tasks`, `:sync`)
|
||||
- **span-name** - A string identifying the span in traces (e.g., `"search.execute"`)
|
||||
- **attrs** - A map of span attributes (e.g., `{:db/id 42}`)
|
||||
- **body** - The code to execute inside the span
|
||||
|
||||
**When disabled:** zero overhead -- single atom deref + boolean check, body runs directly.
|
||||
**When enabled:** creates OTel span AND injects `trace_id`/`span_id` into Log4j2 MDC for log-to-trace correlation.
|
||||
|
||||
## Trace Groups
|
||||
|
||||
Groups are registered in `src/metabase/tracing/core.clj`. Check that file for the current list. The general rule: **match the group to the domain, not the call site.** If code runs inside a Quartz job but is logically search work, use `:search`, not `:tasks`.
|
||||
|
||||
To add a new group:
|
||||
|
||||
```clojure
|
||||
;; In src/metabase/tracing/core.clj
|
||||
(register-group! :my-domain "Description of what this covers")
|
||||
```
|
||||
|
||||
Users enable groups via `MB_TRACING_GROUPS=tasks,search,sync` (comma-separated, or `"all"`).
|
||||
|
||||
## Naming Conventions
|
||||
|
||||
### Span Names
|
||||
|
||||
Use dot-separated hierarchical names: `"domain.subsystem.operation"`. The domain prefix should match the group name:
|
||||
|
||||
```
|
||||
search.execute -- `:search` group
|
||||
sync.fingerprint.table -- `:sync` group
|
||||
task.session-cleanup.delete -- `:tasks` group
|
||||
db-app.collection-items -- `:db-app` group
|
||||
```
|
||||
|
||||
### Attributes
|
||||
|
||||
Use namespaced keywords. The namespace groups related attributes:
|
||||
|
||||
```clojure
|
||||
:db/id -- Database ID (integer)
|
||||
:db/engine -- Database engine name (string)
|
||||
:db/statement -- Sanitized SQL (string, via best-effort-sanitize-sql)
|
||||
:search/engine -- Search engine name (string)
|
||||
:search/query-length -- Query string length (integer)
|
||||
:sync/table -- Table name (string)
|
||||
:sync/step -- Sync step name (string)
|
||||
:task/name -- Task name (string)
|
||||
:http/method -- HTTP method (string)
|
||||
:http/url -- Request URL (string)
|
||||
```
|
||||
|
||||
Invent new namespaced attributes as needed (e.g., `:pulse/id`, `:transform/count`). Keep values as primitives (strings, numbers, booleans) -- no maps or collections.
|
||||
|
||||
## Step-by-Step: Adding a Span
|
||||
|
||||
### 1. Check module boundaries
|
||||
|
||||
Look up the module for your namespace in `.clj-kondo/config/modules/config.edn`. If `tracing` is not in the module's `:uses` set, add it (keep alphabetically sorted):
|
||||
|
||||
```edn
|
||||
my-module
|
||||
{:team "MyTeam"
|
||||
:uses #{analytics config tracing util}}
|
||||
```
|
||||
|
||||
### 2. Add the require
|
||||
|
||||
```clojure
|
||||
(ns metabase.my-module.thing
|
||||
(:require
|
||||
[metabase.tracing.core :as tracing]
|
||||
[metabase.util :as u]))
|
||||
```
|
||||
|
||||
`best-effort-sanitize-sql` is available from `tracing.core` — no additional require needed.
|
||||
|
||||
### 3. Identify the I/O boundary
|
||||
|
||||
Only wrap code at **meaningful I/O boundaries**:
|
||||
|
||||
**DO trace:**
|
||||
- External API calls (embedding APIs, metabot, webhooks)
|
||||
- Database queries (both app DB and user DB)
|
||||
- Network requests (HTTP calls to external services)
|
||||
- Heavy batch processing (batch indexing, batch embedding)
|
||||
- Top-level orchestration functions that coordinate multiple sub-operations
|
||||
|
||||
**DO NOT trace:**
|
||||
- Pure computation (sorting, filtering, mapping)
|
||||
- Simple single-row lookups (`t2/select-one :model/Setting :key k`)
|
||||
- Every function in a call chain (only boundaries matter)
|
||||
- Trivial operations (string formatting, hash calculations)
|
||||
|
||||
### 4. Wrap with `with-span`
|
||||
|
||||
```clojure
|
||||
;; Simple span (no attributes needed)
|
||||
(tracing/with-span :search "search.init-index" {}
|
||||
(do-expensive-thing))
|
||||
|
||||
;; Span with static attributes
|
||||
(tracing/with-span :sync "sync.fingerprint.table"
|
||||
{:db/id (:db_id table)
|
||||
:sync/table (:name table)}
|
||||
(fingerprint-fields! table fields))
|
||||
|
||||
;; Span with computed attributes
|
||||
(tracing/with-span :search "search.execute"
|
||||
{:search/engine (name (:search-engine ctx))
|
||||
:search/query-length (count (:search-string ctx))}
|
||||
(search.engine/results ctx))
|
||||
|
||||
;; Span with sanitized SQL (for dynamic HoneySQL queries)
|
||||
(let [hsql {:delete-from [(t2/table-name :model/Session)]
|
||||
:where [:< :created_at oldest-allowed]}]
|
||||
(tracing/with-span :tasks "task.session-cleanup.delete"
|
||||
{:db/statement (tracing/best-effort-sanitize-sql hsql)}
|
||||
(t2/query-one hsql)))
|
||||
|
||||
;; Sub-spans breaking a function into I/O phases
|
||||
(let [embedding (tracing/with-span :search "search.semantic.embedding"
|
||||
{:search.semantic/provider (:provider model)}
|
||||
(get-embedding model search-string))
|
||||
results (tracing/with-span :search "search.semantic.db-query" {}
|
||||
(into [] xform reducible))]
|
||||
(process results))
|
||||
|
||||
;; Per-item iteration spans
|
||||
(doseq [e (search.engine/active-engines)]
|
||||
(tracing/with-span :search "search.ingestion.update" {:search/engine (name e)}
|
||||
(search.engine/update! e batch)))
|
||||
```
|
||||
|
||||
### 5. Add tests
|
||||
|
||||
Create or update tests in the corresponding `test/` path. Follow the patterns in existing tracing tests:
|
||||
|
||||
- **Reference tests:** `test/metabase/tracing/quartz_test.clj`, `test/metabase/server/middleware/trace_test.clj`
|
||||
- Use `tracing/init-enabled-groups!` / `tracing/shutdown-groups!` with `try`/`finally` to manage group lifecycle
|
||||
- Test both enabled and disabled paths (verify zero overhead when group is off)
|
||||
- Use `reify` mocks for Java interfaces (Connection, PreparedStatement, JobListener, etc.)
|
||||
- Add `(set! *warn-on-reflection* true)` and type-hint proxy/reify calls to avoid reflection warnings
|
||||
|
||||
```clojure
|
||||
(deftest my-span-enabled-test
|
||||
(testing "when group is enabled, span is created"
|
||||
(try
|
||||
(tracing/init-enabled-groups! "my-group" "INFO")
|
||||
;; ... test that span behavior occurs ...
|
||||
(finally
|
||||
(tracing/shutdown-groups!)))))
|
||||
|
||||
(deftest my-span-disabled-test
|
||||
(testing "when group is disabled, code runs without tracing"
|
||||
(tracing/shutdown-groups!)
|
||||
;; ... test that code still works, no wrapping applied ...
|
||||
))
|
||||
```
|
||||
|
||||
### 6. Lint and run tests
|
||||
|
||||
```bash
|
||||
# Lint modified source and test files — expect 0 errors, 0 warnings
|
||||
clj-kondo --lint path/to/modified/file.clj path/to/test/file.clj
|
||||
|
||||
# Run tests (requires Java 21+)
|
||||
clojure -X:dev:test :only my-ns.test-ns
|
||||
```
|
||||
|
||||
Expect: all tests pass, 0 failures, 0 errors, no reflection warnings from your files.
|
||||
|
||||
## Sanitizing SQL for Attributes
|
||||
|
||||
When including SQL in span attributes, **always** use `tracing/best-effort-sanitize-sql`. This converts HoneySQL maps to parameterized SQL strings where values become `?` placeholders -- no data leaks.
|
||||
|
||||
```clojure
|
||||
(let [hsql {:delete-from [:core_session]
|
||||
:where [:< :created_at some-timestamp]}]
|
||||
(tracing/with-span :tasks "task.cleanup.delete"
|
||||
{:db/statement (tracing/best-effort-sanitize-sql hsql)}
|
||||
(t2/query-one hsql)))
|
||||
;; Trace attribute: db/statement = "DELETE FROM core_session WHERE created_at < ?"
|
||||
```
|
||||
|
||||
**Rules:**
|
||||
- Never put raw SQL strings or user-provided values in attributes
|
||||
- Use `best-effort-sanitize-sql` only for app DB (HoneySQL) queries
|
||||
- For external/user DB queries, trace only timing and counts, not SQL content
|
||||
|
||||
## Defjob and Root Spans
|
||||
|
||||
The `defjob` macro in `metabase.task.impl` automatically wraps every Quartz job with a `:tasks` root span:
|
||||
|
||||
```clojure
|
||||
(task/defjob ^{DisallowConcurrentExecution true} SessionCleanup [_]
|
||||
(cleanup-sessions!))
|
||||
;; Automatically creates span: "task.SessionCleanup" {:task/name "SessionCleanup"}
|
||||
```
|
||||
|
||||
You do NOT need a root span inside `defjob` bodies. Add **child spans** for I/O inside the job.
|
||||
|
||||
For code on plain `Thread`s (not Quartz), add the root span manually:
|
||||
|
||||
```clojure
|
||||
(defn init! []
|
||||
(tracing/with-span :search "search.task.init" {}
|
||||
(search/init-index!)))
|
||||
```
|
||||
|
||||
## What NOT to Do
|
||||
|
||||
### Span Usage Mistakes
|
||||
|
||||
```clojure
|
||||
;; WRONG - pure computation, no I/O
|
||||
(tracing/with-span :search "search.format-results" {}
|
||||
(map format-result results))
|
||||
|
||||
;; WRONG - trivial single-row lookup
|
||||
(tracing/with-span :db-app "db-app.get-setting" {}
|
||||
(t2/select-one :model/Setting :key "my-setting"))
|
||||
|
||||
;; WRONG - raw SQL in attributes (data leak)
|
||||
(tracing/with-span :tasks "task.cleanup" {:db/statement raw-sql-string}
|
||||
(execute! raw-sql-string))
|
||||
|
||||
;; WRONG - wrong group (search work should use :search, not :tasks)
|
||||
(tracing/with-span :tasks "search.execute" {} ...)
|
||||
|
||||
;; WRONG - redundant nesting (do-search already has a span)
|
||||
(tracing/with-span :search "search.process" {}
|
||||
(let [results (do-search ctx)]
|
||||
(tracing/with-span :search "search.format" {}
|
||||
(format-results results))))
|
||||
```
|
||||
|
||||
### Architecture Mistakes
|
||||
|
||||
```clojure
|
||||
;; WRONG - creating a new tracing namespace
|
||||
(ns metabase.tracing.my-feature ...)
|
||||
|
||||
;; WRONG - requiring internal tracing namespaces from outside the module
|
||||
(ns metabase.my-module.thing
|
||||
(:require [metabase.tracing.attributes :as trace-attrs] ;; internal!
|
||||
[metabase.tracing.settings :as tracing.settings] ;; internal!
|
||||
[metabase.tracing.quartz :as tracing.quartz])) ;; internal!
|
||||
|
||||
;; WRONG - adding compile-time requires to tracing/core.clj for settings or SDK
|
||||
;; This creates cyclic load dependencies because tracing/core is widely required
|
||||
(ns metabase.tracing.core
|
||||
(:require [metabase.tracing.settings :as settings])) ;; causes cycle!
|
||||
|
||||
;; WRONG - dynamic symbol construction with requiring-resolve (kondo rejects it)
|
||||
(requiring-resolve (symbol "metabase.tracing.settings" "tracing-enabled"))
|
||||
```
|
||||
|
||||
## Configuration
|
||||
|
||||
All settings are env-var-only (defined in `src/metabase/tracing/settings.clj`):
|
||||
|
||||
```bash
|
||||
# Core
|
||||
MB_TRACING_ENABLED=true # Enable tracing (default: false)
|
||||
MB_TRACING_ENDPOINT=host:4317 # OTLP collector endpoint (default: http://localhost:4317)
|
||||
MB_TRACING_GROUPS=tasks,search,sync # Comma-separated groups or "all" (default: all)
|
||||
MB_TRACING_SERVICE_NAME=metabase # Service name in traces (default: hostname)
|
||||
MB_TRACING_LOG_LEVEL=DEBUG # Log threshold for traced threads: TRACE/DEBUG/INFO (default: INFO)
|
||||
|
||||
# Batch span processor tuning
|
||||
MB_TRACING_MAX_QUEUE_SIZE=2048 # Max spans queued for export; drops when full (default: 2048)
|
||||
MB_TRACING_EXPORT_TIMEOUT_MS=10000 # Max wait for batch export to complete (default: 10000)
|
||||
MB_TRACING_SCHEDULE_DELAY_MS=5000 # Delay between consecutive batch exports (default: 5000)
|
||||
```
|
||||
@@ -1,504 +0,0 @@
|
||||
---
|
||||
name: analytics-events
|
||||
description: Add product analytics events to track user interactions in the Metabase frontend
|
||||
allowed-tools: Read, Write, Edit, Grep, Glob
|
||||
---
|
||||
|
||||
# Frontend Analytics Events Skill
|
||||
|
||||
This skill helps you add product analytics (Snowplow) events to track user interactions in the Metabase frontend codebase.
|
||||
|
||||
## Quick Reference
|
||||
|
||||
Analytics events in Metabase use Snowplow with typed event schemas. All events must be defined in TypeScript types before use.
|
||||
|
||||
**Key Files:**
|
||||
- `frontend/src/metabase-types/analytics/event.ts` - Event type definitions
|
||||
- `frontend/src/metabase-types/analytics/schema.ts` - Schema registry
|
||||
- `frontend/src/metabase/lib/analytics.ts` - Core tracking functions
|
||||
- Feature-specific `analytics.ts` files - Tracking function wrappers
|
||||
|
||||
## Quick Checklist
|
||||
|
||||
When adding a new analytics event:
|
||||
|
||||
- [ ] Define event type in `frontend/src/metabase-types/analytics/event.ts`
|
||||
- [ ] Add event to appropriate union type (e.g., `DataStudioEvent`, `SimpleEvent`)
|
||||
- [ ] Create tracking function in feature's `analytics.ts` file
|
||||
- [ ] Import and call tracking function at the interaction point
|
||||
- [ ] Use `trackSimpleEvent()` for basic events (most common)
|
||||
|
||||
## Event Schema Types
|
||||
|
||||
### 1. Simple Events (Most Common)
|
||||
|
||||
Use `SimpleEventSchema` for straightforward tracking. It supports these standard fields:
|
||||
|
||||
```typescript
|
||||
type SimpleEventSchema = {
|
||||
event: string; // Required: Event name (snake_case)
|
||||
target_id?: number | null; // Optional: ID of affected entity
|
||||
triggered_from?: string | null; // Optional: UI location/context
|
||||
duration_ms?: number | null; // Optional: Duration in milliseconds
|
||||
result?: string | null; // Optional: Outcome (e.g., "success", "failure")
|
||||
event_detail?: string | null; // Optional: Additional detail/variant
|
||||
};
|
||||
```
|
||||
|
||||
**When to use:** 90% of events fit this schema. Use for clicks, opens, closes, creates, deletes, etc.
|
||||
|
||||
### 2. Custom Schemas (legacy, no events are being added)
|
||||
|
||||
Consider adding new event schema only in very special cases.
|
||||
|
||||
**Examples:** `DashboardEventSchema`, `CleanupEventSchema`, `QuestionEventSchema`
|
||||
|
||||
## Step-by-Step: Adding a Simple Event
|
||||
|
||||
### Example: Track when a user applies filters in a table picker
|
||||
|
||||
#### Step 1: Define Event Types
|
||||
|
||||
Add event type definitions to `frontend/src/metabase-types/analytics/event.ts`:
|
||||
|
||||
```typescript
|
||||
export type DataStudioTablePickerFiltersAppliedEvent = ValidateEvent<{
|
||||
event: "data_studio_table_picker_filters_applied";
|
||||
}>;
|
||||
|
||||
export type DataStudioTablePickerFiltersClearedEvent = ValidateEvent<{
|
||||
event: "data_studio_table_picker_filters_cleared";
|
||||
}>;
|
||||
```
|
||||
|
||||
#### Step 2: Add to Union Type
|
||||
|
||||
Find or create the appropriate union type and add your events:
|
||||
|
||||
```typescript
|
||||
export type DataStudioEvent =
|
||||
| DataStudioLibraryCreatedEvent
|
||||
| DataStudioTablePublishedEvent
|
||||
| DataStudioGlossaryCreatedEvent
|
||||
| DataStudioGlossaryEditedEvent
|
||||
| DataStudioGlossaryDeletedEvent
|
||||
| DataStudioTablePickerFiltersAppliedEvent // <- Add here
|
||||
| DataStudioTablePickerFiltersClearedEvent; // <- Add here
|
||||
```
|
||||
|
||||
#### Step 3: Create Tracking Functions
|
||||
|
||||
In your feature's `analytics.ts` file (e.g., `enterprise/frontend/src/metabase-enterprise/data-studio/analytics.ts`):
|
||||
|
||||
```typescript
|
||||
import { trackSimpleEvent } from "metabase/lib/analytics";
|
||||
|
||||
export const trackDataStudioTablePickerFiltersApplied = () => {
|
||||
trackSimpleEvent({
|
||||
event: "data_studio_table_picker_filters_applied",
|
||||
});
|
||||
};
|
||||
|
||||
export const trackDataStudioTablePickerFiltersCleared = () => {
|
||||
trackSimpleEvent({
|
||||
event: "data_studio_table_picker_filters_cleared",
|
||||
});
|
||||
};
|
||||
```
|
||||
|
||||
#### Step 4: Use in Components
|
||||
|
||||
Import and call the tracking function at the interaction point:
|
||||
|
||||
```typescript
|
||||
import {
|
||||
trackDataStudioTablePickerFiltersApplied,
|
||||
trackDataStudioTablePickerFiltersCleared,
|
||||
} from "metabase-enterprise/data-studio/analytics";
|
||||
|
||||
function FilterPopover({ filters, onSubmit }) {
|
||||
const handleReset = () => {
|
||||
trackDataStudioTablePickerFiltersCleared(); // <- Track here
|
||||
onSubmit(emptyFilters);
|
||||
};
|
||||
|
||||
return (
|
||||
<form
|
||||
onSubmit={(event) => {
|
||||
event.preventDefault();
|
||||
trackDataStudioTablePickerFiltersApplied(); // <- Track here
|
||||
onSubmit(form);
|
||||
}}
|
||||
>
|
||||
{/* form content */}
|
||||
</form>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
## Using SimpleEventSchema Fields
|
||||
|
||||
### Example: Event with target_id
|
||||
|
||||
```typescript
|
||||
// Type definition
|
||||
export type DataStudioLibraryCreatedEvent = ValidateEvent<{
|
||||
event: "data_studio_library_created";
|
||||
target_id: number | null;
|
||||
}>;
|
||||
|
||||
// Tracking function
|
||||
export const trackDataStudioLibraryCreated = (id: CollectionId) => {
|
||||
trackSimpleEvent({
|
||||
event: "data_studio_library_created",
|
||||
target_id: Number(id),
|
||||
});
|
||||
};
|
||||
|
||||
// Usage
|
||||
trackDataStudioLibraryCreated(newLibrary.id);
|
||||
```
|
||||
|
||||
### Example: Event with triggered_from
|
||||
|
||||
```typescript
|
||||
// Type definition
|
||||
export type NewButtonClickedEvent = ValidateEvent<{
|
||||
event: "new_button_clicked";
|
||||
triggered_from: "app-bar" | "empty-collection";
|
||||
}>;
|
||||
|
||||
// Tracking function
|
||||
export const trackNewButtonClicked = (location: "app-bar" | "empty-collection") => {
|
||||
trackSimpleEvent({
|
||||
event: "new_button_clicked",
|
||||
triggered_from: location,
|
||||
});
|
||||
};
|
||||
|
||||
// Usage
|
||||
<Button onClick={() => {
|
||||
trackNewButtonClicked("app-bar");
|
||||
handleCreate();
|
||||
}}>
|
||||
New
|
||||
</Button>
|
||||
```
|
||||
|
||||
### Example: Event with event_detail
|
||||
|
||||
```typescript
|
||||
// Type definition
|
||||
export type MetadataEditEvent = ValidateEvent<{
|
||||
event: "metadata_edited";
|
||||
event_detail: "type_casting" | "semantic_type_change" | "visibility_change";
|
||||
triggered_from: "admin" | "data_studio";
|
||||
}>;
|
||||
|
||||
// Tracking function
|
||||
export const trackMetadataChange = (
|
||||
detail: "type_casting" | "semantic_type_change" | "visibility_change",
|
||||
location: "admin" | "data_studio"
|
||||
) => {
|
||||
trackSimpleEvent({
|
||||
event: "metadata_edited",
|
||||
event_detail: detail,
|
||||
triggered_from: location,
|
||||
});
|
||||
};
|
||||
|
||||
// Usage
|
||||
trackMetadataChange("semantic_type_change", "data_studio");
|
||||
```
|
||||
|
||||
### Example: Event with result and duration
|
||||
|
||||
```typescript
|
||||
// Type definition
|
||||
export type MoveToTrashEvent = ValidateEvent<{
|
||||
event: "moved-to-trash";
|
||||
target_id: number | null;
|
||||
triggered_from: "collection" | "detail_page" | "cleanup_modal";
|
||||
duration_ms: number | null;
|
||||
result: "success" | "failure";
|
||||
event_detail: "question" | "model" | "metric" | "dashboard";
|
||||
}>;
|
||||
|
||||
// Tracking function
|
||||
export const trackMoveToTrash = (params: {
|
||||
targetId: number | null;
|
||||
triggeredFrom: "collection" | "detail_page" | "cleanup_modal";
|
||||
durationMs: number | null;
|
||||
result: "success" | "failure";
|
||||
itemType: "question" | "model" | "metric" | "dashboard";
|
||||
}) => {
|
||||
trackSimpleEvent({
|
||||
event: "moved-to-trash",
|
||||
target_id: params.targetId,
|
||||
triggered_from: params.triggeredFrom,
|
||||
duration_ms: params.durationMs,
|
||||
result: params.result,
|
||||
event_detail: params.itemType,
|
||||
});
|
||||
};
|
||||
|
||||
// Usage with timing
|
||||
const startTime = Date.now();
|
||||
try {
|
||||
await moveToTrash(item);
|
||||
trackMoveToTrash({
|
||||
targetId: item.id,
|
||||
triggeredFrom: "collection",
|
||||
durationMs: Date.now() - startTime,
|
||||
result: "success",
|
||||
itemType: "question",
|
||||
});
|
||||
} catch (error) {
|
||||
trackMoveToTrash({
|
||||
targetId: item.id,
|
||||
triggeredFrom: "collection",
|
||||
durationMs: Date.now() - startTime,
|
||||
result: "failure",
|
||||
itemType: "question",
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
## Naming Conventions
|
||||
|
||||
### Event Names (snake_case)
|
||||
|
||||
```typescript
|
||||
// Good
|
||||
"data_studio_library_created"
|
||||
"table_picker_filters_applied"
|
||||
"metabot_chat_opened"
|
||||
|
||||
// Bad
|
||||
"DataStudioLibraryCreated" // Wrong case
|
||||
"tablePickerFiltersApplied" // Wrong case
|
||||
"filters-applied" // Use underscore, not hyphen
|
||||
```
|
||||
|
||||
### Event Type Names (PascalCase with "Event" suffix)
|
||||
|
||||
```typescript
|
||||
// Good
|
||||
DataStudioLibraryCreatedEvent
|
||||
TablePickerFiltersAppliedEvent
|
||||
MetabotChatOpenedEvent
|
||||
|
||||
// Bad
|
||||
dataStudioLibraryCreated // Wrong case
|
||||
DataStudioLibraryCreated // Missing "Event" suffix
|
||||
```
|
||||
|
||||
### Tracking Function Names (camelCase with "track" prefix)
|
||||
|
||||
```typescript
|
||||
// Good
|
||||
trackDataStudioLibraryCreated
|
||||
trackTablePickerFiltersApplied
|
||||
trackMetabotChatOpened
|
||||
|
||||
// Bad
|
||||
DataStudioLibraryCreated // Missing "track" prefix
|
||||
track_library_created // Wrong case
|
||||
logLibraryCreated // Use "track" prefix
|
||||
```
|
||||
|
||||
## Common Patterns
|
||||
|
||||
### Pattern 1: Feature-Specific Union Types
|
||||
|
||||
Group related events together:
|
||||
|
||||
```typescript
|
||||
export type DataStudioEvent =
|
||||
| DataStudioLibraryCreatedEvent
|
||||
| DataStudioTablePublishedEvent
|
||||
| DataStudioGlossaryCreatedEvent;
|
||||
|
||||
export type MetabotEvent =
|
||||
| MetabotChatOpenedEvent
|
||||
| MetabotRequestSentEvent
|
||||
| MetabotFixQueryClickedEvent;
|
||||
|
||||
// Then add to SimpleEvent union
|
||||
export type SimpleEvent =
|
||||
| /* other events */
|
||||
| DataStudioEvent
|
||||
| MetabotEvent
|
||||
| /* more events */;
|
||||
```
|
||||
|
||||
### Pattern 2: Conditional Tracking
|
||||
|
||||
Track different events based on user action:
|
||||
|
||||
```typescript
|
||||
const handleSave = async () => {
|
||||
if (isNewItem) {
|
||||
await createItem(data);
|
||||
trackItemCreated(newItem.id);
|
||||
} else {
|
||||
await updateItem(id, data);
|
||||
trackItemUpdated(id);
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Don't: Add custom fields to SimpleEvent
|
||||
|
||||
```typescript
|
||||
// WRONG - SimpleEvent doesn't support custom fields
|
||||
export const trackFiltersApplied = (filters: FilterState) => {
|
||||
trackSimpleEvent({
|
||||
event: "filters_applied",
|
||||
data_layer: filters.dataLayer, // ❌ Not in SimpleEventSchema
|
||||
data_source: filters.dataSource, // ❌ Not in SimpleEventSchema
|
||||
with_owner: filters.hasOwner, // ❌ Not in SimpleEventSchema
|
||||
});
|
||||
};
|
||||
|
||||
// RIGHT - Use only standard SimpleEventSchema fields
|
||||
export const trackFiltersApplied = () => {
|
||||
trackSimpleEvent({
|
||||
event: "filters_applied",
|
||||
});
|
||||
};
|
||||
|
||||
// Or use event_detail for a single variant
|
||||
export const trackFilterApplied = (filterType: string) => {
|
||||
trackSimpleEvent({
|
||||
event: "filter_applied",
|
||||
event_detail: filterType, // ✓ "data_layer", "data_source", etc.
|
||||
});
|
||||
};
|
||||
```
|
||||
|
||||
### Don't: Forget to add event to union type
|
||||
|
||||
```typescript
|
||||
// Define the event
|
||||
export type NewFeatureClickedEvent = ValidateEvent<{
|
||||
event: "new_feature_clicked";
|
||||
}>;
|
||||
|
||||
// ❌ WRONG - Forgot to add to SimpleEvent union
|
||||
// Event won't be recognized by TypeScript
|
||||
|
||||
// ✓ RIGHT - Add to appropriate union
|
||||
export type SimpleEvent =
|
||||
| /* other events */
|
||||
| NewFeatureClickedEvent;
|
||||
```
|
||||
|
||||
### Don't: Mix up event name formats
|
||||
|
||||
```typescript
|
||||
// WRONG
|
||||
event: "dataStudioLibraryCreated" // camelCase
|
||||
event: "data-studio-library-created" // kebab-case
|
||||
event: "Data_Studio_Library_Created" // Mixed case
|
||||
|
||||
// RIGHT
|
||||
event: "data_studio_library_created" // snake_case
|
||||
```
|
||||
|
||||
### Don't: Track PII or sensitive data
|
||||
|
||||
```typescript
|
||||
// WRONG - Don't track user emails, names, or sensitive data
|
||||
trackSimpleEvent({
|
||||
event: "user_logged_in",
|
||||
event_detail: user.email, // ❌ PII
|
||||
});
|
||||
|
||||
// RIGHT - Track non-sensitive identifiers only
|
||||
trackSimpleEvent({
|
||||
event: "user_logged_in",
|
||||
target_id: user.id, // ✓ Just the ID
|
||||
});
|
||||
```
|
||||
|
||||
### Don't: Forget to track both success and failure
|
||||
|
||||
```typescript
|
||||
// WRONG - Only tracking success
|
||||
try {
|
||||
await saveData();
|
||||
trackDataSaved();
|
||||
} catch (error) {
|
||||
// ❌ No tracking for failure case
|
||||
}
|
||||
|
||||
// RIGHT - Track both outcomes
|
||||
try {
|
||||
await saveData();
|
||||
trackDataSaved({ result: "success" });
|
||||
} catch (error) {
|
||||
trackDataSaved({ result: "failure" });
|
||||
}
|
||||
```
|
||||
|
||||
## Testing Analytics Events
|
||||
|
||||
While developing, you can verify events are firing:
|
||||
|
||||
1. **Check browser console** - When `SNOWPLOW_ENABLED=true` in dev, events are logged
|
||||
2. **Use shouldLogAnalytics** - Set in `metabase/env` to see all analytics in console
|
||||
3. **Check Snowplow debugger** - Browser extension for Snowplow events
|
||||
|
||||
Example console output:
|
||||
|
||||
```
|
||||
[SNOWPLOW EVENT | event sent:true], data_studio_table_picker_filters_applied
|
||||
```
|
||||
|
||||
## File Organization
|
||||
|
||||
### Where to put tracking functions:
|
||||
|
||||
```
|
||||
Feature-specific analytics functions:
|
||||
frontend/src/metabase/{feature}/analytics.ts
|
||||
enterprise/frontend/src/metabase-enterprise/{feature}/analytics.ts
|
||||
|
||||
Event type definitions (all in one place):
|
||||
frontend/src/metabase-types/analytics/event.ts
|
||||
|
||||
Core tracking utilities:
|
||||
frontend/src/metabase/lib/analytics.ts
|
||||
```
|
||||
|
||||
## Real-World Examples
|
||||
|
||||
See these files for reference:
|
||||
|
||||
- **Simple events**: `enterprise/frontend/src/metabase-enterprise/data-studio/analytics.ts`
|
||||
- **Events with variants**: `frontend/src/metabase/dashboard/analytics.ts`
|
||||
- **Complex events**: `frontend/src/metabase/query_builder/analytics.js`
|
||||
- **Event type examples**: `frontend/src/metabase-types/analytics/event.ts`
|
||||
|
||||
## Workflow Summary
|
||||
|
||||
1. **Identify the user interaction** to track
|
||||
2. **Decide on event name** (snake_case, descriptive)
|
||||
3. **Define event type** in `event.ts` using `ValidateEvent`
|
||||
4. **Add to union type** (create feature union if needed)
|
||||
5. **Create tracking function** in feature's `analytics.ts`
|
||||
6. **Import and call** at the interaction point
|
||||
7. **Test** that events fire correctly
|
||||
|
||||
## Tips
|
||||
|
||||
- **Be specific** - `filters_applied` is better than `action_performed`
|
||||
- **Use past tense** - `library_created` not `create_library`
|
||||
- **Group related events** - Create feature-specific event union types
|
||||
- **Track meaningful actions** - Not every click needs tracking
|
||||
- **Consider the data** - What would you want to analyze later?
|
||||
- **Stay consistent** - Follow existing naming patterns in the codebase
|
||||
- **Document context** - Use `triggered_from` to track where the action happened
|
||||
@@ -1,144 +0,0 @@
|
||||
---
|
||||
name: clojure-review
|
||||
description: Review Clojure and ClojureScript code changes for compliance with Metabase coding standards, style violations, and code quality issues. Use when reviewing pull requests or diffs containing Clojure/ClojureScript code.
|
||||
allowed-tools: Read, Grep, Bash, Glob
|
||||
---
|
||||
|
||||
# Clojure Code Review Skill
|
||||
|
||||
@./../_shared/clojure-style-guide.md
|
||||
@./../_shared/clojure-commands.md
|
||||
|
||||
## Review guidelines
|
||||
|
||||
**What to flag:**
|
||||
|
||||
- Check compliance with the Metabase Clojure style guide (included above)
|
||||
- If `CLOJURE_STYLE_GUIDE.adoc` exists in the working directory, also check compliance with the community Clojure style guide
|
||||
- Flag all style guide violations
|
||||
|
||||
**What NOT to post:**
|
||||
|
||||
- Do not post comments congratulating someone for trivial changes or for following style guidelines
|
||||
- Do not post comments confirming things "look good" or telling them they did something correctly
|
||||
- Only post comments about style violations or potential issues
|
||||
|
||||
Example bad code review comments to avoid:
|
||||
|
||||
> This TODO comment is properly formatted with author and date - nice work!
|
||||
|
||||
> Good addition of limit 1 to the query - this makes the test more efficient without changing its behavior.
|
||||
|
||||
> The kondo ignore comment is appropriately placed here
|
||||
|
||||
> Test name properly ends with -test as required by the style guide.
|
||||
|
||||
**Special cases:**
|
||||
|
||||
- Do not post comments about missing parentheses (these will be caught by the linter)
|
||||
|
||||
## Quick review checklist
|
||||
|
||||
Use this to scan through changes efficiently:
|
||||
|
||||
### Naming
|
||||
|
||||
- [ ] Descriptive names (no `tbl`, `zs'`)
|
||||
- [ ] Pure functions named as nouns describing their return value
|
||||
- [ ] `kebab-case` for all variables and functions
|
||||
- [ ] Side-effect functions end with `!`
|
||||
- [ ] No namespace-alias repetition in function names
|
||||
|
||||
### Documentation
|
||||
|
||||
- [ ] Public vars in `src` or `enterprise/backend/src` have useful docstrings
|
||||
- [ ] Docstrings use Markdown conventions
|
||||
- [ ] References use `[[other-var]]` not backticks
|
||||
- [ ] `TODO` comments include author and date: `;; TODO (Name 2025-01-01) -- description`
|
||||
|
||||
### Code Organization
|
||||
|
||||
- [ ] Everything `^:private` unless used elsewhere
|
||||
- [ ] No `declare` when avoidable (public functions near end)
|
||||
- [ ] Functions under 20 lines when possible
|
||||
- [ ] No blank, non-comment lines within definition forms (except pairwise constructs in `let`/`cond`)
|
||||
- [ ] Lines ≤ 120 characters
|
||||
|
||||
### Tests
|
||||
|
||||
- [ ] Separate `deftest` forms for distinct test cases
|
||||
- [ ] Pure tests marked `^:parallel`
|
||||
- [ ] Test names end in `-test` or `-test-<number>`
|
||||
|
||||
### Modules
|
||||
|
||||
- [ ] Correct module patterns (OSS: `metabase.<module>.*`, EE: `metabase-enterprise.<module>.*`)
|
||||
- [ ] API endpoints in `<module>.api` namespaces
|
||||
- [ ] Public API in `<module>.core` with Potemkin
|
||||
- [ ] No cheating module linters with `:clj-kondo/ignore [:metabase/modules]`
|
||||
|
||||
### REST API
|
||||
|
||||
- [ ] Response schemas present (`:- <schema>`)
|
||||
- [ ] Query params use kebab-case, bodies use `snake_case`
|
||||
- [ ] Routes use singular nouns (e.g., `/api/dashboard/:id`)
|
||||
- [ ] `GET` has no side effects (except analytics)
|
||||
- [ ] Malli schemas detailed and complete
|
||||
- [ ] All new endpoints have tests
|
||||
|
||||
### MBQL
|
||||
|
||||
- [ ] No raw MBQL manipulation outside `lib`, `lib-be`, or `query-processor` modules
|
||||
- [ ] Uses Lib and MBQL 5, not legacy MBQL
|
||||
|
||||
### Database
|
||||
|
||||
- [ ] Model and table names are singular nouns
|
||||
- [ ] Uses `t2/select-one-fn` instead of selecting full rows for one column
|
||||
- [ ] Logic in Toucan methods, not helper functions
|
||||
|
||||
### Drivers
|
||||
|
||||
- [ ] New multimethods documented in `docs/developers-guide/driver-changelog.md`
|
||||
- [ ] Passes `driver` argument to other driver methods (no hardcoded driver names)
|
||||
- [ ] Minimal logic in `read-column-thunk`
|
||||
|
||||
### Miscellaneous
|
||||
|
||||
- [ ] Example data is bird-themed when possible
|
||||
- [ ] Kondo linter suppressions use proper format (not `#_:clj-kondo/ignore` keyword form)
|
||||
|
||||
## Pattern matching table
|
||||
|
||||
Quick scan for common issues:
|
||||
|
||||
| Pattern | Issue |
|
||||
| -------------------------------------------- | ----------------------------------------------------------- |
|
||||
| `calculate-age`, `get-user` | Pure functions should be nouns: `age`, `user` |
|
||||
| `update-db`, `save-model` | Missing `!` for side effects: `update-db!`, `save-model!` |
|
||||
| `snake_case_var` | Should use kebab-case |
|
||||
| Public var without docstring | Add docstring explaining purpose |
|
||||
| `;; TODO fix this` | Missing author/date: `;; TODO (Name 2025-01-01) -- description` |
|
||||
| `(defn foo ...)` in namespace used elsewhere | Should be `(defn ^:private foo ...)` |
|
||||
| Function > 20 lines | Consider breaking up into smaller functions |
|
||||
| `/api/dashboards/:id` | Use singular: `/api/dashboard/:id` |
|
||||
| Query params with `snake_case` | Use kebab-case for query params |
|
||||
| New API endpoint without tests | Add tests for the endpoint |
|
||||
|
||||
## Feedback format examples
|
||||
|
||||
**For style violations:**
|
||||
|
||||
> This pure function should be named as a noun describing its return value. Consider `user` instead of `get-user`.
|
||||
|
||||
**For missing documentation:**
|
||||
|
||||
> This public var needs a docstring explaining its purpose, inputs, and outputs.
|
||||
|
||||
**For organization issues:**
|
||||
|
||||
> This function is only used in this namespace, so it should be marked `^:private`.
|
||||
|
||||
**For API conventions:**
|
||||
|
||||
> Query parameters should use kebab-case. Change `user_id` to `user-id`.
|
||||
@@ -1,69 +0,0 @@
|
||||
---
|
||||
name: clojure-write
|
||||
description: Guide Clojure and ClojureScript development using REPL-driven workflow, coding conventions, and best practices. Use when writing, developing, or refactoring Clojure/ClojureScript code.
|
||||
---
|
||||
|
||||
# Clojure Development Skill
|
||||
|
||||
## Tool Preference
|
||||
|
||||
When `clojure-mcp` tools are available (e.g., `clojure_eval`, `clojure_edit`), **always use them**
|
||||
instead of shell commands like `./bin/mage -repl`. The MCP tools provide:
|
||||
- Direct REPL integration without shell escaping issues
|
||||
- Better error messages and feedback
|
||||
- Structural Clojure editing that prevents syntax errors
|
||||
|
||||
Only fall back to `./bin/mage` commands when clojure-mcp is not available.
|
||||
|
||||
@./../_shared/development-workflow.md
|
||||
@./../_shared/clojure-style-guide.md
|
||||
@./../_shared/clojure-commands.md
|
||||
|
||||
## REPL-Driven Development Workflow
|
||||
|
||||
- Start with small, fundamental functions:
|
||||
- Identify the core features or functionalities required for your task.
|
||||
- Break each feature down into the smallest, most basic functions that can be developed and tested independently.
|
||||
- Write and test in the REPL:
|
||||
- Write the code for each small function directly in the REPL (Read-Eval-Print Loop).
|
||||
- Test it thoroughly with a variety of inputs, including typical use cases and relevant edge cases, to ensure it
|
||||
behaves as expected.
|
||||
- Integrate into source code:
|
||||
- Once a function works correctly in the REPL, move it from the REPL environment into your source code files (e.g.,
|
||||
within appropriate namespaces).
|
||||
- Gradually increase complexity:
|
||||
- Build upon tested, basic functions to create more complex functions or components.
|
||||
- Compose smaller functions together, testing each new composition in the REPL to verify correctness step by step.
|
||||
- Ensure dependency testing:
|
||||
- Make sure every function is fully tested in the REPL before it is depended upon by other functions.
|
||||
- This ensures that each layer of your application is reliable before you build on it.
|
||||
- Use the REPL fully:
|
||||
- Use the REPL as your primary tool to experiment with different approaches, iterate quickly, and get immediate
|
||||
feedback on your code.
|
||||
- Follow functional programming principles:
|
||||
- Keep functions small, focused, and composable.
|
||||
- Use Clojure's functional programming features—like immutability, higher-order functions, and the standard
|
||||
library—to write concise, effective code.
|
||||
|
||||
## How to Evaluate Code
|
||||
|
||||
### Bottom-up Dev Loop
|
||||
|
||||
1. Write code into a file.
|
||||
2. Evaluate the file's namespace and make sure it loads correctly with:
|
||||
|
||||
```
|
||||
./bin/mage -repl --namespace metabase.app-db.connection
|
||||
```
|
||||
|
||||
3. Call functions in the namespace with test inputs, and observe that the outputs are correct
|
||||
Feel free to copy these REPL session trials into actual test cases using `deftest` and `is`.
|
||||
4. Once you know these functions are good, return to 1, and compose them into the task that you need to build.
|
||||
|
||||
## Critical Rules for Editing
|
||||
|
||||
- Be careful with parentheses counts when editing Clojure code
|
||||
- After EVERY change to Clojure code, verify readability with `-check-readable`
|
||||
- End all files with a newline
|
||||
- When editing tabular code, where the columns line up, try to keep them aligned
|
||||
- Spaces on a line with nothing after it is not allowed
|
||||
@@ -1,163 +0,0 @@
|
||||
---
|
||||
name: docs-review
|
||||
description: Review documentation changes for compliance with the Metabase writing style guide. Use when reviewing pull requests, files, or diffs containing documentation markdown files.
|
||||
allowed-tools: Read, Grep, Bash, Glob
|
||||
---
|
||||
|
||||
# Documentation Review Skill
|
||||
|
||||
@./../_shared/metabase-style-guide.md
|
||||
|
||||
## Review mode detection
|
||||
|
||||
**IMPORTANT: Before starting the review, determine which mode to use:**
|
||||
|
||||
1. **PR review mode**: If the `mcp__github__create_pending_pull_request_review` tool is available, you are reviewing a GitHub PR
|
||||
- Use the pending review workflow to post all issues as one cohesive review
|
||||
- Follow the workflow steps in "PR review mode format" below
|
||||
|
||||
2. **Local review mode**: If the MCP tool is NOT available, output issues in the conversation
|
||||
- Format all issues in a numbered markdown list (as described in "Feedback format" below)
|
||||
|
||||
## Review process
|
||||
|
||||
1. **Detect review mode** - Check if `mcp__github__create_pending_pull_request_review` is available
|
||||
2. Read the changes through once to understand intent
|
||||
3. Check all issues that violate style guide or significantly impact readability
|
||||
4. Only flag issues worth mentioning - if it won't make a material difference to the reader, skip it
|
||||
5. **REQUIRED: Number ALL feedback sequentially** - Start from Issue 1 and increment for each issue found
|
||||
|
||||
## Review checklist
|
||||
|
||||
Run through the diff looking for these issues:
|
||||
|
||||
**Tone and voice:**
|
||||
|
||||
- [ ] Formal/corporate language ("utilize" not "use", "offerings", etc.)
|
||||
- [ ] "Users" instead of "people" or "companies"
|
||||
- [ ] Excessive exclamation points or overly peppy tone
|
||||
- [ ] Telling readers something is cool instead of showing them
|
||||
|
||||
**Structure and clarity:**
|
||||
|
||||
- [ ] Important information buried instead of leading
|
||||
- [ ] Verbose text that adds little value
|
||||
- [ ] Paragraphs without clear purpose
|
||||
- [ ] Vague headings that don't convey the point
|
||||
- [ ] Instructions explain "why" before telling "what to do"
|
||||
- [ ] Tasks described as "easy" or "simple"
|
||||
|
||||
**Links and references:**
|
||||
|
||||
- [ ] Linking the word "here" instead of descriptive text
|
||||
- [ ] Links in headings (unless entire heading is a link)
|
||||
|
||||
**Formatting:**
|
||||
|
||||
- [ ] Ampersands as "and" substitute (except proper nouns)
|
||||
- [ ] Inconsistent list formatting
|
||||
|
||||
**Code and examples:**
|
||||
|
||||
- [ ] Code examples that don't work or would error
|
||||
- [ ] Commands not in execution order
|
||||
- [ ] Full-width screenshots instead of scoped UI elements
|
||||
- [ ] Excessive or unnecessary images
|
||||
|
||||
**Sentence construction:**
|
||||
|
||||
- [ ] Overuse of pronouns when introducing new terms
|
||||
|
||||
## Quick scan table
|
||||
|
||||
| Pattern | Issue |
|
||||
| ----------------------------- | --------------------------------------------- |
|
||||
| we can do X, our feature | Should be "Metabase" or "it" |
|
||||
| click here, read more here | Need descriptive link text |
|
||||
| easy, simple, just | Remove condescending qualifiers |
|
||||
| users | Should be "people" or "companies" if possible |
|
||||
|
||||
## Feedback format
|
||||
|
||||
**MANDATORY REQUIREMENT: Every single issue MUST be numbered sequentially starting from Issue 1.**
|
||||
|
||||
This numbered format is NON-NEGOTIABLE. It allows users to efficiently reference specific issues (e.g., "fix issues 1, 3, and 5") and track which feedback has been addressed.
|
||||
|
||||
### Local review mode format
|
||||
|
||||
When outputting issues in the conversation (local mode), use this format:
|
||||
|
||||
```markdown
|
||||
## Issues
|
||||
|
||||
**Issue 1: [Brief title]**
|
||||
Line X: Succinct description of the issue
|
||||
[code or example]
|
||||
Suggested fix or succinct explanation
|
||||
|
||||
**Issue 2: [Brief title]**
|
||||
Line Y: Description of the issue
|
||||
Suggested fix or explanation
|
||||
|
||||
**Issue 3: [Brief title]**
|
||||
...
|
||||
```
|
||||
|
||||
**Examples:**
|
||||
|
||||
> **Issue 1: Formal tone**
|
||||
> Line 15: This could be more conversational. Consider: "You can't..." instead of "You cannot..."
|
||||
|
||||
> **Issue 2: Vague heading**
|
||||
> Line 8: The heading could be more specific. Try stating the point directly: "Run migrations before upgrading" vs "Upgrade process"
|
||||
|
||||
### PR review mode format
|
||||
|
||||
When posting to GitHub (PR mode), use the **pending review workflow**:
|
||||
|
||||
**Workflow steps:**
|
||||
|
||||
1. **Start a review**: Use `mcp__github__create_pending_pull_request_review` to begin a pending review
|
||||
- This creates a draft review that won't be visible until submitted
|
||||
|
||||
2. **Get diff information**: Use `mcp__github__get_pull_request_diff` to understand the code changes and line numbers
|
||||
- This helps you determine the correct file paths and line numbers for comments
|
||||
|
||||
3. **Identify ALL issues**: Read through all changes and identify every issue worth mentioning
|
||||
- Collect all issues before posting any comments
|
||||
- Number them sequentially (Issue 1, Issue 2, Issue 3, etc.)
|
||||
|
||||
4. **Add review comments**: Use `mcp__github__add_pull_request_review_comment_to_pending_review` for each issue
|
||||
- **CRITICAL**: Post ALL comments in a SINGLE response using multiple tool calls in parallel
|
||||
- Each comment should reference a specific file path and line number from the diff
|
||||
- Start each comment body with `**Issue N: [Brief title]**`
|
||||
- Include the description and suggested fix
|
||||
|
||||
5. **Submit the review**: Use `mcp__github__submit_pending_pull_request_review` to publish all comments at once
|
||||
- Use event type `"COMMENT"` (NOT "REQUEST_CHANGES") to make it non-blocking
|
||||
- **Do NOT include a body message** - Leave the body empty or omit it entirely
|
||||
- All comments will appear together as one cohesive review
|
||||
|
||||
**Comment format example:**
|
||||
|
||||
```
|
||||
**Issue 1: Formal tone**
|
||||
|
||||
This could be more conversational. Consider: "You can't..." instead of "You cannot..."
|
||||
```
|
||||
|
||||
**IMPORTANT**:
|
||||
- Each issue gets its own review comment attached to the pending review
|
||||
- Number ALL comments sequentially (Issue 1, Issue 2, Issue 3, etc.)
|
||||
- Always start the comment body with `**Issue N: [Brief title]**`
|
||||
- **MUST add all comments in parallel in a single response** - Do NOT add them one after another in separate responses
|
||||
- Do NOT output a summary message to the conversation - only post GitHub review comments
|
||||
- When submitting the review, do NOT include a body parameter (or leave it empty) to avoid cluttering the PR with summary text
|
||||
- The review will appear as a single review with multiple comments when submitted
|
||||
|
||||
## Final check
|
||||
|
||||
1. Remove any issues from your assessment that won't make a material difference to the reader if addressed. Only flag issues worth the author's time to fix.
|
||||
2. **Verify all issues are numbered sequentially** starting from Issue 1 with no gaps in numbering.
|
||||
3. Confirm the format exactly matches: `**Issue N: [Brief title]**` where N is the issue number.
|
||||
4. **In PR mode**: Verify each issue was posted as a separate GitHub comment (not output to conversation).
|
||||
@@ -1,96 +0,0 @@
|
||||
---
|
||||
name: docs-write
|
||||
description: Write documentation following Metabase's conversational, clear, and user-focused style. Use when creating or editing documentation files (markdown, MDX, etc.).
|
||||
allowed-tools: Read, Write, Grep, Bash, Glob
|
||||
---
|
||||
|
||||
# Documentation Writing Skill
|
||||
|
||||
@./../_shared/metabase-style-guide.md
|
||||
|
||||
## When writing documentation
|
||||
|
||||
### Start here
|
||||
|
||||
1. **Who is this for?** Match complexity to audience. Don't oversimplify hard things or overcomplicate simple ones.
|
||||
2. **What do they need?** Get them to the answer fast. Nobody wants to be in docs longer than necessary.
|
||||
3. **What did you struggle with?** Those common questions you had when learning? Answer them (without literally including the question).
|
||||
|
||||
### Writing process
|
||||
|
||||
**Draft:**
|
||||
|
||||
- Write out the steps/explanation as you'd tell a colleague
|
||||
- Lead with what to do, then explain why
|
||||
- Use headings that state your point: "Set SAML before adding users" not "SAML configuration timing"
|
||||
|
||||
**Edit:**
|
||||
|
||||
- Read aloud. Does it sound like you talking? If it's too formal, simplify.
|
||||
- Cut anything that doesn't directly help the reader
|
||||
- Check each paragraph has one clear purpose
|
||||
- Verify examples actually work (don't give examples that error)
|
||||
|
||||
**Polish:**
|
||||
|
||||
- Make links descriptive (never "here")
|
||||
- Backticks only for code/variables, **bold** for UI elements
|
||||
- American spelling, serial commas
|
||||
- Keep images minimal and scoped tight
|
||||
|
||||
**Format:**
|
||||
|
||||
- Run prettier on the file after making edits: `bun run prettier --write <file-path>`
|
||||
- This ensures consistent formatting across all documentation
|
||||
|
||||
### Common patterns
|
||||
|
||||
**Instructions:**
|
||||
|
||||
```markdown
|
||||
Run:
|
||||
\`\`\`
|
||||
command-to-run
|
||||
\`\`\`
|
||||
|
||||
Then:
|
||||
\`\`\`
|
||||
next-command
|
||||
\`\`\`
|
||||
|
||||
This ensures you're getting the latest changes.
|
||||
```
|
||||
|
||||
Not: "(remember to run X before Y...)" buried in a paragraph.
|
||||
|
||||
**Headings:**
|
||||
|
||||
- "Use environment variables for configuration" ✅
|
||||
- "Environment variables" ❌ (too vague)
|
||||
- "How to use environment variables for configuration" ❌ (too wordy)
|
||||
|
||||
**Links:**
|
||||
|
||||
- "Check out the [SAML documentation](link)" ✅
|
||||
- "Read the docs [here](link)" ❌
|
||||
|
||||
### Watch out for
|
||||
|
||||
- Describing tasks as "easy" (you don't know the reader's context)
|
||||
- Using "we" when talking about Metabase features (use "Metabase" or "it")
|
||||
- Formal language: "utilize", "reference", "offerings"
|
||||
- Too peppy: multiple exclamation points
|
||||
- Burying the action in explanation
|
||||
- Code examples that don't work
|
||||
- Numbers that will become outdated
|
||||
|
||||
### Quick reference
|
||||
|
||||
| Write This | Not This |
|
||||
| -------------------------- | ------------------ |
|
||||
| people, companies | users |
|
||||
| summarize | aggregate |
|
||||
| take a look at | reference |
|
||||
| can't, don't | cannot, do not |
|
||||
| **Filter** button | \`Filter\` button |
|
||||
| Check out [the docs](link) | Click [here](link) |
|
||||
@@ -1,260 +0,0 @@
|
||||
---
|
||||
name: e2e-test-create
|
||||
description: >
|
||||
Analyze React component source code to understand UI structure,
|
||||
then generate idiomatic Cypress E2E tests following Metabase conventions.
|
||||
Falls back to Playwright MCP browser exploration only when code reading
|
||||
and screenshot debugging are insufficient.
|
||||
disable-model-invocation: true
|
||||
allowed-tools:
|
||||
- Bash
|
||||
- Read
|
||||
- Write
|
||||
- Grep
|
||||
- Glob
|
||||
- Skill
|
||||
- "mcp__playwright__*"
|
||||
---
|
||||
|
||||
# Code-Reading-First → Generate Cypress Tests (Metabase)
|
||||
|
||||
You are writing Cypress E2E tests for the **Metabase** codebase.
|
||||
Before generating ANY test code, you MUST analyze React component source code
|
||||
to understand DOM structure, selectors, and user flows.
|
||||
|
||||
## Phase 0 — Research
|
||||
|
||||
1. Read existing helpers before writing anything:
|
||||
- `e2e/support/helpers/` — all shared helpers (restore, signInAs, openOrdersTable, etc.)
|
||||
- `e2e/support/cypress_sample_database.ts` — table/field schema constants (ORDERS, PRODUCTS, etc.)
|
||||
- `e2e/support/cypress_sample_instance_data.ts` — instance-specific IDs (ORDERS_DASHBOARD_ID, NORMAL_USER_ID, etc.)
|
||||
2. Glob `e2e/test/scenarios/` to find the closest existing spec to the area under test.
|
||||
Study its patterns — match them exactly.
|
||||
3. Glob `frontend/src/metabase/` to find React components for the feature area.
|
||||
|
||||
## Phase 1 — Code Analysis
|
||||
|
||||
Read React component source to understand DOM structure. No browser needed — source code has everything.
|
||||
|
||||
1. **Find relevant components**: Glob and grep `frontend/src/metabase/` for the feature area.
|
||||
2. **Extract selectors**: Grep for `data-testid` in relevant components.
|
||||
3. **Note visible text**: Read component JSX for button labels, headings, placeholders.
|
||||
4. **Note aria attributes**: Grep for `aria-label` in relevant components.
|
||||
5. **Understand user flows**: Read event handlers (onClick, onSubmit, onChange) to understand interactions.
|
||||
6. **Find API calls**: Grep for `Api.use`, `fetch`, `useQuery`, endpoint definitions to identify API calls to intercept.
|
||||
7. **Cross-reference with existing specs**: Find specs in the same area and reuse their proven selectors and `cy.intercept` patterns.
|
||||
|
||||
## Phase 2 — Start Backend
|
||||
|
||||
Use `MB_EDITION=oss` by default. Only use `MB_EDITION=ee` when the user explicitly asks to write an enterprise test.
|
||||
|
||||
Start the backend using `run_in_background: true` (NOT `&`).
|
||||
`bin/e2e-backend` automatically detects if a backend is already running and reuses it.
|
||||
```bash
|
||||
MB_EDITION=oss bin/e2e-backend
|
||||
```
|
||||
|
||||
Do NOT manually generate snapshots by running unrelated test specs.
|
||||
The `bun test-cypress` runner has `GENERATE_SNAPSHOTS: true` by default and automatically
|
||||
generates snapshots before running any spec. When running tests in Phase 4 via the `/e2e-test` skill,
|
||||
snapshots will be generated on the first run if they don't already exist.
|
||||
|
||||
Restore clean test data:
|
||||
```bash
|
||||
curl -sf -X POST http://localhost:4000/api/testing/restore/default
|
||||
```
|
||||
|
||||
## Phase 3 — Generate Cypress Spec
|
||||
|
||||
### File location & naming
|
||||
|
||||
Place specs in `e2e/test/scenarios/<area>/` mirroring the URL structure.
|
||||
Name: `<feature>.cy.spec.js` (NOT `.cy.spec.ts`).
|
||||
|
||||
### Metabase conventions (mandatory)
|
||||
|
||||
- **Helpers via `cy.H`**: All helpers are accessed via `const { H } = cy;` — NOT via direct imports from `e2e/support/helpers`.
|
||||
|
||||
```js
|
||||
const { H } = cy;
|
||||
|
||||
describe("feature name", () => {
|
||||
beforeEach(() => {
|
||||
H.restore();
|
||||
cy.signInAsAdmin();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
- **Sample Database schema** (table/field definitions): Import from `cypress_sample_database`.
|
||||
|
||||
```js
|
||||
import { ORDERS, ORDERS_ID, PRODUCTS } from "e2e/support/cypress_sample_database";
|
||||
```
|
||||
|
||||
- **Instance data** (dashboard IDs, user IDs, etc.): Import from `cypress_sample_instance_data`.
|
||||
|
||||
```js
|
||||
import { ORDERS_DASHBOARD_ID } from "e2e/support/cypress_sample_instance_data";
|
||||
```
|
||||
|
||||
- **Selectors** (priority order):
|
||||
1. `cy.findByText()` / `cy.findByLabelText()` / `cy.findByRole()` — from `@testing-library/cypress`
|
||||
2. `cy.findByTestId()` — for `data-testid` attributes
|
||||
3. `cy.get("[data-testid='...']")` — fallback
|
||||
4. NEVER use positional selectors, CSS class names, or XPaths.
|
||||
|
||||
- **Navigation helpers**: Use existing helpers like `H.openOrdersTable()`, `H.openNativeEditor()`,
|
||||
`H.visitDashboard(id)`, `H.visitQuestion(id)` instead of raw `cy.visit()` chains.
|
||||
Grep `e2e/support/helpers/` to discover what's available for your area.
|
||||
|
||||
- **API setup over UI setup**: Use `cy.request()` or existing API helpers to set up state.
|
||||
Only use the UI for the flow you're actually testing.
|
||||
|
||||
- **Assertions**: Assert on visible text, URL, aria state — not DOM structure.
|
||||
|
||||
- **Waits**: Never use `cy.wait(ms)`. Use `cy.intercept()` + `cy.wait("@alias")` for API calls,
|
||||
or `cy.findByText().should("be.visible")` for DOM readiness.
|
||||
|
||||
- **Isolation**: Each `it()` block must be independently runnable.
|
||||
Don't depend on state from a previous `it()`.
|
||||
|
||||
### Spec structure template
|
||||
|
||||
```js
|
||||
const { H } = cy;
|
||||
import { ORDERS_DASHBOARD_ID } from "e2e/support/cypress_sample_instance_data";
|
||||
import { ORDERS, ORDERS_ID } from "e2e/support/cypress_sample_database";
|
||||
|
||||
describe("area > sub-area > feature (#issue-number)", () => {
|
||||
beforeEach(() => {
|
||||
H.restore();
|
||||
cy.signInAsAdmin();
|
||||
});
|
||||
|
||||
it("should do the primary happy-path thing", () => {
|
||||
// test
|
||||
});
|
||||
|
||||
it("should handle the edge case", () => {
|
||||
// test
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### Intercepts
|
||||
|
||||
When you identified API calls during code analysis, stub or wait on them:
|
||||
|
||||
```js
|
||||
cy.intercept("POST", "/api/dataset").as("dataset");
|
||||
// ... trigger action ...
|
||||
cy.wait("@dataset");
|
||||
```
|
||||
|
||||
## Phase 4 — Validate
|
||||
|
||||
After generating specs:
|
||||
1. Check that all imported helpers exist (Grep `e2e/support/helpers/`).
|
||||
2. **You MUST use the `/e2e-test` skill** to run tests — do NOT run `bun test-cypress` directly.
|
||||
The `/e2e-test` skill handles edition selection, snapshot management, and correct env vars.
|
||||
`/e2e-test GREP="should do the thing" --spec e2e/test/scenarios/<path>`
|
||||
If you created multiple `it()` blocks, run each one individually to isolate failures.
|
||||
|
||||
## Phase 5 — Fix Failures (up to 2 attempts)
|
||||
|
||||
When a test fails, **try to fix it from Cypress output first**:
|
||||
1. Read the failure screenshot (path printed under `(Screenshots)`).
|
||||
2. Read the error message and code frame from the console output.
|
||||
3. Fix the test and re-run (back to Phase 4, step 2).
|
||||
|
||||
If you cannot diagnose the issue after 2 attempts, proceed to Phase 6.
|
||||
|
||||
## Phase 6 — Playwright Fallback
|
||||
|
||||
Only reach this phase after 2 failed fix attempts from Phase 5. The backend is already running.
|
||||
|
||||
Restore clean test data:
|
||||
```bash
|
||||
curl -sf -X POST http://localhost:4000/api/testing/restore/default
|
||||
```
|
||||
|
||||
**Bypass CSP** headers before navigating (Metabase serves strict CSP that blocks dev server scripts).
|
||||
Use `browser_run_code` to set this up:
|
||||
|
||||
```js
|
||||
async (page) => {
|
||||
// Strip CSP headers so the page loads (mirrors Cypress chromeWebSecurity: false)
|
||||
await page.context().route('**/*', async (route) => {
|
||||
const response = await route.fetch();
|
||||
const headers = { ...response.headers() };
|
||||
delete headers['content-security-policy'];
|
||||
delete headers['content-security-policy-report-only'];
|
||||
await route.fulfill({ response, headers });
|
||||
});
|
||||
|
||||
// Sign in via API
|
||||
const response = await page.request.post('http://localhost:4000/api/session', {
|
||||
data: { username: 'admin@metabase.test', password: '12341234' }
|
||||
});
|
||||
const session = await response.json();
|
||||
await page.context().addCookies([{
|
||||
name: 'metabase.DEVICE',
|
||||
value: session.id,
|
||||
domain: 'localhost',
|
||||
path: '/'
|
||||
}]);
|
||||
|
||||
await page.goto('http://localhost:4000');
|
||||
await page.waitForLoadState('networkidle');
|
||||
return 'signed in';
|
||||
}
|
||||
```
|
||||
|
||||
**Maintain an observation log incrementally.** After EVERY significant Playwright interaction,
|
||||
IMMEDIATELY append what you observed to the scratch file BEFORE performing the next interaction:
|
||||
|
||||
```bash
|
||||
cat >> /tmp/e2e-observations.md << 'OBSERVATION'
|
||||
## [Page/Flow name]
|
||||
- URL: /question/notebook#...
|
||||
- Clicked: "Box plot" button → visible text "Box plot", role: radio
|
||||
- Selectors: data-testid="viz-type-button", findByText("Box plot")
|
||||
- API call: POST /api/dataset (triggered on viz change)
|
||||
- Key state: after selecting viz type, summary sidebar shows metric picker
|
||||
OBSERVATION
|
||||
```
|
||||
|
||||
For each page/flow:
|
||||
- Take an accessibility snapshot (`browser_snapshot`).
|
||||
- Click through interactive elements, fill forms, trigger modals.
|
||||
- **Append to the observation log immediately after each step**: URLs, visible text, aria labels, `data-testid` attrs, API calls.
|
||||
- Screenshot key states.
|
||||
|
||||
After exploration:
|
||||
1. Read back your observation log: `cat /tmp/e2e-observations.md`
|
||||
2. Fix the test using observed selectors and behavior.
|
||||
3. Re-run the test (back to Phase 4).
|
||||
4. Clean up: `rm -f /tmp/e2e-observations.md`
|
||||
|
||||
## Phase 7 — Cleanup
|
||||
|
||||
After all tests pass (or after giving up on fixing failures), **always kill the backend on port 4000**:
|
||||
```bash
|
||||
lsof -ti:4000 | xargs kill 2>/dev/null || true
|
||||
```
|
||||
|
||||
Do NOT use broad `pkill` patterns — there may be other Metabase instances on different ports.
|
||||
The backend process started in Phase 2 will NOT be killed automatically when the Claude session ends.
|
||||
Leaving it running wastes resources and can interfere with future sessions. Always clean up.
|
||||
|
||||
## What NOT to do
|
||||
|
||||
- Do NOT use Playwright as the first step — always analyze source code first.
|
||||
- Do NOT kill the backend between phases — it stays running throughout.
|
||||
- Do NOT invent selectors you didn't find in source code or observe in the browser.
|
||||
- Do NOT hardcode database IDs — import from `cypress_sample_database` or `cypress_sample_instance_data`.
|
||||
- Do NOT use `cy.wait(1000)` or any numeric wait.
|
||||
- Do NOT create setup flows through the UI when an API helper exists.
|
||||
- Do NOT put tests in `cypress/e2e/` — Metabase uses `e2e/test/scenarios/`.
|
||||
- Do NOT import helpers directly from `e2e/support/helpers` — use `const { H } = cy;`.
|
||||
@@ -1,78 +0,0 @@
|
||||
---
|
||||
name: e2e-test
|
||||
description: Run Cypress E2E tests, analyze failures including screenshots, and stress test for flakiness
|
||||
disable-model-invocation: false
|
||||
---
|
||||
|
||||
Run Cypress E2E test: $ARGUMENTS
|
||||
|
||||
## Edition selection
|
||||
|
||||
**Default: `MB_EDITION=oss`** — runs without enterprise tokens, simpler and faster.
|
||||
|
||||
Only use `MB_EDITION=ee` when the user explicitly asks to write or run an enterprise test.
|
||||
|
||||
**For `MB_EDITION=ee`:**
|
||||
These tokens must be exported in the shell before starting Claude Code:
|
||||
- `CYPRESS_MB_ALL_FEATURES_TOKEN` — all features (most EE tests need this)
|
||||
- `CYPRESS_MB_PRO_SELF_HOSTED_TOKEN` — pro self-hosted (permissions, SSO, sandboxing)
|
||||
- `CYPRESS_MB_STARTER_CLOUD_TOKEN` — starter plan (rarely needed)
|
||||
- `CYPRESS_MB_PRO_CLOUD_TOKEN` — pro cloud (rarely needed)
|
||||
|
||||
**NEVER echo, print, or log token values. Only check if they are set:**
|
||||
```bash
|
||||
echo "ALL_FEATURES: ${CYPRESS_MB_ALL_FEATURES_TOKEN:+set}" && echo "PRO_SELF_HOSTED: ${CYPRESS_MB_PRO_SELF_HOSTED_TOKEN:+set}" && echo "STARTER: ${CYPRESS_MB_STARTER_CLOUD_TOKEN:+set}" && echo "PRO_CLOUD: ${CYPRESS_MB_PRO_CLOUD_TOKEN:+set}"
|
||||
```
|
||||
|
||||
If tokens are missing, tell the user: "EE tokens are not set. Either export them and restart Claude Code, or add `MB_EDITION=oss` to run OSS-only tests."
|
||||
|
||||
## Running
|
||||
|
||||
First check if snapshots exist:
|
||||
```bash
|
||||
ls e2e/snapshots/default.sql 2>/dev/null && echo "snapshots exist" || echo "snapshots missing"
|
||||
```
|
||||
|
||||
If snapshots exist, skip regeneration for speed:
|
||||
```bash
|
||||
MB_EDITION=oss CYPRESS_VIDEO=false CYPRESS_RETRIES=0 CYPRESS_GUI=false GENERATE_SNAPSHOTS=false bun test-cypress $ARGUMENTS
|
||||
```
|
||||
|
||||
If snapshots are missing (first run), let the runner generate them:
|
||||
```bash
|
||||
MB_EDITION=oss CYPRESS_VIDEO=false CYPRESS_RETRIES=0 CYPRESS_GUI=false bun test-cypress $ARGUMENTS
|
||||
```
|
||||
|
||||
When running enterprise tests, replace `MB_EDITION=oss` with `MB_EDITION=ee` in the commands above.
|
||||
|
||||
If `$ARGUMENTS` is empty, ask the user which spec to run.
|
||||
|
||||
If the user provides a test name or fuzzy name instead of a spec path, find the spec file:
|
||||
```bash
|
||||
find e2e/test/scenarios -name "*FUZZY_NAME*.cy.spec.*" -type f
|
||||
```
|
||||
Then run with `--spec "path/to/matched.cy.spec.js"`.
|
||||
|
||||
To filter by test name, use the `GREP` env var (not `--env grep=`, which breaks on commas):
|
||||
```bash
|
||||
GREP="test name here" MB_EDITION=oss bun test-cypress --spec path/to/spec.js
|
||||
```
|
||||
|
||||
## Stress test (flaky detection)
|
||||
|
||||
When the user asks to verify a test is not flaky:
|
||||
|
||||
```bash
|
||||
MB_EDITION=oss bin/e2e-stress-test $ARGUMENTS
|
||||
```
|
||||
|
||||
Set `E2E_STRESS_RUNS=N` for more iterations (default: 5). Stops on first failure and prints screenshot paths. Read those screenshots to analyze the failure.
|
||||
|
||||
## Analyzing failures
|
||||
|
||||
When tests fail:
|
||||
|
||||
1. Read the **screenshot** for each failure — paths are printed in Cypress output under `(Screenshots)`, typically at `cypress/screenshots/<spec-name>/<test-name> (failed).png`
|
||||
2. Read the error message and code frame from the console output
|
||||
3. Identify the root cause: is it a test bug, a product bug, or a timing issue?
|
||||
4. Suggest a fix with specific code changes
|
||||
@@ -1,114 +0,0 @@
|
||||
---
|
||||
name: mutation-testing
|
||||
description: Run mutation testing on a Clojure namespace, generate tests to kill surviving mutations, and open draft PRs with Linear issue tracking.
|
||||
---
|
||||
|
||||
# Mutation Testing Skill
|
||||
|
||||
This skill runs mutation testing end-to-end: generates a coverage report, groups functions, shells out to `claude -p` to write tests, verifies mutations are killed, and creates draft PRs with Linear issues.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- A running nREPL connected to the Metabase dev environment
|
||||
- `LINEAR_API_KEY` environment variable set with a valid Linear personal API key
|
||||
- `gh` CLI authenticated with GitHub
|
||||
- `claude` CLI available on PATH
|
||||
|
||||
## Reference Files
|
||||
|
||||
- `dev/src/dev/coverage.clj` — mutation testing engine (generates reports, runs mutations)
|
||||
- `dev/src/dev/mutation_testing.clj` — orchestration, Linear API, PR helpers
|
||||
|
||||
## Invocation
|
||||
|
||||
The argument is a Clojure namespace, optionally followed by a base branch:
|
||||
|
||||
```
|
||||
/mutation-testing metabase.lib.order-by
|
||||
/mutation-testing metabase.lib.order-by --base-branch release-x.52.x
|
||||
/mutation-testing metabase.lib.order-by --project-id abc-123
|
||||
/mutation-testing metabase.lib.order-by --base-branch release-x.52.x --project-id abc-123
|
||||
```
|
||||
|
||||
- `--base-branch`: defaults to the value in config (set via `set-config!`), or `"master"` if not configured.
|
||||
- `--project-id`: if provided, issues are added to this existing Linear project instead of creating a new one.
|
||||
|
||||
## Steps
|
||||
|
||||
### 1. Parse arguments
|
||||
|
||||
Parse `$ARGUMENTS` to extract:
|
||||
- **namespace** (required) — the first positional argument
|
||||
- **`--base-branch`** (optional) — if present, the next argument is the base branch name
|
||||
- **`--project-id`** (optional) — if present, the next argument is an existing Linear project ID
|
||||
|
||||
### 2. Load and configure
|
||||
|
||||
```clojure
|
||||
(require '[dev.mutation-testing :as mut-test] :reload)
|
||||
(require '[dev.coverage :as cov] :reload)
|
||||
```
|
||||
|
||||
If this is the first invocation (or the REPL was restarted), set up the Linear team:
|
||||
|
||||
```clojure
|
||||
(mut-test/list-teams!)
|
||||
(mut-test/set-config! {:team-id "<id>"})
|
||||
```
|
||||
|
||||
### 3. Run
|
||||
|
||||
```clojure
|
||||
(mut-test/run! '<target-ns>)
|
||||
;; Or with options:
|
||||
(mut-test/run! '<target-ns> {:base-branch "release-x.52.x"})
|
||||
(mut-test/run! '<target-ns> {:project-id "abc-123"})
|
||||
(mut-test/run! '<target-ns> {:base-branch "release-x.52.x" :project-id "abc-123"})
|
||||
```
|
||||
|
||||
Pass the opts map with `:base-branch` and/or `:project-id` if those flags were provided in the arguments.
|
||||
|
||||
This will:
|
||||
1. Generate a baseline mutation testing report
|
||||
2. Create a Linear project for the namespace
|
||||
3. Group functions by coverage relationships
|
||||
4. For each group: create branch → invoke Claude to write tests → verify → retry if needed → commit & push → create Linear issue → create draft PR
|
||||
5. Print a summary with PR links
|
||||
|
||||
### 4. Handle failures
|
||||
|
||||
If a group fails mid-processing, `run!` catches the error and continues to the next group. To retry a single group manually:
|
||||
|
||||
```clojure
|
||||
;; Get the parsed namespace info
|
||||
(def parsed (mut-test/parse-namespace '<target-ns>))
|
||||
|
||||
;; Run coverage to get the data
|
||||
(def coverage-results (cov/test-namespace (:target-ns parsed) [(:test-ns parsed)]))
|
||||
|
||||
;; Group and find the one you want
|
||||
(def groups (mut-test/group-functions coverage-results))
|
||||
|
||||
;; Process just that group
|
||||
(mut-test/process-group! parsed (nth groups <index>))
|
||||
```
|
||||
|
||||
### 5. Verify the prompt (dry run)
|
||||
|
||||
To inspect what Claude will see without invoking it:
|
||||
|
||||
```clojure
|
||||
(def parsed (mut-test/parse-namespace '<target-ns>))
|
||||
(def coverage-results (cov/test-namespace (:target-ns parsed) [(:test-ns parsed)]))
|
||||
(def groups (mut-test/group-functions coverage-results))
|
||||
(println (mut-test/build-test-prompt
|
||||
(merge (select-keys parsed [:target-ns :test-ns :source-path :test-path])
|
||||
(select-keys (first groups) [:fn-names :mutations]))))
|
||||
```
|
||||
|
||||
## Configuration
|
||||
|
||||
- **Team ID**: Set once per REPL session via `(mut-test/list-teams!)` and `(mut-test/set-config! {:team-id "..."})`
|
||||
- **Project ID**: Set automatically when `run!` calls `create-project-for-namespace!`
|
||||
- **Base Branch**: Defaults to `"master"`. Override via `(mut-test/set-config! {:base-branch "release-x.52.x"})` or pass `{:base-branch "..."}` as the second arg to `run!`
|
||||
- **Project ID**: If set (via `set-config!` or `run!` opts), `run!` reuses the existing Linear project instead of creating a new one
|
||||
@@ -1,20 +0,0 @@
|
||||
---
|
||||
name: typescript-review
|
||||
description: Review TypeScript and JavaScript code changes for compliance with Metabase coding standards, style violations, and code quality issues. Use when reviewing pull requests or diffs containing TypeScript/JavaScript code.
|
||||
allowed-tools: Read, Grep, Bash, Glob
|
||||
---
|
||||
|
||||
# TypeScript/JavaScript Code Review Skill
|
||||
|
||||
@./../_shared/typescript-commands.md
|
||||
|
||||
## Code Review Guidelines
|
||||
|
||||
Review pull requests with a focus on:
|
||||
|
||||
- Readability and maintainability
|
||||
- Appropriate test coverage
|
||||
- Compliance with project coding standards and conventions
|
||||
- Code quality and best practices
|
||||
- Type safety and proper TypeScript usage
|
||||
- React best practices (when applicable)
|
||||
@@ -1,9 +0,0 @@
|
||||
---
|
||||
name: typescript-write
|
||||
description: Write TypeScript and JavaScript code following Metabase coding standards and best practices. Use when developing or refactoring TypeScript/JavaScript code.
|
||||
---
|
||||
|
||||
# TypeScript/JavaScript Development Skill
|
||||
|
||||
@./../_shared/development-workflow.md
|
||||
@./../_shared/typescript-commands.md
|
||||
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -1,147 +0,0 @@
|
||||
{:linters
|
||||
{:metabase/test-helpers-use-non-thread-safe-functions
|
||||
;; TODO -- seems silly to maintain different blacklists and whitelists here than we use for the deftest `^:parallel`
|
||||
;; checker...
|
||||
{:explicitly-safe
|
||||
#{;; these toucan methods might actually set global values if it's used outside of a transaction,
|
||||
;; but since mt/with-temp runs in a transaction, so we'll ignore them in this case.
|
||||
toucan2.core/delete!
|
||||
toucan2.core/update!
|
||||
toucan2.core/insert!
|
||||
toucan2.core/insert-returning-instance!
|
||||
toucan2.core/insert-returning-instances!
|
||||
toucan2.core/insert-returning-pk!
|
||||
toucan2.core/insert-returning-pks!
|
||||
clojure.core.async/<!!
|
||||
clojure.core.async/<!
|
||||
clojure.core.async/>!!
|
||||
clojure.core.async/>!
|
||||
clojure.core.async/alts!
|
||||
clojure.core.async/alts!!
|
||||
clojure.core.async/close!
|
||||
clojure.core.async/poll!
|
||||
clojure.core.memoize/memo-clear!
|
||||
clojure.core/conj!
|
||||
clojure.core/persistent!
|
||||
clojure.core/reset!
|
||||
clojure.core/swap!
|
||||
clojure.core/volatile!
|
||||
clojure.core/vreset!
|
||||
clojure.core/vswap!
|
||||
clojure.java.jdbc/execute!
|
||||
methodical.core/add-aux-method-with-unique-key!
|
||||
methodical.core/remove-aux-method-with-unique-key!
|
||||
next.jdbc/execute!
|
||||
|
||||
;; Definitely thread safe
|
||||
metabase.test.util.dynamic-redefs/patch-vars!
|
||||
|
||||
;; TODO: most of these symbols shouldn't be here, we should go through them and
|
||||
;; find the functions/macros that use them and make sure their names end with !
|
||||
;; best way to do this is try remove each of these and rely on kondo output to find places where it's used
|
||||
clojure.test/grant-collection-perms!
|
||||
clojure.test/grant-collection-perms-fn!
|
||||
clojure.test/grant-perms-fn!
|
||||
clojure.test/purge-old-entries!
|
||||
clojure.test/revoke-collection-perms!
|
||||
clojure.test/save-results!
|
||||
metabase-enterprise.advanced-permissions.models.permissions/update-db-download-permissions!
|
||||
metabase-enterprise.internal-user/install-internal-user!
|
||||
metabase-enterprise.sso.integrations.saml-test/call-with-login-attributes-cleared!
|
||||
metabase.actions.actions/perform-action!
|
||||
metabase.actions.models/insert!
|
||||
metabase.analytics.snowplow-test/fake-track-event-impl!
|
||||
metabase.analytics.snowplow/track-event-impl!
|
||||
metabase.app-db.core/finish-db-setup!
|
||||
metabase.app-db.core/migrate!
|
||||
metabase.app-db.core/setup-db!
|
||||
metabase.app-db.schema-migrations-test.impl/run-migrations-in-range!
|
||||
metabase.app-db.setup/migrate!
|
||||
metabase.app-db.setup/setup-db!
|
||||
metabase.channel.email-test/reset-inbox!
|
||||
metabase.channel.email/send-email!
|
||||
metabase.cmd.dump-to-h2/dump-to-h2!
|
||||
metabase.cmd.load-from-h2/load-from-h2!
|
||||
metabase.content-verification.models.moderation-review/create-review!
|
||||
metabase.core.core/ensure-audit-db-installed!
|
||||
metabase.driver.mongo-test/create-database-from-row-maps!
|
||||
metabase.driver.postgres-test/create-enums-db!
|
||||
metabase.driver.postgres-test/drop-if-exists-and-create-db!
|
||||
metabase.driver.sql-jdbc.execute/execute-statement!
|
||||
metabase.indexed-entities.models.model-index/add-values!
|
||||
metabase.driver.sql-jdbc.connection.ssh-tunnel-test/start-mock-servers!
|
||||
metabase.driver.sql-jdbc.connection.ssh-tunnel-test/stop-mock-servers!
|
||||
metabase.indexed-entities.task.index-values/job-init!
|
||||
metabase.model-persistence.models.persisted-info/ready-database!
|
||||
metabase.model-persistence.task.persist-refresh/job-init!
|
||||
metabase.model-persistence.task.persist-refresh/refresh-tables!
|
||||
metabase.model-persistence.task.persist-refresh/schedule-persistence-for-database!
|
||||
metabase.models.on-demand-test/add-dashcard-with-parameter-mapping!
|
||||
metabase.permissions.core/grant-application-permissions!
|
||||
metabase.permissions.core/grant-collection-read-permissions!
|
||||
metabase.permissions.core/grant-collection-readwrite-permissions!
|
||||
metabase.permissions.core/grant-full-data-permissions!
|
||||
metabase.permissions.core/grant-native-readwrite-permissions!
|
||||
metabase.permissions.core/grant-permissions!
|
||||
metabase.permissions.core/revoke-application-permissions!
|
||||
metabase.permissions.core/revoke-data-perms!
|
||||
metabase.permissions.core/update-data-perms-graph!
|
||||
metabase.permissions.core/update-group-permissions!
|
||||
metabase.permissions.models.collection.graph-test/clear-graph-revisions!
|
||||
metabase.permissions.models.collection.graph-test/do-with-n-temp-users-with-personal-collections!
|
||||
metabase.permissions.models.permissions/grant-application-permissions!
|
||||
metabase.permissions.models.permissions/grant-collection-read-permissions!
|
||||
metabase.permissions.models.permissions/grant-collection-readwrite-permissions!
|
||||
metabase.permissions.models.permissions/grant-full-data-permissions!
|
||||
metabase.permissions.models.permissions/grant-native-readwrite-permissions!
|
||||
metabase.permissions.models.permissions/grant-permissions!
|
||||
metabase.permissions.models.permissions/revoke-application-permissions!
|
||||
metabase.permissions.models.permissions/revoke-data-perms!
|
||||
metabase.permissions.models.permissions/update-data-perms-graph!
|
||||
metabase.permissions.models.permissions/update-group-permissions!
|
||||
metabase.permissions.test-util/with-restored-perms!
|
||||
metabase.public-sharing-rest.api-test/add-card-to-dashboard!
|
||||
metabase.pulse.send/send-notifications!
|
||||
metabase.pulse.send/send-pulse!
|
||||
metabase.query-processor.streaming.interface/begin!
|
||||
metabase.query-processor.streaming.interface/finish!
|
||||
metabase.query-processor.streaming.interface/write-row!
|
||||
metabase.sample-data.impl/try-to-extract-sample-database!
|
||||
metabase.settings.models.setting-test/test-user-local-allowed-setting!
|
||||
metabase.settings.models.setting-test/test-user-local-only-setting!
|
||||
metabase.settings.models.setting.cache/restore-cache!
|
||||
metabase.settings.models.setting/set!
|
||||
metabase.settings.models.setting/validate-settings-formatting!
|
||||
metabase.setup.core/create-token!
|
||||
metabase.sso.ldap-test-util/start-ldap-server!
|
||||
metabase.sync.core/sync-database!
|
||||
metabase.sync.sync-metadata.fields.sync-metadata/update-field-metadata-if-needed!
|
||||
metabase.sync.sync-metadata/sync-db-metadata!
|
||||
metabase.sync.util-test/sync-database!
|
||||
metabase.sync.util/store-sync-summary!
|
||||
metabase.task.core/delete-task!
|
||||
metabase.task.impl/delete-task!
|
||||
metabase.test.data.bigquery-cloud-sdk/execute!
|
||||
metabase.test.data.impl.get-or-create/set-test-db-permissions!
|
||||
metabase.test.data.impl/copy-db-tables-and-fields!
|
||||
metabase.test.data.impl/get-or-create-database!
|
||||
metabase.test.data.impl/get-or-create-default-dataset!
|
||||
metabase.test.data.interface/create-db!
|
||||
metabase.test.data.interface/destroy-db!
|
||||
metabase.test.data.oracle/create-user!
|
||||
metabase.test.data.oracle/drop-user!
|
||||
metabase.test.data.sql-jdbc.load-data/make-insert!
|
||||
metabase.test.data.users/clear-cached-session-tokens!
|
||||
metabase.test.initialize/do-initialization!
|
||||
metabase.test.initialize/initialize-if-needed!
|
||||
metabase.test.util.log/ensure-unique-logger!
|
||||
metabase.test.util.log/set-ns-log-level!
|
||||
metabase.test.util/do-with-temp-env-var-value!
|
||||
metabase.test.util/restore-raw-setting!
|
||||
metabase.test.util/upsert-raw-setting!
|
||||
metabase.test/initialize-if-needed!
|
||||
metabase.test/test-helpers-set-global-values!
|
||||
metabase.test/with-temp-env-var-value!
|
||||
metabase.upload.impl-test/set-local-infile!
|
||||
metabase.util.files/create-dir-if-not-exists!
|
||||
metabase.warehouse-schema.models.field-values/create-or-update-full-field-values!}}}}
|
||||
5
.clj-kondo/macros/common.clj
Normal file
5
.clj-kondo/macros/common.clj
Normal file
@@ -0,0 +1,5 @@
|
||||
(ns macros.common
|
||||
"Helpers for common Metabase macro shapes so we don't have to define the same macroexpansions over and over.")
|
||||
|
||||
(defn ignore-unused [symb]
|
||||
(vary-meta symb assoc :clj-kondo/ignore [:unused-binding]))
|
||||
12
.clj-kondo/macros/metabase/api/card_test.clj
Normal file
12
.clj-kondo/macros/metabase/api/card_test.clj
Normal file
@@ -0,0 +1,12 @@
|
||||
(ns macros.metabase.api.card-test
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-ordered-items
|
||||
[collection model-and-name-syms & body]
|
||||
`(let ~(into []
|
||||
(comp (partition-all 2)
|
||||
(mapcat (fn [[model binding]]
|
||||
[(macros.common/ignore-unused binding) model])))
|
||||
model-and-name-syms)
|
||||
~collection
|
||||
~@body))
|
||||
17
.clj-kondo/macros/metabase/api/collection_test.clj
Normal file
17
.clj-kondo/macros/metabase/api/collection_test.clj
Normal file
@@ -0,0 +1,17 @@
|
||||
(ns macros.metabase.api.collection-test
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-collection-hierarchy [bindings & body]
|
||||
`(let [~(macros.common/ignore-unused 'a) nil
|
||||
~(macros.common/ignore-unused 'b) nil
|
||||
~(macros.common/ignore-unused 'c) nil
|
||||
~(macros.common/ignore-unused 'd) nil
|
||||
~(macros.common/ignore-unused 'e) nil
|
||||
~(macros.common/ignore-unused 'f) nil
|
||||
~(macros.common/ignore-unused 'g) nil]
|
||||
~bindings
|
||||
~@body))
|
||||
|
||||
(defmacro with-some-children-of-collection [collection-or-id-or-nil & body]
|
||||
`(let [~(macros.common/ignore-unused '&ids) ~collection-or-id-or-nil]
|
||||
~@body))
|
||||
7
.clj-kondo/macros/metabase/api/common.clj
Normal file
7
.clj-kondo/macros/metabase/api/common.clj
Normal file
@@ -0,0 +1,7 @@
|
||||
(ns macros.metabase.api.common)
|
||||
|
||||
(defmacro define-routes
|
||||
"Macro for api.common/define-routes"
|
||||
[& args]
|
||||
`(do (def ~'routes "docstring" nil)
|
||||
~@args))
|
||||
13
.clj-kondo/macros/metabase/api/embed_test.clj
Normal file
13
.clj-kondo/macros/metabase/api/embed_test.clj
Normal file
@@ -0,0 +1,13 @@
|
||||
(ns macros.metabase.api.embed-test)
|
||||
|
||||
(defmacro with-embedding-enabled-and-temp-card-referencing
|
||||
[table-kw field-kw [card-binding] & body]
|
||||
`(let [~card-binding [~table-kw ~field-kw]]
|
||||
~@body))
|
||||
|
||||
(defmacro with-embedding-enabled-and-temp-dashcard-referencing
|
||||
[table-kw field-kw [dash-binding card-binding dashcard-binding] & body]
|
||||
`(let [~dash-binding [~table-kw ~field-kw]
|
||||
~(or card-binding '_) nil
|
||||
~(or dashcard-binding '_) nil]
|
||||
~@body))
|
||||
13
.clj-kondo/macros/metabase/api/public_test.clj
Normal file
13
.clj-kondo/macros/metabase/api/public_test.clj
Normal file
@@ -0,0 +1,13 @@
|
||||
(ns macros.metabase.api.public-test)
|
||||
|
||||
(defmacro with-sharing-enabled-and-temp-card-referencing
|
||||
[table-kw field-kw [card-binding] & body]
|
||||
`(let [~card-binding [~table-kw ~field-kw]]
|
||||
~@body))
|
||||
|
||||
(defmacro with-sharing-enabled-and-temp-dashcard-referencing
|
||||
[table-kw field-kw [dash-binding card-binding dashcard-binding] & body]
|
||||
`(let [~dash-binding [~table-kw ~field-kw]
|
||||
~(or card-binding '_) nil
|
||||
~(or dashcard-binding '_) nil]
|
||||
~@body))
|
||||
@@ -1,24 +0,0 @@
|
||||
(ns macros.metabase.collections-rest.api-test)
|
||||
|
||||
(defmacro with-collection-hierarchy! [bindings & body]
|
||||
`(let [~'a nil
|
||||
~'b nil
|
||||
~'c nil
|
||||
~'d nil
|
||||
~'e nil
|
||||
~'f nil
|
||||
~'g nil]
|
||||
~'a
|
||||
~'b
|
||||
~'c
|
||||
~'d
|
||||
~'e
|
||||
~'f
|
||||
~'g
|
||||
~bindings
|
||||
~@body))
|
||||
|
||||
(defmacro with-some-children-of-collection! [collection-or-id-or-nil & body]
|
||||
`(let [~'&ids ~collection-or-id-or-nil]
|
||||
~'&ids
|
||||
~@body))
|
||||
@@ -1,13 +0,0 @@
|
||||
(ns macros.metabase.embedding-rest.api.embed-test)
|
||||
|
||||
(defmacro with-embedding-enabled-and-temp-card-referencing!
|
||||
[table-kw field-kw [card-binding] & body]
|
||||
`(let [~card-binding [~table-kw ~field-kw]]
|
||||
~@body))
|
||||
|
||||
(defmacro with-embedding-enabled-and-temp-dashcard-referencing!
|
||||
[table-kw field-kw [dash-binding card-binding dashcard-binding] & body]
|
||||
`(let [~dash-binding [~table-kw ~field-kw]
|
||||
~(or card-binding '_) nil
|
||||
~(or dashcard-binding '_) nil]
|
||||
~@body))
|
||||
@@ -0,0 +1,4 @@
|
||||
(ns macros.metabase.models.params.chain-filter-test)
|
||||
|
||||
(defmacro chain-filter [& _]
|
||||
`(metabase.models.params.chain-filter/chain-filter nil nil))
|
||||
@@ -0,0 +1,9 @@
|
||||
(ns macros.metabase.models.query-analysis-test
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-test-setup [& body]
|
||||
`(let [~(macros.common/ignore-unused 'card-id) 1
|
||||
~(macros.common/ignore-unused 'table-id) 2
|
||||
~(macros.common/ignore-unused 'tax-id) 3
|
||||
~(macros.common/ignore-unused 'total-id) 4]
|
||||
~@body))
|
||||
9
.clj-kondo/macros/metabase/models/user_test.clj
Normal file
9
.clj-kondo/macros/metabase/models/user_test.clj
Normal file
@@ -0,0 +1,9 @@
|
||||
(ns macros.metabase.models.user-test)
|
||||
|
||||
(defmacro with-groups [bindings & body]
|
||||
`(let ~(into []
|
||||
(comp (partition-all 3)
|
||||
(mapcat (fn [[group-binding properties members]]
|
||||
[group-binding [properties members]])))
|
||||
bindings)
|
||||
~@body))
|
||||
@@ -1,4 +0,0 @@
|
||||
(ns macros.metabase.parameters.chain-filter-test)
|
||||
|
||||
(defmacro chain-filter [& _]
|
||||
`(metabase.parameters.chain-filter/chain-filter nil nil))
|
||||
@@ -1,13 +0,0 @@
|
||||
(ns macros.metabase.public-sharing-rest.api-test)
|
||||
|
||||
(defmacro with-sharing-enabled-and-temp-card-referencing!
|
||||
[table-kw field-kw [card-binding] & body]
|
||||
`(let [~card-binding [~table-kw ~field-kw]]
|
||||
~@body))
|
||||
|
||||
(defmacro with-sharing-enabled-and-temp-dashcard-referencing!
|
||||
[table-kw field-kw [dash-binding card-binding dashcard-binding] & body]
|
||||
`(let [~dash-binding [~table-kw ~field-kw]
|
||||
~(or card-binding '_) nil
|
||||
~(or dashcard-binding '_) nil]
|
||||
~@body))
|
||||
@@ -1,13 +0,0 @@
|
||||
(ns macros.metabase.queries-rest.api.card-test)
|
||||
|
||||
(defmacro with-ordered-items
|
||||
[collection model-and-name-syms & body]
|
||||
`(let ~(into []
|
||||
(comp (partition-all 2)
|
||||
(mapcat (fn [[model binding]]
|
||||
[binding model])))
|
||||
model-and-name-syms)
|
||||
~collection
|
||||
~@(->> (partition-all 2 model-and-name-syms)
|
||||
(map second))
|
||||
~@body))
|
||||
@@ -1,12 +0,0 @@
|
||||
(ns macros.metabase.query-analysis.models.query-analysis-test)
|
||||
|
||||
(defmacro with-test-setup [& body]
|
||||
`(let [~'card-id 1
|
||||
~'table-id 2
|
||||
~'tax-id 3
|
||||
~'total-id 4]
|
||||
~'card-id
|
||||
~'table-id
|
||||
~'tax-id
|
||||
~'total-id
|
||||
~@body))
|
||||
@@ -1,18 +0,0 @@
|
||||
(ns macros.metabase.query-analysis.task.test-setup)
|
||||
|
||||
(defmacro with-test-setup!
|
||||
"Set up the data required to test the Query Analysis related tasks"
|
||||
[& body]
|
||||
`(let [~'c1 1
|
||||
~'c2 2
|
||||
~'c3 3
|
||||
~'c4 4
|
||||
~'archived 5
|
||||
~'invalid 6]
|
||||
~'c1
|
||||
~'c2
|
||||
~'c3
|
||||
~'c4
|
||||
~'archived
|
||||
~'invalid
|
||||
~@body))
|
||||
13
.clj-kondo/macros/metabase/related_test.clj
Normal file
13
.clj-kondo/macros/metabase/related_test.clj
Normal file
@@ -0,0 +1,13 @@
|
||||
(ns macros.metabase.related-test
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-world [& body]
|
||||
`(let [~(macros.common/ignore-unused 'collection-id) nil
|
||||
~(macros.common/ignore-unused 'metric-id-a) nil
|
||||
~(macros.common/ignore-unused 'metric-id-b) nil
|
||||
~(macros.common/ignore-unused 'segment-id-a) nil
|
||||
~(macros.common/ignore-unused 'segment-id-b) nil
|
||||
~(macros.common/ignore-unused 'card-id-a) nil
|
||||
~(macros.common/ignore-unused 'card-id-b) nil
|
||||
~(macros.common/ignore-unused 'card-id-c) nil]
|
||||
~@body))
|
||||
8
.clj-kondo/macros/metabase/shared/util/namespaces.clj
Normal file
8
.clj-kondo/macros/metabase/shared/util/namespaces.clj
Normal file
@@ -0,0 +1,8 @@
|
||||
(ns macros.metabase.shared.util.namespaces)
|
||||
|
||||
(defmacro import-fn
|
||||
"Kondo macro replacement for [[metabase.shared.util.namespaces/import-fn]]."
|
||||
([sym]
|
||||
`(def ~(-> sym name symbol) "docstring" ~sym))
|
||||
([sym name]
|
||||
`(def ~name "docstring" ~sym)))
|
||||
@@ -0,0 +1,12 @@
|
||||
(ns macros.metabase.task.setup.query-analysis-setup
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-test-setup!
|
||||
"Set up the data required to test the Query Analysis related tasks"
|
||||
[& body]
|
||||
`(let [~(macros.common/ignore-unused 'c1) 1
|
||||
~(macros.common/ignore-unused 'c2) 2
|
||||
~(macros.common/ignore-unused 'c3) 3
|
||||
~(macros.common/ignore-unused 'c4) 4
|
||||
~(macros.common/ignore-unused 'arch) 5]
|
||||
~@body))
|
||||
5
.clj-kondo/macros/metabase/test/persistence.clj
Normal file
5
.clj-kondo/macros/metabase/test/persistence.clj
Normal file
@@ -0,0 +1,5 @@
|
||||
(ns macros.metabase.test.persistence)
|
||||
|
||||
(defmacro with-persistence-enabled [[persist-fn-sym] & body]
|
||||
`(clojure.core/let [~persist-fn-sym (fn [])]
|
||||
~@body))
|
||||
@@ -1,9 +0,0 @@
|
||||
(ns macros.metabase.users.models.user-test)
|
||||
|
||||
(defmacro with-groups! [bindings & body]
|
||||
`(let ~(into []
|
||||
(comp (partition-all 3)
|
||||
(mapcat (fn [[group-binding properties members]]
|
||||
[group-binding [properties members]])))
|
||||
bindings)
|
||||
~@body))
|
||||
@@ -1,8 +0,0 @@
|
||||
(ns macros.metabase.util.namespaces)
|
||||
|
||||
(defmacro import-fn
|
||||
"Kondo macro replacement for [[metabase.util.namespaces/import-fn]]."
|
||||
([sym]
|
||||
`(def ~(-> sym name symbol) "docstring" ~sym))
|
||||
([sym name]
|
||||
`(def ~name "docstring" ~sym)))
|
||||
16
.clj-kondo/macros/metabase/xrays/domain_entities/malli.clj
Normal file
16
.clj-kondo/macros/metabase/xrays/domain_entities/malli.clj
Normal file
@@ -0,0 +1,16 @@
|
||||
(ns macros.metabase.xrays.domain-entities.malli)
|
||||
|
||||
(defmacro define-getters-and-setters [schema & defs]
|
||||
`(do
|
||||
(comment ~schema) ;; Reference the schema
|
||||
~@(for [[sym _path] (partition 2 defs)]
|
||||
`(do
|
||||
(defn ~(vary-meta sym
|
||||
assoc :export true)
|
||||
"docs" [_x#] nil)
|
||||
(defn ~(vary-meta (symbol (str "with-" (name sym)))
|
||||
assoc :export true)
|
||||
"docs" [x# _new#] x#)
|
||||
(defn ~(vary-meta (symbol (str (name sym) "-js"))
|
||||
assoc :export true)
|
||||
"docs" [x#] x#)))))
|
||||
@@ -1,20 +0,0 @@
|
||||
(ns macros.metabase.xrays.related-test)
|
||||
|
||||
(defmacro with-world [& body]
|
||||
`(let [~'collection-id nil
|
||||
~'metric-id-a nil
|
||||
~'metric-id-b nil
|
||||
~'segment-id-a nil
|
||||
~'segment-id-b nil
|
||||
~'card-id-a nil
|
||||
~'card-id-b nil
|
||||
~'card-id-c nil]
|
||||
~'collection-id
|
||||
~'metric-id-a
|
||||
~'metric-id-b
|
||||
~'segment-id-a
|
||||
~'segment-id-b
|
||||
~'card-id-a
|
||||
~'card-id-b
|
||||
~'card-id-c
|
||||
~@body))
|
||||
@@ -0,0 +1,6 @@
|
||||
(ns macros.metabase-enterprise.advanced-permissions.api.util-test
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-impersonations! [impersonations-and-attributes-map & body]
|
||||
`(let [~(macros.common/ignore-unused '&group) ~impersonations-and-attributes-map]
|
||||
~@body))
|
||||
@@ -1,5 +0,0 @@
|
||||
(ns macros.metabase-enterprise.database-routing.e2e-test)
|
||||
|
||||
(defmacro with-temp-dbs! [bindings & body]
|
||||
`(let [~@(interleave bindings (repeat nil))]
|
||||
~@body))
|
||||
@@ -1,6 +0,0 @@
|
||||
(ns macros.metabase-enterprise.impersonation.util-test)
|
||||
|
||||
(defmacro with-impersonations! [impersonations-and-attributes-map & body]
|
||||
`(let [~'&group ~impersonations-and-attributes-map]
|
||||
~'&group
|
||||
~@body))
|
||||
@@ -1,18 +1,12 @@
|
||||
(ns macros.metabase-enterprise.query-reference-validation.api-test)
|
||||
(ns macros.metabase-enterprise.query-reference-validation.api-test
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-test-setup! [& body]
|
||||
`(let [~'card-1 1
|
||||
~'card-2 2
|
||||
~'card-3 3
|
||||
~'card-4 4
|
||||
~'card-5 5
|
||||
~'coll-2 100
|
||||
~'coll-3 200]
|
||||
~'card-1
|
||||
~'card-2
|
||||
~'card-3
|
||||
~'card-4
|
||||
~'card-5
|
||||
~'coll-2
|
||||
~'coll-3
|
||||
(defmacro with-test-setup [& body]
|
||||
`(let [~(macros.common/ignore-unused 'card-1) 1
|
||||
~(macros.common/ignore-unused 'card-2) 2
|
||||
~(macros.common/ignore-unused 'card-3) 3
|
||||
~(macros.common/ignore-unused 'card-4) 4
|
||||
~(macros.common/ignore-unused 'card-5) 5
|
||||
~(macros.common/ignore-unused 'coll-2) 100
|
||||
~(macros.common/ignore-unused 'coll-3) 200]
|
||||
~@body))
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
(ns macros.metabase-enterprise.sandbox.test-util)
|
||||
(ns macros.metabase-enterprise.sandbox.test-util
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-gtaps! [gtaps-definition & body]
|
||||
`(let [~'&group ~gtaps-definition]
|
||||
~'&group
|
||||
`(let [~(macros.common/ignore-unused '&group) ~gtaps-definition]
|
||||
~@body))
|
||||
|
||||
@@ -1,112 +1,59 @@
|
||||
(ns macros.metabase-enterprise.serialization.test-util)
|
||||
(ns macros.metabase-enterprise.serialization.test-util
|
||||
(:require [macros.common]))
|
||||
|
||||
(defmacro with-world [& body]
|
||||
`(let [~'db-id nil
|
||||
~'table-id nil
|
||||
~'table nil
|
||||
~'table-id-categories nil
|
||||
~'table-id-users nil
|
||||
~'table-id-checkins nil
|
||||
~'venues-pk-field-id nil
|
||||
~'numeric-field-id nil
|
||||
~'name-field-id nil
|
||||
~'latitude-field-id nil
|
||||
~'longitude-field-id nil
|
||||
~'category-field-id nil
|
||||
~'category-pk-field-id nil
|
||||
~'date-field-id nil
|
||||
~'users-pk-field-id nil
|
||||
~'user-id-field-id nil
|
||||
~'checkins->venues-field-id nil
|
||||
~'last-login-field-id nil
|
||||
~'collection-id nil
|
||||
~'collection-id-nested nil
|
||||
~'user-id-temp nil
|
||||
~'personal-collection-id nil
|
||||
~'pc-felicia-nested-id nil
|
||||
~'pc-nested-id nil
|
||||
~'pc-deeply-nested-id nil
|
||||
~'metric-id nil
|
||||
~'segment-id nil
|
||||
~'dashboard-id nil
|
||||
~'root-dashboard-id nil
|
||||
~'card-id nil
|
||||
~'card-arch-id nil
|
||||
~'card-id-root nil
|
||||
~'card-id-nested nil
|
||||
~'card-id-nested-query nil
|
||||
~'card-id-native-query nil
|
||||
~'card-id-pivot-table nil
|
||||
~'dashcard-id nil
|
||||
~'dashcard-top-level-click-id nil
|
||||
~'dashcard-with-click-actions nil
|
||||
~'dashcard-with-textbox-id nil
|
||||
~'card-id-root-to-collection nil
|
||||
~'card-id-collection-to-root nil
|
||||
~'pulse-id nil
|
||||
~'pulsecard-root-id nil
|
||||
~'pulsecard-collection-id nil
|
||||
~'card-id-template-tags nil
|
||||
~'card-id-filter-agg nil
|
||||
~'card-id-temporal-unit nil
|
||||
~'snippet-id nil
|
||||
~'snippet-collection-id nil
|
||||
~'snippet-nested-collection-id nil
|
||||
~'nested-snippet-id nil
|
||||
~'card-id-with-native-snippet nil
|
||||
~'card-join-card-id nil]
|
||||
~'db-id
|
||||
~'table-id
|
||||
~'table
|
||||
~'table-id-categories
|
||||
~'table-id-users
|
||||
~'table-id-checkins
|
||||
~'venues-pk-field-id
|
||||
~'numeric-field-id
|
||||
~'name-field-id
|
||||
~'latitude-field-id
|
||||
~'longitude-field-id
|
||||
~'category-field-id
|
||||
~'category-pk-field-id
|
||||
~'date-field-id
|
||||
~'users-pk-field-id
|
||||
~'user-id-field-id
|
||||
~'checkins->venues-field-id
|
||||
~'last-login-field-id
|
||||
~'collection-id
|
||||
~'collection-id-nested
|
||||
~'user-id-temp
|
||||
~'personal-collection-id
|
||||
~'pc-felicia-nested-id
|
||||
~'pc-nested-id
|
||||
~'pc-deeply-nested-id
|
||||
~'metric-id
|
||||
~'segment-id
|
||||
~'dashboard-id
|
||||
~'root-dashboard-id
|
||||
~'card-id
|
||||
~'card-arch-id
|
||||
~'card-id-root
|
||||
~'card-id-nested
|
||||
~'card-id-nested-query
|
||||
~'card-id-native-query
|
||||
~'card-id-pivot-table
|
||||
~'dashcard-id
|
||||
~'dashcard-top-level-click-id
|
||||
~'dashcard-with-click-actions
|
||||
~'dashcard-with-textbox-id
|
||||
~'card-id-root-to-collection
|
||||
~'card-id-collection-to-root
|
||||
~'pulse-id
|
||||
~'pulsecard-root-id
|
||||
~'pulsecard-collection-id
|
||||
~'card-id-template-tags
|
||||
~'card-id-filter-agg
|
||||
~'card-id-temporal-unit
|
||||
~'snippet-id
|
||||
~'snippet-collection-id
|
||||
~'snippet-nested-collection-id
|
||||
~'nested-snippet-id
|
||||
~'card-id-with-native-snippet
|
||||
~'card-join-card-id
|
||||
`(let [~(macros.common/ignore-unused 'db-id) nil
|
||||
~(macros.common/ignore-unused 'table-id) nil
|
||||
~(macros.common/ignore-unused 'table) nil
|
||||
~(macros.common/ignore-unused 'table-id-categories) nil
|
||||
~(macros.common/ignore-unused 'table-id-users) nil
|
||||
~(macros.common/ignore-unused 'table-id-checkins) nil
|
||||
~(macros.common/ignore-unused 'venues-pk-field-id) nil
|
||||
~(macros.common/ignore-unused 'numeric-field-id) nil
|
||||
~(macros.common/ignore-unused 'name-field-id) nil
|
||||
~(macros.common/ignore-unused 'latitude-field-id) nil
|
||||
~(macros.common/ignore-unused 'longitude-field-id) nil
|
||||
~(macros.common/ignore-unused 'category-field-id) nil
|
||||
~(macros.common/ignore-unused 'category-pk-field-id) nil
|
||||
~(macros.common/ignore-unused 'date-field-id) nil
|
||||
~(macros.common/ignore-unused 'users-pk-field-id) nil
|
||||
~(macros.common/ignore-unused 'user-id-field-id) nil
|
||||
~(macros.common/ignore-unused 'checkins->venues-field-id) nil
|
||||
~(macros.common/ignore-unused 'last-login-field-id) nil
|
||||
~(macros.common/ignore-unused 'collection-id) nil
|
||||
~(macros.common/ignore-unused 'collection-id-nested) nil
|
||||
~(macros.common/ignore-unused 'user-id-temp) nil
|
||||
~(macros.common/ignore-unused 'personal-collection-id) nil
|
||||
~(macros.common/ignore-unused 'pc-felicia-nested-id) nil
|
||||
~(macros.common/ignore-unused 'pc-nested-id) nil
|
||||
~(macros.common/ignore-unused 'pc-deeply-nested-id) nil
|
||||
~(macros.common/ignore-unused 'metric-id) nil
|
||||
~(macros.common/ignore-unused 'segment-id) nil
|
||||
~(macros.common/ignore-unused 'dashboard-id) nil
|
||||
~(macros.common/ignore-unused 'root-dashboard-id) nil
|
||||
~(macros.common/ignore-unused 'card-id) nil
|
||||
~(macros.common/ignore-unused 'card-arch-id) nil
|
||||
~(macros.common/ignore-unused 'card-id-root) nil
|
||||
~(macros.common/ignore-unused 'card-id-nested) nil
|
||||
~(macros.common/ignore-unused 'card-id-nested-query) nil
|
||||
~(macros.common/ignore-unused 'card-id-native-query) nil
|
||||
~(macros.common/ignore-unused 'card-id-pivot-table) nil
|
||||
~(macros.common/ignore-unused 'dashcard-id) nil
|
||||
~(macros.common/ignore-unused 'dashcard-top-level-click-id) nil
|
||||
~(macros.common/ignore-unused 'dashcard-with-click-actions) nil
|
||||
~(macros.common/ignore-unused 'dashcard-with-textbox-id) nil
|
||||
~(macros.common/ignore-unused 'card-id-root-to-collection) nil
|
||||
~(macros.common/ignore-unused 'card-id-collection-to-root) nil
|
||||
~(macros.common/ignore-unused 'pulse-id) nil
|
||||
~(macros.common/ignore-unused 'pulsecard-root-id) nil
|
||||
~(macros.common/ignore-unused 'pulsecard-collection-id) nil
|
||||
~(macros.common/ignore-unused 'card-id-template-tags) nil
|
||||
~(macros.common/ignore-unused 'card-id-filter-agg) nil
|
||||
~(macros.common/ignore-unused 'card-id-temporal-unit) nil
|
||||
~(macros.common/ignore-unused 'snippet-id) nil
|
||||
~(macros.common/ignore-unused 'snippet-collection-id) nil
|
||||
~(macros.common/ignore-unused 'snippet-nested-collection-id) nil
|
||||
~(macros.common/ignore-unused 'nested-snippet-id) nil
|
||||
~(macros.common/ignore-unused 'card-id-with-native-snippet) nil
|
||||
~(macros.common/ignore-unused 'card-join-card-id) nil]
|
||||
~@body))
|
||||
|
||||
@@ -1,38 +1,329 @@
|
||||
(ns hooks.clojure.core
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.set :as set]
|
||||
[clojure.string :as str]))
|
||||
|
||||
(defn- all-uppercase? [symb]
|
||||
(let [s (str symb)]
|
||||
(= s #_{:clj-kondo/ignore [:discouraged-var]} (str/upper-case s))))
|
||||
(defn- node->qualified-symbol [node]
|
||||
(try
|
||||
(when (hooks/token-node? node)
|
||||
(let [sexpr (hooks/sexpr node)]
|
||||
(when (symbol? sexpr)
|
||||
(let [resolved (hooks/resolve {:name sexpr})]
|
||||
(when-not (= :clj-kondo/unknown-namespace (:ns resolved))
|
||||
(symbol (name (:ns resolved)) (name (:name resolved))))))))
|
||||
;; some symbols like `*count/Integer` aren't resolvable.
|
||||
(catch Exception _
|
||||
nil)))
|
||||
|
||||
(defn- has-underscores? [symb]
|
||||
(str/includes? (str symb) "_"))
|
||||
(def ^:private symbols-allowed-in-fns-not-ending-in-an-exclamation-point
|
||||
'#{;; these toucan methods might actually set global values if it's used outside of a transaction,
|
||||
;; but since mt/with-temp runs in a transaction, so we'll ignore them in this case.
|
||||
toucan2.core/delete!
|
||||
toucan2.core/update!
|
||||
toucan2.core/insert!
|
||||
toucan2.core/insert-returning-instance!
|
||||
toucan2.core/insert-returning-instances!
|
||||
toucan2.core/insert-returning-pk!
|
||||
toucan2.core/insert-returning-pks!
|
||||
clojure.core.async/<!!
|
||||
clojure.core.async/>!!
|
||||
clojure.core.async/alts!!
|
||||
clojure.core.async/close!
|
||||
clojure.core.async/poll!
|
||||
clojure.core.memoize/memo-clear!
|
||||
clojure.core/conj!
|
||||
clojure.core/persistent!
|
||||
clojure.core/reset!
|
||||
clojure.core/swap!
|
||||
clojure.core/volatile!
|
||||
clojure.core/vreset!
|
||||
clojure.core/vswap!
|
||||
clojure.java.jdbc/execute!
|
||||
methodical.core/add-aux-method-with-unique-key!
|
||||
methodical.core/remove-aux-method-with-unique-key!
|
||||
next.jdbc/execute!
|
||||
|
||||
(defn- check-symbol-is-kebab-case [symbol-node]
|
||||
(let [symb (hooks/sexpr symbol-node)]
|
||||
(when (or (all-uppercase? symb)
|
||||
(has-underscores? symb))
|
||||
(hooks/reg-finding! (assoc (meta symbol-node)
|
||||
:message "Use kebab-case for function and variable names; don't use special notation for constants."
|
||||
:type :metabase/check-def-kebab-case)))))
|
||||
;; Definitely thread safe
|
||||
metabase.test.util.dynamic-redefs/patch-vars!
|
||||
|
||||
(defn lint-def* [{:keys [node]}]
|
||||
(let [[_def & args] (:children node)
|
||||
name-symbol (some (fn [arg]
|
||||
(when (and (hooks/token-node? arg)
|
||||
(symbol? (hooks/sexpr arg)))
|
||||
arg))
|
||||
args)]
|
||||
(check-symbol-is-kebab-case name-symbol)))
|
||||
;; TODO: most of these symbols shouldn't be here, we should go through them and
|
||||
;; find the functions/macros that use them and make sure their names end with !
|
||||
;; best way to do this is try remove each of these and rely on kondo output to find places where it's used
|
||||
clojure.test/grant-collection-perms!
|
||||
clojure.test/grant-collection-perms-fn!
|
||||
clojure.test/grant-perms-fn!
|
||||
clojure.test/purge-old-entries!
|
||||
clojure.test/revoke-collection-perms!
|
||||
clojure.test/save-results!
|
||||
metabase-enterprise.advanced-permissions.models.permissions/update-db-download-permissions!
|
||||
metabase-enterprise.internal-user/install-internal-user!
|
||||
metabase-enterprise.sso.integrations.saml-test/call-with-login-attributes-cleared!
|
||||
metabase.actions/perform-action!
|
||||
metabase.analytics.snowplow-test/fake-track-event-impl!
|
||||
metabase.analytics.snowplow/track-event-impl!
|
||||
metabase.api.public-test/add-card-to-dashboard!
|
||||
metabase.cmd.dump-to-h2/dump-to-h2!
|
||||
metabase.cmd.load-from-h2/load-from-h2!
|
||||
metabase.core/ensure-audit-db-installed!
|
||||
metabase.db.schema-migrations-test.impl/run-migrations-in-range!
|
||||
metabase.db.setup/migrate!
|
||||
metabase.db.setup/setup-db!
|
||||
metabase.db/migrate!
|
||||
metabase.db/setup-db!
|
||||
metabase.driver.mongo-test/create-database-from-row-maps!
|
||||
metabase.driver.postgres-test/create-enums-db!
|
||||
metabase.driver.postgres-test/drop-if-exists-and-create-db!
|
||||
metabase.driver.sql-jdbc.execute/execute-statement!
|
||||
metabase.email-test/reset-inbox!
|
||||
metabase.email/send-email!
|
||||
metabase.models.action/insert!
|
||||
metabase.models.collection.graph-test/clear-graph-revisions!
|
||||
metabase.models.collection.graph-test/do-with-n-temp-users-with-personal-collections!
|
||||
metabase.models.field-values/create-or-update-full-field-values!
|
||||
metabase.models.model-index/add-values!
|
||||
metabase.models.moderation-review/create-review!
|
||||
metabase.models.on-demand-test/add-dashcard-with-parameter-mapping!
|
||||
metabase.models.permissions/grant-application-permissions!
|
||||
metabase.models.permissions/grant-collection-read-permissions!
|
||||
metabase.models.permissions/grant-collection-readwrite-permissions!
|
||||
metabase.models.permissions/grant-full-data-permissions!
|
||||
metabase.models.permissions/grant-native-readwrite-permissions!
|
||||
metabase.models.permissions/grant-permissions!
|
||||
metabase.models.permissions/revoke-application-permissions!
|
||||
metabase.models.permissions/revoke-data-perms!
|
||||
metabase.models.permissions/update-data-perms-graph!
|
||||
metabase.models.permissions/update-group-permissions!
|
||||
metabase.models.persisted-info/ready-database!
|
||||
metabase.models.revision/revert!
|
||||
metabase.models.setting-test/test-user-local-allowed-setting!
|
||||
metabase.models.setting-test/test-user-local-only-setting!
|
||||
metabase.models.setting.cache/restore-cache!
|
||||
metabase.models.setting/set!
|
||||
metabase.models.setting/validate-settings-formatting!
|
||||
metabase.permissions.test-util/with-restored-perms!
|
||||
metabase.pulse/send-notifications!
|
||||
metabase.pulse/send-pulse!
|
||||
metabase.query-processor.streaming.interface/begin!
|
||||
metabase.query-processor.streaming.interface/finish!
|
||||
metabase.query-processor.streaming.interface/write-row!
|
||||
metabase.sample-data/try-to-extract-sample-database!
|
||||
metabase.setup/create-token!
|
||||
metabase.sync.sync-metadata.fields.sync-metadata/update-field-metadata-if-needed!
|
||||
metabase.sync.sync-metadata/sync-db-metadata!
|
||||
metabase.sync.util-test/sync-database!
|
||||
metabase.sync.util/store-sync-summary!
|
||||
metabase.sync/sync-database!
|
||||
metabase.task.index-values/job-init!
|
||||
metabase.task.persist-refresh/job-init!
|
||||
metabase.task.persist-refresh/refresh-tables!
|
||||
metabase.task.persist-refresh/schedule-persistence-for-database!
|
||||
metabase.task/delete-task!
|
||||
metabase.test.data.bigquery-cloud-sdk/execute!
|
||||
metabase.test.data.impl/copy-db-tables-and-fields!
|
||||
metabase.test.data.impl/get-or-create-database!
|
||||
metabase.test.data.impl/get-or-create-default-dataset!
|
||||
metabase.test.data.impl.get-or-create/set-test-db-permissions!
|
||||
metabase.test.data.interface/create-db!
|
||||
metabase.test.data.interface/destroy-db!
|
||||
metabase.test.data.oracle/create-user!
|
||||
metabase.test.data.oracle/drop-user!
|
||||
metabase.test.data.sql-jdbc.load-data/make-insert!
|
||||
metabase.test.data.users/clear-cached-session-tokens!
|
||||
metabase.test.initialize/do-initialization!
|
||||
metabase.test.initialize/initialize-if-needed!
|
||||
metabase.test.integrations.ldap/start-ldap-server!
|
||||
metabase.test.util.log/ensure-unique-logger!
|
||||
metabase.test.util.log/set-ns-log-level!
|
||||
metabase.test.util/do-with-temp-env-var-value!
|
||||
metabase.test.util/restore-raw-setting!
|
||||
metabase.test.util/upsert-raw-setting!
|
||||
metabase.test/initialize-if-needed!
|
||||
metabase.test/test-helpers-set-global-values!
|
||||
metabase.test/with-temp-env-var-value!
|
||||
metabase.upload-test/set-local-infile!
|
||||
metabase.util.files/create-dir-if-not-exists!
|
||||
metabase.util.ssh-test/start-mock-servers!
|
||||
metabase.util.ssh-test/stop-mock-servers!})
|
||||
|
||||
(defn lint-def [x]
|
||||
(lint-def* x)
|
||||
x)
|
||||
(defn- end-with-exclamation?
|
||||
[s]
|
||||
(str/ends-with? s "!"))
|
||||
|
||||
(defn- non-thread-safe-form-should-end-with-exclamation*
|
||||
[{[defn-or-defmacro form-name] :children, :as node}]
|
||||
(when-not (and (:string-value form-name)
|
||||
(end-with-exclamation? (:string-value form-name)))
|
||||
(letfn [(walk [f form]
|
||||
(f form)
|
||||
(doseq [child (:children form)]
|
||||
(walk f child)))]
|
||||
(walk (fn [form]
|
||||
(when-let [qualified-symbol (node->qualified-symbol form)]
|
||||
(when (and (not (contains? symbols-allowed-in-fns-not-ending-in-an-exclamation-point qualified-symbol))
|
||||
(end-with-exclamation? qualified-symbol))
|
||||
(hooks/reg-finding! (assoc (meta form-name)
|
||||
:message (format "The name of this %s should end with `!` because it contains calls to non thread safe form `%s`."
|
||||
(:string-value defn-or-defmacro) qualified-symbol)
|
||||
:type :metabase/test-helpers-use-non-thread-safe-functions)))))
|
||||
node))
|
||||
node))
|
||||
|
||||
(defn non-thread-safe-form-should-end-with-exclamation
|
||||
"Used to ensure defn and defmacro in test namespace to have name ending with `!` if it's non-thread-safe.
|
||||
A function or a macro can be defined as 'not thread safe' when their funciton name ends with a `!`.
|
||||
|
||||
Only used in tests to identify thread-safe/non-thread-safe test helpers. See #37126"
|
||||
[{:keys [node cljc lang]}]
|
||||
(when (or (not cljc)
|
||||
(= lang :clj))
|
||||
(non-thread-safe-form-should-end-with-exclamation* node))
|
||||
{:node node})
|
||||
|
||||
(comment
|
||||
(defn x []
|
||||
(let [node (-> "(def ^:private TYPE->MODEL {\"document\" :model/Document})"
|
||||
hooks/parse-string)]
|
||||
(lint-def {:node node}))))
|
||||
(require '[clj-kondo.core :as clj-kondo])
|
||||
(def form (str '(defmacro a
|
||||
[x]
|
||||
`(fun-call x))))
|
||||
|
||||
(def form "(defmacro a
|
||||
[x]
|
||||
`(some! ~x))")
|
||||
|
||||
(def form "(defun f
|
||||
[x]
|
||||
(let [g! (fn [] 1)]
|
||||
(g!)))")
|
||||
|
||||
(str (hooks/parse-string form))
|
||||
(hooks/sexpr (hooks/parse-string form))
|
||||
|
||||
(binding [hooks/*reload* true]
|
||||
(-> form
|
||||
(with-in-str (clj-kondo/run! {:lint ["-"]}))
|
||||
:findings))
|
||||
|
||||
(do (non-thread-safe-form-should-end-with-exclamation* (hooks/parse-string form)) nil))
|
||||
|
||||
(defn- ns-form-node->require-node [ns-form-node]
|
||||
(some (fn [node]
|
||||
(when (and (hooks/list-node? node)
|
||||
(let [first-child (first (:children node))]
|
||||
(and (hooks/keyword-node? first-child)
|
||||
(= (hooks/sexpr first-child) :require))))
|
||||
node))
|
||||
(:children ns-form-node)))
|
||||
|
||||
(defn- lint-require-shapes [ns-form-node]
|
||||
(doseq [node (-> ns-form-node
|
||||
ns-form-node->require-node
|
||||
:children
|
||||
rest)]
|
||||
(cond
|
||||
(not (hooks/vector-node? node))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "All :required namespaces should be wrapped in vectors [:metabase/require-shape-checker]"
|
||||
:type :metabase/require-shape-checker))
|
||||
|
||||
(hooks/vector-node? (second (:children node)))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "Don't use prefix forms inside :require [:metabase/require-shape-checker]"
|
||||
:type :metabase/require-shape-checker)))))
|
||||
|
||||
(defn- require-node->namespace-symb-nodes [require-node]
|
||||
(let [[_ns & args] (:children require-node)]
|
||||
(into []
|
||||
;; prefixed namespace forms are NOT SUPPORTED!!!!!!!!1
|
||||
(keep (fn [node]
|
||||
(cond
|
||||
(hooks/vector-node? node)
|
||||
;; propagate the metadata attached to this vector in case there's a `:clj-kondo/ignore` form.
|
||||
(vary-meta (first (:children node)) (partial merge (meta require-node) (meta node)))
|
||||
|
||||
;; this should also be dead code since we require requires to be vectors
|
||||
(hooks/token-node? node)
|
||||
(vary-meta node (partial merge (meta require-node)))
|
||||
|
||||
:else
|
||||
(printf "Don't know how to figure out what namespace is being required in %s\n" (pr-str node)))))
|
||||
args)))
|
||||
|
||||
(defn- ns-form-node->ns-symb [ns-form-node]
|
||||
(some-> (some (fn [node]
|
||||
(when (and (hooks/token-node? node)
|
||||
(not= (hooks/sexpr node) 'ns))
|
||||
node))
|
||||
(:children ns-form-node))
|
||||
hooks/sexpr))
|
||||
|
||||
(defn- module
|
||||
"E.g.
|
||||
|
||||
(module 'metabase.qp.middleware.wow) => 'metabase.qp"
|
||||
[ns-symb]
|
||||
(some-> (re-find #"^metabase\.[^.]+" (str ns-symb)) symbol))
|
||||
|
||||
(defn- ignored-namespace? [ns-symb config]
|
||||
(some
|
||||
(fn [pattern-str]
|
||||
(re-find (re-pattern pattern-str) (str ns-symb)))
|
||||
(:ignored-namespace-patterns config)))
|
||||
|
||||
(defn- module-api-namespaces
|
||||
"Set API namespaces for a given module. `:any` means you can use anything, there are no API namespaces for this
|
||||
module (yet). If unspecified, the default is just the namespace with the same name as the module e.g.
|
||||
`metabase.db`."
|
||||
[module config]
|
||||
(let [module-config (get-in config [:api-namespaces module])]
|
||||
(cond
|
||||
(= module-config :any)
|
||||
nil
|
||||
|
||||
(set? module-config)
|
||||
module-config
|
||||
|
||||
:else
|
||||
#{module})))
|
||||
|
||||
(defn- lint-modules [ns-form-node config]
|
||||
(let [ns-symb (ns-form-node->ns-symb ns-form-node)]
|
||||
(when-not (ignored-namespace? ns-symb config)
|
||||
(when-let [current-module (module ns-symb)]
|
||||
(let [allowed-modules (get-in config [:allowed-modules current-module])
|
||||
required-namespace-symb-nodes (-> ns-form-node
|
||||
ns-form-node->require-node
|
||||
require-node->namespace-symb-nodes)]
|
||||
(doseq [node required-namespace-symb-nodes
|
||||
:let [clj-kondo-ignore (some-> (meta node) :clj-kondo/ignore hooks/sexpr set)]
|
||||
:when (not (contains? clj-kondo-ignore :metabase/ns-module-checker))
|
||||
:let [required-namespace (hooks/sexpr node)
|
||||
required-module (module required-namespace)]
|
||||
;; ignore stuff not in a module i.e. non-Metabase stuff.
|
||||
:when required-module
|
||||
:let [in-current-module? (= required-module current-module)]
|
||||
:when (not in-current-module?)
|
||||
:let [allowed-module? (or (= allowed-modules :any)
|
||||
(contains? (set allowed-modules) required-module))
|
||||
module-api-namespaces (module-api-namespaces required-module config)
|
||||
allowed-module-namespace? (or (empty? module-api-namespaces)
|
||||
(contains? module-api-namespaces required-namespace))]]
|
||||
(when-let [error (cond
|
||||
(not allowed-module?)
|
||||
(format "Module %s should not be used in the %s module. [:metabase/ns-module-checker :allowed-modules %s]"
|
||||
required-module
|
||||
current-module
|
||||
current-module)
|
||||
|
||||
(not allowed-module-namespace?)
|
||||
(format "Namespace %s is not an allowed external API namespace for the %s module. [:metabase/ns-module-checker :api-namespaces %s]"
|
||||
required-namespace
|
||||
required-module
|
||||
required-module))]
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message error
|
||||
:type :metabase/ns-module-checker)))))))))
|
||||
|
||||
(defn lint-ns [x]
|
||||
(lint-require-shapes (:node x))
|
||||
(lint-modules (:node x) (get-in x [:config :linters :metabase/ns-module-checker]))
|
||||
x)
|
||||
|
||||
@@ -1,52 +0,0 @@
|
||||
(ns hooks.clojure.core.def
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.string :as str]))
|
||||
|
||||
(defn- uppercase-name?
|
||||
"Whether the symbol is clearly an uppercase name like `TYPE-CONSTANTS` or `SOME_CONSTANT` or `TYPE->MODEL`."
|
||||
[s]
|
||||
(and (re-find #"[A-Z0-9]+(?:[-_]>?[A-Z0-9]+)+" s)
|
||||
;; ignore stuff like `->SCREAMING_SNAKE_CASE`... this is ok
|
||||
(not (str/includes? s "SCREAMING_SNAKE_CASE"))))
|
||||
|
||||
(defn- has-underscores? [s]
|
||||
(and (str/includes? s "_")
|
||||
;; ignore stuff like `->snake_case` and `->SCREAMING_SNAKE_CASE`
|
||||
(not (str/includes? s "snake_case"))
|
||||
(not (str/includes? s "SCREAMING_SNAKE_CASE"))))
|
||||
|
||||
(defn- check-symbol-is-kebab-case [symbol-node]
|
||||
(let [symb (hooks/sexpr symbol-node)
|
||||
s (str symb)]
|
||||
(cond
|
||||
(uppercase-name? s)
|
||||
(hooks/reg-finding!
|
||||
(assoc (meta symbol-node)
|
||||
:message "Use lower-case names for functions and variables; don't use special notation for constants. [:metabase/check-def-check-not-all-uppercase]"
|
||||
:type :metabase/check-def-check-not-uppercase-name))
|
||||
|
||||
(has-underscores? s)
|
||||
(hooks/reg-finding!
|
||||
(assoc (meta symbol-node)
|
||||
:message "Use kebab-case names for functions and variables. [:metabase/check-def-no-underscores]"
|
||||
:type :metabase/check-def-no-underscores)))))
|
||||
|
||||
(defn lint-def* [{:keys [node]}]
|
||||
(let [[_def & args] (:children node)
|
||||
name-symbol (some (fn [arg]
|
||||
(when (and (hooks/token-node? arg)
|
||||
(symbol? (hooks/sexpr arg)))
|
||||
arg))
|
||||
args)]
|
||||
(check-symbol-is-kebab-case name-symbol)))
|
||||
|
||||
(defn lint-def [x]
|
||||
(lint-def* x)
|
||||
x)
|
||||
|
||||
(comment
|
||||
(defn x []
|
||||
(let [node (-> "(def ^:private TYPE->MODEL {\"document\" :model/Document})"
|
||||
hooks/parse-string)]
|
||||
(lint-def {:node node}))))
|
||||
@@ -1,7 +0,0 @@
|
||||
(ns hooks.clojure.core.defmacro
|
||||
(:require
|
||||
[hooks.clojure.core.def]))
|
||||
|
||||
(defn lint-defmacro [x]
|
||||
(hooks.clojure.core.def/lint-def* x)
|
||||
x)
|
||||
@@ -1,54 +0,0 @@
|
||||
(ns hooks.clojure.core.defmulti
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.string :as str]
|
||||
[hooks.clojure.core.def]
|
||||
[hooks.common]))
|
||||
|
||||
(defn- check-arglists [report-node arglists]
|
||||
(letfn [(reg-bad-arglists! []
|
||||
(hooks/reg-finding!
|
||||
(assoc (meta report-node)
|
||||
:message ":arglists should be a quoted list of vectors [:metabase/check-defmulti-arglists]"
|
||||
:type :metabase/check-defmulti-arglists)))
|
||||
(reg-bad-arg! []
|
||||
(hooks/reg-finding!
|
||||
(assoc (meta report-node)
|
||||
:message ":arglists should contain actual arg names, not underscore (unused) symbols [:metabase/check-defmulti-arglists]"
|
||||
:type :metabase/check-defmulti-arglists)))
|
||||
(underscore-arg? [arg]
|
||||
(and (symbol? arg)
|
||||
(str/starts-with? arg "_")))
|
||||
(check-arglist [arglist]
|
||||
(cond
|
||||
(not (vector? arglist)) (reg-bad-arglists!)
|
||||
(some underscore-arg? arglist) (reg-bad-arg!)))]
|
||||
(if-not (and (seq? arglists)
|
||||
(= (first arglists) 'quote)
|
||||
(seq (second arglists)))
|
||||
(reg-bad-arglists!)
|
||||
(let [[_quote arglists] arglists]
|
||||
(doseq [arglist arglists]
|
||||
(check-arglist arglist))))))
|
||||
|
||||
(defn- defmulti-check-for-arglists-metadata
|
||||
"Make sure a [[defmulti]] has an attribute map with `:arglists` metadata."
|
||||
[node]
|
||||
(let [[_defmulti _symb & args] (:children node)
|
||||
[_docstring & args] (if (hooks/string-node? (first args))
|
||||
args
|
||||
(cons nil args))
|
||||
attr-map (when (hooks/map-node? (first args))
|
||||
(first args))
|
||||
arglists (some-> attr-map hooks/sexpr :arglists seq)]
|
||||
(if-not (seq? arglists) ; should be a list or at least list-like
|
||||
(hooks/reg-finding!
|
||||
(assoc (meta node)
|
||||
:message "All defmultis should have an attribute map with :arglists metadata. [:metabase/check-defmulti-arglists]"
|
||||
:type :metabase/check-defmulti-arglists))
|
||||
(check-arglists attr-map arglists))))
|
||||
|
||||
(defn lint-defmulti [x]
|
||||
(defmulti-check-for-arglists-metadata (:node x))
|
||||
(hooks.clojure.core.def/lint-def* x)
|
||||
x)
|
||||
@@ -1,82 +0,0 @@
|
||||
(ns hooks.clojure.core.defn
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.string :as str]
|
||||
[hooks.clojure.core.def]
|
||||
[hooks.common]))
|
||||
|
||||
(defn- end-with-exclamation?
|
||||
[s]
|
||||
(str/ends-with? s "!"))
|
||||
|
||||
(defn- explicitly-safe? [config qualified-symbol]
|
||||
(contains? (get-in config [:linters :metabase/test-helpers-use-non-thread-safe-functions :explicitly-safe])
|
||||
qualified-symbol))
|
||||
|
||||
(defn- explicitly-unsafe? [config qualified-symbol]
|
||||
(contains? (get-in config [:linters :metabase/validate-deftest :parallel/unsafe]) qualified-symbol))
|
||||
|
||||
(defn- unsafe? [config qualified-symbol]
|
||||
(and (or (end-with-exclamation? qualified-symbol)
|
||||
(explicitly-unsafe? config qualified-symbol))
|
||||
(not (explicitly-safe? config qualified-symbol))))
|
||||
|
||||
(defn- non-thread-safe-form-should-end-with-exclamation*
|
||||
[{[defn-or-defmacro form-name] :children, :as node} config]
|
||||
(when-not (and (:string-value form-name)
|
||||
(end-with-exclamation? (:string-value form-name)))
|
||||
(letfn [(walk [f form]
|
||||
(f form)
|
||||
(doseq [child (:children form)]
|
||||
(walk f child)))
|
||||
(check-node [form]
|
||||
(when-let [qualified-symbol (hooks.common/node->qualified-symbol form)]
|
||||
(when (unsafe? config qualified-symbol)
|
||||
(hooks/reg-finding!
|
||||
(assoc (meta form-name)
|
||||
:message (format "The name of this %s should end with `!` because it contains calls to non thread safe form `%s`. [:metabase/test-helpers-use-non-thread-safe-functions]"
|
||||
(:string-value defn-or-defmacro) qualified-symbol)
|
||||
:type :metabase/test-helpers-use-non-thread-safe-functions)))))]
|
||||
(walk check-node node))
|
||||
node))
|
||||
|
||||
(defn non-thread-safe-form-should-end-with-exclamation
|
||||
"Used to ensure defn and defmacro in test namespace to have name ending with `!` if it's non-thread-safe.
|
||||
A function or a macro can be defined as 'not thread safe' when their funciton name ends with a `!`.
|
||||
|
||||
Only used in tests to identify thread-safe/non-thread-safe test helpers. See #37126"
|
||||
[{:keys [node cljc lang config]}]
|
||||
(when (or (not cljc)
|
||||
(= lang :clj))
|
||||
(non-thread-safe-form-should-end-with-exclamation* node config))
|
||||
{:node node})
|
||||
|
||||
(defn lint-defn [x]
|
||||
(non-thread-safe-form-should-end-with-exclamation x)
|
||||
(hooks.clojure.core.def/lint-def* x)
|
||||
x)
|
||||
|
||||
(comment
|
||||
(require '[clj-kondo.core :as clj-kondo])
|
||||
(def form (str '(defmacro a
|
||||
[x]
|
||||
`(fun-call x))))
|
||||
|
||||
(def form "(defmacro a
|
||||
[x]
|
||||
`(some! ~x))")
|
||||
|
||||
(def form "(defun f
|
||||
[x]
|
||||
(let [g! (fn [] 1)]
|
||||
(g!)))")
|
||||
|
||||
(str (hooks/parse-string form))
|
||||
(hooks/sexpr (hooks/parse-string form))
|
||||
|
||||
(binding [hooks/*reload* true]
|
||||
(-> form
|
||||
(with-in-str (clj-kondo/run! {:lint ["-"]}))
|
||||
:findings))
|
||||
|
||||
(do (non-thread-safe-form-should-end-with-exclamation* (hooks/parse-string form)) nil))
|
||||
@@ -1,155 +0,0 @@
|
||||
(ns hooks.clojure.core.ns
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.string :as str]
|
||||
[hooks.common]
|
||||
[hooks.common.modules :as modules]))
|
||||
|
||||
(defn- ns-form-node->require-node [ns-form-node]
|
||||
(some (fn [node]
|
||||
(when (and (hooks/list-node? node)
|
||||
(let [first-child (first (:children node))]
|
||||
(and (hooks/keyword-node? first-child)
|
||||
(= (hooks/sexpr first-child) :require))))
|
||||
node))
|
||||
(:children ns-form-node)))
|
||||
|
||||
(defn- lint-require-shapes [ns-form-node]
|
||||
(doseq [node (-> ns-form-node
|
||||
ns-form-node->require-node
|
||||
:children
|
||||
rest)]
|
||||
(cond
|
||||
(not (hooks/vector-node? node))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "All :required namespaces should be wrapped in vectors [:metabase/require-shape-checker]"
|
||||
:type :metabase/require-shape-checker))
|
||||
|
||||
(hooks/vector-node? (second (:children node)))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "Don't use prefix forms inside :require [:metabase/require-shape-checker]"
|
||||
:type :metabase/require-shape-checker)))))
|
||||
|
||||
(defn- lint-requires-on-new-lines [ns-form-node]
|
||||
(let [[require-keyword first-require] (-> ns-form-node
|
||||
ns-form-node->require-node
|
||||
:children)]
|
||||
(when-let [require-keyword-line (:row (meta require-keyword))]
|
||||
(when-let [first-require-line (:row (meta first-require))]
|
||||
(when (= require-keyword-line first-require-line)
|
||||
(hooks/reg-finding! (assoc (meta first-require)
|
||||
:message "Put your requires on a newline from the :require keyword [:metabase/require-shape-checker]"
|
||||
:type :metabase/require-shape-checker)))))))
|
||||
|
||||
(defn- require-node->namespace-symb-nodes [require-node]
|
||||
(let [[_ns & args] (:children require-node)]
|
||||
(into []
|
||||
;; prefixed namespace forms are NOT SUPPORTED!!!!!!!!1
|
||||
(keep (fn [node]
|
||||
(cond
|
||||
(hooks/vector-node? node)
|
||||
;; propagate the metadata attached to this vector in case there's a `:clj-kondo/ignore` form.
|
||||
(let [symbol-node (first (:children node))]
|
||||
(hooks.common/merge-ignored-linters symbol-node require-node node))
|
||||
|
||||
;; this should also be dead code since we require requires to be vectors
|
||||
(hooks/token-node? node)
|
||||
(hooks.common/merge-ignored-linters node require-node)
|
||||
|
||||
:else
|
||||
(printf "Don't know how to figure out what namespace is being required in %s\n" (pr-str node)))))
|
||||
args)))
|
||||
|
||||
(defn- ns-form-node->ns-symb-node [ns-form-node]
|
||||
(some (fn [node]
|
||||
(when (and (hooks/token-node? node)
|
||||
(not= (hooks/sexpr node) 'ns))
|
||||
node))
|
||||
(:children ns-form-node)))
|
||||
|
||||
(defn- ns-form-node->ns-symb [ns-form-node]
|
||||
(some-> (ns-form-node->ns-symb-node ns-form-node)
|
||||
hooks/sexpr))
|
||||
|
||||
(defn- lint-modules [ns-form-node config]
|
||||
(let [ns-symb (ns-form-node->ns-symb ns-form-node)]
|
||||
(when-not (modules/ignored-namespace? config ns-symb)
|
||||
(when-let [current-module (modules/module ns-symb)]
|
||||
(let [required-namespace-symb-nodes (-> ns-form-node
|
||||
ns-form-node->require-node
|
||||
require-node->namespace-symb-nodes)]
|
||||
(doseq [node required-namespace-symb-nodes
|
||||
:when (not (contains? (hooks.common/ignored-linters node) :metabase/modules))
|
||||
:let [required-namespace (hooks/sexpr node)
|
||||
error (modules/usage-error config current-module required-namespace)]
|
||||
:when error]
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message error
|
||||
:type :metabase/modules))))))))
|
||||
|
||||
(defn- lint-namespace-name [ns-form-node]
|
||||
(when-let [ns-symb-node (ns-form-node->ns-symb-node ns-form-node)]
|
||||
(when-not (contains? (hooks.common/ignored-linters ns-symb-node) :metabase/namespace-name)
|
||||
(let [parts (str/split (str (hooks/sexpr ns-symb-node)) #"\.")]
|
||||
(when (and (<= (count parts) 2)
|
||||
(#{"metabase" "metabase-enterprise"} (first parts))
|
||||
;; exclude test namespaces for now
|
||||
(not (str/ends-with? (last parts) "test")))
|
||||
(hooks/reg-finding! (assoc (meta ns-symb-node)
|
||||
:message "Metabase namespaces should have the form metabase[-enterprise].<module>.* [:metabase/namespace-name]"
|
||||
:type :metabase/namespace-name)))))))
|
||||
|
||||
(defn- ns-form-node->import-node [ns-form-node]
|
||||
(some (fn [node]
|
||||
(when (and (hooks/list-node? node)
|
||||
(let [first-child (first (:children node))]
|
||||
(and (hooks/keyword-node? first-child)
|
||||
(= (hooks/sexpr first-child) :import))))
|
||||
node))
|
||||
(:children ns-form-node)))
|
||||
|
||||
(defn- jsqlparser-import?
|
||||
"Check if a symbol or string represents a jsqlparser import."
|
||||
[s]
|
||||
(str/starts-with? (str s) "net.sf.jsqlparser"))
|
||||
|
||||
(defn- lint-jsqlparser-imports [ns-form-node]
|
||||
(when-let [import-node (ns-form-node->import-node ns-form-node)]
|
||||
(doseq [node (rest (:children import-node))
|
||||
:when (not (contains? (hooks.common/ignored-linters node) :metabase/no-jsqlparser-imports))]
|
||||
(cond
|
||||
;; Handle (net.sf.jsqlparser.foo Bar Baz) form
|
||||
(hooks/list-node? node)
|
||||
(let [package-node (first (:children node))
|
||||
package-name (when package-node (str (hooks/sexpr package-node)))]
|
||||
(when (jsqlparser-import? package-name)
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "Don't import net.sf.jsqlparser classes directly. This code should be agnostic to the parsing library. [:metabase/no-jsqlparser-imports]"
|
||||
:type :metabase/no-jsqlparser-imports))))
|
||||
|
||||
;; Handle net.sf.jsqlparser.foo.Bar form (single class import)
|
||||
(hooks/token-node? node)
|
||||
(when (jsqlparser-import? (hooks/sexpr node))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "Don't import net.sf.jsqlparser classes directly. This code should be agnostic to the parsing library. [:metabase/no-jsqlparser-imports]"
|
||||
:type :metabase/no-jsqlparser-imports)))))))
|
||||
|
||||
(defn lint-ns [x]
|
||||
(doto (:node x)
|
||||
lint-require-shapes
|
||||
lint-requires-on-new-lines
|
||||
(lint-modules (modules/config x))
|
||||
lint-namespace-name
|
||||
lint-jsqlparser-imports)
|
||||
x)
|
||||
|
||||
(comment
|
||||
(lint-ns
|
||||
{:node (hooks/parse-string
|
||||
(pr-str
|
||||
'(ns hooks.clojure.core.ns
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.string :as str]
|
||||
[hooks.common]
|
||||
[hooks.common.modules :as modules]))))}))
|
||||
@@ -1,75 +0,0 @@
|
||||
(ns hooks.clojure.core.require
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common.modules :as modules]))
|
||||
|
||||
(defn- lint* [node current-ns config required-namespaces]
|
||||
{:pre [(map? node)
|
||||
(simple-symbol? current-ns)
|
||||
(map? config)
|
||||
(sequential? required-namespaces)
|
||||
(every? simple-symbol? required-namespaces)]}
|
||||
(when-not (modules/ignored-namespace? config current-ns)
|
||||
;; ignore namespaces outside of the module system
|
||||
(when-let [current-module (modules/module current-ns)]
|
||||
(doseq [required-namespace required-namespaces]
|
||||
;; ignore namespaces outside of the module system.
|
||||
(when (modules/module required-namespace)
|
||||
(when-let [error (modules/usage-error config current-module required-namespace)]
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message error
|
||||
:type :metabase/modules))))))))
|
||||
|
||||
(defn- unwrap-require [node]
|
||||
(case (hooks/tag node)
|
||||
;; some.namespace of some.namespace/some-var
|
||||
:token
|
||||
(let [sexpr (hooks/sexpr node)]
|
||||
(if (qualified-symbol? sexpr)
|
||||
(symbol (namespace sexpr))
|
||||
sexpr))
|
||||
|
||||
;; (quote some.namespace)
|
||||
:list
|
||||
(let [[x y] (:children node)]
|
||||
(when (and (hooks/token-node? x)
|
||||
(= (hooks/sexpr x) 'quote))
|
||||
(recur y)))
|
||||
|
||||
;; 'some.namespace
|
||||
:quote
|
||||
(recur (first (:children node)))
|
||||
|
||||
;; something like [some.namespace :as whatever]
|
||||
:vector
|
||||
(recur (first (:children node)))))
|
||||
|
||||
(comment
|
||||
(unwrap-require (-> "(requiring-resolve 'metabase.notification.payload.execute/execute-dashboard)"
|
||||
hooks/parse-string
|
||||
:children
|
||||
second)))
|
||||
|
||||
(defn- lint-require* [node current-ns config]
|
||||
(let [[_require & args] (:children node)
|
||||
required-namespaces (->> (keep unwrap-require args)
|
||||
;; `require` runs through `cc/load-libs` which allows
|
||||
;; #{:as :reload :reload-all :require :use :verbose :refer :as-alias}
|
||||
;; as args. If you use a REPL tool that applies clj-kondo linting (e.g., Clojure MCP)
|
||||
;; you may wish to `:reload` or `:reload-all` a namespace. So we shouldn't assume all
|
||||
;; args are keywords:
|
||||
(remove keyword?))]
|
||||
(lint* node current-ns config required-namespaces)))
|
||||
|
||||
(defn- lint-requiring-resolve* [node current-ns config]
|
||||
(let [[_requiring-resolve symb-node] (:children node)
|
||||
required-namespace (unwrap-require symb-node)]
|
||||
(lint* node current-ns config [required-namespace])))
|
||||
|
||||
(defn lint-require [x]
|
||||
(lint-require* (:node x) (:ns x) (modules/config x))
|
||||
x)
|
||||
|
||||
(defn lint-requiring-resolve [x]
|
||||
(lint-requiring-resolve* (:node x) (:ns x) (modules/config x))
|
||||
x)
|
||||
@@ -2,10 +2,116 @@
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.string :as str]
|
||||
[hooks.common]
|
||||
[hooks.common.modules :as modules]))
|
||||
[hooks.common]))
|
||||
|
||||
(defn- warn-about-disallowed-parallel-forms [form config]
|
||||
(def ^:private disallowed-parallel-forms
|
||||
"Things you should not be allowed to use inside parallel tests. Besides these, anything ending in `!` not whitelisted
|
||||
in [[allowed-parallel-forms]] is not allowed."
|
||||
'#{clojure.core/alter-var-root
|
||||
clojure.core/with-redefs
|
||||
clojure.core/with-redefs-fn
|
||||
metabase-enterprise.sandbox.test-util/with-gtaps!
|
||||
metabase-enterprise.sandbox.test-util/with-gtaps-for-user!
|
||||
metabase-enterprise.sandbox.test-util/with-user-attributes
|
||||
metabase-enterprise.test/with-gtaps!
|
||||
metabase-enterprise.test/with-gtaps-for-user!
|
||||
metabase-enterprise.test/with-user-attributes
|
||||
metabase.actions.test-util/with-actions
|
||||
metabase.actions.test-util/with-actions-disabled
|
||||
metabase.actions.test-util/with-actions-enabled
|
||||
metabase.actions.test-util/with-actions-test-data
|
||||
metabase.actions.test-util/with-actions-test-data-and-actions-enabled
|
||||
metabase.actions.test-util/with-actions-test-data-tables
|
||||
metabase.analytics.snowplow-test/with-fake-snowplow-collector
|
||||
metabase.email-test/with-expected-messages
|
||||
metabase.email-test/with-fake-inbox
|
||||
metabase.test.data.users/with-group
|
||||
metabase.test.data.users/with-group-for-user
|
||||
metabase.test.persistence/with-persistence-enabled
|
||||
metabase.test.util.log/with-log-level
|
||||
metabase.test.util.log/with-log-messages-for-level
|
||||
metabase.test.util.misc/with-single-admin-user
|
||||
metabase.test.util/with-all-users-permission
|
||||
metabase.test.util/with-discarded-collections-perms-changes
|
||||
metabase.test.util/with-env-keys-renamed-by
|
||||
metabase.test.util/with-locale
|
||||
metabase.test.util/with-non-admin-groups-no-root-collection-for-namespace-perms
|
||||
metabase.test.util/with-non-admin-groups-no-root-collection-perms
|
||||
metabase.test.util/with-temp-env-var-value
|
||||
metabase.test.util/with-temp-vals-in-db
|
||||
metabase.test.util/with-temporary-raw-setting-values
|
||||
metabase.test.util/with-temporary-setting-values
|
||||
metabase.test.util/with-user-in-groups
|
||||
metabase.test/with-actions
|
||||
metabase.test/with-actions-disabled
|
||||
metabase.test/with-actions-enabled
|
||||
metabase.test/with-actions-test-data
|
||||
metabase.test/with-actions-test-data-and-actions-enabled
|
||||
metabase.test/with-actions-test-data-tables
|
||||
metabase.test/with-all-users-permission
|
||||
metabase.test/with-discarded-collections-perms-changes
|
||||
metabase.test/with-env-keys-renamed-by
|
||||
metabase.test/with-expected-messages
|
||||
metabase.test/with-fake-inbox
|
||||
metabase.test/with-group
|
||||
metabase.test/with-group-for-user
|
||||
metabase.test/with-locale
|
||||
metabase.test/with-log-level
|
||||
metabase.test/with-log-messages-for-level
|
||||
metabase.test/with-non-admin-groups-no-root-collection-for-namespace-perms
|
||||
metabase.test/with-non-admin-groups-no-root-collection-perms
|
||||
metabase.test/with-persistence-enabled
|
||||
metabase.test/with-single-admin-user
|
||||
metabase.test/with-temp-env-var-value
|
||||
metabase.test/with-temp-vals-in-db
|
||||
metabase.test/with-temporary-raw-setting-values
|
||||
metabase.test/with-temporary-setting-values
|
||||
metabase.test/with-user-in-groups})
|
||||
|
||||
;;; TODO -- we should disallow `metabase.test/user-http-request` with any method other than `:get`
|
||||
|
||||
(def ^:private allowed-parallel-forms
|
||||
"These fns are destructive, but are probably fine inside ^:parallel tests because it usually means you're doing
|
||||
something to an atom or something like that."
|
||||
'#{clojure.core/assoc!
|
||||
clojure.core/compare-and-set!
|
||||
clojure.core/conj!
|
||||
clojure.core/disj!
|
||||
clojure.core/dissoc!
|
||||
clojure.core/persistent!
|
||||
clojure.core/pop!
|
||||
clojure.core/reset!
|
||||
clojure.core/reset-vals!
|
||||
clojure.core/run!
|
||||
clojure.core/swap!
|
||||
clojure.core/swap-vals!
|
||||
clojure.core/volatile!
|
||||
clojure.core/vreset!
|
||||
clojure.core/vswap!
|
||||
clojure.core.async/<!
|
||||
clojure.core.async/<!!
|
||||
clojure.core.async/>!
|
||||
clojure.core.async/>!!
|
||||
clojure.core.async/alt!
|
||||
clojure.core.async/alt!!
|
||||
clojure.core.async/alts!
|
||||
clojure.core.async/alts!!
|
||||
clojure.core.async/close!
|
||||
clojure.core.async/ioc-alts!
|
||||
clojure.core.async/offer!
|
||||
clojure.core.async/onto-chan!
|
||||
clojure.core.async/onto-chan!!
|
||||
clojure.core.async/poll!
|
||||
clojure.core.async/put!
|
||||
clojure.core.async/take!
|
||||
clojure.core.async/to-chan!
|
||||
clojure.core.async/to-chan!!
|
||||
metabase.driver.sql-jdbc.execute/execute-prepared-statement!
|
||||
metabase.pulse/send-pulse!
|
||||
metabase.query-processor.store/store-database!
|
||||
next.jdbc/execute!})
|
||||
|
||||
(defn- warn-about-disallowed-parallel-forms [form]
|
||||
(letfn [(error! [form message]
|
||||
(hooks/reg-finding! (assoc (meta form)
|
||||
:message message
|
||||
@@ -13,12 +119,12 @@
|
||||
(f [form]
|
||||
(when-let [qualified-symbol (hooks.common/node->qualified-symbol form)]
|
||||
(cond
|
||||
(contains? (:parallel/unsafe config) qualified-symbol)
|
||||
(error! form (format "%s is not allowed inside a ^:parallel test or test fixture [:metabase/validate-deftest]" qualified-symbol))
|
||||
(disallowed-parallel-forms qualified-symbol)
|
||||
(error! form (format "%s is not allowed inside a ^:parallel test or test fixture" qualified-symbol))
|
||||
|
||||
(and (not (contains? (:parallel/safe config) qualified-symbol))
|
||||
(and (not (allowed-parallel-forms qualified-symbol))
|
||||
(str/ends-with? (name qualified-symbol) "!"))
|
||||
(error! form (format "destructive functions like %s are not allowed inside a ^:parallel test or test fixture. If this should be allowed, add it to the whitelist in the Kondo config file [:metabase/validate-deftest]"
|
||||
(error! form (format "destructive functions like %s are not allowed inside a ^:parallel test or test fixture. If this should be allowed, add it to the whitelist in .clj-kondo/hooks/clojure/test.clj"
|
||||
qualified-symbol)))))
|
||||
(walk [form]
|
||||
(f form)
|
||||
@@ -29,7 +135,7 @@
|
||||
(defn- deftest-check-parallel
|
||||
"1. Check if test is marked ^:parallel / ^:synchronized correctly
|
||||
2. Make sure disallowed forms are not used in ^:parallel tests"
|
||||
[{[_ test-name & body] :children, :as _node} config]
|
||||
[{[_ test-name & body] :children, :as _node}]
|
||||
(let [test-metadata (:meta test-name)
|
||||
metadata-sexprs (map hooks/sexpr test-metadata)
|
||||
combined-metadata (transduce
|
||||
@@ -46,9 +152,9 @@
|
||||
(hooks/reg-finding! (assoc (meta test-name)
|
||||
:message "Test should not be marked both ^:parallel and ^:synchronized"
|
||||
:type :metabase/validate-deftest)))
|
||||
;; only when the custom `:metabase/deftest-not-marked-parallel-or-synchronized` is enabled: complain if tests are
|
||||
;; not explicitly marked `^:parallel` or `^:synchronized`. This is mostly to encourage people to mark everything
|
||||
;; `^:parallel` in places like `metabase.lib` tests unless there is a really good reason not to.
|
||||
;; only when the custom `:metabase/deftest-not-marked-parallel` is enabled: complain if tests are not explicitly
|
||||
;; marked `^:parallel` or `^:synchronized`. This is mostly to encourage people to mark everything `^:parallel` in
|
||||
;; places like `metabase.lib` tests unless there is a really good reason not to.
|
||||
(when-not (or parallel? synchronized?)
|
||||
(hooks/reg-finding!
|
||||
(assoc (meta test-name)
|
||||
@@ -56,95 +162,31 @@
|
||||
:type :metabase/deftest-not-marked-parallel-or-synchronized)))
|
||||
(when parallel?
|
||||
(doseq [form body]
|
||||
(warn-about-disallowed-parallel-forms form config)))))
|
||||
(warn-about-disallowed-parallel-forms form)))))
|
||||
|
||||
(def ^:private number-of-lines-for-a-test-to-be-considered-horrifically-long
|
||||
200)
|
||||
|
||||
(defn- deftest-check-not-horrifically-long
|
||||
[node {:keys [max-length], :as _config}]
|
||||
[node]
|
||||
(let [{:keys [row end-row]} (meta node)]
|
||||
(when (and row end-row)
|
||||
(let [num-lines (- end-row row)]
|
||||
(when (and max-length
|
||||
(>= num-lines max-length))
|
||||
(when (>= num-lines number-of-lines-for-a-test-to-be-considered-horrifically-long)
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message (str (format "This test is horrifically long, it's %d lines! 😱 " num-lines)
|
||||
"Do you really want to try to debug it if it fails? 💀 "
|
||||
"Split it up into smaller tests! 🥰")
|
||||
:type :metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests)))))))
|
||||
|
||||
(defn- ignore? [node error-type]
|
||||
(contains? (hooks.common/ignored-linters node) error-type))
|
||||
|
||||
(defn deftest-check-no-driver-keywords [node {:keys [drivers], :as _config}]
|
||||
;; fail fast after we see the first error, where we see one hardcoded driver name there are likely several more and we
|
||||
;; don't need multiple warnings for a single test.
|
||||
(letfn [(f [node]
|
||||
(when (and (hooks/keyword-node? node)
|
||||
(set? drivers)
|
||||
(contains? drivers (hooks/sexpr node)))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message (format "Do not hardcode driver name %s in driver tests! [:metabase/disallow-hardcoded-driver-names-in-tests]"
|
||||
(hooks/sexpr node))
|
||||
:type :metabase/disallow-hardcoded-driver-names-in-tests))
|
||||
::error))
|
||||
(walk [node]
|
||||
(cond
|
||||
(ignore? node :metabase/disallow-hardcoded-driver-names-in-tests)
|
||||
nil
|
||||
|
||||
(= (f node) ::error)
|
||||
::error
|
||||
|
||||
:else
|
||||
(reduce
|
||||
(fn [_acc child]
|
||||
(when (= (walk child) ::error)
|
||||
(reduced ::error)))
|
||||
nil
|
||||
(:children node))))]
|
||||
(walk node)))
|
||||
|
||||
(defn- test-namespace-lives-outside-of-module-system? [ns-symb]
|
||||
(some (fn [prefix]
|
||||
(str/starts-with? (name ns-symb) prefix))
|
||||
["build-drivers."
|
||||
"build."
|
||||
"i18n." ; bin/i18n
|
||||
"lint-migrations-file-test"
|
||||
"main-test" ; bin/release-list
|
||||
"metabase.deps-edn-test"
|
||||
"metabase.driver."
|
||||
"metabase.test."
|
||||
"metabase.test.data."
|
||||
"metabuild-common."])) ; utils for build scripts
|
||||
|
||||
(let [warned-namespaces (atom #{})]
|
||||
(defn deftest-check-in-valid-module
|
||||
"Check whether a `deftest` lives in a known module -- important so our system to only run a subset of tests based on
|
||||
which modules have changes picks them up correctly. (DEV-1125)"
|
||||
[{:keys [node], ns-symb :ns, :as input}]
|
||||
(when (and ns-symb
|
||||
(not (test-namespace-lives-outside-of-module-system? ns-symb))
|
||||
;; only warn once per namespace
|
||||
(not (contains? @warned-namespaces ns-symb)))
|
||||
(swap! warned-namespaces conj ns-symb)
|
||||
(let [known-modules (set (keys (:metabase/modules (modules/config input))))
|
||||
current-module (modules/module ns-symb)]
|
||||
(when (or (not current-module)
|
||||
(not (contains? known-modules current-module)))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message (format "All tests must live in a known module; %s is not a known module" (pr-str current-module))
|
||||
:type :metabase/tests-must-live-in-known-modules)))))))
|
||||
|
||||
(defn deftest [{:keys [node cljc lang config], :as input}]
|
||||
(defn deftest [{:keys [node cljc lang]}]
|
||||
;; run [[deftest-check-parallel]] only once... if this is a `.cljc` file only run it for the `:clj` analysis, no point
|
||||
;; in running it twice.
|
||||
(when (or (not cljc)
|
||||
(= lang :clj))
|
||||
(deftest-check-parallel node (get-in config [:linters :metabase/validate-deftest])))
|
||||
(deftest-check-not-horrifically-long node (get-in config [:linters :metabase/i-like-making-cams-eyes-bleed-with-horrifically-long-tests]))
|
||||
(deftest-check-no-driver-keywords node (get-in config [:linters :metabase/disallow-hardcoded-driver-names-in-tests]))
|
||||
(deftest-check-in-valid-module input)
|
||||
input)
|
||||
(deftest-check-parallel node))
|
||||
(deftest-check-not-horrifically-long node)
|
||||
{:node node})
|
||||
|
||||
;;; this is a hacky way to determine whether these namespaces are required in the `ns` form or not... basically `:ns`
|
||||
;;; will come back as `nil` if they are not.
|
||||
@@ -180,11 +222,20 @@
|
||||
|
||||
nil)))))))
|
||||
|
||||
(defn- warn-about-schema= [{[_is assertion-node] :children, :as _is-node}]
|
||||
(let [{[assertion-symb-node] :children} assertion-node]
|
||||
(when (and (hooks/token-node? assertion-symb-node)
|
||||
(= (hooks/sexpr assertion-symb-node) 'schema=))
|
||||
(hooks/reg-finding! (assoc (meta assertion-symb-node)
|
||||
:message "Use =? or malli= instead of schema="
|
||||
:type :metabase/warn-about-schema=)))))
|
||||
|
||||
(defn is [{:keys [node lang]}]
|
||||
(when (= lang :cljs)
|
||||
(warn-about-missing-test-expr-requires-in-cljs node))
|
||||
(warn-about-schema= node)
|
||||
{:node node})
|
||||
|
||||
(defn use-fixtures [{:keys [node config]}]
|
||||
(warn-about-disallowed-parallel-forms node config)
|
||||
(defn use-fixtures [{:keys [node]}]
|
||||
(warn-about-disallowed-parallel-forms node)
|
||||
{:node node})
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
(ns hooks.common
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.pprint]
|
||||
[clojure.set :as set]))
|
||||
[clojure.pprint]))
|
||||
|
||||
(defn with-macro-meta
|
||||
"When introducing internal nodes (let, defn, etc) it is important to provide a meta of an existing token
|
||||
@@ -10,83 +9,10 @@
|
||||
[new-node hook-node]
|
||||
(with-meta new-node (meta (first (:children hook-node)))))
|
||||
|
||||
;;; Kondo stores information about ignored linters as rewrite-clj forms matching how they are specified in the code, so
|
||||
;;; either
|
||||
;;;
|
||||
;;; <vector-node [<token-node :discouraged-namespace>]>
|
||||
;;;
|
||||
;;; or
|
||||
;;;
|
||||
;;; <token-node :discouraged-namespace>
|
||||
;;;
|
||||
;;; these are stored in
|
||||
;;;
|
||||
;;; (-> node meta :clj-kondo/ignore :linters).
|
||||
;;;
|
||||
;;; These are not a convenient shape(s) to work with, so the next few functions abstract that shape out and work with a
|
||||
;;; set of keywords like
|
||||
;;;
|
||||
;;; #{:discouraged-namespace}
|
||||
;;;
|
||||
;;; instead.
|
||||
|
||||
(defn- unwrap-ignored-linters [linters]
|
||||
(cond
|
||||
(or (hooks/vector-node? linters)
|
||||
(hooks/list-node? linters))
|
||||
(into #{}
|
||||
(map hooks/sexpr)
|
||||
(:children linters))
|
||||
|
||||
(hooks/keyword-node? linters)
|
||||
#{(hooks/sexpr linters)}
|
||||
|
||||
(and (set? linters)
|
||||
(every? keyword? linters))
|
||||
linters
|
||||
|
||||
(and (sequential? linters)
|
||||
(every? keyword? linters))
|
||||
(into #{} linters)
|
||||
|
||||
(keyword? linters)
|
||||
#{linters}))
|
||||
|
||||
(defn- wrap-ignored-linters [linters]
|
||||
(hooks/vector-node (mapv hooks/keyword-node linters)))
|
||||
|
||||
(defn ignored-linters
|
||||
"Return a set of keywords all ignored linters associated with a node by looking at its metadata."
|
||||
[node]
|
||||
(or (some-> node meta :clj-kondo/ignore :linters unwrap-ignored-linters)
|
||||
;; deal with the old pre-2024.09 shape mentioned in https://github.com/clj-kondo/clj-kondo/issues/2414
|
||||
(some-> node :meta first hooks/sexpr :clj-kondo/ignore unwrap-ignored-linters)))
|
||||
|
||||
(defn update-ignored-linters
|
||||
"Update the `:clj-kondo/ignore` linters in `node`'s metadata. `linters` should be a set of keywords."
|
||||
[node f & args]
|
||||
(letfn [(apply-f [linters]
|
||||
(apply f linters args))
|
||||
(update-linters [linters]
|
||||
(-> (or (unwrap-ignored-linters linters)
|
||||
#{})
|
||||
apply-f
|
||||
wrap-ignored-linters))]
|
||||
(vary-meta node update-in [:clj-kondo/ignore :linters] update-linters)))
|
||||
|
||||
(defn merge-ignored-linters
|
||||
"Merge the ignored linters from `source-nodes` into `node`. You can use this to propagate
|
||||
ignored linters from parent nodes to child nodes."
|
||||
[node & source-nodes]
|
||||
(if-let [linters (not-empty (transduce (map ignored-linters) set/union #{} source-nodes))]
|
||||
(update-ignored-linters node set/union linters)
|
||||
node))
|
||||
|
||||
;;; This stuff is to help debug hooks. Trace a function and it will pretty-print the before & after sexprs.
|
||||
;;;
|
||||
;;; (hooks.common/trace #'calculate-bird-scarcity)
|
||||
|
||||
#_{:clj-kondo/ignore [:discouraged-var]}
|
||||
(defn- trace* [f]
|
||||
{:pre [(fn? f)]}
|
||||
(fn traced-fn [node]
|
||||
@@ -116,14 +42,13 @@
|
||||
;;;; Common hook definitions
|
||||
|
||||
(defn do*
|
||||
"This is the same idea as [[clojure.core/do]] but doesn't cause Kondo to complain about redundant dos or unused
|
||||
values."
|
||||
"This is the same idea as [[clojure.core/do]] but doesn't cause Kondo to complain about redundant dos or unused values."
|
||||
[{{[_ & args] :children, :as node} :node}]
|
||||
(let [node* (-> (hooks/list-node
|
||||
(list*
|
||||
(update-ignored-linters (hooks/token-node 'do) conj :redundant-do)
|
||||
(with-meta (hooks/token-node 'do) {:clj-kondo/ignore [:redundant-do]})
|
||||
(for [arg args]
|
||||
(update-ignored-linters arg conj :unused-value))))
|
||||
(vary-meta arg update :clj-kondo/ignore #(conj (vec %) :unused-value)))))
|
||||
(with-meta (meta node)))]
|
||||
{:node node*}))
|
||||
|
||||
@@ -363,8 +288,8 @@
|
||||
(-> (hooks/vector-node
|
||||
[(hooks/vector-node (map :model binding-infos))
|
||||
(-> (hooks/list-node (list* (hooks/token-node `let)
|
||||
(hooks/vector-node (mapcat (juxt :binding :value) binding-infos))
|
||||
body))
|
||||
(hooks/vector-node (mapcat (juxt :binding :value) binding-infos))
|
||||
body))
|
||||
(with-meta (meta body)))])
|
||||
(with-meta (meta body)))))
|
||||
|
||||
@@ -439,58 +364,14 @@
|
||||
where the first arg should be linted and appear to be used."
|
||||
[{{[_ arg & body] :children} :node}]
|
||||
(let [node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node [(hooks/token-node (gensym "_"))
|
||||
arg])
|
||||
body))]
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node [(hooks/token-node (gensym "_"))
|
||||
arg])
|
||||
body))]
|
||||
{:node node*}))
|
||||
|
||||
(defn with-used-first-two-args
|
||||
"For macros like
|
||||
|
||||
(with-limit-and-offset limit offset
|
||||
...)
|
||||
|
||||
=>
|
||||
|
||||
(let [_123 limit
|
||||
[_456 offset]
|
||||
...)
|
||||
|
||||
where the first arg should be linted and appear to be used."
|
||||
[{{[_ x y & body] :children} :node}]
|
||||
(let [node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node [(hooks/token-node (gensym "_")) x
|
||||
(hooks/token-node (gensym "_")) y])
|
||||
body))]
|
||||
{:node node*}))
|
||||
|
||||
(defn with-vec-first-binding
|
||||
"For macros like
|
||||
|
||||
(with-temp-file [binding \"temp.txt\" \"content\"]
|
||||
(slurp binding))
|
||||
|
||||
=>
|
||||
(let [binding nil]
|
||||
(slurp binding))
|
||||
|
||||
where the first arg is a vector of bindings."
|
||||
[{{[_ {[x & _args] :children} & body] :children} :node}]
|
||||
(let [node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node
|
||||
[(or x (hooks/token-node '_)) (hooks/token-node 'nil)])
|
||||
body))]
|
||||
{:node node*}))
|
||||
|
||||
(defn node->qualified-symbol
|
||||
"If the node is a symbol, return it as a qualified symbol if it's not already. Otherwise return nil."
|
||||
[node]
|
||||
(defn node->qualified-symbol [node]
|
||||
(try
|
||||
(when (hooks/token-node? node)
|
||||
(let [sexpr (hooks/sexpr node)]
|
||||
@@ -500,8 +381,8 @@
|
||||
(and resolved (:ns resolved))
|
||||
(symbol (name (:ns resolved)) (name (:name resolved)))
|
||||
|
||||
;; if it wasn't resolved but is still qualified it's probably using the full namespace name rather than an
|
||||
;; alias.
|
||||
;; if it wasn't resolved but is still qualified it's probably using the full namespace name rather than an
|
||||
;; alias.
|
||||
(qualified-symbol? sexpr)
|
||||
sexpr)))))
|
||||
;; some symbols like `*count/Integer` aren't resolvable.
|
||||
@@ -526,12 +407,3 @@
|
||||
|
||||
;; should be 1
|
||||
(format-string-specifier-count "%-02d"))
|
||||
|
||||
(defn add-lsp-ignore-unused-public-var-metadata
|
||||
"Add
|
||||
|
||||
^{:clj-kondo/ignore [:clojure-lsp/unused-public-var]}
|
||||
|
||||
metadata to a node to suppress LSP warnings."
|
||||
[node]
|
||||
(update-ignored-linters node conj :clojure-lsp/unused-public-var))
|
||||
|
||||
@@ -1,116 +0,0 @@
|
||||
(ns hooks.common.modules
|
||||
(:require
|
||||
[clojure.string :as str]))
|
||||
|
||||
(defn ignored-namespace? [config ns-symb]
|
||||
(some
|
||||
(fn [pattern-str]
|
||||
(re-find (re-pattern pattern-str) (str ns-symb)))
|
||||
(:ignored-namespace-patterns config)))
|
||||
|
||||
(defn config [{:keys [config], :as _hook-input}]
|
||||
(merge (get-in config [:linters :metabase/modules])
|
||||
(select-keys config [:metabase/modules])))
|
||||
|
||||
(defn module
|
||||
"E.g.
|
||||
|
||||
(module 'metabase.qp.middleware.wow) => 'qp
|
||||
(module 'metabase-enterprise.whatever.core) => enterprise/whatever"
|
||||
[ns-symb]
|
||||
{:pre [(simple-symbol? ns-symb)]}
|
||||
;; treat something like `metabase.driver-test` (for a module that hasn't fully been updated to use `.core`
|
||||
;; namespaces) as being in the `driver` module
|
||||
(let [ns-symb (if (str/ends-with? (name ns-symb) "-test")
|
||||
(symbol (str/replace (name ns-symb) #"-test$" ""))
|
||||
ns-symb)]
|
||||
(or (some->> (re-find #"^metabase-enterprise\.([^.]+)" (str ns-symb))
|
||||
second
|
||||
(symbol "enterprise"))
|
||||
(some-> (re-find #"^metabase\.([^.]+)" (str ns-symb))
|
||||
second
|
||||
symbol))))
|
||||
|
||||
(defn- module-api-namespaces
|
||||
"Set of API namespace symbols for a given module. `:any` means you can use anything, there are no API namespaces for
|
||||
this module (yet). If unspecified, the default is just the `<module>.core` namespace."
|
||||
[config module]
|
||||
(let [module-config (get-in config [:metabase/modules module :api])]
|
||||
(cond
|
||||
(= module-config :any)
|
||||
nil
|
||||
|
||||
(set? module-config)
|
||||
module-config
|
||||
|
||||
:else
|
||||
(let [ns-prefix (if (= (namespace module) "enterprise")
|
||||
(str "metabase-enterprise." (name module))
|
||||
(name module))]
|
||||
#{(symbol (str ns-prefix ".api"))
|
||||
(symbol (str ns-prefix ".core"))
|
||||
(symbol (str ns-prefix ".init"))}))))
|
||||
|
||||
(defn- module-friends
|
||||
[config module]
|
||||
"Set of modules that are `:friends` of `module`, i.e. allowed to use *any* namespace from the module, not just the
|
||||
designated [[module-api-namespaces]]."
|
||||
(set (get-in config [:metabase/modules module :friends])))
|
||||
|
||||
(defn allowed-modules
|
||||
"Set of namespace symbols that `module` is allowed to use. `:any` means it's allowed to use anything."
|
||||
[config module]
|
||||
(get-in config [:metabase/modules module :uses]))
|
||||
|
||||
(defn allowed-module? [config module required-module]
|
||||
(let [allowed-modules (allowed-modules config module)]
|
||||
(or (= allowed-modules :any)
|
||||
(contains? (set allowed-modules) required-module))))
|
||||
|
||||
(defn- allowed-module-namespace? [config current-module ns-symb]
|
||||
(let [module (module ns-symb)
|
||||
module-api-namespaces (module-api-namespaces config module)
|
||||
module-friends (module-friends config module)]
|
||||
(or (empty? module-api-namespaces)
|
||||
(contains? module-api-namespaces ns-symb)
|
||||
(contains? module-friends current-module))))
|
||||
|
||||
(defn- rest-module? [module]
|
||||
(str/ends-with? module "-rest"))
|
||||
|
||||
(defn- routes-module? [module]
|
||||
(str/ends-with? module "-routes"))
|
||||
|
||||
(defn- core-module? [module]
|
||||
(str/ends-with? module "core"))
|
||||
|
||||
(defn usage-error
|
||||
"Find usage errors when a `required-namespace` is required in the `current-module`. Returns a string describing the
|
||||
error type if there is one, otherwise `nil` if there are no errors."
|
||||
[config current-module required-namespace]
|
||||
;; ignore stuff not in a module i.e. non-Metabase stuff.
|
||||
(when-let [required-module (module required-namespace)]
|
||||
(when-not (= current-module required-module)
|
||||
(cond
|
||||
(not (allowed-module? config current-module required-module))
|
||||
(format "Module %s should not be used in the %s module. [:metabase/modules %s :uses]"
|
||||
required-module
|
||||
current-module
|
||||
current-module)
|
||||
|
||||
(not (allowed-module-namespace? config current-module required-namespace))
|
||||
(format "Namespace %s is not an allowed external API namespace for the %s module. [:metabase/modules %s :api]"
|
||||
required-namespace
|
||||
required-module
|
||||
required-module)
|
||||
|
||||
;; (for now) rest modules are allowed to use one another; `routes` is ok because it collects routes together
|
||||
;; and `core` is ok because [[metabase.core.init]] might need to init some of the `-routes` modules'
|
||||
;; namespaces
|
||||
(and (not ((some-fn rest-module? routes-module? core-module?) current-module))
|
||||
(rest-module? required-module))
|
||||
(format "Do not use -rest modules (%s) in non-rest modules (%s) -- move things from %s to %s if needed"
|
||||
required-module
|
||||
current-module
|
||||
required-module
|
||||
(symbol (str/replace required-module #"-rest$" "")))))))
|
||||
@@ -1,26 +0,0 @@
|
||||
(ns hooks.malli.error
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common]))
|
||||
|
||||
(defn humanize [x]
|
||||
(letfn [(check-node [node]
|
||||
(when (hooks/list-node? node)
|
||||
(let [[_ child] (:children node)]
|
||||
(when (hooks/list-node? child)
|
||||
(let [[symb] (:children child)]
|
||||
(when-let [qualified-symb (hooks.common/node->qualified-symbol symb)]
|
||||
(when ('#{malli.core/validate metabase.util.malli.registry/validate}
|
||||
qualified-symb)
|
||||
(hooks/reg-finding! (assoc (meta symb)
|
||||
:message "Use malli.error/humanize with explain, NOT with validate. [:metabase/check-me-humanize]"
|
||||
:type :metabase/check-me-humanize)))))))))]
|
||||
(check-node (:node x))
|
||||
x))
|
||||
|
||||
(comment
|
||||
(humanize {:node (hooks/parse-string
|
||||
(pr-str '(malli.error/humanize
|
||||
(malli.core/validate
|
||||
metabase.sync.sync-metadata.dbms-version/DBMSVersion
|
||||
metabase.driver.druid.sync-test/dbms-version))))}))
|
||||
@@ -0,0 +1,28 @@
|
||||
(ns hooks.metabase.api.automagic-dashboards-test
|
||||
(:require [clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(comment
|
||||
|
||||
(def input (hooks/parse-string
|
||||
"(with-indexed-model [{:keys [model model-index model-index-value]}
|
||||
{:query :some-query}]
|
||||
:foo :bar :body)"))
|
||||
|
||||
(println (hooks/sexpr input))
|
||||
|
||||
(let [f (load-file ".clj-kondo/hooks/metabase/api/automagic_dashboards_test.clj")]
|
||||
(f {:node input})))
|
||||
|
||||
(defn with-indexed-model
|
||||
[{node :node}]
|
||||
(let [[_macro-call binding-and-query-info & body] (:children node)
|
||||
[binding query-info] (:children binding-and-query-info)
|
||||
node* (hooks/list-node
|
||||
[(hooks/token-node 'do-with-testing-model)
|
||||
query-info
|
||||
(hooks/list-node
|
||||
(list* (hooks/token-node 'fn)
|
||||
(hooks/vector-node [binding])
|
||||
body))])]
|
||||
#_(println (hooks/sexpr node*))
|
||||
{:node node*}))
|
||||
45
.clj-kondo/src/hooks/metabase/api/common.clj
Normal file
45
.clj-kondo/src/hooks/metabase/api/common.clj
Normal file
@@ -0,0 +1,45 @@
|
||||
(ns hooks.metabase.api.common
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as api]
|
||||
[clojure.string :as str]))
|
||||
|
||||
(defn route-fn-name
|
||||
"route fn hook"
|
||||
[method route]
|
||||
(let [route (if (vector? route) (first route) route)]
|
||||
(-> (str (name method) route)
|
||||
(str/replace #"/" "_")
|
||||
symbol)))
|
||||
|
||||
(defn defendpoint
|
||||
[arg]
|
||||
(letfn [(update-defendpoint [node]
|
||||
(let [[_defendpoint method route & body] (:children node)]
|
||||
(api/list-node
|
||||
(list
|
||||
(api/token-node 'do)
|
||||
(api/token-node (symbol "compojure.core" (str (api/sexpr method))))
|
||||
(-> (api/list-node
|
||||
(list*
|
||||
(api/token-node 'clojure.core/defn)
|
||||
(api/token-node (route-fn-name (api/sexpr method) (api/sexpr route)))
|
||||
body))
|
||||
(with-meta (meta node)))))))]
|
||||
(update arg :node update-defendpoint)))
|
||||
|
||||
(comment
|
||||
(-> {:node (-> '(api/defendpoint POST "/:id/copy"
|
||||
"Copy a `Card`, with the new name 'Copy of _name_'"
|
||||
[id]
|
||||
{id [:maybe ms/PositiveInt]}
|
||||
(let [orig-card (api/read-check Card id)
|
||||
new-name (str (trs "Copy of ") (:name orig-card))
|
||||
new-card (assoc orig-card :name new-name)]
|
||||
(-> (card/create-card! new-card @api/*current-user*)
|
||||
hydrate-card-details
|
||||
(assoc :last-edit-info (last-edit/edit-information-for-user @api/*current-user*)))))
|
||||
pr-str
|
||||
api/parse-string)}
|
||||
defendpoint
|
||||
:node
|
||||
api/sexpr))
|
||||
@@ -1,149 +0,0 @@
|
||||
(ns hooks.metabase.api.macros
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as api]
|
||||
[clojure.string :as str]))
|
||||
|
||||
(defn- validate-route!
|
||||
"Make sure the route uses kebab-case rather than snake_case."
|
||||
[route]
|
||||
(letfn [(validate-route-string! [route-string]
|
||||
(when (and (api/string-node? route-string)
|
||||
(str/includes? (api/sexpr route-string) "_"))
|
||||
(api/reg-finding! (assoc (meta route-string)
|
||||
:message "REST API routes and query params should use kebab-case, not snake_case"
|
||||
:type :metabase/validate-defendpoint-route-uses-kebab-case))))]
|
||||
(cond
|
||||
(api/string-node? route)
|
||||
(validate-route-string! route)
|
||||
;; otherwise route is a vector like ["/abc/:d/" :d <regex>]
|
||||
(api/vector-node? route)
|
||||
(validate-route-string! (first (:children route))))))
|
||||
|
||||
(defn- validate-query-params!
|
||||
"Make sure query params use kebab-case rather than snake_case.
|
||||
|
||||
This only works on inline schemas for now. If you define the schema elsewhere then we'd have to implement this
|
||||
validation in Clojure-land since we can't resolve the schemas in Kondo. It should however still work on complicated
|
||||
schemas like
|
||||
|
||||
[:merge <map-1> <map-2>]"
|
||||
[query-param-schema-node]
|
||||
(letfn [(map-schema? [node]
|
||||
(and (api/vector-node? node)
|
||||
(when-let [first-child (first (:children node))]
|
||||
(and (api/keyword-node? first-child)
|
||||
(= (api/sexpr first-child) :map)))))
|
||||
(map-schema-keys [{:keys [children], :as _node}]
|
||||
(let [child-schemas (if (api/map-node? (second children))
|
||||
(drop 2 children)
|
||||
(drop 1 children))]
|
||||
(keep (fn [child-schema]
|
||||
(when (api/vector-node? child-schema)
|
||||
(when-let [k (first (:children child-schema))]
|
||||
(when (api/keyword-node? k)
|
||||
k))))
|
||||
child-schemas)))
|
||||
(validate-map-schema! [form]
|
||||
;; only warn once per endpoint -- just once on the first bad node
|
||||
(when-let [bad-node (some (fn [key-node]
|
||||
(when (str/includes? (str (api/sexpr key-node)) "_")
|
||||
key-node))
|
||||
(map-schema-keys form))]
|
||||
(api/reg-finding! (assoc (meta bad-node)
|
||||
:message "REST API routes and query params should use kebab-case, not snake_case"
|
||||
:type :metabase/validate-defendpoint-query-params-use-kebab-case))))
|
||||
(validate! [node]
|
||||
(cond
|
||||
(and (api/vector-node? node)
|
||||
(map-schema? node))
|
||||
(validate-map-schema! node)
|
||||
|
||||
(api/vector-node? node)
|
||||
(doseq [child (:children node)]
|
||||
(validate! child))))]
|
||||
(validate! query-param-schema-node)))
|
||||
|
||||
(defn defendpoint
|
||||
[arg]
|
||||
(letfn [(update-defendpoint [node]
|
||||
(let [[defendpoint method route & more] (:children node)
|
||||
[result-schema & more] (if (= (api/sexpr (first more)) :-)
|
||||
(drop 1 more)
|
||||
(cons nil more))
|
||||
[_docstring & more] (if (api/string-node? (first more))
|
||||
more
|
||||
(cons nil more))
|
||||
[metadata & more] (if (api/map-node? (first more))
|
||||
more
|
||||
(cons nil more))
|
||||
[params & body] more
|
||||
[bindings schemas] (when (api/vector-node? params)
|
||||
(loop [bindings [], schemas [], [x y z & more] (:children params)]
|
||||
(cond
|
||||
(not x)
|
||||
[bindings schemas]
|
||||
|
||||
(and (api/keyword-node? y)
|
||||
(= (api/sexpr y) :-))
|
||||
(recur (conj bindings x) (conj schemas z) more)
|
||||
|
||||
:else
|
||||
(recur (conj bindings x) (conj schemas nil) (list* y z more)))))]
|
||||
(validate-route! route)
|
||||
(when-not result-schema
|
||||
(api/reg-finding! (assoc (meta node)
|
||||
:message "All REST API endpoints should have a detailed response schema"
|
||||
:type :metabase/validate-defendpoint-has-response-schema)))
|
||||
(let [[_route-param-schema query-param-schema _body-param-schema] schemas]
|
||||
(when query-param-schema
|
||||
(validate-query-params! query-param-schema)))
|
||||
(-> (api/list-node
|
||||
(list
|
||||
(api/token-node 'do)
|
||||
defendpoint
|
||||
method
|
||||
route
|
||||
(api/list-node
|
||||
(list*
|
||||
(api/token-node 'do)
|
||||
(filter some? (list* metadata result-schema schemas))))
|
||||
(api/list-node
|
||||
(list*
|
||||
(api/token-node `let)
|
||||
(api/vector-node (into []
|
||||
cat
|
||||
[(mapcat (fn [a-binding]
|
||||
[a-binding (api/map-node {})])
|
||||
(take 4 bindings))
|
||||
(mapcat (fn [a-binding]
|
||||
[a-binding (api/list-node (list
|
||||
(api/token-node 'clojure.core/constantly)
|
||||
(api/token-node 'nil)))])
|
||||
(drop 4 bindings))]))
|
||||
body))))
|
||||
(with-meta (meta node)))))]
|
||||
(update arg :node update-defendpoint)))
|
||||
|
||||
(comment
|
||||
(defn -defendpoint [form]
|
||||
(-> {:node (-> form pr-str api/parse-string)}
|
||||
defendpoint
|
||||
:node
|
||||
api/sexpr))
|
||||
|
||||
(defn x []
|
||||
(-defendpoint '(api.macros/defendpoint :get "/:id/syncable_schemas" ; snake_case route should trigger a warning
|
||||
"Returns a list of all syncable schemas found for the database `id`."
|
||||
[{:keys [id]} :- [:map
|
||||
[:id ms/PositiveInt]]
|
||||
;; snake_case query parameter, should trigger a warning
|
||||
{:keys [filter_by]} :- [:map
|
||||
[:filter_by {:optional true} :string]]]
|
||||
(let [db (get-database id)]
|
||||
(api/check-403 (or (:is_attached_dwh db)
|
||||
(and (mi/can-write? db)
|
||||
(mi/can-read? db))))
|
||||
(->> db
|
||||
(driver/syncable-schemas (:engine db))
|
||||
(vec)
|
||||
(sort)))))))
|
||||
17
.clj-kondo/src/hooks/metabase/api/search_test.clj
Normal file
17
.clj-kondo/src/hooks/metabase/api/search_test.clj
Normal file
@@ -0,0 +1,17 @@
|
||||
(ns hooks.metabase.api.search-test
|
||||
(:require [clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn with-search-items-in-collection [{{[_ & args] :children, :as node} :node}]
|
||||
;; (println \newline)
|
||||
;; (clojure.pprint/pprint (hooks/sexpr node))
|
||||
(let [[created-items-sym search-string & body] args
|
||||
node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node
|
||||
[created-items-sym (hooks/token-node 'nil)])
|
||||
search-string
|
||||
body))]
|
||||
;; (println '=>)
|
||||
;; (clojure.pprint/pprint (hooks/sexpr node*))
|
||||
{:node node*}))
|
||||
@@ -1,14 +0,0 @@
|
||||
(ns hooks.metabase.app-db.schema-migrations-test.impl
|
||||
(:require [clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn test-migrations
|
||||
[{{[_ migration-range {[binding] :children} & body] :children} :node}]
|
||||
(let [node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node `let)
|
||||
(hooks/vector-node
|
||||
[binding (hooks/list-node (list (hooks/token-node `fn)
|
||||
(hooks/vector-node [(hooks/token-node '&) (hooks/token-node '_args)])))])
|
||||
migration-range
|
||||
body))]
|
||||
{:node node*}))
|
||||
@@ -1,25 +0,0 @@
|
||||
(ns hooks.metabase.channel.template.handlebars-helper
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common]))
|
||||
|
||||
(defn defhelper
|
||||
"defhelper hook"
|
||||
[{:keys [node]}]
|
||||
(let [[_ helper-name description argvec & body] (:children node)
|
||||
helper-fn-name (symbol (str (hooks/sexpr helper-name) "*"))]
|
||||
{:node (-> (hooks/list-node
|
||||
(list (hooks/token-node 'do)
|
||||
(hooks/list-node
|
||||
(list (hooks/token-node 'defn)
|
||||
(hooks/token-node helper-fn-name)
|
||||
description
|
||||
argvec
|
||||
(hooks/list-node body)))
|
||||
(hooks/list-node
|
||||
(list (hooks/token-node 'defn)
|
||||
helper-name
|
||||
description
|
||||
argvec
|
||||
(hooks/list-node body)))))
|
||||
(with-meta (meta node)))}))
|
||||
@@ -0,0 +1,14 @@
|
||||
(ns hooks.metabase.db.schema-migrations-test.impl
|
||||
(:require [clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn test-migrations
|
||||
[{{[_ migration-range {[binding] :children} & body] :children} :node}]
|
||||
(let [node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node `let)
|
||||
(hooks/vector-node
|
||||
[binding (hooks/list-node (list (hooks/token-node `fn)
|
||||
(hooks/vector-node [(hooks/token-node '&) (hooks/token-node '_args)])))])
|
||||
migration-range
|
||||
body))]
|
||||
{:node node*}))
|
||||
@@ -1,18 +0,0 @@
|
||||
(ns hooks.metabase.driver.mongo-test
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common]))
|
||||
|
||||
(defn with-describe-table-for-sample
|
||||
"Forward bindings for results of functions used in mongo's describe-table implementation."
|
||||
[{{[_ _ & body] :children} :node}]
|
||||
(let [node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node
|
||||
[(hooks/token-node 'dbfields) (hooks/token-node '(delay nil))
|
||||
(hooks/token-node 'ftree) (hooks/token-node '(delay nil))
|
||||
(hooks/token-node 'nested-fields) (hooks/token-node '(delay nil))])
|
||||
body))]
|
||||
;; Avoid warnings when not all from the dbfields, ftree and nested-fields are used.
|
||||
{:node (hooks.common/update-ignored-linters node* conj :unused-binding)}))
|
||||
@@ -33,36 +33,18 @@
|
||||
(defclause [ag:var var] field-or-expression FieldOrExpressionDef)
|
||||
=>
|
||||
(def ag:var [FieldOrExpressionDef])"
|
||||
[{:keys [node], :as x}]
|
||||
(let [[_defclause clause-name & arg-specs] (:children node)]
|
||||
(assoc x :node (-> (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node 'do)
|
||||
(-> (hooks/keyword-node (keyword "metabase.legacy-mbql.schema" (name (hooks/sexpr clause-name))))
|
||||
(with-meta (meta clause-name))
|
||||
(hooks/reg-keyword! 'metabase.legacy-mbql.schemas.macros/defclause))
|
||||
(collect-defclause-body-schemas arg-specs)))
|
||||
(with-meta (meta node))))))
|
||||
|
||||
(defn defclause*
|
||||
"e.g.
|
||||
|
||||
(defclause [ag:var var] field-or-expression FieldOrExpressionDef)
|
||||
=>
|
||||
(def ag:var [FieldOrExpressionDef])"
|
||||
[{:keys [node], :as x}]
|
||||
(let [[_defclause clause-name schema] (:children node)]
|
||||
(assoc x :node (-> (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node 'do)
|
||||
(-> (hooks/keyword-node (keyword "metabase.legacy-mbql.schema" (name (hooks/sexpr clause-name))))
|
||||
(with-meta (meta clause-name))
|
||||
(hooks/reg-keyword! 'metabase.legacy-mbql.schemas.macros/defclause*))
|
||||
schema))
|
||||
(with-meta (meta node))))))
|
||||
[{:keys [node]}]
|
||||
(let [[_defclause clause-name-form & arg-specs] (:children node)]
|
||||
{:node (-> (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node 'def)
|
||||
(unwrap-defclause-clause-name clause-name-form)
|
||||
(hooks/string-node "Docstring.")
|
||||
(collect-defclause-body-schemas arg-specs)))
|
||||
(with-meta (meta node)))}))
|
||||
|
||||
(comment
|
||||
(defn- expand-defclause [form]
|
||||
(defn- defclause* [form]
|
||||
(hooks/sexpr
|
||||
(:node
|
||||
(defclause
|
||||
@@ -73,35 +55,15 @@
|
||||
form)))}))))
|
||||
|
||||
(defn- x []
|
||||
(expand-defclause '(defclause var
|
||||
field-or-expression FieldOrExpressionDef)))
|
||||
(defclause* '(defclause [ag:var var]
|
||||
field-or-expression FieldOrExpressionDef)))
|
||||
;; =>
|
||||
(do :metabase.legacy-mbql.schema/var [FieldOrExpressionDef])
|
||||
(def ag:var "Docstring." [FieldOrExpressionDef])
|
||||
|
||||
(defn- y []
|
||||
(expand-defclause '(defclause var
|
||||
x (some-list-schema)
|
||||
y (optional FieldOrExpressionDef)
|
||||
z (rest (some-other-schema)))))
|
||||
(defclause* '(defclause var
|
||||
x (some-list-schema)
|
||||
y (optional FieldOrExpressionDef)
|
||||
z (rest (some-other-schema)))))
|
||||
;; =>
|
||||
(do :metabase.legacy-mbql.schema/var [(some-list-schema) FieldOrExpressionDef (some-other-schema)]))
|
||||
|
||||
(defn one-of [{:keys [node], :as x}]
|
||||
(let [[_one-of & schema-nodes] (:children node)
|
||||
node' (-> (hooks/vector-node
|
||||
(mapv (fn [schema-node]
|
||||
(if (and (hooks/token-node? schema-node)
|
||||
(simple-symbol? (hooks/sexpr schema-node)))
|
||||
(-> (hooks/keyword-node (keyword "mteabase.legacy-mbql.schema" (name (hooks/sexpr schema-node))))
|
||||
(with-meta (meta schema-node)))
|
||||
schema-node))
|
||||
schema-nodes))
|
||||
(with-meta (meta node)))]
|
||||
(assoc x :node node')))
|
||||
|
||||
(comment
|
||||
(defn- expand-one-of [form]
|
||||
(hooks/sexpr (:node (one-of {:node (hooks/parse-string (with-out-str (clojure.pprint/pprint form)))}))))
|
||||
|
||||
(defn- z []
|
||||
(expand-one-of '(one-of absolute-datetime relative-datetime time))))
|
||||
(def var "Docstring." [(some-list-schema) FieldOrExpressionDef (some-other-schema)]))
|
||||
|
||||
@@ -4,9 +4,9 @@
|
||||
(defn with-testing-against-standard-queries [{{[_ & args] :children} :node}]
|
||||
(let [[query-sym & body] args
|
||||
node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node
|
||||
[query-sym (hooks/token-node 'nil)])
|
||||
body))]
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node
|
||||
[query-sym (hooks/token-node 'nil)])
|
||||
body))]
|
||||
{:node node*}))
|
||||
|
||||
@@ -1,19 +0,0 @@
|
||||
(ns hooks.metabase.model-persistence.test-util
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn with-persistence-enabled! [x]
|
||||
(letfn [(update-node [node]
|
||||
(let [[_symb bindings & body] (:children node)
|
||||
[fn-binding] (:children bindings)]
|
||||
(-> (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node `let)
|
||||
(hooks/vector-node
|
||||
[fn-binding (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node `fn)
|
||||
(hooks/vector-node [])))])
|
||||
body))
|
||||
(with-meta (meta node)))))]
|
||||
(update x :node update-node)))
|
||||
@@ -1,7 +1,5 @@
|
||||
(ns hooks.metabase.models.interface
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common]))
|
||||
(:require [clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn define-hydration-method
|
||||
"This is used for both [[metabase.models.interface/define-simple-hydration-method]] and
|
||||
@@ -16,6 +14,5 @@
|
||||
(hooks/token-node 'defn)
|
||||
fn-name
|
||||
fn-tail))))
|
||||
(with-meta (meta node))
|
||||
hooks.common/add-lsp-ignore-unused-public-var-metadata)]
|
||||
(with-meta (update (meta node) :clj-kondo/ignore #(hooks/vector-node (cons :clojure-lsp/unused-public-var (:children %))))))]
|
||||
{:node node}))
|
||||
|
||||
264
.clj-kondo/src/hooks/metabase/models/setting.clj
Normal file
264
.clj-kondo/src/hooks/metabase/models/setting.clj
Normal file
@@ -0,0 +1,264 @@
|
||||
(ns hooks.metabase.models.setting
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common :as common]))
|
||||
|
||||
(def ^:private ignored-implicit-export?
|
||||
'#{active-users-count
|
||||
admin-email
|
||||
analytics-uuid
|
||||
anon-tracking-enabled
|
||||
api-key
|
||||
attachment-table-row-limit
|
||||
audit-max-retention-days
|
||||
bcc-enabled?
|
||||
breakout-bin-width
|
||||
check-for-updates
|
||||
cloud-gateway-ips
|
||||
config-from-file-sync-databases
|
||||
custom-homepage
|
||||
custom-homepage-dashboard
|
||||
database-enable-actions
|
||||
deprecation-notice-version
|
||||
dismissed-custom-dashboard-toast
|
||||
email-configured?
|
||||
email-from-address
|
||||
email-from-name
|
||||
email-reply-to
|
||||
email-smtp-host
|
||||
email-smtp-password
|
||||
email-smtp-port
|
||||
email-smtp-security
|
||||
email-smtp-username
|
||||
embedding-app-origin
|
||||
embedding-secret-key
|
||||
enable-password-login
|
||||
enable-public-sharing
|
||||
enable-query-caching
|
||||
engines
|
||||
enum-cardinality-threshold
|
||||
follow-up-email-sent
|
||||
google-auth-auto-create-accounts-domain
|
||||
google-auth-client-id
|
||||
google-auth-configured
|
||||
google-auth-enabled
|
||||
has-sample-database?
|
||||
has-user-setup
|
||||
help-link
|
||||
help-link-custom-destination
|
||||
instance-creation
|
||||
is-hosted?
|
||||
is-metabot-enabled
|
||||
jdbc-data-warehouse-max-connection-pool-size
|
||||
jdbc-data-warehouse-unreturned-connection-timeout-seconds
|
||||
jwt-attribute-email
|
||||
jwt-attribute-firstname
|
||||
jwt-attribute-groups
|
||||
jwt-attribute-lastname
|
||||
jwt-configured
|
||||
jwt-enabled
|
||||
jwt-group-mappings
|
||||
jwt-group-sync
|
||||
jwt-identity-provider-uri
|
||||
jwt-user-provisioning-enabled?
|
||||
jwt-shared-secret
|
||||
last-acknowledged-version
|
||||
last-used-native-database-id
|
||||
ldap-attribute-email
|
||||
ldap-attribute-firstname
|
||||
ldap-attribute-lastname
|
||||
ldap-bind-dn
|
||||
ldap-configured?
|
||||
ldap-enabled
|
||||
ldap-group-base
|
||||
ldap-group-mappings
|
||||
ldap-group-membership-filter
|
||||
ldap-group-sync
|
||||
ldap-host
|
||||
ldap-password
|
||||
ldap-port
|
||||
ldap-user-provisioning-enabled?
|
||||
ldap-security
|
||||
ldap-sync-user-attributes
|
||||
ldap-sync-user-attributes-blacklist
|
||||
ldap-user-base
|
||||
ldap-user-filter
|
||||
load-analytics-content
|
||||
map-tile-server-url
|
||||
metabot-default-embedding-model
|
||||
metabot-feedback-url
|
||||
metabot-get-prompt-templates-url
|
||||
metabot-prompt-generator-token-limit
|
||||
multi-setting-read-only
|
||||
notebook-native-preview-shown
|
||||
notebook-native-preview-sidebar-width
|
||||
notification-link-base-url
|
||||
num-metabot-choices
|
||||
openai-api-key
|
||||
openai-available-models
|
||||
openai-model
|
||||
openai-organization
|
||||
other-sso-enabled?
|
||||
password-complexity
|
||||
persist-models-enabled
|
||||
persisted-model-refresh-cron-schedule
|
||||
premium-embedding-token
|
||||
prometheus-server-port
|
||||
query-caching-max-kb
|
||||
query-caching-max-ttl
|
||||
query-caching-min-ttl
|
||||
query-caching-ttl-ratio
|
||||
redirect-all-requests-to-https
|
||||
reset-token-ttl-hours
|
||||
retired-setting
|
||||
retry-initial-interval
|
||||
retry-max-attempts
|
||||
retry-max-interval-millis
|
||||
retry-multiplier
|
||||
retry-randomization-factor
|
||||
saml-application-name
|
||||
saml-attribute-email
|
||||
saml-attribute-firstname
|
||||
saml-attribute-group
|
||||
saml-attribute-lastname
|
||||
saml-configured
|
||||
saml-enabled
|
||||
saml-group-mappings
|
||||
saml-group-sync
|
||||
saml-identity-provider-certificate
|
||||
saml-identity-provider-issuer
|
||||
saml-identity-provider-uri
|
||||
saml-keystore-alias
|
||||
saml-keystore-password
|
||||
saml-keystore-path
|
||||
saml-user-provisioning-enabled?
|
||||
send-email-on-first-login-from-new-device
|
||||
send-new-sso-user-admin-email?
|
||||
session-cookie-samesite
|
||||
session-cookies
|
||||
session-timeout
|
||||
setup-token
|
||||
show-database-syncing-modal
|
||||
show-metabase-links
|
||||
site-url
|
||||
site-uuid
|
||||
site-uuid-for-premium-features-token-checks
|
||||
site-uuid-for-unsubscribing-url
|
||||
site-uuid-for-version-info-fetching
|
||||
slack-app-token
|
||||
slack-cached-channels-and-usernames
|
||||
slack-channels-and-usernames-last-updated
|
||||
slack-files-channel
|
||||
slack-token
|
||||
slack-token-valid?
|
||||
snowplow-available
|
||||
snowplow-enabled
|
||||
snowplow-url
|
||||
sql-jdbc-fetch-size
|
||||
ssh-heartbeat-interval-sec
|
||||
ssl-certificate-public-key
|
||||
startup-time-millis
|
||||
token-features
|
||||
token-status
|
||||
toucan-name
|
||||
uncached-setting
|
||||
user-visibility
|
||||
version
|
||||
version-info
|
||||
version-info-last-checked})
|
||||
|
||||
(defn- defsetting-lint [node setting-name docstring options-list]
|
||||
(let [anon-binding (common/with-macro-meta (hooks/token-node '_) node)
|
||||
;; (defn my-setting [] ...)
|
||||
getter-node (-> (list
|
||||
(hooks/token-node 'defn)
|
||||
setting-name
|
||||
(hooks/string-node "Docstring.")
|
||||
(hooks/vector-node []))
|
||||
hooks/list-node
|
||||
(with-meta (meta node)))
|
||||
;; (defn my-setting! [_x] ...)
|
||||
setter-node (-> (list
|
||||
(hooks/token-node 'defn)
|
||||
(with-meta
|
||||
(hooks/token-node (symbol (str (hooks/sexpr setting-name) \!)))
|
||||
(meta setting-name))
|
||||
(hooks/string-node "Docstring.")
|
||||
(hooks/vector-node [(hooks/token-node '_value-or-nil)]))
|
||||
hooks/list-node
|
||||
(with-meta (update (meta node) :clj-kondo/ignore #(hooks/vector-node (cons :clojure-lsp/unused-public-var (:children %))))))]
|
||||
|
||||
(when (nil? (second (drop-while (comp not #{[:k :export?]} first) options-list)))
|
||||
(when-not (contains? ignored-implicit-export? (:value setting-name))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "Setting definition must provide an explicit value for :export? indicating whether the setting should be exported or not with serialization."
|
||||
:type :metabase/defsetting-must-specify-export))))
|
||||
|
||||
{:node (-> (list
|
||||
(hooks/token-node 'let)
|
||||
;; include description and the options map so they can get validated as well.
|
||||
(hooks/vector-node
|
||||
[anon-binding docstring
|
||||
anon-binding (hooks/map-node options-list)])
|
||||
getter-node
|
||||
setter-node)
|
||||
hooks/list-node
|
||||
(with-meta (meta node)))}))
|
||||
|
||||
(defn defsetting
|
||||
"Rewrite a [[metabase.models.defsetting]] form like
|
||||
|
||||
(defsetting my-setting \"Description\" :type :boolean)
|
||||
|
||||
as
|
||||
|
||||
(let [_ \"Description\"
|
||||
_ {:type :boolean}]
|
||||
(defn my-setting \"Docstring.\" [])
|
||||
(defn my-setting! \"Docstring.\" [_value-or-nil]))
|
||||
|
||||
for linting purposes."
|
||||
[{:keys [node]}]
|
||||
(let [[setting-name docstring & options] (rest (:children node))]
|
||||
(defsetting-lint node setting-name docstring options)))
|
||||
|
||||
(defn define-multi-setting
|
||||
"Rewrite a [[metabase.models.define-multi-setting]] form like
|
||||
|
||||
(defsetting my-setting \"Description\" :key :type :boolean)
|
||||
|
||||
as
|
||||
|
||||
(let [_ \"Description\"
|
||||
_ {:type :boolean, :multi-thunk :key}]
|
||||
(defn my-setting \"Docstring.\" [])
|
||||
(defn my-setting! \"Docstring.\" [_value-or-nil]))
|
||||
|
||||
for linting purposes."
|
||||
[{:keys [node]}]
|
||||
(let [[setting-name docstring thunk & options] (rest (:children node))]
|
||||
(defsetting-lint node setting-name docstring (concat options [(hooks/token-node :multi-thunk) thunk]))))
|
||||
|
||||
(comment
|
||||
(defn- defsetting* [form]
|
||||
(hooks/sexpr
|
||||
(:node
|
||||
(defsetting
|
||||
{:node
|
||||
(hooks/parse-string
|
||||
(with-out-str
|
||||
#_{:clj-kondo/ignore [:unresolved-namespace]}
|
||||
(clojure.pprint/pprint
|
||||
form)))}))))
|
||||
|
||||
(defn x []
|
||||
(defsetting*
|
||||
'(defsetting active-users-count
|
||||
(deferred-tru "Cached number of active users. Refresh every 5 minutes.")
|
||||
:visibility :admin
|
||||
:type :integer
|
||||
:default 0
|
||||
:getter (fn []
|
||||
(if-not ((requiring-resolve 'metabase.db/db-is-set-up?))
|
||||
0
|
||||
(cached-active-users-count)))))))
|
||||
@@ -1,65 +0,0 @@
|
||||
(ns hooks.metabase.premium-features.defenterprise
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common]))
|
||||
|
||||
(defn defenterprise [{node :node}]
|
||||
(let [[_defenterprise fn-name & args] (:children node)
|
||||
[docstring & args] (if (hooks/string-node? (first args))
|
||||
args
|
||||
(cons nil args))
|
||||
[_enterprise-namespace & args] (if (hooks/token-node? (first args))
|
||||
args
|
||||
(cons nil args))
|
||||
[options fn-tail] (loop [options (hooks/map-node []), [x y & more :as args] args]
|
||||
(if (hooks/keyword-node? x)
|
||||
(recur (update options :children concat [x y]) more)
|
||||
[options args]))]
|
||||
{:node (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node
|
||||
[(hooks/token-node '_options) options])
|
||||
(-> (hooks/list-node
|
||||
(concat
|
||||
(filter some?
|
||||
(list (hooks/token-node 'defn)
|
||||
fn-name
|
||||
docstring))
|
||||
fn-tail))
|
||||
(with-meta (meta node))
|
||||
hooks.common/add-lsp-ignore-unused-public-var-metadata)))}))
|
||||
|
||||
(comment
|
||||
(defn- defenterprise* [form]
|
||||
(hooks/sexpr
|
||||
(:node
|
||||
(defenterprise
|
||||
{:node
|
||||
(hooks/parse-string
|
||||
(with-out-str
|
||||
(clojure.pprint/pprint
|
||||
form)))}))))
|
||||
|
||||
(defn- x []
|
||||
(defenterprise*
|
||||
'(defenterprise score-result
|
||||
"Scoring implementation that adds score for items in official collections."
|
||||
:feature :any
|
||||
:fallback score-result-fallback
|
||||
[result]
|
||||
(conj (scoring/weights-and-scores result)
|
||||
{:weight 2
|
||||
:score (official-collection-score result)
|
||||
:name "official collection score"}
|
||||
{:weight 2
|
||||
:score (verified-score result)
|
||||
:name "verified"}))))
|
||||
|
||||
(defn- y []
|
||||
(defenterprise*
|
||||
'(defenterprise score-result
|
||||
"Score a result, returning a collection of maps with score and weight."
|
||||
metabase-enterprise.search.scoring
|
||||
[result]
|
||||
(weights-and-scores result)))))
|
||||
@@ -0,0 +1,63 @@
|
||||
(ns hooks.metabase.public-settings.premium-features
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn defenterprise [{node :node}]
|
||||
(let [[_defenterprise fn-name & args] (:children node)
|
||||
[docstring & args] (if (hooks/string-node? (first args))
|
||||
args
|
||||
(cons nil args))
|
||||
[_enterprise-namespace & args] (if (hooks/token-node? (first args))
|
||||
args
|
||||
(cons nil args))
|
||||
[options fn-tail] (loop [options (hooks/map-node []), [x y & more :as args] args]
|
||||
(if (hooks/keyword-node? x)
|
||||
(recur (update options :children concat [x y]) more)
|
||||
[options args]))]
|
||||
{:node (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node
|
||||
[(hooks/token-node '_options) options])
|
||||
(-> (hooks/list-node
|
||||
(concat
|
||||
(filter some?
|
||||
(list (hooks/token-node 'defn)
|
||||
fn-name
|
||||
docstring))
|
||||
fn-tail))
|
||||
(with-meta (update (meta node) :clj-kondo/ignore #(hooks/vector-node (cons :clojure-lsp/unused-public-var (:children %))))))))}))
|
||||
|
||||
(comment
|
||||
(defn- defenterprise* [form]
|
||||
(hooks/sexpr
|
||||
(:node
|
||||
(defenterprise
|
||||
{:node
|
||||
(hooks/parse-string
|
||||
(with-out-str
|
||||
(clojure.pprint/pprint
|
||||
form)))}))))
|
||||
|
||||
(defn- x []
|
||||
(defenterprise*
|
||||
'(defenterprise score-result
|
||||
"Scoring implementation that adds score for items in official collections."
|
||||
:feature :any
|
||||
:fallback score-result-fallback
|
||||
[result]
|
||||
(conj (scoring/weights-and-scores result)
|
||||
{:weight 2
|
||||
:score (official-collection-score result)
|
||||
:name "official collection score"}
|
||||
{:weight 2
|
||||
:score (verified-score result)
|
||||
:name "verified"}))))
|
||||
|
||||
(defn- y []
|
||||
(defenterprise*
|
||||
'(defenterprise score-result
|
||||
"Score a result, returning a collection of maps with score and weight."
|
||||
metabase-enterprise.search.scoring
|
||||
[result]
|
||||
(weights-and-scores result)))))
|
||||
@@ -1,16 +0,0 @@
|
||||
(ns hooks.metabase.query-processor.expressions-test
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.metabase.test.data :as data]))
|
||||
|
||||
(defn calculate-bird-scarcity
|
||||
[{{[_ formula filter-clause] :children} :node}]
|
||||
(let [formula* (data/replace-$id-special-tokens formula)
|
||||
filter-clause* (or (some-> filter-clause data/replace-$id-special-tokens)
|
||||
(hooks/token-node 'nil))
|
||||
node* (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node 'do)
|
||||
formula*
|
||||
filter-clause*))]
|
||||
{:node node*}))
|
||||
@@ -0,0 +1,16 @@
|
||||
(ns hooks.metabase.query-processor-test.expressions-test
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.metabase.test.data :as data]))
|
||||
|
||||
(defn calculate-bird-scarcity
|
||||
[{{[_ formula filter-clause] :children} :node}]
|
||||
(let [formula* (data/replace-$id-special-tokens formula)
|
||||
filter-clause* (or (some-> filter-clause data/replace-$id-special-tokens)
|
||||
(hooks/token-node 'nil))
|
||||
node* (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node 'do)
|
||||
formula*
|
||||
filter-clause*))]
|
||||
{:node node*}))
|
||||
@@ -1,17 +0,0 @@
|
||||
(ns hooks.metabase.search.api-test
|
||||
(:require [clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn with-search-items-in-collection [{{[_ & args] :children, :as node} :node}]
|
||||
;; (println \newline)
|
||||
;; (clojure.pprint/pprint (hooks/sexpr node))
|
||||
(let [[created-items-sym search-string & body] args
|
||||
node* (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node
|
||||
[created-items-sym (hooks/token-node 'nil)])
|
||||
search-string
|
||||
body))]
|
||||
;; (println '=>)
|
||||
;; (clojure.pprint/pprint (hooks/sexpr node*))
|
||||
{:node node*}))
|
||||
@@ -1,308 +0,0 @@
|
||||
(ns hooks.metabase.settings.models.setting
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common :as common]))
|
||||
|
||||
(defn lint-defsetting-namespace [node context]
|
||||
(when-not (re-matches #"^metabase(?:-enterprise)?\.[^\.]+\.settings$" (name (:ns context)))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "All defsettings should live in metabase[-enterprise].<module>.settings namespaces"
|
||||
:type :metabase/defsetting-namespace))))
|
||||
|
||||
;;; TODO -- move this into a Kondo config file in `.clj-kondo/config/`
|
||||
(def ^:private ignored-implicit-export?
|
||||
'#{active-users-count
|
||||
admin-email
|
||||
analytics-uuid
|
||||
anon-tracking-enabled
|
||||
api-key
|
||||
attachment-table-row-limit
|
||||
audit-max-retention-days
|
||||
bcc-enabled?
|
||||
breakout-bin-width
|
||||
check-for-updates
|
||||
cloud-gateway-ips
|
||||
config-from-file-sync-databases
|
||||
custom-homepage
|
||||
custom-homepage-dashboard
|
||||
database-enable-actions
|
||||
deprecation-notice-version
|
||||
dismissed-custom-dashboard-toast
|
||||
email-configured?
|
||||
email-from-address
|
||||
email-from-name
|
||||
email-reply-to
|
||||
email-smtp-host
|
||||
email-smtp-password
|
||||
email-smtp-port
|
||||
email-smtp-security
|
||||
email-smtp-username
|
||||
embedding-app-origin
|
||||
embedding-secret-key
|
||||
enable-password-login
|
||||
enable-public-sharing
|
||||
enable-query-caching
|
||||
engines
|
||||
enum-cardinality-threshold
|
||||
follow-up-email-sent
|
||||
google-auth-auto-create-accounts-domain
|
||||
google-auth-client-id
|
||||
google-auth-configured
|
||||
google-auth-enabled
|
||||
has-sample-database?
|
||||
has-user-setup
|
||||
help-link
|
||||
help-link-custom-destination
|
||||
instance-creation
|
||||
is-hosted?
|
||||
jdbc-data-warehouse-max-connection-pool-size
|
||||
jdbc-data-warehouse-unreturned-connection-timeout-seconds
|
||||
jwt-attribute-email
|
||||
jwt-attribute-firstname
|
||||
jwt-attribute-groups
|
||||
jwt-attribute-lastname
|
||||
jwt-configured
|
||||
jwt-enabled
|
||||
jwt-group-mappings
|
||||
jwt-group-sync
|
||||
jwt-identity-provider-uri
|
||||
jwt-user-provisioning-enabled?
|
||||
jwt-shared-secret
|
||||
last-acknowledged-version
|
||||
last-used-native-database-id
|
||||
ldap-attribute-email
|
||||
ldap-attribute-firstname
|
||||
ldap-attribute-lastname
|
||||
ldap-bind-dn
|
||||
ldap-configured?
|
||||
ldap-enabled
|
||||
ldap-group-base
|
||||
ldap-group-mappings
|
||||
ldap-group-membership-filter
|
||||
ldap-group-sync
|
||||
ldap-host
|
||||
ldap-password
|
||||
ldap-port
|
||||
ldap-user-provisioning-enabled?
|
||||
ldap-security
|
||||
ldap-sync-user-attributes
|
||||
ldap-sync-user-attributes-blacklist
|
||||
ldap-user-base
|
||||
ldap-user-filter
|
||||
load-analytics-content
|
||||
map-tile-server-url
|
||||
metabot-default-embedding-model
|
||||
metabot-feedback-url
|
||||
metabot-get-prompt-templates-url
|
||||
metabot-prompt-generator-token-limit
|
||||
multi-setting-read-only
|
||||
notebook-native-preview-sidebar-width
|
||||
notification-link-base-url
|
||||
num-metabot-choices
|
||||
openai-api-key
|
||||
openai-available-models
|
||||
openai-model
|
||||
openai-organization
|
||||
other-sso-enabled?
|
||||
password-complexity
|
||||
persist-models-enabled
|
||||
persisted-model-refresh-cron-schedule
|
||||
premium-embedding-token
|
||||
prometheus-server-port
|
||||
query-caching-max-kb
|
||||
query-caching-max-ttl
|
||||
query-caching-min-ttl
|
||||
query-caching-ttl-ratio
|
||||
redirect-all-requests-to-https
|
||||
reset-token-ttl-hours
|
||||
retired-setting
|
||||
retry-initial-interval
|
||||
retry-max-retries
|
||||
retry-max-interval-millis
|
||||
retry-multiplier
|
||||
retry-jitter-factor
|
||||
saml-application-name
|
||||
saml-attribute-email
|
||||
saml-attribute-firstname
|
||||
saml-attribute-group
|
||||
saml-attribute-lastname
|
||||
saml-configured
|
||||
saml-enabled
|
||||
saml-group-mappings
|
||||
saml-group-sync
|
||||
saml-identity-provider-certificate
|
||||
saml-identity-provider-issuer
|
||||
saml-identity-provider-uri
|
||||
saml-keystore-alias
|
||||
saml-keystore-password
|
||||
saml-keystore-path
|
||||
saml-user-provisioning-enabled?
|
||||
sdk-iframe-embed-setup-settings
|
||||
send-email-on-first-login-from-new-device
|
||||
send-new-sso-user-admin-email?
|
||||
session-cookie-samesite
|
||||
session-cookies
|
||||
session-timeout
|
||||
setup-token
|
||||
show-database-syncing-modal
|
||||
show-metabase-links
|
||||
site-url
|
||||
site-uuid
|
||||
site-uuid-for-premium-features-token-checks
|
||||
site-uuid-for-unsubscribing-url
|
||||
site-uuid-for-version-info-fetching
|
||||
slack-app-token
|
||||
slack-cached-channels-and-usernames
|
||||
slack-channels-and-usernames-last-updated
|
||||
slack-files-channel
|
||||
slack-token
|
||||
slack-token-valid?
|
||||
snowplow-available
|
||||
snowplow-enabled
|
||||
snowplow-url
|
||||
sql-jdbc-fetch-size
|
||||
ssh-heartbeat-interval-sec
|
||||
ssl-certificate-public-key
|
||||
startup-time-millis
|
||||
token-features
|
||||
token-status
|
||||
toucan-name
|
||||
trial-banner-dismissal-timestamp
|
||||
uncached-setting
|
||||
user-visibility
|
||||
version
|
||||
version-info
|
||||
version-info-last-checked})
|
||||
|
||||
(defn- lint-defsetting-export [node]
|
||||
(let [[_defsetting setting-name _docstring & options] (:children node)]
|
||||
(when (nil? (second (drop-while (comp not #{[:k :export?]} first) options)))
|
||||
(when-not (contains? ignored-implicit-export? (:value setting-name))
|
||||
(hooks/reg-finding! (assoc (meta node)
|
||||
:message "Setting definition must provide an explicit value for :export? indicating whether the setting should be exported or not with serialization."
|
||||
:type :metabase/defsetting-must-specify-export))))))
|
||||
|
||||
(defn- defsetting-replacement-node [node]
|
||||
(let [[_defsetting setting-name docstring & options-list] (:children node)
|
||||
anon-binding (common/with-macro-meta (hooks/token-node '_) node)
|
||||
;; (defn my-setting [] ...)
|
||||
getter-node (-> (list
|
||||
(hooks/token-node 'defn)
|
||||
setting-name
|
||||
(hooks/string-node "Docstring.")
|
||||
(hooks/vector-node []))
|
||||
hooks/list-node
|
||||
(with-meta (meta node)))
|
||||
;; (defn my-setting! [_x] ...)
|
||||
setter-node (-> (list
|
||||
(hooks/token-node 'defn)
|
||||
(with-meta
|
||||
(hooks/token-node (symbol (str (hooks/sexpr setting-name) \!)))
|
||||
(meta setting-name))
|
||||
(hooks/string-node "Docstring.")
|
||||
(hooks/vector-node [(hooks/token-node '_value-or-nil)]))
|
||||
hooks/list-node
|
||||
(with-meta (meta node))
|
||||
common/add-lsp-ignore-unused-public-var-metadata)]
|
||||
(-> (hooks/list-node
|
||||
(list
|
||||
(hooks/token-node 'let)
|
||||
;; include description and the options map so they can get validated as well.
|
||||
(hooks/vector-node
|
||||
[anon-binding docstring
|
||||
anon-binding (hooks/map-node options-list)])
|
||||
(hooks/reg-keyword! (-> (hooks/keyword-node (keyword (hooks/sexpr setting-name)))
|
||||
(with-meta (meta setting-name)))
|
||||
'metabase.settings.models.setting/defsetting)
|
||||
getter-node
|
||||
setter-node))
|
||||
(with-meta (meta node)))))
|
||||
|
||||
(defn defsetting
|
||||
"Rewrite a [[metabase.models.defsetting]] form like
|
||||
|
||||
(defsetting my-setting \"Description\" :type :boolean)
|
||||
|
||||
as
|
||||
|
||||
(let [_ \"Description\"
|
||||
_ {:type :boolean}]
|
||||
(defn my-setting \"Docstring.\" [])
|
||||
(defn my-setting! \"Docstring.\" [_value-or-nil]))
|
||||
|
||||
for linting purposes."
|
||||
[{:keys [node], :as context}]
|
||||
(lint-defsetting-namespace node context)
|
||||
(lint-defsetting-export node)
|
||||
(update context :node defsetting-replacement-node))
|
||||
|
||||
(defn define-multi-setting-replacement-node [node]
|
||||
(let [[defsetting setting-name docstring thunk & options] (:children node)]
|
||||
(-> (hooks/list-node
|
||||
(list*
|
||||
defsetting
|
||||
setting-name
|
||||
docstring
|
||||
(hooks/token-node :multi-thunk) thunk
|
||||
options))
|
||||
(with-meta (meta node))
|
||||
defsetting-replacement-node)))
|
||||
|
||||
(defn define-multi-setting
|
||||
"Rewrite a [[metabase.models.define-multi-setting]] form like
|
||||
|
||||
(defsetting my-setting \"Description\" :key :type :boolean)
|
||||
|
||||
as
|
||||
|
||||
(let [_ \"Description\"
|
||||
_ {:type :boolean, :multi-thunk :key}]
|
||||
(defn my-setting \"Docstring.\" [])
|
||||
(defn my-setting! \"Docstring.\" [_value-or-nil]))
|
||||
|
||||
for linting purposes."
|
||||
[{:keys [node], :as context}]
|
||||
(update context :node define-multi-setting-replacement-node))
|
||||
|
||||
(defn define-multi-setting-impl
|
||||
[context]
|
||||
(letfn [(update-node [node]
|
||||
(let [[_define-multi-setting-impl setting-name & more] (:children node)]
|
||||
(-> (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'do)
|
||||
;; For a qualified setting name like `oss-namespace/setting`, then leave the symbol as is, so the
|
||||
;; usage of that namespace/var gets recorded. Otherwise for an unqualified name convert it to a
|
||||
;; keyword so we don't get unresolved var errors.
|
||||
(if (and (hooks/token-node? setting-name)
|
||||
(qualified-symbol? (hooks/sexpr setting-name)))
|
||||
setting-name
|
||||
(-> (hooks/keyword-node (keyword (hooks/sexpr setting-name)))
|
||||
(with-meta (meta setting-name))))
|
||||
more))
|
||||
(with-meta (meta node)))))]
|
||||
(update context :node update-node)))
|
||||
|
||||
(comment
|
||||
(defn- defsetting* [form]
|
||||
(hooks/sexpr
|
||||
(:node
|
||||
(defsetting
|
||||
{:node
|
||||
(hooks/parse-string
|
||||
(with-out-str
|
||||
#_{:clj-kondo/ignore [:unresolved-namespace]}
|
||||
(clojure.pprint/pprint
|
||||
form)))}))))
|
||||
|
||||
(defn x []
|
||||
(defsetting*
|
||||
'(defsetting active-users-count
|
||||
(deferred-tru "Cached number of active users. Refresh every 5 minutes.")
|
||||
:visibility :admin
|
||||
:type :integer
|
||||
:default 0
|
||||
:getter (fn []
|
||||
(if-not ((requiring-resolve 'metabase.app-db.core/db-is-set-up?))
|
||||
0
|
||||
(cached-active-users-count)))))))
|
||||
@@ -45,35 +45,37 @@
|
||||
:unqualified/unknown)))
|
||||
|
||||
(defn dataset
|
||||
[{{[_ dataset & body] :children, :as node} :node this-ns :ns}]
|
||||
(let [node' (case (dataset-type this-ns dataset)
|
||||
;; non-symbol, qualified symbols, and unqualified symbols from the current namespace/let-bound can all
|
||||
;; get converted from something like
|
||||
;;
|
||||
;; (dataset whatever
|
||||
;; ...)
|
||||
;;
|
||||
;; to
|
||||
;;
|
||||
;; (let [_ whatever]
|
||||
;; ...)
|
||||
(:non-symbol :qualified :unqualified/local-def)
|
||||
(hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'let)
|
||||
(hooks/vector-node [(hooks/token-node '_) dataset])
|
||||
body))
|
||||
[{{[_ dataset & body] :children} :node this-ns :ns}]
|
||||
(let [noop (constantly nil)
|
||||
body (case (dataset-type this-ns dataset)
|
||||
;; non-symbol, qualified symbols, and unqualified symbols from the current namespace/let-bound can all
|
||||
;; get converted from something like
|
||||
;;
|
||||
;; (dataset whatever
|
||||
;; ...)
|
||||
;;
|
||||
;; to
|
||||
;;
|
||||
;; (let [_ whatever]
|
||||
;; ...)
|
||||
(:non-symbol :qualified :unqualified/local-def)
|
||||
(list* (hooks/token-node 'let)
|
||||
(hooks/vector-node [(hooks/token-node '_) dataset])
|
||||
body)
|
||||
|
||||
;; for ones that came from the dataset defs namespace or ones whose origin is unknown, just ignore them
|
||||
;; and generate a form like
|
||||
;;
|
||||
;; (do ...)
|
||||
(:unqualified/from-dataset-defs-namespace :unqualified/unknown)
|
||||
(hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node `do)
|
||||
body)))]
|
||||
{:node (with-meta node' (meta node))}))
|
||||
;; for ones that came from the dataset defs namespace or ones whose origin is unknown, just ignore them
|
||||
;; and generate a `print` form:
|
||||
;;
|
||||
;; (print ...)
|
||||
;;
|
||||
;; (this used to be a `do` form (which makes a lot more semantic sense), but that resulted in warnings
|
||||
;; about unused values
|
||||
(:unqualified/from-dataset-defs-namespace :unqualified/unknown)
|
||||
(list* (hooks/token-node noop)
|
||||
body))]
|
||||
{:node (with-meta (hooks/list-node (with-meta body
|
||||
(meta dataset)))
|
||||
(meta dataset))}))
|
||||
|
||||
(defn- special-token-node?
|
||||
"Whether this node is one of the special symbols like `$field`."
|
||||
|
||||
@@ -1,49 +0,0 @@
|
||||
(ns hooks.metabase.test.data.datasets
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clojure.set :as set]
|
||||
[hooks.common]))
|
||||
|
||||
(defn- only-core-drivers?
|
||||
"Whether we're for sure only testing against the 'core' drivers that are also app DB types: `:postgres`, `:mysql`, or
|
||||
`:h2`. Tests like these are ok to mark `:mb/driver-tests` since we always run the full test suite against app DB
|
||||
drivers anyway."
|
||||
[node]
|
||||
(let [drivers (hooks/sexpr (second (:children node)))
|
||||
drivers (cond
|
||||
(keyword? drivers) #{drivers}
|
||||
(set? drivers) drivers
|
||||
:else nil)]
|
||||
(when drivers
|
||||
(empty? (set/difference drivers #{:h2 :mysql :postgres})))))
|
||||
|
||||
(defn- validate-mb-driver-tests [x]
|
||||
(when-not (:mb/driver-tests (meta (:ns x)))
|
||||
(hooks/reg-finding!
|
||||
(assoc (meta (:ns x))
|
||||
:message (str "Namespaces that run driver tests (test-driver or test-drivers) should be marked"
|
||||
" ^:mb/driver-tests [:metabase/validate-mb-driver-tests]")
|
||||
:type :metabase/validate-mb-driver-tests))))
|
||||
|
||||
(defn- validate-ns-tags [x]
|
||||
(when-not (only-core-drivers? (:node x))
|
||||
(validate-mb-driver-tests x)))
|
||||
|
||||
(defn test-drivers [x]
|
||||
(validate-ns-tags x)
|
||||
(letfn [(update-node [node]
|
||||
(let [[test-drivers drivers-expr & body] (:children node)]
|
||||
(-> (hooks/list-node
|
||||
(list*
|
||||
(hooks/token-node 'do)
|
||||
test-drivers
|
||||
(-> drivers-expr
|
||||
(hooks.common/update-ignored-linters conj :unused-value))
|
||||
body))
|
||||
(with-meta (meta node)))))]
|
||||
(update x :node update-node)))
|
||||
|
||||
(defn test-driver
|
||||
"We can use the same hook for `test-driver` as we use for `test-drivers`."
|
||||
[x]
|
||||
(test-drivers x))
|
||||
@@ -1,14 +0,0 @@
|
||||
(ns hooks.metabase.test.gentest
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn iterate*
|
||||
"(iterate {:as limit-spec} [s1 val1 & more] body-form body-form...)"
|
||||
[{{:keys [children] :as node} :node :as orig}]
|
||||
(let [[token limit-spec bindings & body-nodes] children
|
||||
node* (-> (hooks/list-node
|
||||
(list token limit-spec
|
||||
(hooks/list-node (list* (hooks/token-node 'let) bindings
|
||||
body-nodes))))
|
||||
(with-meta (meta node)))]
|
||||
(assoc orig :node node*)))
|
||||
@@ -1,6 +1,6 @@
|
||||
(ns hooks.metabase.test.util
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]))
|
||||
(:require [clj-kondo.hooks-api :as hooks]
|
||||
[hooks.common :as common]))
|
||||
|
||||
(defn- namespaced-symbol-node? [node]
|
||||
(when (hooks/token-node? node)
|
||||
@@ -27,11 +27,11 @@
|
||||
(hooks/vector-node (into []
|
||||
(mapcat (fn [[setting-name v]]
|
||||
(concat
|
||||
[(with-meta (hooks/token-node '_) (meta setting-name)) v]
|
||||
[(with-meta (hooks/token-node '_) (meta setting-name)) v]
|
||||
;; if the setting name is namespace-qualified add a `_`
|
||||
;; entry for it too.
|
||||
(when (namespaced-symbol-node? setting-name)
|
||||
[(with-meta (hooks/token-node '_) (meta setting-name)) setting-name]))))
|
||||
(when (namespaced-symbol-node? setting-name)
|
||||
[(with-meta (hooks/token-node '_) (meta setting-name)) setting-name]))))
|
||||
(partition 2 (:children bindings))))
|
||||
bindings)]
|
||||
{:node (-> (list*
|
||||
|
||||
@@ -1,15 +0,0 @@
|
||||
(ns hooks.metabase.test.util.generators.jvm
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(defn with-random-cards
|
||||
"(with-random-cards <metadata-provider> <integer-count-value> & body-forms)"
|
||||
[{{:keys [children] :as node} :node :as orig}]
|
||||
(let [[token num-cards & body-nodes] children
|
||||
node* (-> (hooks/list-node
|
||||
(list token num-cards
|
||||
(hooks/list-node (list* (hooks/token-node 'let)
|
||||
(hooks/vector-node [(hooks/token-node '&cards) (hooks/list-node ())])
|
||||
body-nodes))))
|
||||
(with-meta (meta node)))]
|
||||
(assoc orig :node node*)))
|
||||
@@ -15,17 +15,3 @@
|
||||
actual-arg-count)
|
||||
:type :format))))))
|
||||
x)
|
||||
|
||||
(defn case-enum
|
||||
"Lint `(case-enum ...)` like `(condp = ...)`."
|
||||
[x]
|
||||
(letfn [(update-node [node]
|
||||
(let [[_case-enum x & more] (:children node)]
|
||||
(-> (api/list-node
|
||||
(list*
|
||||
(api/token-node 'clojure.core/condp)
|
||||
(api/token-node 'clojure.core/=)
|
||||
x
|
||||
more))
|
||||
(with-meta (meta node)))))]
|
||||
(update x :node update-node)))
|
||||
|
||||
@@ -1,74 +0,0 @@
|
||||
(ns hooks.metabase.util.i18n
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as api]
|
||||
[clojure.string :as str]
|
||||
[hooks.common]))
|
||||
|
||||
(defn- valid-apostrophes?
|
||||
"Returns true if `s` contains only doubled quotes (''') or complete placeholder-escaping quotes ('{...}'), false otherwise."
|
||||
[s]
|
||||
(loop [chars (seq s)]
|
||||
(if (empty? chars)
|
||||
true
|
||||
(let [c (first chars)]
|
||||
(if (= c \')
|
||||
(cond
|
||||
;; Doubled quotes: skip both
|
||||
(and (next chars) (= (second chars) \'))
|
||||
(recur (nthrest chars 2))
|
||||
|
||||
;; Placeholder escape. We allow for intentional escaping, like either '{0}` or '{{`, but not 'arbitrary
|
||||
;; strings' So basically, just require the *next* character to be a `{`, find the next single-quote, and
|
||||
;; continue on from there.
|
||||
;;
|
||||
;; Technically a string like "foo bar's happy but i'm not" is totally fine, but we'll say it's invalid
|
||||
;; because... that inner string literal is probably not what we actually want. So since we've never done
|
||||
;; this intentionally as far as I can tell, let's just remove the possibility to avoid ambiguity.
|
||||
(= (second chars) \{)
|
||||
(let [at-closing (drop-while #(not= \' %) (next chars))]
|
||||
(if (seq at-closing)
|
||||
(recur (next at-closing))
|
||||
false))
|
||||
;; Unpaired single quote: invalid
|
||||
:else
|
||||
false)
|
||||
(recur (rest chars)))))))
|
||||
|
||||
(defn format-str-arg-node->str
|
||||
[node]
|
||||
(cond
|
||||
;; it's just a string literal
|
||||
(api/string-node? node) (api/sexpr node)
|
||||
;; it's `(str "some" "strings")`
|
||||
(and (api/list-node? node)
|
||||
(= 'str (api/sexpr (first (:children node))))
|
||||
(every? api/string-node? (rest (:children node))))
|
||||
(apply str (map api/sexpr (rest (:children node))))
|
||||
|
||||
;; it's something else, no good
|
||||
:else
|
||||
(do
|
||||
(api/reg-finding!
|
||||
(assoc (meta node)
|
||||
:message "Must be a string literal or a call to `str` with string literal arguments."
|
||||
:type :metabase/validate-string-or-str-args-to-i18n))
|
||||
nil)))
|
||||
|
||||
(defn strict-apostrophes
|
||||
"Checks the node to make sure it's not a i18n call that will result in dropped apostrophes."
|
||||
[{:keys [node]}]
|
||||
(let [format-str-arg (second (:children node))
|
||||
;; `format-str-arg` must be either a `"string"` or `(str "a call " "to" "str")` (to allow for splitting long
|
||||
;; strings into shorter lines)
|
||||
format-str (format-str-arg-node->str format-str-arg)]
|
||||
(when-not (valid-apostrophes? format-str)
|
||||
(api/reg-finding!
|
||||
(assoc (meta node)
|
||||
:message "Format string contains invalid single quote usage. Use '' for literals or '{...}' for escaping."
|
||||
:type :metabase/validate-escaped-single-quotes-in-i18n))))
|
||||
{:node node})
|
||||
|
||||
(def tru strict-apostrophes)
|
||||
(def trs strict-apostrophes)
|
||||
(def deferred-tru strict-apostrophes)
|
||||
(def deferred-trs strict-apostrophes)
|
||||
@@ -109,6 +109,8 @@
|
||||
actual-arg-count)
|
||||
:type :metabase/validate-logging)))))
|
||||
|
||||
|
||||
|
||||
(defn infof
|
||||
"Valid logf, debugf, etc."
|
||||
[{:keys [node], :as x}]
|
||||
|
||||
@@ -1,26 +0,0 @@
|
||||
(ns hooks.metabase.util.log.capture
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as api]))
|
||||
|
||||
(defn with-log-messages-for-level [x]
|
||||
(letfn [(update-binding [[lhs-node _rhs-node]]
|
||||
[lhs-node (api/list-node
|
||||
(list
|
||||
(api/token-node 'clojure.core/fn)
|
||||
(api/vector-node [])))])
|
||||
(update-bindings [bindings-node]
|
||||
(-> (api/vector-node
|
||||
(into []
|
||||
(comp (partition-all 2)
|
||||
(mapcat update-binding))
|
||||
(:children bindings-node)))
|
||||
(with-meta (meta bindings-node))))
|
||||
(update-node [node]
|
||||
(let [[_with-log-messages-for-level bindings & body] (:children node)]
|
||||
(-> (api/list-node
|
||||
(list*
|
||||
(api/token-node 'clojure.core/let)
|
||||
(update-bindings bindings)
|
||||
body))
|
||||
(with-meta (meta node)))))]
|
||||
(update x :node update-node)))
|
||||
@@ -1,16 +0,0 @@
|
||||
(ns hooks.metabase.util.timing
|
||||
"Custom CLJ Kondo hooks for timing-related linting rules."
|
||||
(:require [clj-kondo.hooks-api :as api]))
|
||||
|
||||
(defn discourage-millis-duration
|
||||
"Discourage slow and unreliable duration calculations using System/currentTimeMillis "
|
||||
[{:keys [node]}]
|
||||
(let [sexpr (api/sexpr node)]
|
||||
(when (and (list? sexpr) (= (first sexpr) '-) (>= (count sexpr) 2))
|
||||
(let [first-arg (second sexpr)]
|
||||
(when (= first-arg (list 'System/currentTimeMillis))
|
||||
(api/reg-finding!
|
||||
(assoc (meta node)
|
||||
:message "Avoid System/currentTimeMillis for calculating durations. Use u/start-timer and u/since-ms for more reliability."
|
||||
:type :metabase/discourage-millis-duration
|
||||
:level :warning)))))))
|
||||
@@ -1,29 +0,0 @@
|
||||
(ns hooks.metabase.xrays.api.automagic-dashboards-test
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]))
|
||||
|
||||
(comment
|
||||
|
||||
(def input (hooks/parse-string
|
||||
"(with-indexed-model! [{:keys [model model-index model-index-value]}
|
||||
{:query :some-query}]
|
||||
:foo :bar :body)"))
|
||||
|
||||
(println (hooks/sexpr input))
|
||||
|
||||
(let [f (load-file ".clj-kondo/hooks/metabase/xrays/api/automagic_dashboards_test.clj")]
|
||||
(f {:node input})))
|
||||
|
||||
(defn with-indexed-model!
|
||||
[{node :node}]
|
||||
(let [[_macro-call binding-and-query-info & body] (:children node)
|
||||
[binding query-info] (:children binding-and-query-info)
|
||||
node* (hooks/list-node
|
||||
[(hooks/token-node 'do-with-indexed-model!)
|
||||
query-info
|
||||
(hooks/list-node
|
||||
(list* (hooks/token-node 'fn)
|
||||
(hooks/vector-node [binding])
|
||||
body))])]
|
||||
#_(println (hooks/sexpr node*))
|
||||
{:node node*}))
|
||||
@@ -1,63 +0,0 @@
|
||||
(ns hooks.clojure.core.defmulti-test
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clj-kondo.impl.utils]
|
||||
[clojure.test :refer :all]
|
||||
[hooks.clojure.core.defmulti]))
|
||||
|
||||
(defn- lint-defmulti [form]
|
||||
(binding [clj-kondo.impl.utils/*ctx* {:config {:linters {:metabase/check-defmulti-arglists {:level :warning}}}
|
||||
:ignores (atom nil)
|
||||
:findings (atom [])
|
||||
:namespaces (atom {})}]
|
||||
(hooks.clojure.core.defmulti/lint-defmulti {:node (hooks/parse-string (pr-str form))})
|
||||
@(:findings clj-kondo.impl.utils/*ctx*)))
|
||||
|
||||
(deftest ^:parallel defmulti-missing-arglists-test
|
||||
(testing "missing arglists"
|
||||
(is (=? [{:message "All defmultis should have an attribute map with :arglists metadata. [:metabase/check-defmulti-arglists]"}]
|
||||
(lint-defmulti '(defmulti expand-visualization
|
||||
(fn [card _ _]
|
||||
(-> card :visualization first))))))))
|
||||
|
||||
(deftest ^:parallel defmulti-invalid-arglists-test
|
||||
(testing "invalid arglists"
|
||||
(are [form] (=? [{:message ":arglists should be a quoted list of vectors [:metabase/check-defmulti-arglists]"}]
|
||||
(lint-defmulti form))
|
||||
'(defmulti expand-visualization
|
||||
{:arglists '()}
|
||||
(fn [card _ _]
|
||||
(-> card :visualization first)))
|
||||
'(defmulti expand-visualization
|
||||
{:arglists ([a])} ; unquoted
|
||||
(fn [card _ _]
|
||||
(-> card :visualization first)))
|
||||
'(defmulti expand-visualization
|
||||
{:arglists '[(a)]}
|
||||
(fn [card _ _]
|
||||
(-> card :visualization first)))
|
||||
'(defmulti expand-visualization
|
||||
{:arglists '((a))}
|
||||
(fn [card _ _]
|
||||
(-> card :visualization first))))))
|
||||
|
||||
(deftest ^:parallel defmulti-arglists-with-underscore-args-test
|
||||
(testing "arglists, but some symbols are _"
|
||||
(are [form] (=? [{:message ":arglists should contain actual arg names, not underscore (unused) symbols [:metabase/check-defmulti-arglists]"}]
|
||||
(lint-defmulti form))
|
||||
'(defmulti expand-visualization
|
||||
{:arglists '([card _ _])}
|
||||
(fn [card _ _]
|
||||
(-> card :visualization first)))
|
||||
'(defmulti expand-visualization
|
||||
{:arglists '([card _bad _arg])}
|
||||
(fn [card _ _]
|
||||
(-> card :visualization first))))))
|
||||
|
||||
(deftest ^:parallel defmulti-arglists-ok-test
|
||||
(testing "good"
|
||||
(is (= []
|
||||
(lint-defmulti '(defmulti expand-visualization
|
||||
{:arglists '([card x y])}
|
||||
(fn [card _ _]
|
||||
(-> card :visualization first))))))))
|
||||
@@ -1,87 +0,0 @@
|
||||
(ns hooks.clojure.core.ns-test
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as hooks]
|
||||
[clj-kondo.impl.utils]
|
||||
[clojure.test :refer :all]
|
||||
[hooks.clojure.core.ns]))
|
||||
|
||||
(defn- lint-ns [form config]
|
||||
(binding [clj-kondo.impl.utils/*ctx* {:config config
|
||||
:ignores (atom nil)
|
||||
:findings (atom [])
|
||||
:namespaces (atom {})}]
|
||||
(hooks.clojure.core.ns/lint-ns {:node (hooks/parse-string
|
||||
(binding [*print-meta* true]
|
||||
(pr-str form)))
|
||||
:config config})
|
||||
@(:findings clj-kondo.impl.utils/*ctx*)))
|
||||
|
||||
(defn- lint-modules [form config]
|
||||
(lint-ns form (assoc-in config [:linters :metabase/modules :level] :warning)))
|
||||
|
||||
(deftest ^:parallel module-checker-allowed-modules-test
|
||||
(are [task-namespace] (=? [{:message "Module task should not be used in the api module. [:metabase/modules api :uses]"
|
||||
:type :metabase/modules}]
|
||||
(lint-modules
|
||||
'(ns metabase.api.search
|
||||
(:require
|
||||
[metabase.request.core :as request]
|
||||
[task-namespace :as task]))
|
||||
'{:metabase/modules {api {:uses #{request}}
|
||||
request {:api #{metabase.request.core}}}}))
|
||||
metabase.task
|
||||
metabase.task.core))
|
||||
|
||||
(deftest ^:parallel module-checker-api-namespaces-test
|
||||
(is (=? [{:message "Namespace metabase.search.config is not an allowed external API namespace for the search module. [:metabase/modules search :api]"
|
||||
:type :metabase/modules}]
|
||||
(lint-modules
|
||||
'(ns metabase.api.search
|
||||
(:require
|
||||
[metabase.search.config :as search.config]
|
||||
[metabase.search.core :as search]))
|
||||
'{:metabase/modules {api {:uses #{search}}
|
||||
search {:api #{metabase.search.core}}}}))))
|
||||
|
||||
(deftest ^:parallel module-checker-friends-test
|
||||
(is (= []
|
||||
(lint-modules
|
||||
'(ns metabase.search-rest.api
|
||||
(:require
|
||||
[metabase.search.config :as search.config]
|
||||
[metabase.search.core :as search]))
|
||||
'{:metabase/modules {search {:api #{metabase.search.core}
|
||||
:friends #{search-rest}}
|
||||
search-rest {:uses #{search}}}}))))
|
||||
|
||||
(deftest ^:parallel module-checker-api-namespaces-ignore-test
|
||||
(is (empty? (lint-modules
|
||||
'(ns metabase.api.search
|
||||
(:require
|
||||
^{:clj-kondo/ignore [:metabase/modules]}
|
||||
[metabase.search.config :as search.config]
|
||||
[metabase.search.core :as search]))
|
||||
'{:metabase/modules {api {:uses #{search}}
|
||||
search {:api #{metabase.search.core}}}}))))
|
||||
|
||||
(deftest ^:parallel module-checker-enterprise-test
|
||||
(is (=? [{:message "Module search should not be used in the enterprise/search module. [:metabase/modules enterprise/search :uses]"
|
||||
:type :metabase/modules}]
|
||||
(lint-modules
|
||||
'(ns metabase-enterprise.search.core
|
||||
(:require
|
||||
[metabase.search.core :as search]))
|
||||
'{:metabase/modules {search {:api #{metabase.search.core}}}}))))
|
||||
|
||||
(deftest ^:parallel require-newlines-test
|
||||
(binding [clj-kondo.impl.utils/*ctx* {:config {:linters {:metabase/require-shape-checker {:level :warnings}}}
|
||||
:ignores (atom nil)
|
||||
:findings (atom [])
|
||||
:namespaces (atom {})}]
|
||||
(hooks.clojure.core.ns/lint-ns {:node (hooks/parse-string "
|
||||
(ns metabase.task.cache
|
||||
(:require [metabase.task.bunny]
|
||||
[metabase.task.rabbit]))")})
|
||||
(is (=? [{:message "Put your requires on a newline from the :require keyword [:metabase/require-shape-checker]",
|
||||
:type :metabase/require-shape-checker}]
|
||||
@(:findings clj-kondo.impl.utils/*ctx*)))))
|
||||
@@ -1,55 +0,0 @@
|
||||
(ns hooks.clojure.test-test
|
||||
(:require
|
||||
[clj-kondo.hooks-api :as api]
|
||||
[clj-kondo.impl.utils]
|
||||
[clojure.edn :as edn]
|
||||
[clojure.test :refer :all]
|
||||
[hooks.clojure.test]))
|
||||
|
||||
(set! *warn-on-reflection* true)
|
||||
|
||||
(defn- deftest-warnings
|
||||
[form]
|
||||
(binding [clj-kondo.impl.utils/*ctx* {:config {:linters {:metabase/disallow-hardcoded-driver-names-in-tests {:level :warning}}}
|
||||
:ignores (atom nil)
|
||||
:findings (atom [])
|
||||
:namespaces (atom {})}]
|
||||
(hooks.clojure.test/deftest {:node (api/parse-string form)
|
||||
:config {:linters
|
||||
{:metabase/disallow-hardcoded-driver-names-in-tests
|
||||
{:drivers
|
||||
#{:athena}}}}})
|
||||
(mapv :message @(:findings clj-kondo.impl.utils/*ctx*))))
|
||||
|
||||
(deftest ^:parallel disallow-hardcoded-driver-names-in-tests-test
|
||||
(is (= []
|
||||
(deftest-warnings
|
||||
"(mt/test-drivers (mt/normal-drivers)
|
||||
(do-something))")))
|
||||
(is (= ["Do not hardcode driver name :athena in driver tests! [:metabase/disallow-hardcoded-driver-names-in-tests]"]
|
||||
(deftest-warnings
|
||||
"(mt/test-drivers (mt/normal-drivers)
|
||||
(when-not (= driver/*driver* :athena)
|
||||
(do-something)))")))
|
||||
(testing "make sure :clj-kondo/ignore is propagated correctly"
|
||||
(is (= []
|
||||
(deftest-warnings
|
||||
"(mt/test-drivers (mt/normal-drivers)
|
||||
#_{:clj-kondo/ignore [:metabase/disallow-hardcoded-driver-names-in-tests]}
|
||||
(when-not (= driver/*driver* :athena)
|
||||
(do-something)))")))))
|
||||
|
||||
(deftest ^:parallel check-driver-keywords-test
|
||||
(testing "Make sure we keep hooks.clojure.test/driver-keywords up to date"
|
||||
(let [driver-keywords (-> (slurp ".clj-kondo/config.edn")
|
||||
edn/read-string
|
||||
(get-in [:linters :metabase/disallow-hardcoded-driver-names-in-tests :drivers]))
|
||||
driver-modules (->> (slurp "modules/drivers/deps.edn")
|
||||
edn/read-string
|
||||
:deps
|
||||
vals
|
||||
(keep (comp keyword :local/root))
|
||||
(into #{}))]
|
||||
(doseq [driver driver-modules]
|
||||
(is (contains? driver-keywords driver)
|
||||
(format "hooks.clojure.test/driver-keywords should contain %s, please add it" driver))))))
|
||||
Some files were not shown because too many files have changed in this diff Show More
Reference in New Issue
Block a user