Analyzer#1345
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDerives class names from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/freezed/lib/src/models.dart (1)
1076-1084:⚠️ Potential issue | 🔴 CriticalInverted synthetic check — throws on plain fields instead of explicit getters.
The condition at line 1079 is inverted. Per the analyzer documentation and the error message intent, this code should reject explicit getters on freezed classes without a private constructor, but currently rejects plain fields instead.
When an explicit getter like
int get x => ...is declared, its PropertyAccessorElement is non-synthetic, andgetter.nonSyntheticreturns the getter itself. When a plainfinal int x;field is declared, its induced getter is synthetic, andgetter.nonSyntheticreturns the FieldElement (not the getter).
Case getter.nonSyntheticnonSynthetic != getterExplicit int get x => …(should throw)getter itself false — no error Plain final int x;(should pass)FieldElement true — throws error The condition is inverted relative to the intended behavior. Change the operator from
!=to==:Proposed fix
if (!shouldUseExtends && field.getter != null && !field.getter!.isAbstract && - field.getter!.nonSynthetic != field.getter) { + field.getter!.nonSynthetic == field.getter) { throw InvalidGenerationSourceError( 'Getters require a MyClass._() constructor', element: field, ); }This regression breaks existing tests: the
@ShouldThrow('Getters require a MyClass._() constructor')cases intest/source_gen_src.dartfor explicit getters (int get regularProperty => 42;) will fail to throw as expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/freezed/lib/src/models.dart` around lines 1076 - 1084, The condition that rejects explicit getters is inverted: in the block checking shouldUseExtends and field.getter you should compare getter.nonSynthetic to the getter with == instead of != so explicit (non-synthetic) getters trigger the InvalidGenerationSourceError('Getters require a MyClass._() constructor', element: field) while synthetic field-induced accessors do not; update the expression involving field.getter!.nonSynthetic and field.getter in the if inside the shouldUseExtends branch accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/freezed/lib/src/models.dart`:
- Around line 1334-1355: The getters constructors and properties currently
switch on body and silently return const [] for unhandled shapes; add a
defensive assertion in the default branch that fails fast with a clear message
when body is an EmptyClassBody (or any unrecognized ClassBody) so callers know
the class shape is unsupported (referencing the getters constructors and
properties and the body switch on BlockClassBody); implement the assertion to
include the actual runtime type (e.g., "Unsupported ClassBody: $runtimeType
(EmptyClassBody not handled) — `@freezed` classes with semicolon are not
supported") so the error is thrown at the point of observation rather than later
in Class construction.
---
Outside diff comments:
In `@packages/freezed/lib/src/models.dart`:
- Around line 1076-1084: The condition that rejects explicit getters is
inverted: in the block checking shouldUseExtends and field.getter you should
compare getter.nonSynthetic to the getter with == instead of != so explicit
(non-synthetic) getters trigger the InvalidGenerationSourceError('Getters
require a MyClass._() constructor', element: field) while synthetic
field-induced accessors do not; update the expression involving
field.getter!.nonSynthetic and field.getter in the if inside the
shouldUseExtends branch accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b22ab6b-2742-4af7-ba61-90876067dcb2
📒 Files selected for processing (5)
packages/freezed/example/test/diagnosticable_test.dartpackages/freezed/example/test/json_test.dartpackages/freezed/lib/src/models.dartpackages/freezed/pubspec.yamlpackages/freezed/test/integration/decorator.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 25-26: The "Check format" step is guarded by if: matrix.channel ==
'stable' but you removed the 'stable' matrix lane (only 'master' remains), so
the formatting check never runs; update the CI so formatting is still enforced
by either restoring a 'stable' matrix entry or changing the step's condition to
target 'master' (e.g., replace the guard on the "Check format" step that
references matrix.channel == 'stable' with matrix.channel == 'master' or remove
the guard to run on all channels), ensuring you modify the matrix.channel
condition and the "Check format" step reference accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 134773b7-9835-4b21-9120-8b0fd9328049
📒 Files selected for processing (4)
.github/workflows/build.ymlpackages/freezed/CHANGELOG.mdpackages/freezed/example/pubspec.yamlpackages/freezed/pubspec.yaml
✅ Files skipped from review due to trivial changes (2)
- packages/freezed/CHANGELOG.md
- packages/freezed/example/pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/freezed/pubspec.yaml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
24-34: Dead matrix config referencing removedstablechannel.With
stablecommented out ofchannel, theexcludeentries on lines 31–34 (and thecacheexpression on line 42) can never match. Consider cleaning these up to avoid confusion, or restore astablelane if it's intended to come back.🧹 Proposed cleanup
channel: # - stable - master dependencies: - get - downgrade - exclude: - - channel: stable - dependencies: downgrade - - package: packages/freezed_lint - channel: stable🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yml around lines 24 - 34, The workflow matrix contains a dead configuration: the channel list no longer includes "stable" but the exclude rules (the exclude block referencing channel: stable and the package: packages/freezed_lint / channel: stable rule) and the cache expression still refer to "stable", so they will never match; either restore "stable" in the channel matrix or remove/simplify the exclude entries and any cache expressions that reference "stable" (look for the "channel:" list, the "exclude:" block, and the cache expression that mentions stable) so the matrix is consistent and there are no unreachable rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build.yml:
- Around line 24-34: The workflow matrix contains a dead configuration: the
channel list no longer includes "stable" but the exclude rules (the exclude
block referencing channel: stable and the package: packages/freezed_lint /
channel: stable rule) and the cache expression still refer to "stable", so they
will never match; either restore "stable" in the channel matrix or
remove/simplify the exclude entries and any cache expressions that reference
"stable" (look for the "channel:" list, the "exclude:" block, and the cache
expression that mentions stable) so the matrix is consistent and there are no
unreachable rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54a0b878-e1f0-4801-b93c-93cb6ff2b511
📒 Files selected for processing (2)
.github/workflows/build.ymlpackages/freezed/test/json_test.dart
✅ Files skipped from review due to trivial changes (1)
- packages/freezed/test/json_test.dart
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/freezed/lib/src/models.dart (1)
1334-1355:⚠️ Potential issue | 🟡 MinorAvoid silently accepting unsupported class-body shapes.
EmptyClassBody/futureClassBodyvariants currently produce empty constructors/properties, which can defer the failure to a less clear assertion or generation path. Prefer failing here with an actionableInvalidGenerationSourceError.Suggested defensive handling
Iterable<ConstructorDeclaration> get constructors { final that = body; switch (that) { case BlockClassBody(): return that.members.whereType<ConstructorDeclaration>(); default: - return const []; + throw InvalidGenerationSourceError( + 'Classes using `@freezed` must use a block class body. ' + 'Unsupported class body: ${that.runtimeType}.', + element: declaredFragment?.element, + node: this, + ); } } Iterable<(FieldDeclaration, VariableDeclaration)> get properties { final that = body; switch (that) { case BlockClassBody(): return that.members .whereType<FieldDeclaration>() .where((e) => !e.isStatic) .expand((e) => e.fields.variables.map((f) => (e, f))); default: - return const []; + throw InvalidGenerationSourceError( + 'Classes using `@freezed` must use a block class body. ' + 'Unsupported class body: ${that.runtimeType}.', + element: declaredFragment?.element, + node: this, + ); } }Verification query:
What are the concrete subclasses of ClassBody returned by ClassDeclaration.body in package:analyzer 12.x, including semicolon class declarations?Optional repository check:
#!/bin/bash set -euo pipefail # Confirm analyzer constraints and inspect current class-body handling/tests. fd pubspec.yaml | xargs rg -n -C2 "analyzer:" rg -n -C3 "BlockClassBody|EmptyClassBody|ClassBody|Iterable<ConstructorDeclaration> get constructors|Iterable<\\(FieldDeclaration" --type dart rg -n -C2 -i "EmptyClassBody|semicolon|empty class|class\\s+[A-Za-z_][A-Za-z0-9_]*[^{};]*;" --type dart🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/freezed/lib/src/models.dart` around lines 1334 - 1355, The getters constructors and properties currently swallow non-BlockClassBody shapes by returning empty lists; change them to detect unsupported ClassBody variants (e.g., EmptyClassBody / semicolon class bodies) and throw an InvalidGenerationSourceError with an actionable message (mentioning the owning ClassDeclaration name and the unexpected body kind) instead of returning const []; update both Iterable<ConstructorDeclaration> get constructors and Iterable<(FieldDeclaration, VariableDeclaration)> get properties to perform this check on body and throw InvalidGenerationSourceError for any non-BlockClassBody to surface clear failures during generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/freezed/lib/src/models.dart`:
- Around line 1334-1355: The getters constructors and properties currently
swallow non-BlockClassBody shapes by returning empty lists; change them to
detect unsupported ClassBody variants (e.g., EmptyClassBody / semicolon class
bodies) and throw an InvalidGenerationSourceError with an actionable message
(mentioning the owning ClassDeclaration name and the unexpected body kind)
instead of returning const []; update both Iterable<ConstructorDeclaration> get
constructors and Iterable<(FieldDeclaration, VariableDeclaration)> get
properties to perform this check on body and throw InvalidGenerationSourceError
for any non-BlockClassBody to surface clear failures during generation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a89ef4bb-e5c1-4948-b19b-752d1ff54dcd
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.gitignorepackages/freezed/example/pubspec.yamlpackages/freezed/lib/src/ast.dartpackages/freezed/lib/src/models.dartpackages/freezed/pubspec.yamlpackages/freezed/test/common_types_test.dartpackages/freezed/test/decorator_test.dartpackages/freezed/test/deep_copy_test.dartpubspec.yaml
✅ Files skipped from review due to trivial changes (3)
- packages/freezed/test/deep_copy_test.dart
- .gitignore
- pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/freezed/example/pubspec.yaml
- packages/freezed/pubspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Tests
Chores
Documentation