Review fixes for the new rules: false positives, Dart 3.10 gating, and 4.2.0 version sync - #261
Merged
juan-campuzano merged 6 commits intoAug 10, 2026
Conversation
…rameters When a parameter is declared as a type parameter (e.g. `T value` in `ValueNotifier<T>`), its substituted context type may be inferred from the argument itself; rewriting the argument to a dot shorthand removes the inference source and the fixed code no longer compiles (DOT_SHORTHAND_MISSING_CONTEXT). Guard on the parameter declaration's type via `baseElement` (available across analyzer 10-14). Also tighten the rule test: assert exact issue lines and replacements via RuleTestHelper.verifyIssues, and add a step that applies every suggested replacement to the fixture and runs `dart analyze` on the result (opted in to Dart 3.10 via `// @Dart=3.10`) to prove the fixes compile
…ldcards A guarded wildcard (`case _ when condition:`) doesn't satisfy exhaustiveness, so the compiler still enforces coverage of the remaining subtypes; it isn't a fallback that hides missing cases and the rule's rationale doesn't apply to it. Require the guarded pattern to have no `when` clause in both switch statements and switch expressions. Also tighten the rule test to assert exact issue lines via RuleTestHelper.verifyIssues and add guarded-wildcard fixtures for both switch forms
…verage The rule suggested Dart 3.10 syntax regardless of the analyzed file's language version, so on pre-3.10 code the warning was unactionable and the auto-fix produced code that does not compile. Bail out unless the unit's featureSet enables dot_shorthands (present across analyzer 10-14). The main fixture opts in via `// @Dart=3.10`; a new pre-3.10 fixture proves the rule stays silent below that. Also add avoid-non-exhaustive-switch-on-sealed-classes and prefer-dot-shorthands to the dart_all preset, and extend coverage: - unit tests for ast_compat.correspondingParameterOf (positional, named, constructor-named, non-argument) - dot-shorthands fixtures for const constructor calls, explicit type arguments, and supertype-typed parameters - init-state rule: exact-line assertions via verifyIssues plus a two-violations-in-one-initState fixture - fix a misplaced deprecated_member_use ignore in ast_compat_test Verified with make test-analyzer-compat-full across analyzer 10.0.1 through 14.1.0
…-typed callbacks The "callback object" check accepted any named argument, so a plain value configuration like `widget.controller.configure(Options(timeout: 5))` was reported as a hardcoded callback configuration. Require at least one named argument whose static type is a function type. Known limitation (documented in the visitor): a tear-off callback that reads widget fields inside its body is still reported, since only the creation expression itself is inspected for `widget` references
Bump tools/analyzer_plugin/pubspec.yaml to 4.2.0, add the 4.2.0 row to the README compatibility table, and note in the changelog that prefer-dot-shorthands only fires on language version 3.10+
… callback bodies A tear-off callback (e.g. `onError: _handleError`) was judged only by the configuration expression itself, so a handler that reads widget fields inside its body was still reported as hardcoded. Follow references to methods of the same State class (tear-offs and calls inside callback literals, transitively) and scan their bodies for `widget` references. Function-typed arguments that resolve outside the class cannot be inspected within a single resolved unit and are conservatively treated as referencing the widget. Fixtures: tear-off reading a widget field, transitive tear-off, and external tear-off (all not flagged); hardcoded tear-off (flagged).
Contributor
|
@Akhrameev thanks! |
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
2 participants
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.
Wow!
Hey @juan-campuzano, really nice set of rules in draft!
While reviewing, I hit a few edge cases.
I found 4 issues; here is their resolution.
prefer-dot-shorthands
ValueNotifier(LogLevel.info)thesubstituted parameter type (
T=LogLevel) matched the context check, but thesuggested
.inforemoves the very argumentTis inferred from, so the fixed codefails with
DOT_SHORTHAND_MISSING_CONTEXT. The rule now skips parameters declaredas type parameters (checked via
baseElement, available on analyzer 10-14).a project with e.g.
sdk: ^3.5.0resolves below 3.10 and the fix would notcompile.
checknow bails out unless the unit'sfeatureSetenablesFeature.dot_shorthands; a// @dart=3.9fixture proves it stays silent.avoid-non-exhaustive-switch-on-sealed-classes
case _ when condition:does not satisfyexhaustiveness, so the compiler still enforces the remaining subtypes and the
rule's rationale does not apply. Both switch-statement and switch-expression
branches now require the guarded pattern to have no
whenclause.avoid-non-configurable-callbacks-in-init-state
named argument, so
widget.controller.configure(Options(timeout: 5))wasreported. It now requires at least one named argument with a function type.
onError: _handleErrorwas reportedeven when
_handleErrorreads widget fields inside its body. The rule now followsreferences to methods of the same State class (transitively) and scans their
bodies; callbacks resolving outside the class are conservatively not reported.
A hardcoded tear-off (body touching no widget field) is still caught.
Coverage and hygiene
ast_compat.correspondingParameterOf(positional, named,constructor-named, non-argument), matching how
isAbstractMethodwas covered.RuleTestHelper.verifyIssueswith exact lines andreplacements; fixtures added for the rules' skip branches (const constructor
calls, explicit type arguments, supertype-typed parameters, multiple violations
in one
initState).dart_allpreset (prefer-dot-shorthandsissafe there now that it no-ops below Dart 3.10).
tools/analyzer_plugin/pubspec.yaml, README compatibilitytable row, and a CHANGELOG note about the 3.10 gate.
Verification
make test-analyzer-compat-fullpasses on all rows (analyzer 10.0.1, 11.0.0,12.1.0, 13.0.0, 13.3.0, 14.0.0, 14.1.0).