Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add dart_frog_lint package #559

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
3429f02
Feat: Add dart_grog_lint package
rrousselGit Mar 7, 2023
b0b95da
Spell check
rrousselGit Mar 7, 2023
df813c9
Merge branch 'main' into Add-lint-package
rrousselGit Mar 7, 2023
f7a066c
Update packages/dart_frog_lint/README.md
rrousselGit Mar 13, 2023
d6b1b18
Merge branch 'main' into Add-lint-package
rrousselGit Mar 13, 2023
6e6b78d
Add docs comment
rrousselGit Mar 15, 2023
d24824f
Merge branch 'main' into Add-lint-package
renancaraujo Mar 21, 2023
6c5f46f
Merge branch 'main' into Add-lint-package
renancaraujo Mar 21, 2023
82b7ae2
Merge branch 'main' into Add-lint-package
renancaraujo Mar 24, 2023
81b3f4a
Merge branch 'main' into Add-lint-package
renancaraujo Mar 27, 2023
7c2a908
Merge branch 'main' into Add-lint-package
rrousselGit Apr 3, 2023
b85d39f
Reorganize example
rrousselGit Apr 3, 2023
8ed36fa
Add parseRoute test
rrousselGit Apr 3, 2023
6dd53eb
Update packages/dart_frog_lint/lib/src/dart_frog_middleware.dart
rrousselGit Apr 4, 2023
74a424f
Update packages/dart_frog_lint/lib/src/dart_frog_entrypoint.dart
rrousselGit Apr 4, 2023
02e9f95
Update packages/dart_frog_lint/lib/src/dart_frog_entrypoint.dart
rrousselGit Apr 4, 2023
1f8ee19
Update packages/dart_frog_lint/lib/src/dart_frog_entrypoint.dart
rrousselGit Apr 4, 2023
baa902e
Update packages/dart_frog_lint/lib/src/dart_frog_entrypoint.dart
rrousselGit Apr 4, 2023
65fa934
Merge branch 'main' into Add-lint-package
alestiago Apr 12, 2023
3a9120e
Merge branch 'main' into Add-lint-package
renancaraujo Apr 18, 2023
a31a015
Update packages/dart_frog_lint/test/src/parse_route_test.dart
rrousselGit Apr 19, 2023
2f339d9
Add issue link about missing exclude patterns
rrousselGit Apr 19, 2023
3a3161e
Add more entrypoint tests
rrousselGit Apr 19, 2023
0af05fe
Add valid onRequest with parameter
rrousselGit Apr 19, 2023
b0114d9
Rename dart_frog_route -> dart_frog_request
rrousselGit Apr 19, 2023
513d9d8
Merge branch 'main' into Add-lint-package
renancaraujo May 19, 2023
4d55491
Merge branch 'main' into Add-lint-package
renancaraujo Jun 6, 2023
28bbbcc
Merge branch 'main' into Add-lint-package
renancaraujo Jul 18, 2023
c236835
Merge branch 'main' into Add-lint-package
rrousselGit Jul 21, 2023
9bebc8b
Merge branch 'main' into Add-lint-package
renancaraujo Aug 3, 2023
2784ae7
Merge branch 'main' into Add-lint-package
tomarra Feb 7, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .github/workflows/dart_frog_lint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
name: dart_frog_lint

on:
pull_request:
paths:
- ".github/workflows/dart_frog_lint.yaml"
- "packages/dart_frog_lint/example/**"
- "packages/dart_frog_lint/lib/**"
- "packages/dart_frog_lint/test/**"
- "packages/dart_frog_lint/pubspec.yaml"
push:
branches:
- main
paths:
- ".github/workflows/dart_frog_lint.yaml"
- "packages/dart_frog_lint/example/**"
- "packages/dart_frog_lint/lib/**"
- "packages/dart_frog_lint/test/**"
- "packages/dart_frog_lint/pubspec.yaml"

jobs:
build:
uses: VeryGoodOpenSource/very_good_workflows/.github/workflows/dart_package.yml@v1
with:
working_directory: packages/dart_frog

custom_lint:
runs-on: ubuntu-latest
defaults:
run:
working-directory: packages/dart_frog_lint

steps:
# Bootstrap project
- uses: actions/[email protected]
with:
fetch-depth: 2
- uses: subosito/[email protected]
- name: Add pub cache bin to PATH
run: echo "$HOME/.pub-cache/bin" >> $GITHUB_PATH
- name: Add pub cache to PATH
run: echo "PUB_CACHE="$HOME/.pub-cache"" >> $GITHUB_ENV
- name: Install dependencies
run: flutter pub get

