diff --git a/.github/workflows/dart_frog_lint.yaml b/.github/workflows/dart_frog_lint.yaml new file mode 100644 index 000000000..bbc200ed3 --- /dev/null +++ b/.github/workflows/dart_frog_lint.yaml @@ -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/checkout@v3.1.0 + with: + fetch-depth: 2 + - uses: subosito/flutter-action@v2.7.1 + - 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 diff --git a/packages/dart_frog_lint/.gitignore b/packages/dart_frog_lint/.gitignore new file mode 100644 index 000000000..cc063e820 --- /dev/null +++ b/packages/dart_frog_lint/.gitignore @@ -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/ \ No newline at end of file diff --git a/packages/dart_frog_lint/CHANGELOG.md b/packages/dart_frog_lint/CHANGELOG.md new file mode 100644 index 000000000..93fbc2511 --- /dev/null +++ b/packages/dart_frog_lint/CHANGELOG.md @@ -0,0 +1,3 @@ +# Unreleased + +Initial release \ No newline at end of file diff --git a/packages/dart_frog_lint/LICENSE b/packages/dart_frog_lint/LICENSE new file mode 100644 index 000000000..7918ffb58 --- /dev/null +++ b/packages/dart_frog_lint/LICENSE @@ -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. \ No newline at end of file diff --git a/packages/dart_frog_lint/README.md b/packages/dart_frog_lint/README.md new file mode 100644 index 000000000..85275102d --- /dev/null +++ b/packages/dart_frog_lint/README.md @@ -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 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 +Future 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 diff --git a/packages/dart_frog_lint/analysis_options.yaml b/packages/dart_frog_lint/analysis_options.yaml new file mode 100644 index 000000000..84e34fba4 --- /dev/null +++ b/packages/dart_frog_lint/analysis_options.yaml @@ -0,0 +1 @@ +include: package:very_good_analysis/analysis_options.4.0.0.yaml diff --git a/packages/dart_frog_lint/coverage_badge.svg b/packages/dart_frog_lint/coverage_badge.svg new file mode 100644 index 000000000..499e98ce2 --- /dev/null +++ b/packages/dart_frog_lint/coverage_badge.svg @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + coverage + coverage + 100% + 100% + + diff --git a/packages/dart_frog_lint/example/.gitignore b/packages/dart_frog_lint/example/.gitignore new file mode 100644 index 000000000..965f204b9 --- /dev/null +++ b/packages/dart_frog_lint/example/.gitignore @@ -0,0 +1 @@ +custom_lint.log \ No newline at end of file diff --git a/packages/dart_frog_lint/example/analysis_options.yaml b/packages/dart_frog_lint/example/analysis_options.yaml new file mode 100644 index 000000000..ff7873384 --- /dev/null +++ b/packages/dart_frog_lint/example/analysis_options.yaml @@ -0,0 +1,7 @@ +include: ../analysis_options.yaml +analyzer: + plugins: + - custom_lint +linter: + rules: + file_names: false diff --git a/packages/dart_frog_lint/example/lib/out.dart b/packages/dart_frog_lint/example/lib/out.dart new file mode 100644 index 000000000..61d0cd9da --- /dev/null +++ b/packages/dart_frog_lint/example/lib/out.dart @@ -0,0 +1 @@ +// An empty file for checking that routes lints don't trigger on lib folders diff --git a/packages/dart_frog_lint/example/main.dart b/packages/dart_frog_lint/example/main.dart new file mode 100644 index 000000000..7355f128e --- /dev/null +++ b/packages/dart_frog_lint/example/main.dart @@ -0,0 +1,7 @@ +import 'dart:io'; + +import 'package:dart_frog/dart_frog.dart'; + +Future run(Handler handler, InternetAddress ip, int port) { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/example/pubspec.yaml b/packages/dart_frog_lint/example/pubspec.yaml new file mode 100644 index 000000000..0589407cd --- /dev/null +++ b/packages/dart_frog_lint/example/pubspec.yaml @@ -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 diff --git a/packages/dart_frog_lint/example/routes/dart_frog_middleware/invalid_parameter_middleware.dart b/packages/dart_frog_lint/example/routes/dart_frog_middleware/invalid_parameter_middleware.dart new file mode 100644 index 000000000..9d56fd5b3 --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_middleware/invalid_parameter_middleware.dart @@ -0,0 +1,6 @@ +import 'package:dart_frog/dart_frog.dart'; + +// expect_lint: dart_frog_middleware +Handler middleware(int handler) { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_middleware/invalid_return_middleware.dart b/packages/dart_frog_lint/example/routes/dart_frog_middleware/invalid_return_middleware.dart new file mode 100644 index 000000000..66d8d02be --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_middleware/invalid_return_middleware.dart @@ -0,0 +1,6 @@ +import 'package:dart_frog/dart_frog.dart'; + +// expect_lint: dart_frog_middleware +String middleware(Handler handler) { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_middleware/missing_middleware.dart b/packages/dart_frog_lint/example/routes/dart_frog_middleware/missing_middleware.dart new file mode 100644 index 000000000..f01ce832f --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_middleware/missing_middleware.dart @@ -0,0 +1,4 @@ +// expect_lint: dart_frog_middleware +import 'dart:async'; + +FutureOr fn() {} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_middleware/no_parameter_middleware.dart b/packages/dart_frog_lint/example/routes/dart_frog_middleware/no_parameter_middleware.dart new file mode 100644 index 000000000..636a884f1 --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_middleware/no_parameter_middleware.dart @@ -0,0 +1,6 @@ +import 'package:dart_frog/dart_frog.dart'; + +// expect_lint: dart_frog_middleware +Handler middleware() { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_middleware/valid_middleware.dart b/packages/dart_frog_lint/example/routes/dart_frog_middleware/valid_middleware.dart new file mode 100644 index 000000000..1b8fa2adc --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_middleware/valid_middleware.dart @@ -0,0 +1,5 @@ +import 'package:dart_frog/dart_frog.dart'; + +Handler middleware(Handler handler) { + return handler; +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/async_response.dart b/packages/dart_frog_lint/example/routes/dart_frog_route/async_response.dart new file mode 100644 index 000000000..68c97fccd --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/async_response.dart @@ -0,0 +1,5 @@ +import 'package:dart_frog/dart_frog.dart'; + +Future onRequest(RequestContext context) async { + throw UnimplementedError(''); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/invalid_context.dart b/packages/dart_frog_lint/example/routes/dart_frog_route/invalid_context.dart new file mode 100644 index 000000000..9656ee8b8 --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/invalid_context.dart @@ -0,0 +1,6 @@ +import 'package:dart_frog/dart_frog.dart'; + +// expect_lint: dart_frog_route +Response onRequest(String context) { + return Response(); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/invalid_response.dart b/packages/dart_frog_lint/example/routes/dart_frog_route/invalid_response.dart new file mode 100644 index 000000000..fbf5e8d6c --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/invalid_response.dart @@ -0,0 +1,6 @@ +import 'package:dart_frog/dart_frog.dart'; + +// expect_lint: dart_frog_route +String onRequest(RequestContext context) { + return ''; +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/missing_on_request.dart b/packages/dart_frog_lint/example/routes/dart_frog_route/missing_on_request.dart new file mode 100644 index 000000000..9768e88db --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/missing_on_request.dart @@ -0,0 +1,4 @@ +// expect_lint: dart_frog_route +import 'dart:async'; + +FutureOr fn() {} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/no_context.dart b/packages/dart_frog_lint/example/routes/dart_frog_route/no_context.dart new file mode 100644 index 000000000..ec041209d --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/no_context.dart @@ -0,0 +1,6 @@ +import 'package:dart_frog/dart_frog.dart'; + +// expect_lint: dart_frog_route +Response onRequest() { + return Response(); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId2].dart b/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId2].dart new file mode 100644 index 000000000..e66011d9f --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId2].dart @@ -0,0 +1,7 @@ +import 'package:dart_frog/dart_frog.dart'; + +// Incorrect parameter type +// expect_lint: dart_frog_route +Response onRequest(RequestContext context, int userId2) { + return Response(); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId3].dart b/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId3].dart new file mode 100644 index 000000000..f80b3d3db --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId3].dart @@ -0,0 +1,5 @@ +import 'package:dart_frog/dart_frog.dart'; + +Response onRequest(RequestContext context, String userId3) { + return Response(); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId].dart b/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId].dart new file mode 100644 index 000000000..6a7f6fbc3 --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/parametrized_route/[userId].dart @@ -0,0 +1,7 @@ +import 'package:dart_frog/dart_frog.dart'; + +// Missing parameter +// expect_lint: dart_frog_route +Response onRequest(RequestContext context) { + return Response(); +} diff --git a/packages/dart_frog_lint/example/routes/dart_frog_route/sync_response.dart b/packages/dart_frog_lint/example/routes/dart_frog_route/sync_response.dart new file mode 100644 index 000000000..822f62a84 --- /dev/null +++ b/packages/dart_frog_lint/example/routes/dart_frog_route/sync_response.dart @@ -0,0 +1,5 @@ +import 'package:dart_frog/dart_frog.dart'; + +Response onRequest(RequestContext context) { + throw UnimplementedError(''); +} diff --git a/packages/dart_frog_lint/lib/dart_frog_lint.dart b/packages/dart_frog_lint/lib/dart_frog_lint.dart new file mode 100644 index 000000000..ea5200a70 --- /dev/null +++ b/packages/dart_frog_lint/lib/dart_frog_lint.dart @@ -0,0 +1,16 @@ +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:dart_frog_lint/src/dart_frog_entrypoint.dart'; +import 'package:dart_frog_lint/src/dart_frog_middleware.dart'; +import 'package:dart_frog_lint/src/dart_frog_request.dart'; + +/// The entrypoint of dart_frog_lint +PluginBase createPlugin() => _DartFrogLintPlugin(); + +class _DartFrogLintPlugin extends PluginBase { + @override + List getLintRules(CustomLintConfigs configs) => [ + const DartFrogRequest(), + const DartFrogMiddleware(), + const DartFrogEntrypoint(), + ]; +} diff --git a/packages/dart_frog_lint/lib/src/dart_frog_entrypoint.dart b/packages/dart_frog_lint/lib/src/dart_frog_entrypoint.dart new file mode 100644 index 000000000..c2ab68d96 --- /dev/null +++ b/packages/dart_frog_lint/lib/src/dart_frog_entrypoint.dart @@ -0,0 +1,90 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:collection/collection.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:dart_frog_lint/src/types.dart'; + +/// {@template dart_frog_lint.request} +/// The definition of `dart_frog_entrypoint` lints. +/// {@endtemplate} +class DartFrogEntrypoint extends DartLintRule { + /// {@macro dart_frog_lint.request} + const DartFrogEntrypoint() + : super( + code: const LintCode( + name: 'dart_frog_entrypoint', + problemMessage: 'Main files should define a valid "run" function.', + ), + ); + + @override + List get filesToAnalyze => ['main.dart']; + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addCompilationUnit((node) { + // Search for a function declaration with the name "run" + final run = + node.declarations.whereType().firstWhereOrNull( + (declaration) => declaration.name.lexeme == 'run', + ); + + if (run == null) { + // No function declaration found with the name "run" + reporter.reportErrorForNode(code, node.directives.firstOrNull ?? node); + return; + } + + if (run.functionExpression.parameters?.parameters.length != 3) { + // Only three parameters are allowed + reporter.reportErrorForNode(code, run); + return; + } + + final handlerType = run.functionExpression.parameters?.parameters + .firstOrNull?.declaredElement?.type; + final ipType = run.functionExpression.parameters?.parameters + .elementAtOrNull(1) + ?.declaredElement + ?.type; + final portType = run.functionExpression.parameters?.parameters + .elementAtOrNull(2) + ?.declaredElement + ?.type; + + if (handlerType == null || !isHandler(handlerType)) { + // The parameter is not a Handler + reporter.reportErrorForNode(code, run); + return; + } + + if (ipType == null || !isInternetAddress(ipType)) { + // The parameter is not an InternetAddress + reporter.reportErrorForNode(code, run); + return; + } + + if (portType?.isDartCoreInt != true) { + // The parameter is not a int + reporter.reportErrorForNode(code, run); + return; + } + + final returnType = run.returnType?.type; + if (returnType == null || + !returnType.isDartAsyncFuture || + !isHttpServer( + (returnType as InterfaceType).typeArguments.single, + )) { + // The parameter is not a HttpServer + reporter.reportErrorForNode(code, run); + return; + } + }); + } +} diff --git a/packages/dart_frog_lint/lib/src/dart_frog_middleware.dart b/packages/dart_frog_lint/lib/src/dart_frog_middleware.dart new file mode 100644 index 000000000..6eeb9a17f --- /dev/null +++ b/packages/dart_frog_lint/lib/src/dart_frog_middleware.dart @@ -0,0 +1,65 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:collection/collection.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:dart_frog_lint/src/types.dart'; + +/// {@template dart_frog_lint.request} +/// The definition of `dart_frog_middleware` lints. +/// {@endtemplate} +class DartFrogMiddleware extends DartLintRule { + /// {@macro dart_frog_lint.request} + const DartFrogMiddleware() + : super( + code: const LintCode( + name: 'dart_frog_middleware', + problemMessage: + 'Middleware files should define a valid "middleware" function.', + ), + ); + + @override + List get filesToAnalyze => ['routes/**_middleware.dart']; + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addCompilationUnit((node) { + // Search for a function declaration with the name "middleware" + final middleware = + node.declarations.whereType().firstWhereOrNull( + (declaration) => declaration.name.lexeme == 'middleware', + ); + + if (middleware == null) { + // No function declaration found with the name "middleware" + reporter.reportErrorForNode(code, node.directives.firstOrNull ?? node); + return; + } + + if (middleware.functionExpression.parameters?.parameters.length != 1) { + // Only one parameter is allowed + reporter.reportErrorForNode(code, middleware); + return; + } + + final handlerType = middleware.functionExpression.parameters?.parameters + .firstOrNull?.declaredElement?.type; + if (handlerType == null || !isHandler(handlerType)) { + // The parameter is not a Handler + reporter.reportErrorForNode(code, middleware); + return; + } + + final returnType = middleware.returnType?.type; + if (returnType == null || !isHandler(returnType)) { + // The parameter is not a Handler + reporter.reportErrorForNode(code, middleware); + return; + } + }); + } +} diff --git a/packages/dart_frog_lint/lib/src/dart_frog_request.dart b/packages/dart_frog_lint/lib/src/dart_frog_request.dart new file mode 100644 index 000000000..8d06082dd --- /dev/null +++ b/packages/dart_frog_lint/lib/src/dart_frog_request.dart @@ -0,0 +1,90 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/error/listener.dart'; +import 'package:collection/collection.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:dart_frog_lint/src/parse_route.dart'; +import 'package:dart_frog_lint/src/types.dart'; + +/// {@template dart_frog_lint.request} +/// The definition of `dart_frog_request` lints. +/// {@endtemplate} +class DartFrogRequest extends DartLintRule { + /// {@macro dart_frog_lint.request} + const DartFrogRequest() + : super( + code: const LintCode( + name: 'dart_frog_route', + problemMessage: + 'Dart files within the "route" directory should define a ' + 'valid "onRequest" function.', + ), + ); + + @override + List get filesToAnalyze => ['routes/**.dart']; + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + // package:glob which filesToAnalyze uses does not seem to support exclude + // patterns, so we have to manually filter out the _middleware.dart files + // See https://github.com/dart-lang/glob/issues/75 + if (resolver.path.endsWith('_middleware.dart')) return; + + context.registry.addCompilationUnit((node) { + // Search for a function declaration with the name "onRequest" + final onRequest = + node.declarations.whereType().firstWhereOrNull( + (declaration) => declaration.name.lexeme == 'onRequest', + ); + + if (onRequest == null) { + // No function declaration found with the name "onRequest" + reporter.reportErrorForNode(code, node.directives.firstOrNull ?? node); + return; + } + + final parameters = onRequest.functionExpression.parameters; + if (parameters == null) { + // Possible syntax error + reporter.reportErrorForNode(code, onRequest); + return; + } + + final contextParameterType = onRequest.functionExpression.parameters + ?.parameters.firstOrNull?.declaredElement?.type; + if (contextParameterType == null || + !requestContextTypeChecker.isExactlyType(contextParameterType)) { + // The onRequest function doesn't have a "RequestContext" parameter + reporter.reportErrorForNode(code, onRequest); + return; + } + + if (!isOnRequestResponse(onRequest.returnType?.type)) { + // The onRequest function doesn't return a "Response" + reporter.reportErrorForNode(code, onRequest); + return; + } + + final route = parseRoute(resolver.path); + if (onRequest.functionExpression.parameters?.parameters.length != + 1 + route.parameters.length) { + // The onRequest function doesn't have the correct number of parameters + reporter.reportErrorForNode(code, onRequest); + return; + } + + for (final parameter in parameters.parameters.skip(1)) { + final parameterType = parameter.declaredElement?.type; + if (parameterType?.isDartCoreString != true) { + // Route parameters should be positional strings + reporter.reportErrorForNode(code, onRequest); + return; + } + } + }); + } +} diff --git a/packages/dart_frog_lint/lib/src/parse_route.dart b/packages/dart_frog_lint/lib/src/parse_route.dart new file mode 100644 index 000000000..949d62c79 --- /dev/null +++ b/packages/dart_frog_lint/lib/src/parse_route.dart @@ -0,0 +1,43 @@ +import 'package:path/path.dart' as p; + +/// A parsed route file. +class RouteFile { + RouteFile._(this.path, this.parameters); + + /// The route path relative to the "routes" directory. + final String path; + + /// The parameter names in the route path. + final List parameters; +} + +/// Decode a route path into a [RouteFile]. +RouteFile parseRoute(String path) { + final routePath = _findEnclosingRouteDirectory(path); + if (routePath == null) { + throw ArgumentError.value( + path, + 'path', + 'The path must be within a "routes" directory.', + ); + } + + final relativePath = p.relative(path, from: routePath); + final split = p.split(relativePath); + final parameters = split + .map(p.basenameWithoutExtension) + .where((element) => element.startsWith('[') && element.endsWith(']')) + .map((e) => e.substring(1, e.length - 1)) + .toList(); + + return RouteFile._(relativePath, parameters); +} + +/// Find the enclosing "routes" directory of a route path. +String? _findEnclosingRouteDirectory(String path) { + final split = p.split(path); + final routeIndex = split.lastIndexOf('routes'); + if (routeIndex == -1) return null; + + return p.joinAll(split.sublist(0, routeIndex)); +} diff --git a/packages/dart_frog_lint/lib/src/types.dart b/packages/dart_frog_lint/lib/src/types.dart new file mode 100644 index 000000000..1ebf975b5 --- /dev/null +++ b/packages/dart_frog_lint/lib/src/types.dart @@ -0,0 +1,54 @@ +import 'package:analyzer/dart/element/type.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; + +/// [TypeChecker] for `RequestContext` +const requestContextTypeChecker = TypeChecker.fromName( + 'RequestContext', + packageName: 'dart_frog', +); + +/// [TypeChecker] for `Response` +const _responseTypeChecker = TypeChecker.fromName( + 'Response', + packageName: 'dart_frog', +); + +/// Checks that [type] is `Response | Future`. +bool isOnRequestResponse(DartType? type) { + if (type == null) return false; + if (_responseTypeChecker.isExactlyType(type)) return true; + + if (!type.isDartAsyncFuture) return false; + + type as InterfaceType; + return _responseTypeChecker.isExactlyType(type.typeArguments.first); +} + +/// [TypeChecker] for `Handler` +const _handlerTypeChecker = TypeChecker.fromName( + 'Handler', + packageName: 'dart_frog', +); + +/// Checks that a type is assignable with `Handler`. +/// +/// Since `Handler` is a typedef, we need check the alias instead of type matchs +bool isHandler(DartType type) { + final alias = type.alias; + if (alias == null) return false; + return _handlerTypeChecker.isExactly(alias.element); +} + +bool _isFromDartSdk(DartType type) => type.element?.library?.isInSdk ?? false; + +/// [TypeChecker] for `InternetAddress` +bool isInternetAddress(DartType type) { + const nameChecker = TypeChecker.fromName('InternetAddress'); + return _isFromDartSdk(type) && nameChecker.isExactlyType(type); +} + +/// [TypeChecker] for `HttpServer` +bool isHttpServer(DartType type) { + const nameChecker = TypeChecker.fromName('HttpServer'); + return _isFromDartSdk(type) && nameChecker.isExactlyType(type); +} diff --git a/packages/dart_frog_lint/pubspec.yaml b/packages/dart_frog_lint/pubspec.yaml new file mode 100644 index 000000000..cfcb7abe4 --- /dev/null +++ b/packages/dart_frog_lint/pubspec.yaml @@ -0,0 +1,25 @@ +name: dart_frog_lint +description: Lint rules and refactorings for dart_frog +version: 0.1.0 +homepage: https://dartfrog.vgv.dev +repository: https://github.com/VeryGoodOpenSource/dart_frog +issue_tracker: https://github.com/VeryGoodOpenSource/dart_frog/issues +documentation: https://dartfrog.vgv.dev/docs/overview + +environment: + sdk: ">=2.19.0 <3.0.0" + +dependencies: + analyzer: ^5.2.0 + analyzer_plugin: ^0.11.2 + collection: ^1.17.1 + custom_lint_builder: ^0.2.10 + glob: ^2.1.1 + meta: ^1.9.0 + path: ^1.8.3 + +dev_dependencies: + dart_frog: + path: ../dart_frog + test: ^1.21.1 + very_good_analysis: ^4.0.0 diff --git a/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/incorrect_ip.dart b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/incorrect_ip.dart new file mode 100644 index 000000000..772f08c78 --- /dev/null +++ b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/incorrect_ip.dart @@ -0,0 +1,9 @@ +import 'dart:io'; + +import 'package:dart_frog/dart_frog.dart'; + +class InternetAddress {} + +Future run(Handler handler, InternetAddress ip, int port) { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/incorrect_sdk_ip.dart b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/incorrect_sdk_ip.dart new file mode 100644 index 000000000..df89c6293 --- /dev/null +++ b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/incorrect_sdk_ip.dart @@ -0,0 +1,7 @@ +import 'dart:io'; + +import 'package:dart_frog/dart_frog.dart'; + +Future run(Handler handler, HttpServer ip, int port) { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_future_result.dart b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_future_result.dart new file mode 100644 index 000000000..f4302a9f4 --- /dev/null +++ b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_future_result.dart @@ -0,0 +1,7 @@ +import 'dart:io'; + +import 'package:dart_frog/dart_frog.dart'; + +HttpServer run(Handler handler, InternetAddress ip, int port) { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_http_server_result.dart b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_http_server_result.dart new file mode 100644 index 000000000..e5ac2132d --- /dev/null +++ b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_http_server_result.dart @@ -0,0 +1,9 @@ +import 'dart:io'; + +import 'package:dart_frog/dart_frog.dart'; + +class HttpServer {} + +Future run(Handler handler, InternetAddress ip, int port) { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_server_io_result.dart b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_server_io_result.dart new file mode 100644 index 000000000..e1ad65c95 --- /dev/null +++ b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_sources/non_server_io_result.dart @@ -0,0 +1,7 @@ +import 'dart:io'; + +import 'package:dart_frog/dart_frog.dart'; + +Future run(Handler handler, InternetAddress ip, int port) { + throw UnimplementedError(); +} diff --git a/packages/dart_frog_lint/test/src/dart_frog_entrypoint_test.dart b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_test.dart new file mode 100644 index 000000000..84763ca70 --- /dev/null +++ b/packages/dart_frog_lint/test/src/dart_frog_entrypoint_test.dart @@ -0,0 +1,103 @@ +import 'dart:io'; + +import 'package:analyzer/dart/analysis/results.dart'; +import 'package:analyzer/dart/analysis/utilities.dart'; +import 'package:dart_frog_lint/src/dart_frog_entrypoint.dart'; +import 'package:test/test.dart'; + +void main() { + group('dart_frog_entrypoint', () { + test( + 'Emits warning if the second parameter a class named InternetAddress ' + 'but not from dart:io', () async { + final file = File( + 'test/src/dart_frog_entrypoint_sources/incorrect_ip.dart', + ).absolute; + + final result = await resolveFile2(path: file.path); + result as ResolvedUnitResult; + + final error = await const DartFrogEntrypoint() + .testRun(result) + .then((e) => e.single); + + expect(error.message, 'Main files should define a valid "run" function.'); + expect(error.offset, 89); + expect(error.length, 103); + }); + + test( + 'Emits warning if the second parameter is class from dart:io but not ' + 'named InternetAddress', () async { + final file = File( + 'test/src/dart_frog_entrypoint_sources/incorrect_sdk_ip.dart', + ).absolute; + + final result = await resolveFile2(path: file.path); + result as ResolvedUnitResult; + + final error = await const DartFrogEntrypoint() + .testRun(result) + .then((e) => e.single); + + expect(error.message, 'Main files should define a valid "run" function.'); + expect(error.offset, 63); + expect(error.length, 98); + }); + + test('Emits warning if the result returns a non-future HttpServer', + () async { + final file = File( + 'test/src/dart_frog_entrypoint_sources/non_future_result.dart', + ).absolute; + + final result = await resolveFile2(path: file.path); + result as ResolvedUnitResult; + + final error = await const DartFrogEntrypoint() + .testRun(result) + .then((e) => e.single); + + expect(error.message, 'Main files should define a valid "run" function.'); + expect(error.offset, 63); + expect(error.length, 95); + }); + + test('Emits warning if the result returns a non Future result', + () async { + final file = File( + 'test/src/dart_frog_entrypoint_sources/non_http_server_result.dart', + ).absolute; + + final result = await resolveFile2(path: file.path); + result as ResolvedUnitResult; + + final error = await const DartFrogEntrypoint() + .testRun(result) + .then((e) => e.single); + + expect(error.message, 'Main files should define a valid "run" function.'); + expect(error.offset, 84); + expect(error.length, 103); + }); + + test( + 'Emits warning if the result returns a non Future,' + ' but the class is not a HttpServer', () async { + final file = File( + 'test/src/dart_frog_entrypoint_sources/non_server_io_result.dart', + ).absolute; + + final result = await resolveFile2(path: file.path); + result as ResolvedUnitResult; + + final error = await const DartFrogEntrypoint() + .testRun(result) + .then((e) => e.single); + + expect(error.message, 'Main files should define a valid "run" function.'); + expect(error.offset, 63); + expect(error.length, 108); + }); + }); +} diff --git a/packages/dart_frog_lint/test/src/parse_route_test.dart b/packages/dart_frog_lint/test/src/parse_route_test.dart new file mode 100644 index 000000000..bec00ec5e --- /dev/null +++ b/packages/dart_frog_lint/test/src/parse_route_test.dart @@ -0,0 +1,36 @@ +import 'package:dart_frog_lint/src/parse_route.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +void main() { + group('parseRoute', () { + test('throws if path is not within a "routes" directory', () { + final path = p.join('path', 'to', 'file.dart'); + expect(() => parseRoute(path), throwsArgumentError); + }); + + test('parses a route path', () { + final path = p.join('routes', 'path', 'to', 'file.dart'); + final route = parseRoute(path); + + expect(route.path, 'routes/path/to/file.dart'); + expect(route.parameters, isEmpty); + }); + + test('parses a route with parameters', () { + final path = p.join('routes', '[path]', 'to', '[file].dart'); + final route = parseRoute(path); + + expect(route.path, 'routes/[path]/to/[file].dart'); + expect(route.parameters, ['path', 'file']); + }); + + test('[] must be placed around the entire parameter name', () { + final path = p.join('routes', 'p[ath]', 'to', '[fil]e.dart'); + final route = parseRoute(path); + + expect(route.path, 'routes/p[ath]/to/[fil]e.dart'); + expect(route.parameters, isEmpty); + }); + }); +}