feat: disco_lint #33
feat: disco_lint #33
Conversation
|
Warning Rate limit exceeded@nank1ro has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
WalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
actor Dev as Developer
participant Editor as Code Editor/<br/>IDE
participant Analyzer as Dart Analyzer
participant Plugin as disco_lint Plugin
participant Assist as WrapWithProviderScope<br/>Assist
participant Widget as Widget Code
Dev->>Editor: Opens Flutter widget file
Analyzer->>Plugin: Loads disco_lint plugin (from analysis_options.yaml)
Plugin->>Analyzer: Registers available assists
Analyzer->>Assist: Assist registered
Dev->>Editor: Places cursor on widget expression
Editor->>Analyzer: Requests available code assists
Analyzer->>Assist: Check applicability (is widget wrappable?)
Assist->>Widget: Analyze widget expression
alt Widget is valid and wrappable
Assist-->>Editor: Assist is applicable
Dev->>Editor: Invokes "Wrap with ProviderScope" assist
Editor->>Assist: Trigger assist compute
Assist->>Assist: Resolve ProviderScope class from disco package
Assist->>Assist: Build replacement widget expression
Assist->>Assist: Insert ProviderScope( providers: [], child: <original_widget> )
Assist->>Widget: Apply code change
Widget-->>Dev: Widget now wrapped with ProviderScope
else Widget is not applicable
Assist-->>Editor: Assist not available
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #33 +/- ##
=======================================
Coverage 99.53% 99.53%
=======================================
Files 9 9
Lines 217 217
=======================================
Hits 216 216
Misses 1 1 🚀 New features to boost your workflow:
|
Deploying flutter-disco with
|
| Latest commit: |
a1aed21
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a406c5f3.flutter-disco.pages.dev |
| Branch Preview URL: | https://feat-disco-lint.flutter-disco.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
packages/disco/example/pubspec.yaml (1)
20-20: Version bump consistency verified through earlier check.This very_good_analysis bump matches the pattern across the repository. The verification requested for examples/bloc/pubspec.yaml applies here as well.
packages/disco/pubspec.yaml (1)
26-26: Version bump consistency verified through earlier check.This very_good_analysis bump completes the coordinated upgrade across the repository. The verification requested for examples/bloc/pubspec.yaml applies here.
🧹 Nitpick comments (5)
packages/disco_lint/.gitignore (1)
1-3: Minimal but adequate.gitignoresetup.The configuration correctly ignores the essential
.dart_tool/directory per Dart conventions. This is sufficient for a package that will have itspubspec.lockcommitted.Consider adding additional patterns for generated build artifacts if desired in the future:
# https://dart.dev/guides/libraries/private-files # Created by `dart pub` .dart_tool/ + build/ + .packagesThis is optional and can be deferred until needed during development.
packages/disco_lint/example/README.md (1)
1-16: Example README is adequate but could be more specific.The README is a standard Flutter getting-started template. For better documentation, consider adding a brief section explaining what the example demonstrates (e.g., how the disco_lint assists work within a Flutter project). However, this is optional and the current content is acceptable for an example project.
packages/disco_lint/pubspec.yaml (1)
2-4: Update placeholder metadata before publishing.The description is a generic placeholder that doesn't reflect the package's purpose as a Dart analyzer plugin providing assists for the disco package. The commented repository URL should also be updated or removed.
-description: A sample command-line application. +description: A Dart analyzer plugin providing assists for the disco package. version: 1.0.0 -# repository: https://github.com/my_org/my_repo +repository: https://github.com/our-creativity/discopackages/disco_lint/lib/src/assists/wrap_with_provider_scope.dart (1)
13-17: Inconsistent assist ID prefix.The assist ID uses
'solidart.wrap_with_provider_scope'but the package is nameddisco_lint. Consider using a consistent prefix like'disco.wrap_with_provider_scope'to align with the package naming convention.@override AssistKind get assistKind => const AssistKind( - 'solidart.wrap_with_provider_scope', + 'disco.wrap_with_provider_scope', 30, 'Wrap with ProviderScope', );packages/disco_lint/lib/src/assists/base/wrap_single_widget.dart (1)
4-5: Consider documenting why the internal import is necessary.While the
ignoredirective is appropriate, a brief comment explaining the dependency would help future maintainers understand the risk of using internal analyzer APIs.-// ignore: implementation_imports +// ignore: implementation_imports +// Required for `findWidgetExpression` extension on AstNode. import 'package:analyzer/src/utilities/extensions/flutter.dart';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
analysis_options.yaml(1 hunks)docs/src/content/docs/examples/auto-route.mdx(1 hunks)examples/auto_route/pubspec.yaml(1 hunks)examples/bloc/pubspec.yaml(1 hunks)examples/preferences/pubspec.yaml(1 hunks)examples/solidart/analysis_options.yaml(0 hunks)examples/solidart/pubspec.yaml(1 hunks)packages/disco/example/pubspec.yaml(1 hunks)packages/disco/pubspec.yaml(1 hunks)packages/disco_lint/.gitignore(1 hunks)packages/disco_lint/CHANGELOG.md(1 hunks)packages/disco_lint/README.md(1 hunks)packages/disco_lint/analysis_options.yaml(1 hunks)packages/disco_lint/example/.gitignore(1 hunks)packages/disco_lint/example/.metadata(1 hunks)packages/disco_lint/example/README.md(1 hunks)packages/disco_lint/example/analysis_options.yaml(1 hunks)packages/disco_lint/example/lib/main.dart(1 hunks)packages/disco_lint/example/pubspec.yaml(1 hunks)packages/disco_lint/example/test/widget_test.dart(1 hunks)packages/disco_lint/lib/disco_lint.dart(1 hunks)packages/disco_lint/lib/main.dart(1 hunks)packages/disco_lint/lib/src/assists/base/wrap_builder.dart(1 hunks)packages/disco_lint/lib/src/assists/base/wrap_single_widget.dart(1 hunks)packages/disco_lint/lib/src/assists/wrap_with_provider_scope.dart(1 hunks)packages/disco_lint/pubspec.yaml(1 hunks)pubspec.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- examples/solidart/analysis_options.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (21)
packages/disco_lint/lib/disco_lint.dart (1)
1-1: Minimal library entrypoint is acceptable for analyzer plugin structure.The empty library directive follows the expected pattern for analyzer plugins where functionality is exposed through the plugin entrypoint (main.dart). Assuming main.dart properly registers the plugin and assists, this is acceptable.
packages/disco_lint/CHANGELOG.md (1)
1-3: LGTM!Standard changelog entry for initial release.
analysis_options.yaml (1)
38-40: Plugin registration looks correct.The disco_lint plugin configuration follows the expected analyzer plugin registration pattern. Assuming packages/disco_lint/lib/main.dart properly implements the plugin interface and registers assists, this configuration is appropriate.
examples/bloc/pubspec.yaml (1)
21-21: The bump from ^9.0.0 to ^10.0.0 is safe. Version 10.0.0 is available on pub.dev with no breaking changes between the two versions—only Dart 3.9.0 support and documentation updates were included in the major release.pubspec.yaml (1)
8-9: LGTM!The workspace entries for the new disco_lint package and its example are correctly configured.
docs/src/content/docs/examples/auto-route.mdx (1)
20-20: LGTM!Documentation properly updated to reflect the auto_route version change in the example project.
packages/disco_lint/example/.gitignore (1)
1-45: LGTM!Standard Flutter project .gitignore file with appropriate patterns for the example project.
packages/disco_lint/example/.metadata (1)
1-30: LGTM!Standard Flutter project metadata file generated by Flutter tooling. The unmanaged files list appropriately excludes custom application code from automatic migration.
examples/solidart/pubspec.yaml (1)
22-22: very_good_analysis ^10.0.0 is valid and available.Version 10.0.0 was released on August 22, 2025, with support for Dart 3.9.0. No breaking changes are present in this version.
examples/auto_route/pubspec.yaml (1)
12-12: Version updates are properly implemented for compatibility.The major version updates are justified and have been correctly implemented in the example:
auto_route: ^11.0.0– Code properly extendsRootStackRouterand uses the newPageRouteInfopatternauto_route_generator: ^10.4.0– Generated code follows v11 conventionsbuild_runner: ^2.10.4andvery_good_analysis: ^10.0.0– Compatible with Flutter SDK ^3.6.0The example avoids all breaking changes: no deprecated navigation methods, no removed
AutoRouterConfig.module, and no guard implementations relying on outdated APIs. No further action needed.packages/disco_lint/analysis_options.yaml (1)
1-30: Standard analysis options configuration.This is the standard Dart package analysis options template including recommended lints. The configuration is appropriate for the disco_lint package itself.
packages/disco_lint/example/test/widget_test.dart (1)
13-29: Standard Flutter counter smoke test.This is the standard Flutter counter app test template, appropriate for demonstrating the example application works correctly.
packages/disco_lint/lib/main.dart (1)
8-17: Clean plugin implementation.The plugin entrypoint follows the standard pattern for Dart analyzer plugins: a private class with a public top-level instance, proper name override, and assist registration. The structure is extensible for adding more assists in the future.
packages/disco_lint/example/analysis_options.yaml (1)
1-28: Standard Flutter analysis options.This is the standard Flutter analysis options template using
flutter_lints, which is appropriate for the Flutter example application.packages/disco_lint/lib/src/assists/wrap_with_provider_scope.dart (1)
4-10: Clean assist configuration.The WrapWithProviderScope class properly configures the wrapping behavior by specifying the widget name, the required import, and the default
providers: []parameter. The implementation is concise and correctly delegates to the base class.packages/disco_lint/lib/src/assists/base/wrap_builder.dart (2)
31-42: Widget wrapping logic is well-structured.The compute method properly validates the widget expression, checks wrapability, and retrieves the builder element before proceeding. The early returns prevent unnecessary work when conditions aren't met.
46-80: Code generation handles formatting correctly.The replacement logic properly manages indentation levels, trailing commas based on code style options, and constructs the builder pattern correctly. The use of
writeReferencefor the builder element ensures proper import handling.packages/disco_lint/example/lib/main.dart (2)
1-6: LGTM!Standard Flutter entry point with correct imports for disco and Flutter.
42-58: LGTM!The StatefulWidget pattern and counter logic are correctly implemented. The example effectively demonstrates the
ProviderScopewrapping that thedisco_lintassist provides.Also applies to: 60-108, 110-126
packages/disco_lint/lib/src/assists/base/wrap_single_widget.dart (2)
7-23: LGTM!The class structure is well-designed with clear required parameters and sensible defaults. The
singleLocationapplicability correctly indicates this correction applies to one location at a time.
25-72: LGTM!The
computemethod correctly handles both single-line and multiline widget wrapping scenarios. The indentation logic properly re-indents the wrapped widget source and formats extra named parameters with appropriate trailing commas for Dart's preferred style.
| import 'package:analyzer/src/dart/ast/extensions.dart'; | ||
| import 'package:analyzer/src/dart/element/type.dart'; | ||
| import 'package:analyzer/src/utilities/extensions/flutter.dart'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Dart analyzer package public API vs internal src directories API stability
💡 Result:
Short answer: Treat anything under lib/src (or any files not re-exported by a library outside lib/src) as internal/unstable and do not depend on it. The analyzer package follows the Dart package-layout conventions: public, supported API lives in libraries under lib (and any names they explicitly export); implementation details live in lib/src and can change between releases (and the analyzer toolset enforces this with lints/diagnostics). Rely only on documented public APIs or explicitly exported members; if you import analyzer/src you risk breakage. [1][2][3]
Key points and evidence
- Dart convention: libraries in lib are public; files in lib/src are implementation and “should never be imported from another package.” Linter/diagnostics exist to flag implementation_imports. [3][4]
- The analyzer package itself marks internal API and can produce diagnostics for exporting/internal annotations; the analyzer changelog and issues show real breakage when code used analyzer/src internals across versions. [1][5][6]
- Use @internal and export-hiding rules: the analyzer enforces invalid_export_of_internal_element and related diagnostics to prevent exposing internal types. [1][2]
Practical recommendations
- Only depend on symbols documented in the analyzer package’s public libraries (the files in lib that the package exposes).
- Don’t import package:analyzer/src/...; enable the implementation_imports lint to catch accidental imports. [3][4]
- If you need functionality not exposed publicly, open an issue/PR on the analyzer repo asking maintainers to add a supported API (or provide a stable wrapper) rather than importing internals. [5][6]
Sources
- Dart package layout, linter rule and implementation_imports guidance. [3][4]
- Diagnostics about internal/export rules (@internal, invalid export). [1][2]
- analyzer changelog and issues demonstrating internal-API breakage. [5][6]
🌐 Web query:
analyzer package Dart pub.dev public API documentation
💡 Result:
The analyzer package public API docs are available here:
- Pub package page (overview + link to docs): pub.dev/packages/analyzer. [1]
- API reference (generated docs): pub.dev/documentation/analyzer/latest/. [2]
Sources:
[1] pub.dev/packages/analyzer
[2] pub.dev/documentation/analyzer/latest/
🌐 Web query:
Flutter widget detection Dart analyzer package public API extensions
💡 Result:
Short answer: use the analyzer package's Element/InterfaceType APIs — get a ClassElement for the type and inspect its supertypes (allSupertypes or supertype) to see if any InterfaceType resolves to Flutter's Widget (or StatefulWidget/StatelessWidget). Do this inside an analyzer plugin or lint. Example pattern:
- Resolve the element for the class (ClassElement).
- Check classElement.allSupertypes.any((t) =>
t.element.name == 'Widget' &&
t.element.library?.source.uri.toString().contains('package:flutter'));
Minimal example (within an analyzer/lint/plugin where you have a ClassElement):
final bool isFlutterWidget(ClassElement c) =>
c.allSupertypes.any((t) =>
t.element.name == 'Widget' &&
t.element.library?.source.uri.toString().contains('package:flutter'));
Notes:
- You can also check for 'StatelessWidget' / 'StatefulWidget' the same way. Use analyzer plugins when running in IDE/analysis; see the analyzer plugin docs for setup. [1][2][3]
Sources:
- Analyzer plugins (writing lints/plugins). [1]
- Analyzer element API (InterfaceElement / ClassElement / allSupertypes). [2]
- Flutter Widget classes (StatelessWidget / StatefulWidget). [3]
References:
[1] https://dart.dev/tools/analyzer-plugins
[2] InterfaceElement / ClassElement docs (analyzer package).
[3] Flutter widgets API (StatelessWidget / Widget).
Replace internal analyzer APIs with public equivalents.
The imports from analyzer/src/* paths violate Dart package conventions. The analyzer package treats anything under lib/src as internal and subject to change between versions; the package maintainers explicitly track and break these APIs across minor releases. Enable the implementation_imports lint to catch such issues.
Public APIs exist for the needed functionality. For example, use ClassElement.allSupertypes and the public InterfaceType API to detect Flutter widgets by traversing the inheritance hierarchy instead of relying on internal extensions.
| for (final namedParam in extraNamedParams) { | ||
| builder.write(indentNew1); | ||
| builder.write('$namedParam: '); | ||
| builder.addSimpleLinkedEdit('variable', namedParam); | ||
| builder.writeln(','); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -A 5 "class WrapWithProviderScope" --type dartRepository: our-creativity/disco
Length of output: 717
🏁 Script executed:
rg "extraNamedParams" --type dart -B 2 -A 2Repository: our-creativity/disco
Length of output: 3863
🏁 Script executed:
cat -n packages/disco_lint/lib/src/assists/base/wrap_builder.dart | head -80Repository: our-creativity/disco
Length of output: 3268
🏁 Script executed:
rg "extraNamedParams:" --type dartRepository: our-creativity/disco
Length of output: 181
🏁 Script executed:
rg "extends WrapBuilder\|extends WrapSingleWidget" --type dart -A 10Repository: our-creativity/disco
Length of output: 46
Bug in named parameter string handling produces malformed output.
The code at lines 56-61 assumes extraNamedParams contains only parameter names, but WrapWithProviderScope passes complete assignment strings like 'providers: [],'. The current loop adds an extra : and a trailing comma, producing malformed output like providers: [],: ,.
for (final namedParam in extraNamedParams) {
builder.write(indentNew1);
- builder.write('$namedParam: ');
- builder.addSimpleLinkedEdit('variable', namedParam);
- builder.writeln(',');
+ builder.write(namedParam);
+ if (!namedParam.endsWith(',')) {
+ builder.write(',');
+ }
+ builder.writeln();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (final namedParam in extraNamedParams) { | |
| builder.write(indentNew1); | |
| builder.write('$namedParam: '); | |
| builder.addSimpleLinkedEdit('variable', namedParam); | |
| builder.writeln(','); | |
| } | |
| for (final namedParam in extraNamedParams) { | |
| builder.write(indentNew1); | |
| builder.write(namedParam); | |
| if (!namedParam.endsWith(',')) { | |
| builder.write(','); | |
| } | |
| builder.writeln(); | |
| } |
🤖 Prompt for AI Agents
In packages/disco_lint/lib/src/assists/base/wrap_builder.dart around lines
56-61, the loop assumes entries in extraNamedParams are bare names but callers
may pass full assignments (e.g. "providers: [],"). Update the loop to detect
whether namedParam already contains a ':' or '=' (or already ends with a comma)
and, if so, write the string as-is (ensuring a single trailing comma) without
adding an extra ": " or linked edit; otherwise treat it as a simple name: write
"name: ", call addSimpleLinkedEdit('variable', name) and then write a single
comma. Ensure trimming/normalizing trailing commas so output is not duplicated.
| dependencies: | ||
| analyzer: ^8.0.0 | ||
| analyzer_plugin: ^0.13.10 | ||
| analysis_server_plugin: ^0.3.3 | ||
| collection: ^1.19.1 | ||
| meta: ^1.17.0 | ||
| path: ^1.9.1 | ||
| source_span: ^1.10.1 | ||
| build: ^4.0.3 | ||
| yaml: ^3.1.3 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
analyzer package pub.dev latest version
💡 Result:
The latest version of the analyzer package on pub.dev is 9.0.0 (published 46 days ago). [1][2]
Sources:
[1] pub.dev — analyzer package.
[2] pub.dev — analyzer (versions).
🌐 Web query:
analyzer_plugin package pub.dev latest version
💡 Result:
The latest published version is analyzer_plugin 0.13.11. [1][2]
Sources:
[1] pub.dev package page for analyzer_plugin.
[2] analyzer_plugin changelog on pub.dev.
🌐 Web query:
analysis_server_plugin package pub.dev latest version
💡 Result:
Latest version: 0.3.4. [1][2]
Sources:
- analysis_server_plugin package page on pub.dev. [1]
- analysis_server_plugin changelog on pub.dev. [2]
Dependencies are appropriate for an analyzer plugin, but consider updating the analyzer constraint.
The selected dependencies (analyzer, analyzer_plugin, analysis_server_plugin) are correct for building a Dart analyzer plugin. However, the analyzer package constraint ^8.0.0 excludes the latest version 9.0.0 (released 46 days ago). Consider updating to ^9.0.0 if compatible with your plugin's implementation. The analyzer_plugin and analysis_server_plugin constraints allow their latest patch versions (0.13.11 and 0.3.4 respectively).
🤖 Prompt for AI Agents
In packages/disco_lint/pubspec.yaml around lines 11 to 20 the analyzer
constraint is pinned to ^8.0.0 which excludes analyzer 9.x; update the analyzer
SDK constraint to ^9.0.0 (or a range that includes 9.x, e.g. ^9.0.0) and, if
needed, bump or verify compatible versions of analyzer_plugin and
analysis_server_plugin (use their latest patch versions like 0.13.11 and 0.3.4)
then run pub get and run tests/static analysis to ensure compatibility.
| @@ -0,0 +1,20 @@ | |||
| This package is a developer tool for users of disco, designed to help stop common issues and simplify repetitive tasks. | |||
|
|
|||
| > I highly recommend using this package to avoid errors and understand how to properly use disco | |||
There was a problem hiding this comment.
In a new PR, I would just reframe this to "> It is highly recommended to use this lint package in combination with Disco."
|
LGTM otherwise ;) |
This PR adds a new package
disco_lintthat provides theWrap with ProviderScopeassist to the Dart analyser.TODOs:
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.