# Finally do some checks
- name: Run custom_lint
run: dart run custom_lint

pana:
uses: VeryGoodOpenSource/very_good_workflows/.github/workflows/pana.yml@v1
with:
working_directory: packages/dart_frog
10 changes: 10 additions & 0 deletions packages/dart_frog_lint/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# See https://www.dartlang.org/guides/libraries/private-files

# Files and directories created by pub
.dart_tool/
.packages
build/
pubspec.lock

# Test related files
coverage/
3 changes: 3 additions & 0 deletions packages/dart_frog_lint/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Unreleased

Initial release
21 changes: 21 additions & 0 deletions packages/dart_frog_lint/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
MIT License

Copyright (c) 2022 Very Good Ventures

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
230 changes: 230 additions & 0 deletions packages/dart_frog_lint/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
Dart_frog_lint is a developer tool for users of dart_frog, to help spot common mistakes.
It helps verify that file conventions are properly respected.

## Table of content

- [Table of content](#table-of-content)
- [Installing dart\_frog\_lint](#installing-dart_frog_lint)
- [Enabling/disabling lints.](#enablingdisabling-lints)
- [Disable one specific rule](#disable-one-specific-rule)
- [Disable all lints by default](#disable-all-lints-by-default)
- [Running dart\_frog\_lint in the terminal/CI](#running-dart_frog_lint-in-the-terminalci)
- [All the lints](#all-the-lints)
- [dart\_frog\_entrypoint](#dart_frog_entrypoint)
- [dart\_frog\_middleware](#dart_frog_middleware)
- [dart\_frog\_route](#dart_frog_route)

## Installing dart_frog_lint

Dart_frog_lint is implemented using [custom_lint]. As such, it uses custom_lint's installation logic.
Long story short:

- Add both dart_frog_lint and custom_lint to your `pubspec.yaml`:
```yaml
dev_dependencies:
custom_lint:
dart_frog_lint:
```
- Enable `custom_lint`'s plugin in your `analysis_options.yaml`:

```yaml
analyzer:
plugins:
- custom_lint
```

## Enabling/disabling lints.

By default when installing dart_frog_lint, most of the lints will be enabled.
To change this, you have a few options.

### Disable one specific rule

You may dislike one of the various lint rules offered by dart_frog_lint.
In that event, you can explicitly disable this lint rule for your project
by modifying the `analysis_options.yaml`

```yaml
analyzer:
plugins:
- custom_lint

custom_lint:
rules:
# Explicitly disable one lint rule
- dart_frog_request: false
```

Note that you can both enable and disable lint rules at once.
This can be useful if your `analysis_options.yaml` includes another one:

```yaml
include: path/to/another/analysis_options.yaml
analyzer:
plugins:
- custom_lint

custom_lint:
rules:
# Enable one rule
- dart_frog_middleware
# Disable another
- dart_frog_request: false
```

### Disable all lints by default

Instead of having all lints on by default and manually disabling lints of your choice,
you can switch to the opposite logic:
Have lints off by default, and manually enable lints.

This can be done in your `analysis_options.yaml` with the following:

```yaml
analyzer:
plugins:
- custom_lint

custom_lint:
# Forcibly disable lint rules by default
enable_all_lint_rules: false
rules:
# You can now enable one specific rule in the "rules" list
- dart_frog_middleware
```

## Running dart_frog_lint in the terminal/CI

Custom lint rules created by dart_frog_lint may not show-up in `dart analyze`.
To fix this, you can run a custom command line: `custom_lint`.

Since your project should already have custom_lint installed
(cf [installing dart_frog_lint](#installing-dart_frog_lint)), then you should be
able to run:

```sh
dart run custom_lint
```

Alternatively, you can globally install `custom_lint`:

```sh
# Install custom_lint for all projects
dart pub global activate custom_lint
# run custom_lint's command line in a project
custom_lint
```

## All the lints
alestiago marked this conversation as resolved.
Show resolved Hide resolved

### dart_frog_entrypoint

The `dart_frog_entrypoint` lint checks that `main.dart` files contain a
valid `run` function. See also [Creating a custom entrypoint](https://dartfrog.vgv.dev/docs/advanced/custom_entrypoint).

**Good**:

```dart
// main.dart
Future<HttpServer> run(Handler handler, InternetAddress ip, int port) {
// TODO
}
```

**Bad**:

```dart
// An empty main.dart file
// The file must contain a top-level function named "run"
```

```dart
// main.dart

// A "run" function is present, but the return value or parameters are incorrect
void run() {}
```

### dart_frog_middleware

The `dart_frog_middleware` lint checks that `routes/*_middleware.dart` files contain a
valid `middleware` function. See also [Middleware](https://dartfrog.vgv.dev/docs/basics/middleware).

**Good**:

```dart
// routes/my_middleware.dart
Handler middleware(Handler handler) {
return (context) async {
// TODO
};
}
```

**Bad**:

```dart
// routes/my_middleware.dart
// The file must contain a valid top-level "middleware" function
```

```dart
// routes/my_middleware.dart
// The file must contain a valid top-level "middleware" function

// A "middleware" function is present, but the return value or parameters are incorrect
void middleware() {}
```

### dart_frog_route

The `dart_frog_route` lint checks that `routes/*.dart` files contain a
valid `onRequest` function. See also [Routes](https://dartfrog.vgv.dev/docs/basics/routes).

**Good**:

```dart
// routes/hello.dart
Response onRequest(RequestContext context) {
// TODO
}
```

```dart
// routes/posts/[id].dart
// Dynamic routes are supported too
Response onRequest(RequestContext context, String id) {
// TODO
}
```

```dart
// routes/example.dart
// Routes can return a Future<T>
Future<Response> onRequest(RequestContext context) async {
// TODO
}
```

**Bad**:

```dart
// routes/hello.dart
// The file must contain a valid top-level "onRequest" function
```

```dart
// routes/hello.dart
// An "onRequest" function is present, but the return value or parameters are incorrect
void onRequest() {}
```

```dart
// routes/posts/[id].dart
// The route is a dynamic route, but onRequest doesn't receive the correct number of parameters.
Response onRequest(RequestContext context) {
// TODO
}
```

[custom_lint]: https://pub.dev/packages/custom_lint
1 change: 1 addition & 0 deletions packages/dart_frog_lint/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include: package:very_good_analysis/analysis_options.4.0.0.yaml
20 changes: 20 additions & 0 deletions packages/dart_frog_lint/coverage_badge.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions packages/dart_frog_lint/example/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
custom_lint.log
7 changes: 7 additions & 0 deletions packages/dart_frog_lint/example/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
include: ../analysis_options.yaml
analyzer:
plugins:
- custom_lint
linter:
rules:
file_names: false
1 change: 1 addition & 0 deletions packages/dart_frog_lint/example/lib/out.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// An empty file for checking that routes lints don't trigger on lib folders
7 changes: 7 additions & 0 deletions packages/dart_frog_lint/example/main.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import 'dart:io';
alestiago marked this conversation as resolved.
Show resolved Hide resolved

import 'package:dart_frog/dart_frog.dart';

Future<HttpServer> run(Handler handler, InternetAddress ip, int port) {
throw UnimplementedError();
}
Comment on lines +1 to +7
Copy link
Contributor

@alestiago alestiago Mar 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we are not testing that the lint rules dart_frog_entrypoint works as expected. In other words, if the DartFrogEntrypoint implementation is altered to have an incorrect behaviour or no behaviour at all, the CI will not complain about it.

We might need another testing strategy for this lint rule? Since we can't really have two main.dart under the same directory?

In here we're using an actual test alongside the example/ testing (note that is very WIP). Despite this, I would still like example/ to include examples of the lint rule failing and being catched by expect_lint.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about changing filesToAnalyze to:

  @override
  List<String> get filesToAnalyze => ['main.dart', 'dart_frog_entrypoint/**.dart'];

Then we could have example/dart_frog_entrypoint/whatever.dart with expect_lint in it?

Copy link
Contributor

@alestiago alestiago Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good to me! However, if someone has a route named dart_frog_entrypoint/index.dart would they get the lint warning? Is there a way to only allow dart_frog_entrypoint/**.dart as a file to analyse only during testing?

15 changes: 15 additions & 0 deletions packages/dart_frog_lint/example/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: dart_frog_lint_example
version: 0.0.1
publish_to: "none"

environment:
sdk: ">=2.17.0 <3.0.0"

dependencies:
dart_frog:
path: ../../dart_frog

dev_dependencies:
dart_frog_lint:
path: ..
very_good_analysis: ^4.0.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import 'package:dart_frog/dart_frog.dart';

// expect_lint: dart_frog_middleware
Handler middleware(int handler) {
throw UnimplementedError();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import 'package:dart_frog/dart_frog.dart';

// expect_lint: dart_frog_middleware
String middleware(Handler handler) {
throw UnimplementedError();
}
Loading
Loading