-
Notifications
You must be signed in to change notification settings - Fork 161
feat: add dart_frog_lint package #559
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
base: main
Are you sure you want to change the base?
Changes from all commits
3429f02
b0b95da
df813c9
f7a066c
d6b1b18
6e6b78d
d24824f
6c5f46f
82b7ae2
81b3f4a
7c2a908
b85d39f
8ed36fa
6dd53eb
74a424f
02e9f95
1f8ee19
baa902e
65fa934
3a9120e
a31a015
2f339d9
3a3161e
0af05fe
b0114d9
513d9d8
4d55491
28bbbcc
c236835
9bebc8b
2784ae7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
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/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# Unreleased | ||
|
||
Initial release |
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. |
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 | ||
|
||
### 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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
include: package:very_good_analysis/analysis_options.4.0.0.yaml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
custom_lint.log |
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 |
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 |
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently we are not testing that the lint rules We might need another testing strategy for this lint rule? Since we can't really have two In here we're using an actual test alongside the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about changing @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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds good to me! However, if someone has a route named |
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(); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.