Skip to content

Commit

Permalink
Add checks with Grit (#3027)
Browse files Browse the repository at this point in the history
  • Loading branch information
Nateowami authored Feb 20, 2025
1 parent b95a799 commit 46214a7
Show file tree
Hide file tree
Showing 11 changed files with 221 additions and 15 deletions.
21 changes: 21 additions & 0 deletions .github/workflows/grit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: Grit check

on:
push:
branches: [master, develop, sf-qa, sf-live]
pull_request:
branches: [master, develop]

permissions: {}

jobs:
grit-check:
runs-on: ubuntu-latest
steps:
- name: Check out code
uses: actions/checkout@v4
with:
persist-credentials: false

- name: grit-check
uses: getgrit/github-action-check@c1ba5f99fd0e46f4cdc35d8dbc4622610ccc1784 # v0
2 changes: 2 additions & 0 deletions .grit/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
.gritmodules*
*.log
3 changes: 3 additions & 0 deletions .grit/grit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
version: 0.0.1
patterns:
- name: github.com/getgrit/stdlib#*
20 changes: 20 additions & 0 deletions .grit/patterns/README.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
This is a .txt file, not a .md file, so Grit won't try to treat it as a pattern

See https://docs.grit.io/guides/config for documentation

Run `grit list` to see available patterns, especially locally defined ones.

Run `grit check` to run patterns (appears to only run those with a defined level, like error or warn)

Run `grit apply <name of pattern>` to apply a pattern (name comes from the markdown file)

Run `grit patterns test` to verify the patterns are correct

The documentation on the linked page states:
If a subheading has a single code block, it represents a test case that should be matched by the pattern. You don't need
to provide a transformed example.

This appears to be incorrect; experimentally it represents a case that *should not* be matched by the pattern. Grit is
intended to migrate code, but you can also write patterns that merely match code without transforming it. This results
in writing a before/after block that is identical, which *looks like* it means nothing should happen, but in reality
appears to mean it should be matched but not transformed.
32 changes: 32 additions & 0 deletions .grit/patterns/bare_verify.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
level: error
---

# Bare verify

`verify(something);` is incomplete; it should be like `verify(something).once();`

```grit
`verify($_);` where {
$filename <: r".*\.spec\.ts$"
}
```

## Verify positive example

```ts
// @filename: some.spec.ts
verify(something);
```

```ts
// @filename: some.spec.ts
verify(something);
```

## Verify negative example

```ts
// @filename: some.spec.ts
verify(something).then(somethingElse);
```
27 changes: 27 additions & 0 deletions .grit/patterns/bare_when.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
level: error
---

# Bare when

`when(something)` is is incomplete; it should be like `when(something).then(somethingElse)`

```grit
`when($_);`
```

## When example

```ts
when(something);
```

```ts
when(something);
```

## When negative example

```ts
when(something).then(somethingElse);
```
31 changes: 31 additions & 0 deletions .grit/patterns/import_from_lodash_es.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
level: warn
---

Imports can safely be made directly from `lodash-es` instead of `lodash-es/...`.

# Lodash import from main

```grit
language js
`import $value from "$path";` => `import { $value } from "lodash-es";` where {
$path <: r"lodash-es/.*"
}
```

## Basic example

Before

```ts
import merge from "lodash-es/merge";
import { merge } from "lodash-es";
```

After

```ts
import { merge } from "lodash-es";
import { merge } from "lodash-es";
```
29 changes: 29 additions & 0 deletions .grit/patterns/no_internal_imports.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
level: error
---

# No internal imports (such as from rxjs/internal)

Imports should not be from directories marked as internal

```grit
language js
`import $anything from "$path"` where $path <: r".*/internal/.*"
```

## Basic example

```ts
import { Observable } from "rxjs/internal/Observable";
```

```ts
import { Observable } from "rxjs/internal/Observable";
```

## Negative example

```ts
import { Observable } from "rxjs";
```
30 changes: 30 additions & 0 deletions .grit/patterns/no_skipped_tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
level: error
---

# No skipped tests (no fdescribe or fit)

fit() and fdescribe() should not be checked into version control

```grit
language js
or {
`fdescribe($test)` => `describe($test)`,
`fit($test)` => `it($test)`
}
```

## Basic example

```ts
fdescribe("MyComponent", () => {
fit("should do something", () => {});
});
```

```ts
describe("MyComponent", () => {
it("should do something", () => {});
});
```
26 changes: 26 additions & 0 deletions .grit/patterns/xforge_common_absolute_only.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
tags: [precommit]
level: warn
---

# Import from xforge-common, not from ../xforge-common

Imports from `xforge-common` should be absolute, not relative

```grit
language js
`import $thing from '$anywhere';` where {
$anywhere <: r".*\.\./(xforge-common/.*)"($path) => `$path`
}
```

## Basic example

```ts
import { UICommonModule } from "../../xforge-common/ui-common.module";
```

```ts
import { UICommonModule } from "xforge-common/ui-common.module";
```
15 changes: 0 additions & 15 deletions .lgtm/javascript-queries/import-relative-to-project-root.ql

This file was deleted.

0 comments on commit 46214a7

Please sign in to comment.